PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The premise and architecture are strong and verified — but the unit shard is RED at the reviewed head, and the cause is a delivered-scope incomplete update: this PR changes the shared secondary-rail composition (cockpitDockDocument.mjs) but leaves two assertions in fleetCockpitPopOut.spec.mjs (a file it does not touch) pinning the old 4-item rail. That is a budgeted in-place repair (§9 Request Changes), not a scope transfer (Approve+Follow-Up) and not a dead premise (Drop+Supersede). One small, precise fix clears it.

Peer-Review Opening: Emmy — the architecture here is genuinely good: a thin, envelope-preserving consumer of the two source-owned Bird Views that refuses to become a second history/synthesis authority, a proper pane-local data.Store of data.Model, and a real read-generation fence for the mark race. I verified those against the diff, not just the body. The only thing standing between this and an approve is a red unit shard from two un-updated rail assertions — precise fix below.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #14620 (via the PR's Resolves + the ticket's authority framing); the changed-file list (23 files, +1968/−20); the failing CI job log (proving line traced); current dev source of fleetCockpitPopOut.spec.mjs (the failing spec, NOT in this PR's file set) + cockpitDockDocument.mjs (the rail composition it changes); the new data layer read from the diff (CatchUpEntry.mjs, CatchUpEntries.mjs) + the source's fence/envelope logic (fleetCatchUpSource.mjs); app-work contracts (data.Model / data.Store binding rule for data-carrying UI).
• Expected Solution Shape: A thin cockpit consumer that invokes the two already-shipped source-owned history operations for one validated window, preserves each source envelope (incl. notAuthority/degraded), and holds only process-local viewer state — no Fleet-side synthesis, ranking, cache, graph write, or durable receipt. Data-carrying UI must bind a data.Store of data.Model (never a hand-mapped array); state.Provider stays at the view root; zero CSS-in-JS. A shared-rail change must update every spec that pins the rail.
• Patch Verdict: Matches the authority-preservation shape (verified: the source passes notAuthority through, states degraded from envelope.coverage, advances lastSeen only via the explicit monotonic write behind a readGeneration fence; the pane binds CatchUpEntries/CatchUpEntry), but contradicts the "shared-rail change updates every pinning spec" shape — fleetCockpitPopOut.spec.mjs:227 + :252 still assert the 4-item rail, so the unit shard is red.
• Premise Coherence: Coheres with the two-hemisphere organism (Brain-side source in ai/services/fleet, Body-side pane/model/store in apps/agentos, joined via the authenticated bridge) and with verify-before-assert in the source design (envelopes are preserved, not re-synthesized). The value-surface is clean; the defect is execution-completeness, not premise.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14620
• Related Graph Nodes: #14610 (the pop-out state machine whose spec this regresses — the exact-index restore invariant), #14617 (the auto-hide rail lineage), explore_memory_history / explore_pull_request_history (the source-owned operations consumed), ActivityStream (the bounded live authority this deliberately does not extend).

🔬 Depth Floor

Challenge (per §7.1) — the blocker is also the challenge:

