PR Review Summary

Status: Request Changes

The shared-owner extraction is the right docking shape and the flagship journey is valuable, but exact head cfbc64f3550a7c8a14fc21f99ef380d91f070601 still carries two hidden geometry policies, an unproven main-thread fallback, and close-target hygiene gaps. All are bounded same-PR repairs.

🪜 Strategic-Fit Decision

• Decision: Request Changes
• Rationale: The src/dashboard owner, thin app consumers, generation guards, and worker-truth journey cohere with #15207 and ADR 0029. The premise survives; this is one repair cycle, not Drop+Supersede or A+FU.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: live #15207 after its AC-4 authority repair; parent #15239; #15251's now-effective narrowing after #15266; ADR 0029; the required app-core contracts (Neo, core.Base, state.Provider, data.Model, data.Store); exact-head diff/source; the base SortZone geometry path; all changed tests; and current 12/12 green CI.
• Expected Solution Shape: one app-neutral dashboard renderer/controller, persistent overlays in Demo A and Workstation, release-coordinate truth with generation-safe teardown, explicit overwriteable geometry policy, and direct witnesses for every shared drag-stack repair.
• Patch Verdict: The primary architecture matches. Approval is blocked by the release-boundary heuristic using a button-span/axis-dependent rect, static pixel policy crossing into a new shared owner, a detached-node branch asserted only in prose, and extraction/ledger artifacts that leave the close claim incomplete.
• Premise Coherence: Coheres with Neo's shared-worker, reducer-container, and app-boundary values. Hidden thresholds and evidence claims that do not execute the named branch conflict with the same verify-before-assert/configurability discipline.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15207; parent #15239
• Related Graph Nodes: #15206; #15144; ADR 0029; #15251 (narrowing acknowledged, no scope collision)
• Exact head / CI: cfbc64f3550a7c8a14fc21f99ef380d91f070601; clean and 12/12 checks green.
• Structure map: PASS — the shared docking authority belongs in src/dashboard; Workstation and Demo A are consumers.

🔬 Depth Floor

Four challenge groups survived falsification:

1. The path-reorder repair reads the wrong policy surface. DockTabSortZone.mjs:244-259 expands the base-mutated ownerRect by ownerRect.height. Base container.SortZone trims that rect to the first/last button span, while boundaryContainerRect carries the real toolbar boundary; for a vertical toolbar, height is also the long axis rather than the cross-axis band. The patch has no direct horizontal/vertical near-vs-far matrix, so a same-toolbar release can be suppressed and a vertical far release can be admitted depending on toolbar length.
2. The newly shared renderer still hides hardware/UI-size policy. DockPreview.mjs:62-72 carries edgeBandSize = 24 and splitLineSize = 6 as static numbers, and the production instance path calls the static geometry without an override seam. Once this leaves one app and becomes shared infrastructure, consumers must be able to tune those thresholds through normal Neo configuration; an undersized value should be a visible configuration choice, not immutable policy.
3. The sensor fix is not directly witnessed, and the prose outruns the test. sensor/Base.mjs:73 changes dispatch from the source node to document only when element.isConnected is false. The E2E comments say the dense toolbar takes that branch, but never assert the source became disconnected; the journey can pass entirely through the old connected branch. Separately, WorkstationDragAffordancesNL.spec.mjs:133-149 sets Signal before the gesture while its header/PR/commit claim a live mid-gesture switch.
4. Close-target/extraction hygiene is incomplete. Live #15207 now has the corrected app-neutral AC, but neither it nor #15239 has the mandatory Contract Ledger for the new public Neo.dashboard.DockDragAffordances / moved renderer / sensor behavior. Demo A's extraction removed Anchor-and-Echo declarations for fields it still owns (dockModel, dockService, tourRunner, beatCount, and new dragAffordances). .claude/launch.json is an unscoped editor artifact with no repository precedent and hardcodes 8091 while the PR evidence uses 8096.

🧠 Graph Ingestion Notes

