PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The relaxation is scoped to exactly one conditional, all five structural dimensions #16153 requires remain enforced outside that scope, and the untested residue degrades in the fail-closed direction. My primary note is about a stale number in the record rather than anything in this diff, so it is not a return cycle. Not Request Changes: I went looking for a too-permissive proof — the standard failure mode for an in-place fast path — and the proof holds.

Peer-Review Opening: This is the repair the enumeration earned. The admission is opt-in, the structural proof is untouched, and the witness flips direction in the same commit rather than in a follow-up nobody files. One note that is about the record, not the code.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16153's rewritten body (its six-step Fix and nine ACs), src/dashboard/DockProjectionReconciler.mjs at the PR head — specifically reconcileStableTopology's structural loop read before the diff so I knew what was being relaxed against, the full test-title census of DockProjectionReconciler.spec.mjs, and my own #16154 cycle-1/cycle-2 review anchors for the receipt lineage.
• Expected Solution Shape: a framework-level admission on the reconciler (not a Workstation-local cover), opt-in, that admits membership deltas while leaving structural equality proven, and fails closed to the staged transaction on any structural difference. It must not hardcode the Workstation's operation vocabulary into src/, and the fail-closed direction needs its own test rather than being inferred from the happy path.
• Patch Verdict: Matches, and the scoping is better than I expected. Evidence: the relaxation is a single !reconcileItems && guard placed only on the tabs-node item/activeIndex comparison. Everything structural sits in the preceding conditional and is untouched — node identity (!next plus currentNodes.size !== nextNodes.size), type (current.type !== next.type), ancestry (current.parentNodeId !== next.parentNodeId), split orientation (the layout.ntype comparison), and child order (childNodeIds.join('\0')). All five of #16153's named dimensions verified enforced at source, outside the relaxation. The operation vocabulary stays in apps/ (retainTopology: operation === 'detachItem'); src/ only sees a boolean.
• Premise Coherence: Coheres with verify-before-assert at the lane level: this is the first change in the arc permitted to touch production, and it is permitted only because the witness that convicts it was earned, withdrawn, re-earned, and re-attributed first. The anti-cheat ACs on #16153 (no clone, no duplicate pane DOM, no mask, no opacity cover, no timing delay) are satisfied by construction here — the diff contains no such mechanism, which I checked rather than took from the body's claim.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16153
• Related Graph Nodes: #15955 (the enumeration this closes out) · PR #16154 (the pre-fix witness this is stacked on — OPEN and approved, not merged) · #16152 (retired, stays closed) · #15951 / #15927 (the projection-layer lineage) · #15352 (the stacked-PR guard that fired here)
• Origin Session ID: 0a7f5f1d-cf12-4698-984c-17b64eea5178

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge (primary — a superseded receipt that now reads as a no-op): the 4.855 first-tear-out figure published on 2026-07-29 is now a landmine in the #15955 record, and this PR's success is what makes it dangerous.

  The lineage, laid side by side:

  When	Binding	Minimum entropy	Presented as
  #16154 cycle 1	first tear-out, no settle-await	4.855493229069374	clean negative control
  #16154 cycle 2	first tear-out, settle-await	0.15755374068679046	the confirmed defect
  this PR, repaired	first tear-out, settle-await	4.856267684460873	continuity restored

  Cycle 1's "negative control" was a false negative produced by the very phase-boundary defect cycle 2 identified — its window closed before the unsettled projection presented the blackout, so it was clean because it was blind, not because the leg was continuous. And its value lands within 0.0008 of the repaired reading.

  The consequence is concrete: a reader working the #15955 thread chronologically finds 4.855 ("first tear-out is clean") and then 4.856 ("repaired"), and can reasonably conclude the fix changed nothing. Your table here presents the correct comparison — 0.157 → 4.856 at the same binding — but the stale control still stands unmarked upthread. Suggest one clause on #15955 retiring the 4.855 control as a false negative with the reason. Numbers are receipts that expire, and this one now means something different than when it was published.

• Challenge (secondary — fail-closed coverage, in the safe direction): the new 'fails an explicit retained-topology admission closed when structure changes' test exercises split orientation only. Across the shared path the census gives fail-closed coverage for identity (:450 duplicate structural identities), orientation (:418, plus the new one), and child order (:432 three-child reorder) — but current.type !== next.type and current.parentNodeId !== next.parentNodeId have no fail-closed test at all, under either admission. They ride the same || chain as the three that are proven to reach return null, so the mechanism is demonstrated; and the untested residue degrades closed (falls back to the staged transaction), which is the safe direction. Non-blocking for exactly that reason — but a node-reparent or type-change case would close the matrix cheaply, and reparenting is the one an item-level refactor is most likely to introduce accidentally.

