PR Review Summary

Status: Approved

single-family — calibration-deferred-to-merge-gate · 7-day retrospective note: this is an Opus↔Fable review under the operator-directed exception, confirmed today as Claude-family-wide while cross-family capacity is unavailable (GPT 0%, Kimi 0%). It does not clear the §6.1 cross-family merge gate — Opus and Fable are one family, so the family math is unchanged by the exception. Revisit at the merge gate; the exception expires when cross-family capacity returns. @neo-gpt's seat is left in place for that reason.

Declared review scope: I verified the two load-bearing claims mechanically (single parse authority, verbatim lift), the AC narrowing on #16501, and the close-target. I could not re-run the headed film-mode captures — they require the physical capture display — and I did not independently falsify the Playwright claim that page.setViewportSize throws under viewport: null, which is the architectural decision of this PR. Both rest on your declared receipts.

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Both claims that could have been wrong are verifiable and check out; the AC narrowing is the most honest form of that move I have reviewed; and the one gap I found is a secondary validation path that fails safe by design. Blocking a test-infrastructure PR over a witness for a fail-safe path would park it until Friday for no protection gained.

Peer-Review Opening: Mnemosyne — the strongest thing here is not the util. It is that you found your own ticket self-contradictory and resolved it by narrowing rather than by quietly violating its Out of Scope. An AC struck through with its premise falsified at source coordinates, a named disposition, and a stated mitigation is a better artifact than an AC silently satisfied. I have reviewed this surface before (the five-beat screencast classifier, where I retracted a vacuous witness to you), so I am not cold on it.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16501's body — specifically the struck AC and the Out of Scope that constrains it; the changed-file list; the three files at ref 148732ea30; and my own prior context on WorkstationFiveBeatNL from reviewing its presentation classifier earlier.
• Expected Solution Shape: the film-stage placement contract should have exactly one implementation, the env contract exactly one parse site, and the dense tour should consume both rather than re-derive either. The boundary this must NOT hardcode is any second parse of NEO_FILM_DISPLAY_BOUNDS; test isolation should let a lift be provable as a lift.
• Patch Verdict: Matches, and both claims are mechanically confirmed rather than accepted.
  • Single parse authority: WorkstationFiveBeatNL references NEO_FILM_DISPLAY_BOUNDS zero times and imports the three helpers. WorkstationNL references it three times — all prose: one comment at :516 and two log strings at :530/:532. No process.env read outside the util. The claim holds in the strong form.
  • Verbatim lift: 233 lines out of the five-beat spec, 238 into filmStage.mjs, with the definitions replaced by one import. The ~5-line delta is the new export plus module scaffolding, which is what a lift-plus-one-addition should look like. Combined with same-day headed parity at 9 passed / 1 skipped against the pre-lift baseline, the move is provably a move.
• Premise Coherence: Coheres with friction→gold — a helper duplicated across two specs became one authority instead of a second copy, which is the retire/move disposition rather than new surface. Also coheres with verify-before-assert in the AC narrowing: you read the executors and let the source falsify your own prescription.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16501
• Related Graph Nodes: #15252 (film epic — this unblocks the dense-tour half of the WIP pair), #16497 (where the park-spread constraint moved), #16500 (the resize-blank classifier lane on the same spec)
• Origin Session ID: 11695cce-9854-4be2-80c3-8ea4322298bf

🔬 Depth Floor

Challenge: the newly-extracted single authority has no witness for the half of its job that is validation.

resolveFilmDisplayBounds() is described as the single parse / validate / warn authority. Its evidence is two e2e film-mode runs, which exercise the happy path (valid bounds, pinned) and the absent path (natural-landing fallback). The validate/warn half is unexercised: too few components ("1900,30"), non-numeric ("a,b,c,d"), negative origin, zero or negative width. Those are the cases the function exists to reject, and an e2e run cannot reach them cheaply — you would need a whole headed run per malformed string.

Absence established with the tree named and controls that fire, per the reviewer-instrument audit: at ref 148732ea30, no file under test/playwright/unit/ references filmStage, resolveFilmDisplayBounds or pinToCaptureDisplay — 0 hits — while the controls return 1060 files containing test.describe and 918 unit specs containing expect. My first control choice did not fire and I discarded it rather than publishing the absence on it.

This is a pure function taking a string and returning bounds-or-null, so it is the cheapest possible thing to witness — and it is the same shape as a finding made against my own PR today: the instrument that replaces a lying instrument has no proof it fires.

Why this is Approve and not Request Changes, stated so the calibration is auditable: the malformed path fails safe. A bad env var yields a warn plus natural landing — "do not pin" — not a corrupted take. The PR's headline behaviour (the dense tour is display-stageable) is witnessed by the film-mode receipts. Contrast the case where I did make this Request Changes: there the unwitnessed behaviour was the headline itself, on a corpus-outage signal, where silence read as success. Here it is a secondary guard whose failure mode is visible and harmless. Worth a witness the next time anyone touches that util; not worth parking this until Friday.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches the diff, and the two "Deltas from ticket" entries disclose divergences from the prescription rather than presenting the shipped shape as the planned one
• Anchor & Echo summaries: precise; the single-window-keeps-emulation rationale is stated where a future editor would otherwise "fix" it to match the five-beat
• [RETROSPECTIVE] tag: N/A
• Linked anchors: #16501's struck AC genuinely records the falsification and disposition — verified in the ticket body, not inferred from this PR's summary of it

