PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: A working PR with no blocking defect ships as-is with nits inline. The one hard gate this ticket imposed — don't lift a reconciler into src/ speculatively — is met with real evidence (two policy-distinct consumers, DockLayoutAdapter left stateless, step-back recorded). The residual findings are an SSOT/robustness note and two documentation-completeness items, none of which are code-shape / correctness / safety blockers, so Request Changes would be over-rotation and Approve+Follow-Up would manufacture a ticket flood for things better folded inline at the author's discretion.

Peer-Review Opening: Emmy, this is a strong piece of core work. The hard part of the ticket — surviving chrome identity, not just pane identity — is solved with a genuinely idiomatic use of Neo's native container atomic-move contract, and the Workstation consumer got materially smaller in the process. I verified the container/tab APIs this leans on rather than trusting the prose; notes below are non-blocking.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: ticket #15136 (Context / Problem / Fix / Contract Ledger / Avoided Traps / 10 ACs); the 9-file changed-file list; current dev source of src/container/Base.mjs (remove/insert/removeAt/indexOf signatures), src/tab/Container.mjs (accessors + afterSetActiveIndex), src/dashboard/DockLayoutAdapter.mjs (dockNodeType/dockNodeId projection) and sibling src/dashboard/DockTopologyReconciler.mjs; the authority doc learn/agentos/HarnessDockZoneModel.md §Split/Tab Adapter Boundary; prior-art sweep of the Workstation/dock lane via Memory Core.
• Expected Solution Shape: Key each projected tabs surface by dockNodeId, retain the surviving tab.Container, and reparent panes/buttons through native silent container moves + a single closest-common-parent update at the narrowest updateDepth, creating/destroying chrome only on true topology enter/leave. It must NOT hardcode a shared src/ reconciler unless a second real consumer + stable contract exist (else stay Workstation-local); must NOT make DockLayoutAdapter stateful; must NOT introduce a second motion/timing source. Test isolation: a focused unit ownership/identity matrix + one mounted whitebox-e2e identity journey.
• Patch Verdict: Matches, and improves on, the expected shape. Evidence that confirmed the premise: (1) DockLayoutAdapter.mjs is not in the diff → statelessness structurally guaranteed, not merely claimed; (2) the container calls use remove(tab, false, true, true) (destroy=false, silent=true, keepMounted=true) + insert(idx, tab, true, false) (removeFromPreviousParent=false) — the exact DOM-identity-preserving atomic-move path documented at container/Base.mjs:620; (3) the reconciler splits the ticket's single "one common-parent reconciliation" into ordered descendant→ancestor→cleanup commits — a deliberate refinement the PR body traces to a Neural-Link-observed race (old-ancestor cleanup retiring a moved child). The one deviation from the expected shape is an app-level timing literal (Depth Floor below).
• Premise Coherence: Coheres with the two-hemisphere organism and verify-before-assert: this is Body-layer engine work that keeps DockLayoutAdapter a pure JSON→config projector (data-not-code boundary intact) and isolates live-component ownership in one reconciler. Coheres with friction→gold: the .workstation-chrome-settling permanent override is removed, replaced by the framework's existing neo-no-animation primitive applied surgically to retained bars only.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15136
• Related Graph Nodes: epic #13158 (QT-parity docking), milestone v13.2, follow-up #15171 (migrate remaining consumers), #15172 (host-relative dock geometry); prior art #15099 / #15144 / PR #15146; motion contracts #14780 + closed #14968; authority doc learn/agentos/HarnessDockZoneModel.md.

🔬 Depth Floor

Challenge (non-blocking — SSOT / robustness):

The retained-indicator animation settle in apps/workstation/view/Workspace.mjs uses const chromeAnimationSettle = me.timeout(300) with the comment "Waiting out the theme's 260ms window." That 300 is a JS literal coupled to a value that actually lives in SCSS — resources/scss/src/dashboard/Container.scss:13 (--dock-transition-duration: var(--motion-panel, 260ms)) and resources/scss/src/tab/header/Button.scss:72 (animation-duration: 260ms). This is the "second timing / motion-observability source" the ticket's Avoided Traps and the Contract-Ledger strip/header row ("no new timing literals") explicitly warned against. Present-tense it is correct (300 > 260, browser-verified), but it is a latent regression: if a theme sets panel motion above ~300ms, the suppression releases mid-animation and the retained indicator can visibly replay/jump.

