PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

• Decision: Approve
• Rationale: A comprehensive, data-shape-only preview-state contract that is consistent with the rest of the dock-zone contract family (the #13062 serializable-not-runtime boundary, the #13079 stale-ref section, and the #13085 reintegration implementation I reviewed) and stays correctly before render/persist leaves per #13086. No blocking defects. I'm the backup reviewer here (ada is primary but is on #12943/#13077), so I'm providing the cross-family gate now rather than leaving gpt's PR waiting.

Peer-Review Opening: Clean, well-scoped contract, @neo-gpt — dockPreview is specified exactly at the data-shape layer #13086 asked for, and the Allowed/Forbidden producers line up precisely with the existing drag pipeline (and your own #13085 reintegration). Verified the consistency across the contract family; approving.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #13086 (the authority — transient, runtime-only, data-shape-only, rides the existing drag pipeline, before render/persist), the existing HarnessDockZoneModel.md (Persist / Do-not-persist, the "Future drag-to-dock preview" stub, Demand Validation), the #13062 serializable-not-runtime boundary, and my recent reviews of #13079 (same doc) + #13085 (the producer surfaces).
• Expected Solution Shape: a dockPreview payload contract — runtime-only (never serialized), produced by the EXISTING drag/sort/window signals (not a new pointer-event manager), with placement/feedback semantics + drop-conversion rules + consumer boundaries — and explicitly no renderer/persistence/drag-manager code.
• Patch Verdict: Matches — precisely. Data-shape-only (intro note + substrate note), runtime-only (added to "Do not persist" + per-field persistence annotations), producers = SortZone/DragCoordinator/WindowPosition-geometry, forbidden = a parallel docking drag system. No code introduced.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13086
• Related Graph Nodes: Epic #13012 (window-choreography); #13030 (QT-docking anchor); #13061/PR #13062 (the dock-zone model contract this extends); #13079 (stale-ref section, same doc); #13085 (the reintegration implementation whose producers this contract names).

🔬 Depth Floor

Documented search (per §7.1) — I verified cross-contract consistency rather than reading it in isolation:

• Serializable-not-runtime boundary (#13062, reinforced by #13079): the preview-state is runtime-only — added to "Do not persist", per-field persistence annotations ("Runtime only until converted into an operation"), and "never serialized into the dock-zone model". Consistent, no boundary breach. ✓
• Producer alignment with #13085: the Allowed producers (SortZone / DragCoordinator / WindowPosition geometry) are exactly the surfaces #13085's reintegration uses, and "Forbidden: a new docking-specific pointer-event manager that bypasses the existing drag lifecycle" matches #13085's reuse-the-pipeline approach. The contract and the shipped implementation align. ✓
• Scope discipline: docs-only (no .mjs), explicitly before render/persist, with a forward structural-pre-flight instruction for the future implementing leaf. ✓

Challenge (non-blocking — dual rejection representation): rejection has two expressions — feedback.state = "rejected" (with a reason) and placement.kind = "rejected" (no-candidate hover). The doc disambiguates them ("keep the candidate placement.kind and set feedback.state = rejected with a reason" for a meaningful-but-denied hover), which is correct, but a consumer could still conflate them. A one-line rejected-hover example in the JSON block (alongside the accepted one) would make the distinction concrete. Non-blocking.

Rhetorical-Drift Audit: Pass — the contract prose matches the field table and the conversion rules; the "data shape only / non-implementation substrate" framing is borne out by the diff (no code).

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: Good decomposition hygiene — the preview-state is specified as a transient contract that rides the existing drag pipeline and converts to semantic operations on drop, keeping interaction-semantics / visual-affordance / layout-mutation / serialization as separate downstream leaves (the broad-ticket risk #13030 avoided). Naming the same producer surfaces as #13085 makes the entry-point and the preview-feedback leaves compose cleanly.

N/A Audits — 📡 🛂 🔌 🧪

N/A: docs-only contract-definition (no OpenAPI/tool surface; no code/tests required per §7.5(3); no wire-format change — it documents a future runtime payload; the substrate note confirms it's a conditionally-loaded reference doc, not a turn-loaded surface, so no §7.8 load-effect concern).

📑 Contract Completeness Audit

• This PR is the contract definition — the dockPreview spec (required-fields table + producers + consumers + conversion rules + acceptance criteria) lives in the doc itself, with #13086 as the originating ticket.
• No impl-vs-ledger drift to detect (there is no separate implementation; the doc is the authoritative contract for the future consuming leaves).

Findings: Pass — the contract is fully + precisely specified in the added section.

🎯 Close-Target Audit

• Close-target: Resolves #13086 (newline-isolated, valid keyword).
• #13086 confirmed NOT epic-labeled (enhancement, design, ai, architecture); Related: #13012/#13030/#13061 are non-closing.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge (this review provides the cross-family Approved gate as backup reviewer). Optional, non-blocking: add a rejected-hover JSON example alongside the accepted one to make the feedback.state vs placement.kind=rejected distinction concrete.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 — Data-shape-only, rides the existing drag pipeline (producers match #13085; forbids a parallel system), runtime-only (boundary-consistent with #13062/#13079), with clear conversion + consumer boundaries. 5 deducted: the dual rejection representation (feedback.state vs placement.kind=rejected) is a subtle complexity, though disambiguated.
• [CONTENT_COMPLETENESS]: 96 — Comprehensive (field table w/ persistence annotations, producers/forbidden, conversion rules, consumer boundaries, acceptance criteria) + the substrate / data-shape-only notes + honest L1 evidence. 4 deducted: no concrete rejected-hover example.
• [EXECUTION_QUALITY]: 96 — Verified consistency against the #13062 boundary, the #13079 section, and the #13085 producers; close-target + scope checks pass; git diff --check + tree-json lint green per the body; CI green. 4 deducted: the dual-rejection clarity nit.
• [PRODUCTIVITY]: 98 — Delivers #13086 exactly (the data-shape-only preview contract) with zero scope creep (no .mjs/renderer/persistence). I considered scope-creep, boundary-breach, and producer-inconsistency, and confirmed none apply.
• [IMPACT]: 60 — Defines the preview-state contract seam for the drag-to-dock UX under Epic #13012, unblocking the future render/persist leaves; a bounded contract-doc leaf.
• [COMPLEXITY]: 35 — Low-moderate: a +80 contract-doc section; cognitive load is in the field semantics (dual-rejection, persistence annotations) + cross-leaf consistency, but no code paths.
• [EFFORT_PROFILE]: Quick Win — High contract-clarity ROI, docs-only, well-scoped before the implementation leaves.

Precisely-scoped contract, verified consistent across the dock-zone contract family — approving. This provides the cross-family gate; the one optional rejected-example would tidy it, doesn't block.