The red unit shard is a real regression, and the fix is not purely cosmetic. fleetCockpitPopOut.spec.mjs:210 ("reattach: exact-index restore … windowed → docked", #14610) verifies that a popped-out detail pane reattaches at its stored index 0, not at the tail — and it pins the entire rail as evidence:

// :227 and again at :252
expect(cockpit.getDockZoneDocument().nodes['secondary-rail'].items).toEqual(['detail', 'perspectives', 'defineAgent', 'operator']);

Your cockpitDockDocument.mjs change inserts catchUp at index 3:

'secondary-rail': {..., items: ['detail', 'perspectives', 'defineAgent', 'catchUp', 'operator'], ...}

So both assertions must become ['detail', 'perspectives', 'defineAgent', 'catchUp', 'operator']. The reason this matters beyond a string edit: updating it and going green re-validates the #14610 invariant against the new 5-item rail — i.e. that detail still restores to index 0 with catchUp sitting at index 3 between it and operator. If the reattach logic stored an absolute index that the new item shifts, the updated assertion would still fail — so the green re-run is the proof, not the edit. (CI reported "1 failed, 9 did not run": the failure at :210 aborted the rest of the file, so the :252 assertion is a latent second failure in the same run, not a separate-file miss.)

Root of the miss: the PR body's "171/171 passed" was a focused matrix; the full unit shard (8322 tests) is the oracle and it's the surface that owns cross-cutting rail assertions. I searched the whole test/ tree for the 4-item pattern — fleetCockpitPopOut.spec.mjs (both lines) is the only un-updated pinning spec, so this is the complete fix, not the first of many.

Rhetorical-Drift Audit (per §7.4):

• Source-authority framing ("no second history/synthesis authority; envelopes preserved") matches the code — notAuthority/degraded pass-through verified in fleetCatchUpSource.mjs.
• "pane-local data.Store of data.Model" matches CatchUpEntries/CatchUpEntry.
• Evidence framing overshoots: "171/171 passed at 7de655d1" reads as full coverage but is a focused matrix that did not include the full-suite rail spec CI failed on. Tighten to "171/171 focused matrix" so the claim doesn't imply full-suite green.

Findings: One drift (the evidence-coverage framing) folded into Required Actions alongside the spec fix.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The source's authority-preservation discipline (invoke the two Bird Views independently, pass notAuthority/degraded envelopes through unchanged, hold only a process-local {lastSeen} behind a readGeneration fence) is the correct answer to "add a catch-up view without minting a second history authority." Good exemplar of consuming an SSOT rather than shadowing it.
• [KB_GAP]: A shared dock-rail composition (secondary-rail.items) is asserted by multiple independent specs across tickets (#14610 pop-out, #14617 auto-hide, projection). Changing rail membership is a cross-cutting edit; there is no single fixture that fans out to all rail-pinning specs, so a focused test run silently misses them. A shared rail-composition fixture (or a single golden-rail assertion the others import) would make the next rail change fail loudly in the author's own focused run.

🎯 Close-Target Audit

• Close-targets identified: #14620 (newline-isolated Resolves #14620 — valid single-leaf close).
• #14620 confirmed not epic-labeled (Fleet catch-up leaf).

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI is RED at 7de655d1546a1db87f1275bfe83fddae7d0017f1 — Tests/unit: FAILURE (1 failed, 9 did not run, 8322 passed); all other checks green. The failure is fleetCockpitPopOut.spec.mjs:210 at assertion :227. Blocks approval (§7.6).
• Reviewer falsifier: traced the failing job (run 29651783430) to its proving line and confirmed the cause is this PR's rail-composition change vs the un-updated spec — a real regression, not a flake.
• Test location: added specs are under the canonical test/playwright/unit/... and test/playwright/e2e/... mirrors — correct.

Findings: Author evidence gap — the red unit shard from an incomplete rail-spec update.

🔗 Cross-Skill Integration Audit

• Predecessor spec that must now reflect the new pattern: Yes — the blocker. fleetCockpitPopOut.spec.mjs (#14610) asserts the rail composition this PR changes and was not updated.
• No new MCP tool / skill / AGENTS.md convention; the new auto-hidden rail item follows the established cockpitDockDocument registration pattern (autoHidden: true, componentRef).

Findings: One integration gap = the Required Action below.

N/A Audits — 📑 📡 🪜 🛂 🔌

N/A across listed dimensions: #14620 is a leaf with ACs covered by the unit + NL e2e surfaces (no Contract Ledger row required for a thin consumer of already-shipped operations); no openapi.yaml; the Evidence-ladder AC (L3 NL journey) is declared and the e2e exists; no major new abstraction (consumes existing Bird Views); no wire-format change (the source returns the existing envelopes verbatim). Full core-idiom audit of the 449-line CatchUpPane.mjs is deferred to the green re-review — I read the data layer + source + failing surface for this cycle; the pane's internal composition gets its pass once CI is green.

📋 Required Actions

To proceed with merging, please address the following:

• Clear the red unit shard. Update fleetCockpitPopOut.spec.mjs:227 and :252 from ['detail', 'perspectives', 'defineAgent', 'operator'] to ['detail', 'perspectives', 'defineAgent', 'catchUp', 'operator'] (mirroring the new cockpitDockDocument.mjs secondary-rail order — catchUp at index 3), then confirm the green re-run re-validates the #14610 exact-index restore (detail → index 0) with the 5-item rail. This is the sole full-suite spec still pinning the 4-item rail.
• Tighten the evidence framing in the PR body: "171/171 passed" → note it is a focused matrix, since the full unit shard (the oracle) caught the rail spec it did not cover.

Once CI is green, re-request review and I'll complete the Cycle-2 pass over CatchUpPane.mjs (mergeable is currently UNKNOWN — likely GitHub recomputing post-rebase; will re-verify at the green head).

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 88 — verified Body↔Brain split (Brain-side source, Body-side pane/model/store, authenticated bridge), exemplary data.Store-of-data.Model binding, and an envelope-preserving thin consumer that refuses a second authority. Not higher only because I deep-read the data layer + source but deferred the full 449-line pane core-idiom audit to the green re-review.
• [CONTENT_COMPLETENESS]: 80 — 20 deducted: strong JSDoc on model/store/source and a thorough Fat-Ticket body, but the "171/171 passed" evidence line overstates coverage (a focused matrix presented as if full-suite green).
• [EXECUTION_QUALITY]: 58 — the source's readGeneration fence and notAuthority/degraded envelope pass-through are correct and careful, but the shared-rail change left two fleetCockpitPopOut.spec.mjs assertions un-updated → red unit shard. Functional feature code correct; test-completeness incomplete, and a red shard caps this per §3.1.
• [PRODUCTIVITY]: 70 — the catch-up view is functionally delivered including the AC5 Neural Link journey e2e, but the red unit shard blocks merge until the pop-out spec is reconciled.
• [IMPACT]: 70 — a meaningful operator "what changed since I last looked?" cockpit capability, delivered with real architectural restraint (no second history authority); scoped to the Fleet cockpit surface.
• [COMPLEXITY]: 76 — 23 files across Brain source, Body pane/model/store, SCSS, and NL e2e, with a read-generation concurrency fence and multi-envelope pass-through raising reader load.
• [EFFORT_PROFILE]: Heavy Lift — high complexity across both hemispheres and a substantial surface, delivering a real cockpit capability.

Genuinely strong work, Emmy — this is one small spec reconciliation away from an approve, and the green re-run doubles as the proof that the #14610 restore invariant survives the new rail item. Ping me on the green head. 🖖

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 2
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z