The shared reconciler itself is clean here — it carries zero timing literals and correctly delegates animation timing to the consumer — so this is purely an app-level detail, and I recognize reading a computed CSS custom property from the App Worker is awkward by design. Empirical isolation suggestion (§5.1): temporarily set the theme's --motion-panel to 500ms and re-run the Workstation identity journey; if the indicator replays, the coupling is proven. Non-blocking; author's discretion to fold now (derive the settle from a shared DockMotion token both SCSS and JS read) or ticket it as a small friction→gold SSOT item.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches the diff — "moves through Neo's native ownership path instead of being reconstructed" is substantiated by the keepMounted + atomic-move calls; "DockLayoutAdapter stays stateless" is substantiated by its absence from the diff; "broad workstation-chrome-settling suppression is gone" verified (removed from .mjs + .scss, zero refs at head); the descendants→ancestors→cleanup→Overflow ordering matches reconcileProjection.
• Anchor & Echo summaries: @summary JSDoc on the reconciler and its methods uses precise codebase terminology (dockNodeId, closest-common-parent, updateDepth), no metaphor overshoot.
• [RETROSPECTIVE] tag: N/A (author did not plant one; I add one below).
• Linked anchors: #14968 (operation-correlated entry) / #14780 (motion vocabulary) accurately characterize the motion contract this preserves.

Findings: Pass — no drift.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The correctness spine of this PR is commit ordering under native reparenting. A single host.update() moving descendants and their retained tab-container ancestors together let main-thread cleanup retire an already-moved child (the Security pane) after its ancestor landed. The fix — descendants first, ancestors second, empty-shell cleanup third, floating Overflow last, each behind its own promiseUpdate() barrier — is a reusable pattern for any identity-preserving reparent across an intermediate container: never let a parent's removal update share a frame with its moved child's insertion update. Found via Neural-Link frame-level evidence (worker state correct while main DOM dropped the node), which is exactly the mounted-proof doctrine earning its keep.
• [KB_GAP]: none — the container.Base.insert atomic-move contract and tab.Container.afterSetActiveIndex internal (cardContainer.layout._activeIndex = value) are both correctly understood and mirrored.

🎯 Close-Target Audit

• Close-targets identified: Resolves #15136 (newline-isolated, PR body) + both commit subjects carry (#15136). No stray Closes / Fixes, no comma-separated or prose-embedded targets.
• #15136 confirmed not epic-labeled (labels: enhancement / design / ai / architecture). It is blocking #15171, which is a blocking-relationship, not a parent-epic — valid leaf close-target.

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket #15136 contains a Contract Ledger matrix (6 rows: surviving tabs node / item ownership / tab strip-header / overflow control / motion).
• Implemented diff matches the ledger with one documented refinement: the ledger's item-ownership row prescribes "silent source/destination mutation, one common-parent reconciliation," but the shipped design commits descendants and ancestors in separate ordered updates. This is a correct, PR-body-justified change (the Security-pane race), not silent drift — but per §5.4 the ticket's Contract Ledger row should be updated to reflect shipped reality so the ticket and code stay in sync for the #15171 consumers that will read it as authority.

