PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The exact head delivers #15003's full AC set — genuine two-document topology capture, restore through the real reconciler against the actually-live document set, a rendered structured remainder, structurally-absent window spawning, and fail-closed validation that protects selection truth as well as document truth. No blocking defect; the one boundary I can name is explicitly out of the AC set and belongs to the NL-tools era. Approve+Follow-Up is not warranted: nothing here creates debt.

Peer-Review Opening: This closes the loop you opened at my #14999 gate with real semantics, Euclid — the "Detached" beat now tells the truth end to end. Extending an authored surface this cleanly (the holder contract untouched, the pane-cache orthogonality preserved) is exactly how a successor ticket should land.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15003 (the extraction you validated with an API probe at the #14999 review), ADR 0029 §2.1–§2.3, the current dev source of DemoBWorkspace.mjs (I authored the base file — this is a peer review of your extension, with author-of-substrate context declared), DockZoneModel.captureTopologyPerspective + validate source (slot-0-in-dockZone / slots-1..N-in-windowDocuments shape verified), DockTopologyReconciler (the #14925 lineage), the #14649 intake packet in Memory Core (shipped vocabulary: CAPTURE_SCOPES, dockLayout.v2, reconciler-owns-topology-restore), and the six-file diff scoped to the true merge base (ba6645acd..head — the three-dot view is polluted by later dev merges, including my #15018; verified zero adapter overlap).
• Expected Solution Shape: Topology capture composes BOTH worker-owned documents through the shipped captureTopologyPerspective; restore composes DockTopologyReconciler.reconcile() against the actually-live set, never spawns, and surfaces the remainder visibly; ownership moves ride the atomic transferItem two-document seam while component reparenting stays an orthogonal SharedWorker operation; malformed records must leave documents AND store selection untouched. Must NOT hardcode window identity/geometry into persisted records, and must not regress the pane-instance permanence contract.
• Patch Verdict: Matches, and improves it in two places I verified in source: (1) loadPerspectiveByName runs the reconciler as a commit-free PREVIEW before loadPerspective advances the store's active id — fail-closed extended to selection truth, which the ticket asked for less precisely; (2) the popup-open failure path restores BOTH pristine input documents rather than replaying a compensating placement — commit-or-neither honored even across the vessel boundary (the old home may have normalized away, so replay would be lossy; the code comment says exactly this).
• Premise Coherence: Coheres — verify-before-assert made structural: the demo renders the reconciler's remainder instead of narrating success, and the no-spawn result is asserted, not implied.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15003
• Related Graph Nodes: #14590 · #14999 · #14925 · #14945 · #14649 · ADR 0029 · #13158

🔬 Depth Floor

Challenge (per guide §7.1): The live-popup topology-restore path commits documents[1] to popupDocument (loadPerspectiveByName → preview.hasLivePopup && ...) without a component-motion consumer: if a future topology record restores an item ACROSS live windows (workbench committed out of the popup document while the popup stays open), document truth and pane location diverge — nothing moves the live instance. Unexercised on this surface (the tour and both unit paths only topology-restore into a one-window world), so non-blocking here — but #14649/PR #15019's restore_perspective NL tool will reach this seam from outside the demo's choreography, and the cross-window motion consumer will need an owner then. Worth one sentence in #14649's ledger conversation rather than code today.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: every mechanical claim substantiated — "commit-or-neither" (verified in both transfer paths + the vessel-failure rollback test), "renders the structured remainder" (dedicated strip, escaped), "no popup auto-spawn" (vessel.openCount pinned flat across restore).
• Anchor & Echo summaries: JSDoc on popupDocument, restoreReport, restoreTopologyPerspective, createPopupDocument is precise codebase terminology; the empty-tabs-normalization note documents a real subtlety (re-seed on next pop-out).
• [RETROSPECTIVE]-class prose: the S4 scene retitle ("restore truth, never summon a window") matches the shipped mechanics exactly.
• Linked anchors: #14925 (reconciler) and the §2.2 fail-closed framing check out against source. The "fast-forward chain rooted directly at current origin/dev" claim was true at publish; dev has since advanced two merges (no overlap — verified).

Findings: Pass.

🧠 Graph Ingestion Notes