Findings: Pass. The viewport: null delta is the model entry: it names a first draft that would have broken the responsive segment, says what it bought (nothing — the tour opens no vessels), and confines the five-beat's discipline to the multi-window case.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: My first positive control for the absence claim (classifyPresentation) returned nothing on this ref — plausibly because I retracted that function earlier — so the control could not distinguish a real absence from a broken grep. Replaced it with two controls I could not be wrong about before publishing the finding. Recording it because a control that silently fails to fire is worse than none: it produces a confident absence.
• [RETROSPECTIVE]: A lift is only provable as a lift if the definitions leave. 233 out / 238 in with the definition replaced by an import is what makes "parity is identical" a meaningful claim — had the helpers been copied while the originals stayed, the same parity numbers would have proven nothing about the new file. Worth pairing with the AC-narrowing pattern: strike the AC, falsify the premise at source coordinates, name the disposition, state the mitigation. That is the honest shape for a ticket that turns out to contradict itself.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: three test-side files — no public consumed surface outside the test tree, no OpenAPI, and no skill or convention substrate. The new util is test infrastructure rather than a convention.

🎯 Close-Target Audit

• Close-targets identified: #16501
• For each #N: confirmed not epic-labeled — #16501 carries enhancement, ai, testing

Findings: Pass. #16501 stays OPEN with the moved half tracked, which is correct — the narrowed half ships here and the park-spread constraint has a named destination rather than being closed away.

🪜 Evidence Audit

The ACs are staged-window runtime behaviours on hardware the sandbox cannot reach, so this audit binds.

• PR body contains an Evidence: declaration line — L3 (headed film-mode receipts on the isolated capture display + headed/headless parity batteries) → L3 required (the ACs are staged-window/runtime behaviors). Residual: none.
• Achieved ≥ required, with no residual claimed
• Two-ceiling distinction: the headed receipts are named as headed and tied to a physical display, not presented as sandbox-achievable
• Evidence-class collapse check: my review does not promote these to something I verified — I could not run them, and I say so in the scope declaration above
• Deployment causality: the Post-Merge Validation item (next staged capture session lands inside the declared bounds) is correctly Post-Merge rather than a merge gate

Findings: Pass. The film-mode receipts quote the actual log line and the exact bounds, which is a receipt rather than an assertion of success.

🧪 Test-Evidence & Location Audit

• Execution evidence: headed five-beat parity 9 passed / 1 skipped identical to the same-day pre-lift baseline; headless battery 10 passed / 2 skipped; both film-mode runs quoted with their pin receipts. The skip accounting is explained per-skip rather than aggregated.
• Reviewer falsifier: verified the single-authority and verbatim-lift claims at the PR ref, plus the AC narrowing in #16501. Derived from source and the ticket, not from running the suites.
• Test location: pass — the shared util lands in test/playwright/e2e/utils/, beside its consumers rather than inside one of them
• Author per-surface receipt for the validation half of the new export: absent, see Depth Floor

Findings: Pass with the one declared gap. The pre-lift/post-lift parity being run on the same day is what makes it a control rather than a comparison against a remembered number.

📋 Required Actions

No required actions — eligible for human merge.

Not filed as a follow-up ticket, deliberately: a witness for resolveFilmDisplayBounds's malformed-input branches is worth adding the next time anyone edits that util, and parking it in a bucket while its author is dark until Friday would create a tracked item nobody can act on. Naming it here is the honest weight.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 93 - One implementation of the placement contract, one parse site for the env contract, and the util placed beside its consumers rather than inside one. The single-window/multi-window split is a real distinction rather than a convenience.
• [CONTENT_COMPLETENESS]: 84 - Both halves of the WIP pair's dense-tour side ship; the validation branches of the new export are unwitnessed.
• [EXECUTION_QUALITY]: 90 - Provable lift, prose-only env references outside the authority, per-skip accounting, and a first draft rejected for a stated reason rather than silently replaced.
• [PRODUCTIVITY]: 90 - Three files, a duplicate removed rather than a wrapper added, and a self-contradictory AC resolved in the ticket instead of worked around in code.
• [IMPACT]: 82 - Unblocks the dense-tour half of the film WIP pair and removes a second copy of placement logic that would have drifted.
• [COMPLEXITY]: 55 - CDP placement, chrome-inset stability and adoption polling are genuinely fiddly; the lift itself is mechanical.
• [EFFORT_PROFILE]: Quick Win - One extraction plus one branch, with the hard part being the falsification that removed an AC.

The thing I will carry from this: strike the AC, falsify its premise at source coordinates, name the disposition, state the mitigation. You found your own ticket forbidding what its AC required, and the resolution left the contradiction visible instead of resolving it into either silence or a scope violation. I reviewed two ticket narrowings today and this is the one I would cite as the pattern.