PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The PR keeps the correct producer/reducer boundary and appears to close the destroyed-owner exception, but the exact-head primary user journey still performs a split. That is the close-target's defining behavioral AC, so it must be repaired in this PR rather than routed to a follow-up.

Peer-Review Opening: The teardown half is narrowly placed and the geometry idea is directionally coherent. The live gesture falsifies the carve-out predicate at this exact head, however, so this is not yet merge-eligible.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Issue #14913; the four-file changed-surface list; current dev versions of DockPreviewProducer, container/tab-header SortZone, and the existing producer spec; ADR 0029 §2.3; HarnessDockZoneModel.md's Drag Integration Boundary and Preview State Contract; MainContainer's existing onDockCrossZoneDrop reducer; and DockCrossZoneDragNL as the authoritative user journey.
• Expected Solution Shape: Header-strip intent should be classified inside the existing pure DockPreviewProducer and committed through previewToOperation; SortZone completion should become teardown-safe. The fix must not hardcode terminal-tabs or fork the drag pipeline, and test isolation must pin both actual rendered geometry and the live pointer journey on an isolated port.
• Patch Verdict: Contradicts the expected behavior at head e2888411d7d5d3ad3b5ea9c4491fe8548290d31b. The implementation disables the 36px carve-out when it is not smaller than the geometric edge band; the real Terminal tabs-node is 44px high, producing a 10.56px band, so the live path deliberately takes the non-carve-out branch and still emits a split.
• Premise Coherence: The boundary choice coheres with verify-before-assert and the no-parallel-drag invariant, but the PR body's L3 red→green claim conflicts with verify-before-assert because the exact-head L3 journey is red.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14913
• Related Graph Nodes: Parent #13158 · Related #14898 · ADR 0029 §2.3 · neo.harness.dockPreview.v1

🔬 Depth Floor

Challenge: The new unit case proves a 400×300 synthetic zone and explicitly preserves edge semantics for a 100×100 small zone, but the production Terminal tabs-node is 402×44. At that real size, band = 0.24 × 44 = 10.56 and tabHeaderCarveOutPx = 36, so tabHeaderCarveOutPx < band is false. The test therefore encodes the opposite branch from the user journey it is meant to protect.

Rhetorical-Drift Audit:

• PR description framing matches the diff's observable effect.
• Anchor & Echo summaries accurately describe the intended producer and teardown boundaries.
• No inflated retrospective tag is present.
• ADR and model-contract anchors establish the claimed preview → operation pipeline.

Findings: Failed. The PR body says DockCrossZoneDragNL is red→green and that L3 evidence is achieved; an independent exact-head run received tabs-strategy-0 instead of terminal-tabs.

🧠 Graph Ingestion Notes

• [KB_GAP]: None found; ADR 0029 §2.3 and HarnessDockZoneModel.md already state the boundary clearly.
• [TOOLING_GAP]: The repo-configured Chrome 150 channel aborted inside the Codex sandbox; the same exact-head test was rerun with the locally installed Playwright Chromium outside the sandbox and reached the product assertion.
• [RETROSPECTIVE]: Synthetic geometry was green while the real gesture stayed red. For pointer-class docking regressions, the same-PR live journey is the falsifier; a unit rect must be derived from, or at least include, the observed rendered scale.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: this bugfix does not change a public wire/schema contract, MCP description, workflow skill, or cross-substrate convention; it adjusts an existing producer threshold and teardown guards.

🎯 Close-Target Audit

• Close-target identified: #14913.
• Confirmed #14913 is not epic-labeled; it is an open bug/regression/testing leaf.

Findings: Pass. The PR body uses one newline-isolated Resolves #14913 target, and the commit carries no stale magic close keyword.

🪜 Evidence Audit

• PR body contains an Evidence: declaration.
• Achieved evidence meets the L3 user-journey requirement.
• No residual is declared.
• The body asks for the correct evidence class rather than promoting unit evidence to L3.

Findings: Evidence-AC mismatch. At exact head e2888411d7d5d3ad3b5ea9c4491fe8548290d31b, the isolated DockCrossZoneDragNL run failed: expected terminal-tabs, received tabs-strategy-0. The destroyed-owner moveTo TypeError did not recur in that run, so the blocker is specifically the still-wrong placement geometry.

🧪 Test-Execution & Location Audit

• Exact PR head checked out locally.
• Existing unit and E2E specs remain in canonical test/playwright/unit and test/playwright/e2e locations.
• Four related unit files ran on one worker: 29 passed.
• The primary DockCrossZoneDragNL journey passes.

Findings: Focused units pass, but the isolated exact-head whitebox journey fails its committed-model assertion. A geometry probe measured the target tabs-node at 402×44 and the Terminal header at y=108..129, directly falsifying the carve-out's small-zone gate.

📋 Required Actions

To proceed with merging, please address the following:

• Make header-strip tab intent win for the geometry the application actually renders, then pin that scale in the producer spec and rerun DockCrossZoneDragNL. Concretely, replace or reshape the tabHeaderCarveOutPx < band gate so a 44px-high tabs-node with a header-center drop resolves tab-into; the exact-head exit proof must move strategy into terminal-tabs. Refresh the PR's Evidence line only after that exact-head run is green.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 88 — the fix stays in the existing producer and SortZone consumer edges, preserves the dock-blind coordinator, and does not fork the drag pipeline; 12 deducted because the geometry rule is not aligned to the actual rendered target surface.
• [CONTENT_COMPLETENESS]: 82 — JSDoc and PR rationale explain both mechanisms, but the evidence statement overclaims a failing L3 journey and the new unit coverage omits the production 44px height.
• [EXECUTION_QUALITY]: 45 — twenty-nine related units pass and the moveTo TypeError was absent, but the defining exact-head E2E assertion fails and the original split behavior remains.
• [PRODUCTIVITY]: 50 — one of the two regression mechanisms appears closed; the primary tab-drop outcome required by #14913 is not.
• [IMPACT]: 78 — this restores or breaks the flagship human docking gesture on the verification surface, but the patch is scoped to one regression path rather than a new subsystem.
• [COMPLEXITY]: 48 — four files combine pure geometry classification with asynchronous teardown safety; the change is contained but crosses unit and live-gesture behavior.
• [EFFORT_PROFILE]: Maintenance — a focused regression repair on an established preview → operation pipeline, with medium risk because rendered geometry and async destruction meet at the gesture boundary.

The teardown guard can remain; the geometry half needs one evidence-bound correction at this head.