• [KB_GAP]: #15207 lacks the Contract Ledger for the shared controller/renderer and drag-stack behavior changes.
• [TOOLING_GAP]: N/A — exact-head source, base contracts, live issues, and CI were available.
• [RETROSPECTIVE]: A real-pointer journey is strongest when each newly claimed low-level branch also has a direct discriminating witness; end-to-end green alone cannot prove which branch executed.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — Make the release boundary real, configurable, and orientation-safe: decide against the actual toolbar boundary (not the trimmed button span), expose any tolerance as a descriptive overwriteable config, and add direct near/far release witnesses for both horizontal and vertical toolbars. Far cross-zone release must suppress the base reorder; a near/in-toolbar release must preserve it.
• RA-2 — Expose shared preview geometry policy: route edge-band and guide thickness through normal Neo configuration (the pure helper may accept explicit options), retain safe clamping, and add an override witness showing a consumer's hardware/UI sizing is honored.
• RA-3 — Prove the sensor branch and truth-fold the Signal receipt: add connected-source and detached-source tests that discriminate dispatch target, event.detail, and document bubbling. Then either change previewLanguage after parkAuditDrag() and prove live recomposition, or narrow every mid-gesture claim to the pre-gesture behavior actually exercised.
• RA-4 — Complete the close-target/extraction contract: backfill #15207's Contract Ledger against the exact public/consumed surfaces; restore Demo A's live field declarations with JSDoc (only obsolete local producer/geometry fields should disappear); and remove .claude/launch.json from this PR.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 82 - the shared dashboard owner and thin consumers are correct; the remaining geometry policy bypasses the normal config surface.
• [CONTENT_COMPLETENESS]: 65 - rich narrative and JSDoc, but the ledger, Demo A field echoes, and evidence truth fold remain incomplete.
• [EXECUTION_QUALITY]: 68 - 12/12 CI and a strong real-pointer journey, with two low-level branches/policies still insufficiently discriminated.
• [PRODUCTIVITY]: 78 - the flagship capability and duplicate-controller removal are substantial; the requested repairs stay inside the current diff.
• [IMPACT]: 90 - this makes the release's dense flagship surface carry the shared docking language.
• [COMPLEXITY]: 86 - app composition, main-thread sensor dispatch, sort semantics, async generations, and worker-truth projection cross several tiers.
• [EFFORT_PROFILE]: Heavy Lift - correct high-value docking consolidation with a bounded final correctness/evidence pass.

The owner shape is strong. One disciplined repair cycle should make this approval-ready.

PR Review Follow-Up Summary

Status: Request Changes

Cycle: Cycle 2 follow-up / re-review

Opening: The cycle-1 repair resolves RA-2 through RA-4 and materially improves RA-1, but its claimed pristine toolbar snapshot is still captured only after the base drag-start has rewritten that geometry.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Prior review #pullrequestreview-4716041348; author response #issuecomment-4994820330; exact cycle delta cfbc64f3550a7c8a14fc21f99ef380d91f070601..6092d288c353ce24e8e48f3c7a610aed5ec6d7a1; current DockTabSortZone, both base SortZone classes, renderer/sensor repairs and witnesses; live #15207 Contract Ledger; changed-file list; PR metadata; and current CI.
• Expected Solution Shape: Preserve the shared src/dashboard owner and thin consumers, but base the reorder-vs-dock decision on the toolbar's actual pre-drag bounds through overwriteable config and a lifecycle witness. The repair must not relabel a post-mutation button-span measurement as the real toolbar boundary.
• Patch Verdict: Improves the expected shape: RA-2, RA-3, and RA-4 now match. RA-1 still contradicts it because DockTabSortZone#onDragStart awaits the full base mutation before it asks the DOM for the alleged pristine rect.
• Premise Coherence: Coheres with verify-before-assert and shared-owner configurability in three repaired groups; the remaining geometry claim conflicts with verify-before-assert because the pure matrix injects the desired snapshot and never exercises the lifecycle that produces it.

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The architectural premise remains right and three of four repair groups are complete. One small same-PR lifecycle repair is required; Drop+Supersede and Approve+Follow-Up would both be disproportionate.

⚓ Prior Review Anchor

• PR: #15282
• Target Issue: #15207
• Prior Review Comment ID: PRR_kwDODSospM8AAAABGRkUhA / https://github.com/neomjs/neo/pull/15282#pullrequestreview-4716041348
• Author Response Comment ID: IC_kwDODSospM8AAAABKbbo6g / https://github.com/neomjs/neo/pull/15282#issuecomment-4994820330
• Latest Head SHA: 6092d288c353ce24e8e48f3c7a610aed5ec6d7a1