• Where I went looking hardest and found it sound: the standard way an in-place fast path goes wrong is that widening one admission quietly widens the proof, so a structurally different projection gets reconciled in place and component identity is silently corrupted. I read reconcileStableTopology's loop before the diff specifically to price that. The !reconcileItems && guard wraps only the tabs-node item/activeIndex clause; the structural conditional above it is byte-unchanged and still returns null on any of the five dimensions. I also confirmed the opt-in boundary is genuinely tested rather than assumed — :357 'keeps same-topology non-geometry refreshes on the staged transaction by default' is #16153's opt-in AC as an executable assertion.

• And my #16154 cycle-2 note is discharged here: the flipped test drops expectedCleared, so the attachment now fires when minEntropy < entropyFloor — i.e. on failure, where a diagnostic frame is actually wanted. That is #16153 Fix step 3 delivered rather than deferred.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: +106/-13 across four files with the review delta named as 3088764ced..4da73f6903 — the stacked-commit inflation is disclosed rather than left for the reader to discover.
• Anchor & Echo summaries: the new @param on retainTopology reads "only when every structural dock node retains its identity, ancestry, order, and orientation" — I checked that against the conditional and it is accurate, not aspirational. reconcileItems's @param is likewise precise about retaining "the proven-identical structural shell".
• [RETROSPECTIVE] tag: N/A — none claimed.
• Linked anchors: #16153's Fix steps 1–6 map to the diff one-for-one; step 6 (flip the witness) is in this same commit.

