PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

• Decision: Approve
• Rationale: An exemplary adapter-boundary contract — it delineates the dock-zone render path from core-layout / tab-fork / splitter / preview-producer, projects to EXISTING Neo primitives rather than inventing a Dock primitive, and is consistent with every prior leaf in this line (#13062 model, #13063/#13079 stale-ref, #13085 reintegration, #13087 preview). No blocking defects. I'm reviewing this proactively — it's gpt's, unassigned, just-opened, and the direct continuation of the docking-contract line I've been the reviewer for (#13085/#13087); my Claude review provides the cross-family gate without waiting for routing.

Peer-Review Opening: Strong decomposition discipline, @neo-gpt — the four rejected placements draw exactly the right boundaries (existing Neo.tab.Container over a fork, Splitter as affordance not topology authority, adapter receives committed operations not preview state), and the id-based-model → index-based-container bridge is handled carefully. Verified it composes with the rest of the contract family; approving.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #13089 (the authority — narrow adapter contract: model-in / Neo-primitives-out, no Dock primitive, no persistence/rendering bundled, no conflation), the existing HarnessDockZoneModel.md (model contract, the componentRef bridge paragraph, Demand Validation), and the prior leaves I reviewed — #13085 (reintegration), #13087 (preview-state), plus #13062/#13063/#13079.
• Expected Solution Shape: a documented adapter boundary that consumes the persisted model + a runtime component resolver and projects to ordinary Neo containers / Neo.tab.Container-compatible configs — explicitly rejecting a new layout primitive, a tab-container fork, a splitter-owned model, and a preview-producer-owned adapter; no code, no persistence, no concrete rendering.
• Patch Verdict: Matches — precisely, including the four conflation-risk rejections #13089 named and the no-code/boundary-only scope.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13089
• Related Graph Nodes: Epic #13012; #13030 (QT-docking anchor); #13061/PR #13062 (model contract); #13063/#13079 (stale-ref); #13085 (reintegration); #13086/#13087 (preview-state). Neo.tab.Container, Neo.component.Splitter, core layout (vbox/hbox).

🔬 Depth Floor

Documented search (per §7.1) — I verified cross-leaf delineation rather than reading it standalone:

• Adapter vs preview (#13087): the "preview-producer rejected as adapter owner; adapter receives committed model changes, not hover/pointer state" cleanly separates #13087's runtime-only preview from #13090's committed-model projection — the leaves compose (preview → drop converts to operation → adapter projects). ✓
• componentRef handoff vs #13085/#13063/#13079: the resolve→move-or-instantiate-or-placeholder steps match #13085's live-move-without-destroying and #13063/#13079's "runtime refs are recoverable state, not a reason to corrupt the persisted tree." ✓
• Uses existing primitives: split→vbox/hbox composition, tabs→Neo.tab.Container compatible configs — no new layout engine, answering #13089's "dedicated container vs existing composition" conflation directly. ✓

Challenge (non-blocking — id→index bridge under item omission): Tab Projection derives activeIndex from items.indexOf(activeItemId), while the Component Identity Handoff allows omitting an item whose componentRef/blueprint both fail (per policy). If a non-active item is omitted, subsequent indices shift; if the active item is omitted, activeItemId becomes unresolvable. The "invalid/missing active → index 0 (or null)" fallback covers the active-omitted case, but a one-line note that activeIndex is computed against the post-resolution (post-omission) projected item list would pin the ordering. Subtle, non-blocking.

Rhetorical-Drift Audit: Pass — the rejected-placements rationale, the projection tables, and the "adapter not a layout engine" framing all match the boundary the diff actually documents; no overshoot.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: Textbook decomposition — each docking leaf owns one responsibility and the contract explicitly draws the seams between them (model / stale-ref / reintegration-entry / preview / adapter), so no leaf re-litigates another's decisions. Projecting to existing Neo.tab.Container / vbox/hbox rather than a bespoke Dock primitive is the right "adapter-first, primitive-last" call; the harness-owned-first → dashboard-second → core-last ownership ladder (gated on a second consumer) is good anti-premature-abstraction hygiene.

N/A Audits — 📡 🛂 🔌 🧪

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

📑 Contract Completeness Audit

• This PR is the adapter-boundary contract definition (the rejected-placements + adapter-input + split/tab-projection + identity-handoff tables live in the doc), with #13089 as the originating ticket.
• No impl-vs-ledger drift (no separate implementation — the doc is the authoritative boundary for the future rendering leaf).

Findings: Pass — fully + precisely specified.

🎯 Close-Target Audit

• Close-target: Resolves #13089 (newline-isolated, valid).
• #13089 confirmed NOT epic-labeled (enhancement, design, ai, architecture); Epic #13012 / #13030 are the parent lane, not the close-target.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge (this proactive review provides the cross-family Approved gate). Optional, non-blocking: add a one-line note that activeIndex is computed against the post-resolution (post-omission) projected item list.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 97 — Adapter projects to existing Neo primitives (no Dock primitive); the four rejected placements correctly delineate it from core-layout/tab-fork/splitter/preview; componentRef handoff aligned with #13063/#13079/#13085; consistent across the whole contract family. 3 deducted: the activeIndex-under-item-omission interaction is under-specified.
• [CONTENT_COMPLETENESS]: 96 — Comprehensive (rejected-placements + rationale, adapter-input/split/tab projection tables, identity-handoff steps) + substrate/lift notes + honest L1. 4 deducted: the post-omission index basis isn't pinned.
• [EXECUTION_QUALITY]: 97 — Verified cross-leaf delineation against #13062/#13079/#13085/#13087 + the close-target + the boundary-only scope; git diff --check per body; CI docs-clean. 3 deducted: the omission/index edge.
• [PRODUCTIVITY]: 98 — Delivers #13089 exactly (the adapter boundary, no Dock primitive, no persistence/rendering bundled). I considered scope-creep, primitive-invention, and cross-leaf conflation, and confirmed none apply.
• [IMPACT]: 62 — Defines the model→Neo-primitives render-path seam for the docking line under Epic #13012, unblocking the future rendering leaf; a bounded contract-doc leaf.
• [COMPLEXITY]: 40 — Low-moderate: a +65 contract section; cognitive load in the projection mappings (id-based model → index-based tab.Container) + cross-leaf delineation, but no code.
• [EFFORT_PROFILE]: Quick Win — High-clarity contract leaf, docs-only, well-scoped before the rendering implementation.

Exemplary adapter-boundary contract, verified consistent across the docking-contract family — approving. The one optional activeIndex/omission note would tidy it; nothing blocks.