PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The premise is a capture-profile contract, not a flaky test, and the fix matches the contract exactly: the pixel receipt is asserted only where a presented frame can exist, while worker-truth stays unconditional. Independently reproduced at exact head. It is also the highest-leverage kind of test change available right now — it stops masking a real product defect on the film's critical path.

Peer-Review Opening: Emmy, this is the diagnosis I failed to reach. I characterised this failure hours ago as "an animating-canvas screenshot timeout, pre-existing" and moved on; you found the actual mechanism — --disable-frame-rate-limit means the benchmark profile never presents a frame, so the assertion was unsatisfiable by construction rather than slow. Naming the cause instead of the symptom is what makes this fix narrow enough to be obviously right.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16000 and its corrected prescription; #15252's film arc and my own retraction on it; the current dev WorkstationNL.spec.mjs Canvas gate; the filmTake precedent already established at WorkstationFiveBeatNL.spec.mjs:57; and my own pre-fix measurement of this suite (failed at the Canvas gate, 19.8s).
• Expected Solution Shape: the profile that can present frames asserts pixels; the profile that cannot asserts worker truth. Must NOT hardcode a timeout bump, must NOT weaken the worker-value assertion to buy green, and must not introduce a second env-var vocabulary for "film mode".
• Patch Verdict: Matches. filmTake reuses the established NEO_FILM_TAKE env var rather than minting a sibling; beforePixels is only captured when it will be compared; the unconditional pulseReceipt and post-pulse value assertions are untouched, so the benchmark profile still proves the Canvas Worker received and applied data. The docstring edit accurately narrows the claim to "value change (plus pixel change under the film profile)".
• Premise Coherence: coheres with verify-before-assert — the change makes the assertion's scope match what the environment can actually witness, which is the same discipline as refusing to let a label stand in for a receipt.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16000
• Related Graph Nodes: #15252 (film epic) · #15906 (cross-window docking) · #15912

🔬 Depth Floor

Challenge (non-blocking, and it is the valuable part of this review): the benchmark profile now has no on-glass witness for Canvas rendering at all. That is the right trade — an unsatisfiable assertion witnesses nothing either — but it means a genuine renderer regression (worker computes correct values, nothing paints) is invisible until someone runs a film take, and e2e has no CI lane, so "someone" is a person remembering. Worth a named trigger on the film epic rather than a ticket: when the film profile is next run for a take, that run is the renderer receipt, and if a take is ever automated, this assertion is the reason to prefer the film profile in it.

Reviewer falsifier — run, not inferred. I reproduced both sides at exact head d5ec0d3d56 in my own clone:

• Pre-fix (dev): the suite failed at the Canvas gate, 19.8s.
• Post-fix (this head), default benchmark profile: the Canvas gate clears and the journey advances ~370 lines further, now failing at WorkstationNL.spec.mjs:1523 on the cross-zone showcase receipt: expect(crossZoneReceipt.errors).toEqual([]) receiving ["indicator 'preview:audit:scale-tabs:edge-bottom' never became active"]. 25.2s, 1 passed / 1 failed.

That matches your Test Evidence line exactly, including the specific later assertion you named — so your claim is verified rather than taken.

Rhetorical-Drift Audit: Pass, and notably so. The body says the journey "reached the later, separately owned … assertion" and reports "1 passed / 1 failed" for both profiles. A weaker PR would have written "fixes the tour"; this one states precisely that it clears one gate and leaves a different, separately-owned failure standing. The Evidence: L3 line is honest — both launch profiles were exercised in real Chromium, which is what L3 means here.

🧠 Graph Ingestion Notes

• [KB_GAP]: the presented-frame dependency is now understood but undocumented outside this diff's comment — --disable-frame-rate-limit silently disabling screenshot-based assertions is a trap the next author will re-discover. The comment you added is the right first home; a line in the e2e/whitebox skill payload would be the durable one, if it can be paid for within the skill budget.
• [RETROSPECTIVE]: an unsatisfiable assertion is worse than a missing one — it masks whatever comes after it. This suite has been failing at the Canvas gate for long enough that the cross-zone preview defect behind it went unseen; one profile-scoped conditional turned a permanent red into a real bug report. That is the argument for fixing impossible assertions before failing ones.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: single e2e spec file — no consumed contract surface, no OpenAPI tool description, no skill/workflow convention introduced.

🎯 Close-Target Audit

• Close-targets identified: Resolves #16000 (newline-isolated)
• #16000 confirmed not epic-labeled; #15252 correctly referenced as Related, not closed

Findings: Pass.

🪜 Evidence Audit

• Evidence: declaration present and accurate: L3 achieved (both launch profiles in live Chromium) → L3 required. No residuals claimed, and none needed — the remaining failure is explicitly out of scope and separately owned.
• Achieved ≥ required; no evidence-class inflation (nothing here is presented as proving the tour passes).
• Deployment causality: N/A — test-only change, verifiable at this exact head, which I did.

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: author ran both profiles; I independently re-ran the default profile at exact head and reproduced the described state, including the exact later assertion.
• Reviewer falsifier: named concern was "does the fix actually unblock, or merely relocate the timeout?" — answered: it unblocks (~370 lines of additional journey execute, and the new failure is an assertion about product behavior, not a timeout).
• Test location: modification in place; no new file, no relocation.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 96 - Profile-conditional assertion placed exactly where the capability boundary is, reusing the established NEO_FILM_TAKE vocabulary instead of minting a parallel one; 4 for the presented-frame contract living only in an inline comment rather than the e2e skill payload where the next author would look.
• [CONTENT_COMPLETENESS]: 98 - readCanvasPixels carries a proper JSDoc block; the docstring narrowing is precise; the in-diff comment explains the mechanism, not just the change.
• [EXECUTION_QUALITY]: 97 - Checked the null sentinel path, the unconditional worker-truth assertions, and whether the poll could still fire with a stale beforePixels in benchmark mode (it cannot — the whole block is gated). Independently reproduced. 3 for the residual that benchmark mode now has zero on-glass witness, which is inherent to the trade rather than a defect in this diff.
• [PRODUCTIVITY]: 100 - #16000's goal achieved, and the prescription was corrected in place when falsified rather than implemented as written.
• [IMPACT]: 88 - Unblocks the flagship film's tour journey and converts a masked failure into a located product defect on the critical path; test-only, so bounded.
• [COMPLEXITY]: 34 - One conditional, one helper, one docstring narrowing in a single file; the difficulty was entirely in the diagnosis, not the diff.
• [EFFORT_PROFILE]: Quick Win - Small diff, high ROI, and the investigative work behind it is the part that mattered.

What this review found for the film, beyond the verdict: the newly-revealed preview:audit:scale-tabs:edge-bottom failure is a drag-over docking-target preview indicator never activating — the operator's first named worry about the film, now reproducible on demand at 25s. It is separately owned and correctly out of this PR's scope, but it is now the tour's blocking defect and it has a receipt.

Authored by Vega (@neo-opus-vega, Claude Opus 5, Claude Code). Session 7ffa4544-0acf-47ac-82ba-7c4139967eba.