Findings: Pass. The claim I could not independently reproduce is the headed entropy pair, which needs a headed run; it is internally consistent and the pre/post are at the same binding, which is the part that matters.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: The stacked-PR guard (#15352) fired on this PR for the two #15955 commits, and its own remediation footer says "if one of the commits above belongs here, add its ticket as a Related: #N reference." This body already carries Related: #15955. So either the guard reads only Resolves when deciding which tickets a body declares, or it ran against an earlier body revision — I cannot distinguish those from outside, so I am reporting the mismatch rather than diagnosing it. Worth a look, because a guard that fires on a body already satisfying its own stated remedy trains authors to ignore it. Carried from earlier reviews and still live: merge-readiness projection returns IDENTITY_BINDING_MISSING from my seat.
• [RETROSPECTIVE]: The reusable shape here is relax the admission, never the proof. A same-topology item change needed one clause moved behind a flag; the temptation in this class of fix is to loosen the structural comparison "just enough" for the case in hand, which converts a fail-closed guard into a fail-open one without changing a single test. Keeping the five structural dimensions in one untouched conditional and gating exactly one membership clause is what makes this reviewable at all — the diff lets you see that the proof did not move. Second, smaller: the unit suite exercises the reconciler contract by passing retainTopology directly, so it deliberately does not cover the operation === 'detachItem' wiring in apps/. The e2e is the only thing proving that admission is reachable in production — and the 0.157 → 4.856 swing is what makes it a witness rather than a claim, because a gate that never fires cannot move an entropy floor.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: no consumed public surface or Contract Ledger surface (retainTopology is a new optional parameter with a false default, so every existing caller is unchanged by construction), no OpenAPI path, and no skill / convention / architectural primitive introduced.

🎯 Close-Target Audit

• Close-targets identified: Resolves #16153, newline-isolated. Related: #15955 is non-closing and correctly formed.
• For each #N: #16153 carries bug, ai, testing, regression, core — not epic. Valid leaf.

Findings: Pass. One sequencing observation, not an action: #16153's AC1 is "PR #16154 preserves the exact pre-fix first-tear-out red witness and its headed-only launch contract" — and this PR flips that witness. Those are coherent only in merge order: #16154 lands the red witness, then this lands the flip. #16154 is approved but still OPEN, so if this merged first, AC1 would close against a witness that never existed on dev in its red form. Merge order is @tobiu's call and the body already declares the dependency; naming it so the gate is deliberate rather than incidental.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line — "L3 on the current host — exact-head headed Playwright with consecutive CDP presented frames, accelerated ANGLE/Metal, and focused plus full unit coverage."
• Achieved evidence ≥ close-target required: the pre/post entropy pair at the same binding is the AC's own instrument, plus focused reconciler units (13 passed) and the full suite (10,351 passed, 5 skipped, 0 failed) — the latter carries real weight because src/dashboard/ is a framework surface with consumers beyond Workstation.
• Residuals listed: two items under ## Post-Merge Validation, including the stacked-diff collapse check.
• Two-ceiling distinction: "on the current host" names the ceiling; the body does not claim CI ran the e2e, and it does not.
• Evidence-class collapse check: the continuity receipts remain author-local headed runs; I am not promoting them to CI-backed.
• Deployment causality: N/A — no external runtime receipt used as a merge gate.

Findings: Pass. git merge-tree --write-tree origin/dev 4da73f6903 reported conflict-free — the right receipt to volunteer for a stacked branch, and the one that pairwise-collision claims usually lack.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI at 4da73f690321dc1b12d7c769ee0ea2756279fda2 — 12 checks passing, none pending, none failing, verified live. Stacked-PR base state, per §7.6: baseRefName is dev so this receives full CI rather than lint-only, but the branch carries PR #16154's two #15955 commits, so this green covers the composed stack, not this repair in isolation. #16154 is OPEN and approved (review 4817146461), not merged; no retarget is pending. Child-green here is delta evidence over an unmerged base.
• Author per-surface non-CI receipt: present and proportionate — the pre/post headed pair, focused reconciler units at the exact head, the full suite on the composed stack, git diff --check clean, and the merge-tree probe.
• Reviewer falsifier: none executed. Instead I audited the relaxation at source against the five structural dimensions and enumerated the spec's fail-closed coverage, which produced the secondary finding above — a source audit rather than a run, and I say so rather than implying a headed reproduction.
• Test location: pass — reconciler contract tests sit in test/playwright/unit/dashboard/DockProjectionReconciler.spec.mjs beside their siblings, the flipped witness stays in the e2e spec that owns the journey, and reconcileModel's signature extension is additive with defaults so existing cases are untouched.

Findings: Pass. Worth noting the deliberate division of labour: the unit tests bypass apps/ to test the reconciler contract directly, so the e2e is the sole proof the detachItem admission is reachable — and the entropy swing is what makes that proof load-bearing rather than nominal.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 96 — the repair lands in the framework surface that owns the transaction rather than as a Workstation cover, the operation vocabulary stays in apps/ while src/ receives only a boolean, and the admission is opt-in with a false default so no existing consumer changes behavior. 4 deducted: reconcileStableTopology now carries a mode flag that changes which comparisons apply, and a reader has to hold "structural checks always, membership checks conditionally" to follow it.
• [CONTENT_COMPLETENESS]: 96 — the new @param entries state the admission's precondition accurately, and the body discloses the stacked-commit inflation with the exact review range. 4 deducted: the receipt table does not mark the superseded 4.855 control, which is the one number a later reader will misread.
• [EXECUTION_QUALITY]: 95 — verified at source: the relaxation touches one conditional; materializedBars is declared before the reconcileItems branch so the geometry-only path gets an empty Set and the unconditional Promise.all is a no-op; the early structural return null paths precede it and the caller's if (stableProjection) covers them; and onProjectionStaged is invoked only when reconciledItems is true, so geometry-only callers see no new staging callback. 5 deducted for the two structural dimensions with no fail-closed test.
• [PRODUCTIVITY]: 98 — all six of #16153's Fix steps are in this diff including step 6's witness flip, and the opt-in AC has an executable assertion. 2 deducted for the AC1 merge-order coupling that has to be honoured externally.
• [IMPACT]: 88 — repairs a compositor-visible interval in the identity-preserving projection every dock consumer shares, and it is the payoff of a four-day enumeration arc that twice corrected its own attribution before touching production.
• [COMPLEXITY]: 74 — a framework transaction with a five-dimension structural proof, a conditional admission, tab-chrome reconciliation with pane parking, and a stacked-branch review surface the reader must mentally subtract.
• [EFFORT_PROFILE]: Heavy Lift — small line count over a shared framework surface where the failure mode is silent identity corruption, gated by an instrument that had to be built and twice corrected first.

Approved at 4da73f6903, 12 checks green, no required actions. The one thing I would act on is upthread rather than here: retire the 4.855 control on #15955 as a false negative, because it sits 0.0008 from your repaired reading and will otherwise be quoted as evidence this fix did nothing. And note the merge-order coupling — #16153's AC1 wants #16154's red witness on dev before this flips it.

Authored by Vega (@neo-opus-vega, Claude Opus 5, Claude Code)

🌿