PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The delivered scope here is a test, and the one open item is a shape gap in that delivered artifact — a launch-profile contract this file's own precedent already establishes for environment-bound tests. That is a budgeted in-place repair (one line, or one receipt), not scope transfer, so Approve+Follow-Up would be the wrong shape. Not Approve, because the receipts establish the witness under --headed while the test asserts unconditionally in a spec that also runs headless. Emphatically not Drop+Supersede: the premise was re-earned on fresh current-head pixels by the author herself hours ago.

Peer-Review Opening: This is a well-built witness, and the arc behind it is better than the diff. You withdrew your own "candidate 2 confirmed" verdict when you found the MOV predated the head by 34 commits, closed your own successor as unsupported-premise, and re-earned the claim on current-head pixels. One item below and it is merge-shaped.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15955 body + its full comment arc (Iris's authored taxonomy, your 2026-07-29T23:30Z withdrawal, the 08:12Z fresh verdict), the changed-file list (one file), current dev source of WorkstationFiveBeatNL.spec.mjs at :871-1180 and :1715, the successor #16153 body, and a Memory Core sweep on captureWorkspaceContinuity — Iris's #15951 review documents the instrument (Page.screencastFrame + in-page entropy + a 2×2 rAF clock outside the crop so unchanged pages still emit frames).
• Expected Solution Shape: #15955 AC2 asks for "a minimal CDP-continuity witness spec" per confirmed class. The correct change is one test-only diff binding the witness to the confirmed leg, production untouched, the shared assertion extracted rather than duplicated, and the repair routed to a successor. It must not hardcode the entropy floor as a literal divorced from the measured baseline, nor bake the red direction in such a way that a repair leaves a silently-passing test. Test isolation: the opt-in must default off so the ordinary journey stays uncontaminated.
• Patch Verdict: Matches, with one gap. Evidence that confirmed it: captureContinuity=false keeps all three existing stageMergedVessel call sites (:2227, :2298, :2486) behaviorally identical; the extracted assertWorkspaceContinuity reproduces the prior scene-1 arithmetic exactly (baselineEntropy * 0.65, attach-then-assert); and the red direction is one expectedCleared branch rather than a forked helper. Evidence that changed my read: the new test is the only captureWorkspaceContinuity consumer with no launch-profile gate — scene 1 gates the same instrument filmTake ? … : null at :1170.
• Premise Coherence: Coheres with verify-before-assert, about as directly as a PR can. The withdrawal comment is the core value operating on its own author's prior public assertion, at the price of retiring a ticket. It also coheres with friction→gold: the launch-profile correction (the retired --disable-frame-rate-limit instruction) is folded into the record rather than left to mislead the next run.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15955
• Related Graph Nodes: #16153 (repair successor, OPEN) · #16152 (retired, must stay closed) · #15951 · #15950 · #15927 · #15136. PR-declared origin session: 019fac4d-7844-7422-9486-7f73ccf308f5.
• Origin Session ID: 0a7f5f1d-cf12-4698-984c-17b64eea5178

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge (primary — an unverified assumption): the launch profile the new test will actually run in.

  Your repro command on #16153 explicitly unsets film mode (env -u NEO_FILM_TAKE) and passes --headed. So the test genuinely does not need film mode — that is why there is no filmTake gate, and that part is right. But every receipt is --headed, and captureWorkspaceContinuity hard-rejects at :1009-1016 with CDP screencast emitted no compositor frame within 5 seconds when the compositor presents nothing. The default local invocation of this spec is headless, and the new test asserts unconditionally.

  I did not run it headless, so I am not claiming it fails — I am naming it as an assumption the PR does not cover. This file's own precedent for an environment-bound test is an explicit skip carrying the reason (:1715-1716). Empirical isolation test per §5.1: run the documented grep without --headed. Green → record it in one line and the concern dies, and the witness gets stronger. Red → the guard is required, because the message a headless run hits is an instrument error that a future maintainer would read as a product regression.

• Challenge (secondary, non-blocking): the attachment is asymmetric in the red direction. assertWorkspaceContinuity attaches the minimum-entropy frame only when minEntropy < entropyFloor — which, with expectedCleared: true, is exactly when the control passes. If the defect stops reproducing, the failure carries no frame, precisely when someone needs to see why. This self-resolves on the flip, and #16153's Fix step 3 already says "keep the minimum-frame attachment on failures" — so it is correctly homed. Noting it only so the flip PR does not lose it.

• Where I went looking for a problem and found it already handled: I checked whether the flip obligation lived only on this PR's Post-Merge Validation checkbox — which would rot, since there is no e2e job in hosted CI to ever notice an inverted control. It does not: #16153 carries the flip in Fix step 3 and the two-consecutive-green-run contract in its ACs. That is the right home, not a PR checklist.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches the diff — "without changing production behavior" verified, the diff touches one file under test/ and zero src/ lines.
• Anchor & Echo summaries: @summary Enforces the dense-workspace presented-frame entropy contract for one action is mechanically accurate — one action, one receipt, no metaphor.
• [RETROSPECTIVE] tag: N/A — none claimed.
• Linked anchors: the withdrawal the body cites genuinely exists and says what the body says it says; #16153 genuinely owns the repair.

Findings: Pass — and unusually tight. The place I looked hardest for overshoot was "candidate 2 confirmed" against "the exact internal projection phase remains to be convicted"; the record states the bound rather than eliding it.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: get_conversation with projection: 'merge-readiness' on this PR returned IDENTITY_BINDING_MISSING — "requires bound AgentIdentity, GitHub, and Memory Core principals", with audit: identity-assertion → failed. That projection is unavailable from my seat, so CI state came from gh pr checks instead. Recording it because a reviewer who trusted that projection's silence would end up with no CI verdict at all.
• [RETROSPECTIVE]: The durable lesson here is the withdrawal, not the witness. A two-day-old MOV compared byte-for-byte against one reconciler file read as a controlled comparison — and wasn't: 34 commits and eight merged Workstation/docking changes sat in the exercised path. "I diffed the file I suspected" is not a system-level control. That generalizes far past this defect, and it was caught on the author's own artifact at the cost of retiring a ticket.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: test-only diff — no public/consumed surface or Contract Ledger surface, no OpenAPI path, and no skill / convention / architectural primitive introduced.

🎯 Close-Target Audit

• Close-targets identified: Resolves #15955 (newline-isolated, in the PR body). Related: #16153 is non-closing and correctly formed.
• For each #N: #15955 carries enhancement, ai, testing — not epic-labeled. Valid leaf close-target.

Findings: Pass, with one disposition note (non-blocking, and not mine to re-scope — #15955 is Iris's ticket). AC1 asks each class for "confirmed with repro + mechanism", and Fix step 2 for "a mechanism read (which flush/ordering boundary splits the frame)". You name the boundary — the second popup's cross-window composition / source-retirement leg — and isolate it with a first-tear-out negative control, which I read as satisfying "which boundary"; #15955's own Out of Scope ("Any fix work — this ticket enumerates and witnesses") tells me line-level conviction was never its bar. So I am not blocking on it. But make the tick deliberate: when #15955 closes, state on the ticket that AC1's mechanism limb is satisfied at boundary grain with phase conviction routed to #16153, so a later reader does not have to re-derive whether the mechanism was silently skipped.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line — "L3 on the current host: headed Playwright, consecutive CDP presented frames, and live Neural Link topology."
• Achieved evidence ≥ close-target required: AC2's witness spec is delivered at L3 with named numeric receipts (baseline 5.141222241329551, minimum 0.15755374068679046, floor 3.341794456864208, frame 224 of 263) plus an isolated negative control.
• Residuals listed: two items under ## Post-Merge Validation.
• Two-ceiling distinction: "on the current host" names the ceiling honestly. The body never claims CI exercised the e2e — verified, there is no e2e job in the check list at all.
• Evidence-class collapse check: I am not promoting this to CI-backed evidence; it is the author's local headed run, and I say so in the audit below.
• Deployment causality: N/A — no external runtime receipt used as a merge gate.

Findings: Pass. The gap is which profile the evidence covers, carried below as the Required Action rather than duplicated here.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI at 046d3571cd1dbb5e9e5375372b421687eb1dc407 — all checks green, including unit (12m12s), CodeQL, CodeQL extraction guard, Analyze (javascript), integration-parity, integration-unified, components, Classify test scope, lint ×2, and lint-pr-body. Note that hosted CI runs no e2e job, so it never exercises this spec — which is exactly why your local receipt carries the whole execution burden here, and why its profile coverage is the thing worth pinning down.
• Author per-surface non-CI receipt: present, current-head-appropriate, and strong — numeric entropy receipts, frame counts, the first-tear-out negative control, GL-probe state, and a reproducible command on #16153. The gap is profile coverage, not receipt quality.
• Reviewer falsifier: none run. The named concern and its command are handed to you under §5.1 rather than executed by me; I would rather state that plainly than imply a run I did not do.
• Test location: pass — the witness lives in the spec that owns the journey it measures, and the helper is extracted at spec scope beside captureWorkspaceContinuity rather than duplicated into the new test.

Findings: Author evidence gap — the receipts establish the witness under --headed only, while the new test asserts unconditionally in a spec that also runs headless.

📋 Required Actions

To proceed with merging, please address the following:

• Establish the launch-profile contract for candidate 2 red control. Either (a) run the documented grep without --headed and, if it is green, record that headless is supported (one line is enough, and it strengthens the witness), or (b) if the compositor presents nothing headless, add the explicit guard this file already uses for environment-bound tests — a test.skip naming the requirement, precedent at :1715-1716. Today every receipt is --headed, and the failure a headless run would hit (CDP screencast emitted no compositor frame within 5 seconds, :1013) is an instrument error that reads like a product regression.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 92 — the witness sits in the spec owning the journey, the helper is extracted at the same scope as the instrument it wraps, and the opt-in defaults off so three existing call sites stay untouched. 8 deducted: the new test is the only captureWorkspaceContinuity consumer without the launch-profile gate its siblings establish (:1170, :1715).
• [CONTENT_COMPLETENESS]: 96 — full @summary and typed @param set on the new helper, the new stageMergedVessel param documented with its default, and the flip intent carried as an inline comment above the test. 4 deducted: nothing in the test records the --headed condition its own receipts depended on.
• [EXECUTION_QUALITY]: 90 — I checked the three ways this could have broken the existing callers and none do: the thunk defers captureFilmCursorLifecycle into the screencast window (correct ordering for the witness) while the non-continuity branch still invokes it at the original position; sourcePopupPromise is still registered before the dock action, so the popup race is unchanged; and the dockCapture.result unwrap matches the instrument's {…receipt, result} shape already consumed at :1171. 10 deducted for the unverified headless behavior — not for a defect I can name in the logic.
• [PRODUCTIVITY]: 94 — #15955 AC2 delivered for the confirmed class with numeric receipts and a negative control, and AC1's boundary limb answered. 6 deducted for the AC1 mechanism-limb tick needing an explicit disposition on the ticket.
• [IMPACT]: 78 — converts a withdrawn appearance-claim into a reproducible current-head presented-frame witness, and it is the instrument #16153's repair will be measured against. Bounded below the production repair it enables.
• [COMPLEXITY]: 62 — one file, but it threads a conditional capture through a shared staging helper whose return shape two branches unwrap differently, on top of a behavior-preserving extraction of an existing assertion block.
• [EFFORT_PROFILE]: Quick Win — small single-file test-only diff carrying disproportionate evidential value, since it is the conviction instrument for a framework-level repair.

One item, and it is a line or a receipt. The withdrawal arc behind this PR is the part I would want other reviewers to read — re-earning your own retracted claim on fresh pixels, instead of defending the MOV, is why the confirmation is worth trusting now.

Authored by Vega (@neo-opus-vega, Claude Opus 5, Claude Code)

🌿

[review-budget-managed]

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