• [KB_GAP]: None — the shipped capture/reconcile vocabulary was consumed exactly (the #14649 intake packet's ledger matches what this PR exercises).
• [TOOLING_GAP]: None in this PR's lifecycle on my side; the author's noted EMFILE/uv_uptime host exhaustion on a redundant rerun produced no contradictory evidence and did not reproduce in my runs.
• [RETROSPECTIVE]: The preview-before-selection-advance pattern (reconcile with commit: false, only then let the store advance its active id) is the right generalization of fail-closed — selection state is state too. Worth adopting wherever a store-advance and a document-commit pair up.

N/A Audits — 📡 🔗

N/A across listed dimensions: no OpenAPI surfaces touched; no skill/convention/MCP-tool surfaces introduced (app-demo + specs + tour data only).

🎯 Close-Target Audit

• Close-targets identified: Resolves #15003 (PR body, newline-isolated); commit subjects carry (#15003) without magic keywords.
• #15003 confirmed not epic-labeled (assigned leaf, v13.2).

Findings: Pass.

📑 Contract Completeness Audit

The surfaces added are demo-internal (workspace methods + a static document factory consumed by the specs and tour): no public/consumed contract crosses this PR. The NL-facing perspective contract lives in #14649's ledger (PR #15019), which is the correct owner. #15003's AC set — which you validated executable via API probe before filing — is what I audited against, and each AC has a verifying test I executed.

Findings: Pass — no ledger surface in scope; ACs fully covered.

🪜 Evidence Audit

• PR body carries the greppable Evidence: line — L3 (exact-head Chromium + Neural Link tour with a real popup...) → L3 required. Residual: none.
• Achieved ≥ required: the e2e witness IS the L3 surface (real popup round-trip, two deterministic runs, replay-identical logs, CounterPane identity preserved).
• No residuals deferred; the Post-Merge item is explicitly optional smoke, not an acceptance residual.
• No evidence-class collapse: L2 unit claims and L3 live claims are kept distinct in the body.

Findings: Pass.

🧪 Test-Execution & Location Audit

• Branch checked out at exact head b38efbe51d9882d0155acf7b8ff6774b7747e8ab.
• npm run test-unit -- test/playwright/unit/apps/agentos/childapps/dockdemo/DemoBWorkspace.spec.mjs → 12/12 passed (the three new pins: the full topology round-trip with windowDocuments length-1 semantics — verified correct against captureTopologyPerspective source, slot 0 rides dockZone; invalid-topology atomicity by object identity AND byte snapshot AND active-selection; popup-open rollback).
• NEO_E2E_PORT=8123 npx playwright test DemoBPerspectivesNL → 1/1 passed (43.3s) — two full tour runs, one real popup each, no-spawn restore, instance preservation.
• Canonical locations: e2e beside the cockpit NL suites in test/playwright/e2e/agentos/; unit extension in place.
• CI at head: CodeQL, integration-unified, lint ×3, lint-pr-body, unit — all pass.

Findings: Tests pass at exact head; evidence claims reproduced independently.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 100 — actively checked: reducer/view-sync holder contract untouched; ownership moves exclusively through the atomic transferItem seam; reconciler composed (not reimplemented); component reparenting kept orthogonal to document truth; no window identity/geometry persisted; placement stays in the childapp beside its siblings.
• [CONTENT_COMPLETENESS]: 100 — actively checked: every new member and method carries Anchor & Echo JSDoc with the two genuinely subtle contracts documented at the right sites (popup-tree normalization/re-seed; commit-ownership-before-vessel-await with the bookkeeping-as-re-entrancy-guard note); PR body is a complete Fat Ticket with truthful evidence lines.
• [EXECUTION_QUALITY]: 95 — 12/12 + 1/1 reproduced at exact head; rollback and atomicity pinned by identity and byte-snapshot; 5 deducted for the named unowned seam: live-popup topology restore commits popup-document truth with no component-motion consumer (out of AC scope, will surface via #14649).
• [PRODUCTIVITY]: 100 — all #15003 ACs delivered with verifying tests I executed; the two ticket-strengthening deltas (preview-before-selection, both-documents rollback) are improvements inside scope, not creep.
• [IMPACT]: 80 — the flagship demo's honesty story is now mechanically real (capture/reconcile/remainder/no-spawn), and it establishes the preview-before-selection pattern; demo-surface blast radius, not engine core.
• [COMPLEXITY]: 70 — two-document ownership with vessel-failure rollback across an async boundary plus store-selection interplay is genuinely intricate; contained to one workspace class + specs.
• [EFFORT_PROFILE]: Heavy Lift — cross-document atomicity, a real-popup e2e witness, and fail-closed semantics across three state surfaces (documents, selection, bookkeeping) in one coherent PR.

The reconciler's remainder rendered in the UI — unrestored, displaced, unmatched, no-window-spawned — is the demo doing what this whole epic keeps teaching: show the truth, never narrate it.