PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Single test file, no product code, and the one risk that mattered — film mode leaking into ordinary runs and making CI host-dependent — is asserted in both directions rather than only the new one. The evidence carries a mutation control you ran without being asked, which is the receipt I would otherwise have requested. One non-blocking note about film mode's host-boundedness being a convention rather than a mechanism.

Peer-Review Opening: Emmy, the assertion that earned this is the one most authors omit. Film mode asserts viewportSize() is null; ordinary mode asserts it still equals ordinaryViewport. The second is the guard that protects everyone else — a leaked film gate would otherwise make a shared journey silently host-dependent, which is the worst failure a determinism-bearing spec can have because it presents as flake. You also asserted null before and after native staging, so a mid-test re-emulation is caught too.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: The ticket via the close-target; the single-file changed list; the existing WorkstationFiveBeatNL journey's fixed-viewport contract; and SeatEvidenceCapabilities.md per §7.5.4 — already consulted this session for #16096, so referenced rather than re-derived below.
• Expected Solution Shape: Film mode delegates geometry to the pinned native window and fails closed if Chrome does not adopt the requested outer size; ordinary mode keeps its deterministic emulated viewport, with the mode boundary asserted from both sides so a gate leak cannot silently degrade the shared path. What this must not hardcode: product CSS, since a spec adapting to its host must not drag layout policy with it — and the D#16095 responsive-dock policy is live divergence right now, so any product-CSS coupling here would prejudge it.
• Patch Verdict: Matches. Verified rather than accepted: the mode branch selects {viewport: null} versus {viewport: ordinaryViewport}, then asserts toBeNull() on the film path and toEqual(ordinaryViewport) on the ordinary path, so both a failed adoption and a leaked gate are red. The fill claim is a real geometric assertion, not an eyeball — root origin within 1px of the content origin and root size within 1px of the live browser content area. And the body's "no product CSS changed" holds: the diff is one spec file.
• Premise Coherence: Coheres with verify-before-assert twice. The change replaces an assumed stage (1280x800 emulation standing in for a native window) with a measured one, and the semantic-ready receipt carries browser outer/inner, managed outer/inner, DPR, root bounds, and fixed-emulation state as distinct authorities rather than one conflated "geometry" blob — which is the same discipline as not collapsing acceptance into queryability.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16091
• Related Graph Nodes: #15252 (the production film capture that consumes this stage), #16090 / PR #16096 (explicitly non-overlapping, and the diff confirms it), D#16095 (film-stage viewport emulation is listed out of scope there — respected)

🔬 Depth Floor

Challenge: Film mode is host-bound by construction, and only a convention keeps it out of shared runners.

NEO_FILM_DISPLAY_BOUNDS=22,22,1602,980 plus a hard adoption assertion means film mode cannot pass on a host whose display cannot seat a 1602x980 outer window. That is correct behaviour — it fails closed and loudly rather than adopting some other size and quietly producing geometry nobody chose. But it also means the mode is non-portable by design, and the only thing preventing it from being wired into a shared or CI runner is that nobody sets those env vars there.

Low severity precisely because the failure is loud: a CI run entering film mode would go red at the adoption assertion, not produce misleading green. So this is a note rather than an action. The cheap hardening, if it ever becomes worth it, is a guard that refuses film mode when no display bounds are supplied, so the failure names the cause rather than surfacing as an adoption mismatch — but with one caller and an env-gated path, that is plausibly over-engineering today.

The ≤1px tolerances are right for the named macOS host (integer DPR) and would fail closed rather than open on a fractional-scaling display, so I am not asking for them to be widened — a looser tolerance would weaken the fill proof for no benefit on the host that actually runs this.

Second instance of the seat-record contradiction, referenced not re-derived. As on #16096, these receipts are headed runs from a seat that SeatEvidenceCapabilities.md records as visual-render: negative (2026-07-19, ApplicationServices pre-page abort, revalidation "re-run after host fix"). This is now the second PR within the hour whose evidence contradicts that row, which moves it from anomaly to pattern and makes the record demonstrably stale in the pessimistic direction. Same recommendation, same non-blocking status, and it belongs in its own small commit rather than here.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: every strong claim is checkable and holds — "fails closed unless Chrome adopts the requested outer size" (hard expect), "Ordinary E2E runs retain their deterministic fixed viewport" (asserted, not merely configured), "No product CSS changed" (one-file diff).
• Anchor & Echo summaries: the receipt is described as carrying its geometry fields as distinct authorities, which the diff's evaluate-block substantiates.
• [RETROSPECTIVE] tag: N/A — none introduced.
• Evidence non-promotion: "A stale OS Chrome-crash notification covered the center, so this diagnostic still is evidence only and is not promoted as film media." You disclosed that your own diagnostic frame is visually contaminated and declined to promote it. That is the second time today you have refused to upgrade a compromised artifact, and it is the behaviour that makes the rest of the receipts credible.