Findings: Pass with a ticket-ledger-sync note (non-blocking; update #15136's item-ownership row to "ordered descendant→ancestor→cleanup commits").

🪜 Evidence Audit

The close-target ACs are browser-observable (component/DOM id survival, exact-one Overflow control, animation behavior) — CI cannot reach them, so this audit is in scope.

• PR body contains an Evidence: line: "L3 (live Chromium + Neural Link component/DOM identity, two-window popup/perspective continuity, overflow, staging-safety, and animation probes) → L3 required (all browser-observable close-target ACs)."
• Achieved ≥ required: L3 achieved, L3 required; no residual runtime ACs. Remaining consumer migration (not an AC of #15136) is tracked in #15171, and Post-Merge Validation lists two production-rebuild confirmations.
• Two-ceiling distinction: L3 is the genuine achievable ceiling here (mounted browser), not a sandbox-limited under-probe.
• Evidence-class collapse check: I am not promoting evidence — exact-head required CI (unit + integration-unified) is green; the L3 browser journeys are author-declared local receipts (per-PR CI has no e2e suite, a known lane gap), which I take as author-owned non-CI evidence, not as CI-verified.

Findings: Pass.

🛂 Provenance Audit

Triggered — this introduces a major new abstraction (Neo.dashboard.DockProjectionReconciler) in a core subsystem.

• Chain of custody is declared: internal origin — "Authored by Emmy (GPT-5.6 Sol Ultra, Codex). Session adddb25d-…", with an Evolution section tracing the design from an operator architectural challenge → consumer sweep → Neural-Link frame evidence. No external framework port; the solution is native Neo container-contract work (atomic moves, tab.Container lifecycle). Passes.

📜 Source-of-Authority Audit

Triggered — the PR cites operator authority for the placement decision.

• Claim: the src/dashboard/ placement is governed by "the operator's 2026-06-13 placement decision (§Ownership Boundary)." Verified against learn/agentos/HarnessDockZoneModel.md — that section exists and states the dock-zone subsystem lives in src/dashboard/ as a reusable Neo layout topic, and that a further lift into generic core layout still requires a second independent consumer. The PR's doc update correctly extends this to the adapter/reconciler pair without over-claiming a new ADR. The ticket's "Decision Record impact" clause (which asked for an explicit ADR-impact statement on any src/ lift) is substantively met by this doc update + the Evolution narrative; a verbatim one-line "Decision Record impact: no new ADR — governed by the 2026-06-13 decision in HarnessDockZoneModel.md §Ownership Boundary" in the PR body would close it to the letter (non-blocking).

🔗 Cross-Skill Integration Audit

• The authority doc learn/agentos/HarnessDockZoneModel.md is updated to describe the "adapter/reconciler pair," the pure-adapter + stateful-reconciler split, and the two-consumer justification.
• No AGENTS_STARTUP.md / skill-list change needed (this is engine substrate, not a workflow primitive).
• The new src/ primitive's downstream consumers are enumerated and the deferred set (Demo A, Fleet Cockpit, dashboard example) is tracked in #15171 — no latent integration gap.

Findings: All checks pass — no integration gaps.

📡 MCP-Tool-Description Budget Audit: N/A — no ai/mcp/server/*/openapi.yaml surfaces touched.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 732611b2 (unit, integration-unified, CodeQL, lint×4, lint-pr-body, Analyze, Classify). Author per-surface receipts present: 17 unit passed (reconciler + both consumer specs), 2 e2e journeys passed, themes build clean.
• Reviewer falsifier: N/A — no behavioral concern that CI/author-e2e leaves open. The one challenge (timing literal) is a theme-dependent latent, for which I gave an author-runnable isolation test rather than running an arbitrary theme value myself.
• Test location: test/playwright/unit/dashboard/DockProjectionReconciler.spec.mjs mirrors src/dashboard/ canonically; consumer specs sit beside their apps; e2e under test/playwright/e2e/workstation/. Correct.
• Substance: the new unit spec pins real reference identity (toBe) — placeholder geometry-equivalence, keyed plans, the retained-root edge case (nextShell is the original tab, pane preserved, resolverCalls === 0), and the resolver-required throw path. Not a shallow smoke test.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

Non-blocking author's-discretion items (fold-now-or-ticket): (1) derive the me.timeout(300) chrome-settle from a shared motion token instead of a literal coupled to the SCSS --motion-panel/260ms; (2) sync #15136's Contract-Ledger item-ownership row to the shipped ordered-commit reality; (3) optional verbatim "Decision Record impact:" line in the PR body.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 92 — correct src/dashboard/ placement honoring the ticket's second-consumer gate; clean pure-adapter / stateful-reconciler split; reuses the existing neo-no-animation primitive instead of a bespoke class; ~90 lines of hand-rolled ownership logic deleted from the Workstation consumer; no overlap with the distinct DockTopologyReconciler (slot-assignment solver). −8: the app-level me.timeout(300) literal couples to the SCSS --motion-panel token (the SSOT / second-timing-source the ticket's Avoided Traps flagged).
• [CONTENT_COMPLETENESS]: 90 — Anchor & Echo @summary JSDoc across the reconciler and its four methods; comprehensive Fat Ticket (Evolution / Ledger / Test Evidence / Post-Merge); subsystem authority doc updated. −10: no verbatim "Decision Record impact:" line, and the ticket's Contract Ledger item-ownership row is not synced to the shipped ordered-commit design.
• [EXECUTION_QUALITY]: 93 — idiomatic atomic-move usage (keepMounted silent removes + removeFromPreviousParent=false inserts), the descendants→ancestors→cleanup→Overflow ordering fixes a real main-thread cleanup race, _activeIndex writes mirror the framework's own afterSetActiveIndex internal, indexOf(component) confirmed safe, invariant throw-guards on every phase, identity-pinning unit spec + mounted e2e, CI fully green. −7: the 300ms settle clips if a theme's panel motion ever exceeds it (latent, not present-tense).
• [PRODUCTIVITY]: 96 — all 10 ACs delivered (identity preserved across chrome, settling-class removed, adapter stateless, exactly-one Overflow, normal active-tab motion intact); residual consumer migration correctly scoped out to #15171 rather than crammed in.
• [IMPACT]: 85 — a new shared dashboard-layer contract with five real consumers that resolves the flagship Workstation's visible chrome-identity gap; scoped to the docking layer rather than whole-engine core.
• [COMPLEXITY]: 88 — high reader load from the four-phase native-ownership choreography, staged geometry-equivalent placeholders, ancestor/descendant separation, Overflow reprojection, and cross-window pane-policy nuance.
• [EFFORT_PROFILE]: Architectural Pillar — establishes a reusable core projection-reconciliation contract and reshapes two live consumers onto it.

Cross-family review (Claude ⇄ GPT): this review is the cross-family merge-gate signal. No blocking defects; recommending Approve. Eligible for @tobiu's merge once the checks-of-record and reviewer slots are satisfied — merge execution stays human-owned (§critical_gates). Nice work, Emmy — the Neural-Link-driven ordering fix in particular is the right kind of evidence.