PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: A high-blast change (it amends an accepted ADR), but it is the right amendment: it resolves a genuine internal contradiction in ADR 0029 rather than introducing new policy, and I verified the two load-bearing claims (write-only removal is reader-safe; the amendment leaves §2.5/§2.6 — consumed by the just-graduated D#15173 and by #15175 — untouched). Nothing important is missing (not Request Changes) and the single residual is optional ADR-trail hygiene (not a must-do, so not Approve+Follow-Up).

Peer-Review Opening: Emmy — good catch, and the right fix shape. The ADR's §2.3 was internally inconsistent (it demanded persisting owningWorkspaceId/fallbackTarget into dockZone.v1 item records before the §2.2 hint layer that should own them exists), and the code was faithfully implementing the contradiction. Refusing to encode speculative state in a finite schema — and amending the ADR to say so — is the correct call.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: the PR body (Decision-Record-impact + Evolution); the full 4-file diff; ADR 0029 §2.3/§2.4 (the amended clauses) and its Status/Ledger; the DockCrossWindowParticipation source; a src/ reader sweep for the removed fields; my own D#15173 [GRADUATION_APPROVED] and #15175 review (both consume ADR 0029 §2.5/§2.6). Note: ticket #15189 has no local markdown index entry yet (unsynced), so the premise is built from the PR body + diff, cross-checked against the code.
• Expected Solution Shape: cross-window transferItem must publish the executor's finite dockZone.v1 documents unchanged; the placement-intent fields must not be stamped into generic item records until the §2.2 hint layer that owns them exists; the ADR clause that mandated the premature stamping must be amended (not silently left contradictory), and any status/echo drift corrected. It must NOT touch §2.5/§2.6 (live consumers), and must NOT remove a field that is actually read anywhere.
• Patch Verdict: Matches. The code removes the two write-only stamps and publishes the executor's documents verbatim; ADR §2.3 becomes a "hint-layer transaction (conditional on that layer existing)" clause + §2.4 mirrors it for transferNode; the Implementation-Ledger CrossWindowDragTarget row is updated to landed reality (#14769/PR #15017); the Status is corrected Proposed→Accepted (the ADR is demonstrably live on dev) with the ArchitectureOverview echo. Verified: the removed fields have zero readers in src/, and §2.5/§2.6 are untouched.
• Premise Coherence: Coheres — it strengthens the ADR's own finite-dockZone.v1 principle by removing a clause that violated it, and it defers the real hint-atomicity to §2.2's future workspace-set transaction rather than encoding not-yet-owned state. Friction→gold: live Demo B topology-capture exposed the contradiction, and the fix corrects the decision record instead of papering over it in app code.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15189
• Related Graph Nodes: ADR 0029 (amended), parent epic #13158, #14772 (downstream two-window journey — retains the post-transfer topology-capture tripwire), landed transfer #14769/PR #15017; adjacent ADR-0029 consumers D#15173 (graduated) + #15175 (§2.5/§2.6, unaffected).

🔬 Depth Floor

Challenge (non-blocking — ADR amendment-trail hygiene):

The amendment rewrites §2.3's "Hint-updating" and §2.4's transferNode clauses in place. The trail is not lost — the PR body declares "amends ADR 0029," the Implementation-Ledger row is updated, and git preserves the change — but the ADR itself carries no inline amendment note (e.g., "Amended 2026-07-15 (#15189 / PR #15190): §2.3/§2.4 hint-atomicity deferred to the §2.2 hint-layer transaction; the landed transfer publishes finite documents only."). For an ADR that other work already cites as authority, an inline one-liner would stop a future reader from quoting the superseded "same-commit hint update" mandate. Minor here because the ADR is being accepted in this same change and the clause was never validly implementable, so an in-place correction is defensible; optional, author's discretion.

Rhetorical-Drift Audit (per guide §7.4):

• "removing the two write-only placement fields" — verified: src/ sweep finds no reader; the fields exist only as the removed stamps + the rewritten JSDoc.
• "publishes the real transferItem() source/target documents unchanged" — verified: the stamping block is deleted; commitTransfer now receives the executor's results directly.
• "amends ADR 0029 … the separate-ledger shape revalidated when a semantic-recovery consumer is actually filed" — matches the §2.3/§2.4 rewrites and the ledger row; the deferral is explicit, not silent.
• Status Proposed→Accepted — consistent with the ADR being live on dev (present at the canonical decisions path) and consumed as binding authority by D#15173 / #15175.

Findings: Pass — framing matches the diff.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The load-bearing move is refusing to persist not-yet-owned state in a finite schema. An ADR clause that mandates writing perspective/workspace fields (owningWorkspaceId, fallbackTarget) into generic dockZone.v1 item records — before the layer that owns those fields exists — is a contradiction the implementation will faithfully reproduce until a live capture (Demo B topology-capture) falsifies it. The correct fix amends the decision record and defers the atomicity to the real owner (§2.2 workspace-set transaction), rather than sanitizing in app code. "Write-only field with zero readers" is the tell that a schema is carrying speculative state.
• [KB_GAP]: none.

🛂 Source-of-Authority Audit

Triggered — the PR amends an accepted ADR and flips its lifecycle status.

• Amendment soundness: the §2.3/§2.4 rewrites resolve an internal contradiction (premature hint-persistence vs the finite-dockZone.v1 contract) and are declared amends ADR 0029 in the body. Sound.
• Blast-radius against live consumers: verified the diff touches only Status/§2.3/§2.4/Ledger — not §2.5 (core-lift) or §2.6 (container contract), which the just-graduated D#15173 and PR #15175 consume. No collision.
• Status flip: Proposed → Accepted is a documentation-accuracy correction (the ADR is live on dev; PR #14425 landed 2026-07-02), not an agent self-accepting an unmerged decision. Consistent with the ADR being consumed as binding authority. (If ADR-0005's lifecycle distinguishes "merged" from a separate ratification step, that's a one-line operator confirm — not a code/design gate.)

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 96c61d45 (Analyze, CodeQL, integration-unified, lint×5, lint-pr-body, unit, Classify). Author receipts: focused cross-window spec 10 passed; dashboard blast-radius 285 passed.
• Reviewer falsifier: N/A — I ran the one falsifier that mattered (the src/ reader sweep for the removed fields → none), which is the correctness crux; CI owns the rest.
• Test location: test/playwright/unit/dashboard/DockCrossWindowParticipation.spec.mjs (canonical).
• Substance: the replacement test asserts the record travels verbatim (toEqual(sourceRecord)), carries no owningWorkspaceId/fallbackTarget, both documents pass DockZoneModel.validate() (schema-finite), and captureTopologyPerspective([...]) succeeds — for both addTab and splitNode. It pins exactly the integration tripwire the loose fields used to fail; not a smoke test.

Findings: Pass.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: no ticket Contract-Ledger to drift against (bug fix; #15189 unsynced, Decision-Record-impact declared inline); no OpenAPI tool surface; no new cross-skill workflow primitive (this narrows an existing ADR clause + removes dead writes). Evidence, Source-of-Authority, and Test-Evidence handled above.

📋 Required Actions

No required actions — eligible for human merge.

One optional, non-blocking item (Depth Floor): an inline ADR amendment-note on §2.3/§2.4 would preserve the decision trail for future citers. Not tracked as a follow-up — it's ADR-hygiene on a clause being corrected as the ADR is accepted, and the trail already exists in the PR body + ledger + git.

📊 Evaluation Metrics

Verdict weights: 30% premise, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.

• [ARCH_ALIGNMENT]: 95 — resolves a real internal ADR contradiction (finite-schema vs premature hint-persistence), removes verified-write-only dead fields (net −12), defers speculative state to the tracked §2.2 layer, and leaves §2.5/§2.6 untouched (no collision with D#15173/#15175). −5: the in-ADR amendment trail is implicit (PR body + ledger + git) rather than an inline ADR note.
• [CONTENT_COMPLETENESS]: 92 — explicit amends ADR 0029 Decision-Record-impact, an Evolution that names the contradiction, Deltas, per-suite Test Evidence, and Post-Merge (the #14772 tripwire + rebase). −8: no inline ADR amendment note; ticket #15189 unsynced at review time.
• [EXECUTION_QUALITY]: 95 — write-only removal verified reader-safe across src/; ADR/code/test mutually consistent; the replacement test proves schema-finiteness + the capture tripwire for both placement shapes. −5: minor.
• [PRODUCTIVITY]: 95 — fixes the bug, corrects the ADR contradiction + status + doc echo, and correctly scopes the §2.2 hint layer as future work.
• [IMPACT]: 80 — amends an accepted governance ADR and restores the finite cross-window dockZone.v1 contract that topology-capture depends on.
• [COMPLEXITY]: 45 — small diff (+56/−44, 4 files); the weight is conceptual (finite-schema vs deferred hint layer), not volume.
• [EFFORT_PROFILE]: Maintenance — a targeted contradiction-fix + ADR correction; high governance/correctness value on a contained surface.

Cross-family review (Claude ⇄ GPT): this review is the cross-family merge-gate signal. No blocking defect; Approve. Eligible for @tobiu's merge once checks-of-record and reviewer slots are satisfied — merge stays human-owned (§critical_gates). Clean ADR correction, Emmy — refusing to persist not-yet-owned state in a finite schema is exactly right.