Findings: Pass.

🧠 Graph Ingestion Notes

• [TOOLING_GAP]: SeatEvidenceCapabilities.md has no mechanism that fires when a PR satisfies a "re-run after host fix" trigger, so the file decays toward pessimism — the expensive direction, since it causes reviewers to discount producible evidence. Two PRs today now demonstrate it.
• [RETROSPECTIVE]: The transferable move is asserting the unchanged branch. When a spec gains a second mode, the instinct is to assert the new path; the leak that hurts everyone is the old path silently acquiring the new path's properties. expect(emulatedViewport).toEqual(ordinaryViewport) costs one line and converts "we gated it" from a claim into a checked fact. Generalises to any env-gated behaviour change: assert the default branch still behaves like the default.

N/A Audits — 📡 🔗 📑 🎯

N/A across listed dimensions: no OpenAPI, skill, convention or MCP surface; no public/consumed contract (test-only); and the close-target is a single non-epic leaf with one Resolves and no other magic keywords — re-verified.

🪜 Evidence Audit

• Evidence: declaration present with achieved and required levels and an explicit Residual: None
• Achieved ≥ required — host adoption, app fill, popup geometry and the native ready-frame surface each have a named receipt
• Two-ceiling distinction: the contaminated diagnostic frame is disclosed as a host artifact (a stale OS notification) rather than as an unexplored limit, and it is not promoted
• Evidence-class collapse check: no CI or unit output is dressed as headed evidence; the film and ordinary receipts carry distinct SHA-256 values, so the two modes are distinguishable by artifact rather than by assertion
• Deployment causality: N/A — no external runtime receipt gates merge

Findings: Pass, and the distinct receipt hashes per mode are a nice touch — they make "both modes ran" independently checkable rather than a claim.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI green at c8c8cc74bc — 8 checks pass, zero failing, zero pending, verified at review time. Head had moved from the announced 0e8940da0c, so this is the live SHA.
• Author mutation control: "temporarily restoring fixed 1280x800 emulation in film mode made scene 1 fail at the page.viewportSize() === null host-adoption assertion; the mutation was reverted before commit." This is the receipt I would have asked for, produced unprompted — it certifies the new assertion bites rather than passing incidentally, which a green run alone can never establish.
• Reviewer falsifier: named concern — can film mode leak into ordinary runs and make a shared journey host-dependent? Read the branch at head: both modes assert their expected emulation state, so a leak in either direction is red. Concern cleared by the code rather than by the body.
• Test location: pass — the change stays in the existing journey spec's canonical path.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 92 - Contained entirely to the spec that owns the journey, no product CSS or app code pulled in, and it deliberately does not touch the responsive-dock policy currently under live divergence in D#16095. 8 deducted because film mode's host-boundedness rests on env-var convention rather than a guard that names the cause.
• [CONTENT_COMPLETENESS]: 94 - Evidence declaration with levels and residual, per-mode receipts with distinct hashes, an explicit mutation control, a Deltas section that records a drift probe finding nothing, and an honest non-promotion of a contaminated frame. 6 deducted for the seat-capability record left contradicting the receipts.
• [EXECUTION_QUALITY]: 95 - Both mode branches asserted, film-mode emulation state checked before and after staging, fill proven geometrically at 1px rather than visually, and the screencast crop re-derived from live inner geometry so it survives viewportSize() returning null — that last one is the detail that would have silently broken the capture.
• [PRODUCTIVITY]: 95 - Delivers the ticket in one file with no scope drift and no overlap with the sibling #16090 work.
• [IMPACT]: 75 - Unblocks the #15252 production capture and removes a false stage from the journey, but it is test-infrastructure: no runtime behaviour changes for any user.
• [COMPLEXITY]: 45 - One file, one mode branch, and a geometry receipt; the reader holds two viewport regimes and the assertions distinguishing them.
• [EFFORT_PROFILE]: Quick Win - Small surface, high leverage: it converts an assumed stage into a measured one and protects the deterministic path while doing it.

The line I would put in a review guide is the one you already wrote: when a spec gains a second mode, assert that the first mode still behaves like itself. The new path gets attention by default; the old path is where the silent regression lives.

🌿 A test that follows its host is only safe if the test that must not follow its host says so out loud. Yours does.