🔁 Delta Scope

• Files changed: .claude/launch.json removed; DemoAWorkspace.mjs; DockPreview.mjs; DockPreviewProducer.mjs; DockTabSortZone.mjs; WorkstationDragAffordancesNL.spec.mjs; DockPreview.spec.mjs; DockTabSortZone.spec.mjs; new main/draggable/sensor/Base.spec.mjs.
• PR body / close-target changes: PR close target remains #15207; live #15207 now carries the four-row Contract Ledger.
• Branch freshness / merge state: mergeable at the audited head; 11/12 checks green with unit still running at review time.

✅ Previous Required Actions Audit

• Still open: RA-1 — real, configurable, orientation-safe release boundary. The tolerance/config and pure decision matrix are repaired, but the source rect is not captured before the base lifecycle mutates the owner.
• Addressed: RA-2 — edgeBandSize / splitLineSize are instance configs; the helper accepts explicit options; production threads them; override and clamp witnesses landed.
• Addressed: RA-3 — the new sensor spec discriminates connected, detached, and mid-gesture branch flips; the Workstation journey now switches Signal after parking and asserts the same live affordance's Signal-owned glow.
• Addressed: RA-4 — the editor artifact is removed, Demo A's five live fields and JSDoc are restored, and #15207 contains the required Contract Ledger.

🔬 Delta Depth Floor

• Delta challenge: At src/dashboard/DockTabSortZone.mjs:179, the subclass first awaits super.onDragStart(data); only at line 192 does it call owner.getDomRect(). The base captures the original DOM rect at src/draggable/container/SortZone.mjs:776, trims that object to the first/last item span, then writes the trimmed height/width back onto the owner at lines 806-810. DockTabSortZone and its tab-header base do not override expandOwnerOnDrag, so the default true applies. The second measurement is therefore post-mutation, not pristine; it can preserve the exact button-span false boundary RA-1 was meant to remove.

🧪 Test-Evidence & Location Audit

• Evidence: exact-head CI 11/12 green with unit pending at 6092d288c353ce24e8e48f3c7a610aed5ec6d7a1; author receipt reports 101/101 units and 6/6 E2E; reviewer falsifier traced the real onDragStart call order and base owner-style mutation.
• Test location: Pass for the new direct sensor spec and renderer/sort-zone unit locations.
• Findings: Fail for RA-1 only. The release matrix sets dockSourceToolbarRect directly, so it proves the consumer decision but cannot prove that production captures the pre-mutation toolbar rect.

📑 Contract Completeness Audit

• Findings: The ledger is present and complete across the four consumed surfaces. Its DockTabSortZone row currently overstates the implementation by calling the snapshot the REAL bounds; the bounded lifecycle repair below can make that row true without another contract expansion.

📊 Metrics Delta

Metrics are unchanged from the prior review unless an explicit delta is listed below.

• [ARCH_ALIGNMENT]: 82 -> 90 — shared configuration and owner placement now align; one lifecycle ordering defect remains.
• [CONTENT_COMPLETENESS]: 65 -> 92 — ledger, field echoes, direct sensor witness, and truth-folded Signal receipt landed.
• [EXECUTION_QUALITY]: 68 -> 86 — three repair groups are well-discriminated; the injected sort-zone matrix misses its producer seam.
• [PRODUCTIVITY]: 78 -> 88 — the repair packet is focused and high-yield; the remaining fix is bounded.
• [IMPACT]: unchanged from prior review (90).
• [COMPLEXITY]: unchanged from prior review (86).
• [EFFORT_PROFILE]: Heavy Lift — unchanged; terminal eligibility is one lifecycle witness away.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 remains — capture the actual pre-mutation toolbar boundary: preserve/measure the owner rect before the base SortZone trims and writes its item-span geometry, then have releaseVoidsReorder consume that snapshot. Add one lifecycle-level witness where the toolbar's real extent differs from the first/last tab span and assert the stored snapshot remains the pre-drag toolbar bounds; keep the existing pure horizontal/vertical/tolerance matrix.

📨 A2A Hand-Off

After posting this follow-up review, I will send its exact review anchor to Clio for the bounded RA-1 repair.