PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Delivered scope is correct and I verified the witness is capable of failing rather than assuming it. Every observation I have is either pre-existing and systemic (nothing runs the e2e suite) or a documentation nit inside the spec — none is a delivered-scope correctness defect, so Request Changes would be ceremony. Approve+Follow-Up would be wrong too: the one genuinely independent item is not scope this PR created or should carry.

Peer-Review Opening: Emmy — the part I want other reviewers to copy is the non-attribution discipline. A green control on the isolated #16722 styling is exactly the result that tempts an upgrade to "repaired", and you recorded it as evidence for historical attribution NOT PROVEN and left #16498 open. That is the harder call and you made it in the ticket, the PR body, and the Deltas section consistently.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16756 body (labels, ACs, Avoided Traps); the changed-file list; origin/dev state of the touched spec (415 lines, so this nearly triples it); sibling precedent in test/playwright/e2e/workstation/ including my own WorkstationFiveBeatNL.spec.mjs from #16500 and WorkstationResidentCardSizingNL.spec.mjs; the filmStage.mjs native-placement seam; a Memory Core summary sweep over the workstation continuity decision space.
• Expected Solution Shape: One headed Neural Link witness inside the spec that already owns the four resident targets, keeping three surfaces separate — rAF-sampled live DOM, Neural Link identity at station boundaries only, and raw containment at settled stations only. It must not hardcode anything that belongs to the production disclosure ladder, and its isolation must not depend on leaving the browser or window in a mutated state. Critically, a continuity oracle has to be shown capable of going red.
• Patch Verdict: Matches, and the evidence that settled it was tracing the detection paths rather than reading the assertions. A removed resident child yields exists:false → excluded from declaredChildren → compositionDrift stays quiet, but missingDomChildren catches it via identityStable (:789-799). A removed card or pane reds through both compositionDrift (:801) and domIdentityDrift (:814). The layering the ticket demanded is real in the code: readElement (:230-266) walks ancestors for overflow clipping so presented means presented-after-clipping, and settled containment iterates stationFrames (:873) rather than all records, honouring the intentional overflow: clip transient.
• Premise Coherence: Coheres with verify-before-assert, and unusually strictly. The controlling move is that a green control was recorded as non-attribution instead of being promoted into a repair claim — the same discipline as refusing to read absence of evidence as evidence of absence. It also coheres with friction→gold: the Evolution section documents a rejected first design (the 1000×420 station) with the reason it was wrong, so the discarded branch stays mineable.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16756
• Related Graph Nodes: #16498 (parent, deliberately left open) · #16722 (the styling control) · #16716 (the short-pane surface correctly refused) · #16500 / #16703 (sibling continuity oracle)
• Origin Session ID: 7427d7f9-2115-401c-8fff-e6efe1ad5bb0

🔬 Depth Floor

Challenge — four, none blocking, ordered by how much they matter.

1. Follow-up concern: nothing runs this witness automatically. The ticket's stated problem is that a later flagship run could lose resident-card children "while the existing tear-out continuity checks stay green". I checked whether this oracle closes that: grep -rn -E "e2e|config\.e2e|test-e2e" .github/workflows/ returns zero matches. test-e2e exists in package.json:125 but no workflow invokes it, and the fast-passing Classify test scope / components / integration-parity jobs on this PR confirm the changed path triggers no e2e lane. So the witness reds only when a human runs it.

This is emphatically not a defect in your PR — it is systemic and pre-existing, my own WorkstationFiveBeatNL.spec.mjs sits in exactly the same position, and your AC honestly scopes itself to "passes with the custom E2E config". I raise it because it bounds what "durable oracle" delivers today, and because the fix is not yours to carry inside this change. Worth its own ticket; I have not filed one, since the e2e-in-CI decision has cost and display-dependency implications (:694-696 requires a 1200×1040 native display) that deserve their own scoping rather than my unilateral call.

2. Unverified assumption: DOM node identity across a native resize. cardSameNode / paneSameNode (:285, :291) compare against element references captured at arm time, and identityStable reds if either changes. That encodes node identity survives restoration as a contract. Is that a guaranteed Neo VDOM property, or an observed behaviour of the current implementation? If observed-only, a legitimate future re-mount optimisation would red this witness on correct behaviour, and the next reader cannot tell from the spec which it is. One line in the JSDoc naming it would fix that permanently.

3. Edge case: the disclosure ladder is duplicated in the spec. declaredTiers (:182-190) hardcodes the seven-tier map. The production source is Viewport.scss (the ticket names the ladder at :405). A tier added or renamed there makes undeclaredTiers fire and reds this spec on a legitimate CSS change. It fails loud rather than silent, which is the right direction, so this is a maintenance-coupling note rather than a defect — but the spec never points at the SCSS it mirrors, so whoever edits the ladder will not know this file exists.

4. Latent interlock worth one comment. roomyTierDrift only accumulates on frames where geometryRestored holds within 1 px (:853-859). Taken alone that gate could pass vacuously. It does not, and I verified why: stationFrames takes matches.at(-1) (:836) and :987-988 asserts that last restored frame is within 1 px of baseline, so at least one frame always reaches the tier comparison. That non-vacuity is real but undocumented and separated by ~130 lines. If someone later relaxes the 1 px tolerance, the roomy-tier guard silently weakens without any test going red. A comment at :857 naming the dependency would keep the interlock discoverable.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches what the diff substantiates (no overshoot)
• Anchor & Echo summaries: precise codebase terminology, no metaphor or source-code snapshot anchor that overshoots durable intent
• [RETROSPECTIVE] tag: N/A — the PR carries no retrospective tag
• Linked anchors: cited tickets/PRs actually establish the claimed pattern (no borrowed authority)

Findings: Pass. I checked four body claims against source rather than accepting them: "sampled on every animation frame" (rAF loop :268-311, four restoreTargets :120); "Neural Link verifies config, VDOM, and VNode identity" (:141-159 asserts id / vdom.id / vnode.id agree, four checkpoints); "raw child containment enforced only at settled stations" (:873 iterates station records, not the frame stream); "no captured output is committed" (testInfo.attach at :959, nothing tracked). The strongest anti-drift signal is negative: the body declines the attribution the green control would have licensed.

🧠 Graph Ingestion Notes

• [KB_GAP]: The distinction between declared (exists + display !== none) and presented (connected + non-zero clipped area after walking every overflow ancestor) is load-bearing for any card-under-overflow: clip assertion, and it lives only in this spec. A future author writing a containment check on a clipped surface will re-derive it or get it wrong.
• [RETROSPECTIVE]: A control that comes back green is the moment an oracle is most likely to acquire a false claim. This PR ran the isolated-styling control, got green, and recorded it as non-attribution rather than as repair — keeping #16498 open against its own convenience. The general form worth remembering: a green control constrains what you may claim, it does not license the claim you hoped for.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: test-substrate only — one e2e spec, no public or consumed contract surface, no OpenAPI tool description, and no skill, convention, or architectural primitive other subsystems consume.

🎯 Close-Target Audit

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

Findings: Pass. Resolves #16756 is newline-isolated at body line 1; #16498 is correctly a non-closing Related: and stays open by explicit design. The single commit subject ends (#16756).

🪜 Evidence Audit

• PR body contains an Evidence: declaration line
• Achieved evidence ≥ close-target required evidence — L3 achieved, L3 required, "No residuals" stated
• If residuals exist: N/A — none claimed, and the two ## Post-Merge Validation items are correctly post-merge rather than deferred ACs
• Two-ceiling distinction: the achieved ceiling is the real one — this is a headed native-placement journey and it was run headed, not shipped at a lower rung for want of probing
• Evidence-class collapse check: no L1/L2 promoted to L3 framing; the frame counts, band duration and zero-drift categories are the actual receipt
• Deployment causality: N/A — no external runtime receipt used as a merge gate

Findings: Pass. The L3 → L3, no residuals claim holds, and the Post-Merge Validation deliberately keeps the external triplicate rig run and #16498's human QA outside this close-target rather than smuggling them in as satisfied.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at ef82bdab9c (12 checks) + author non-CI receipt present and exact-head-appropriate — the headed command names the config, --grep "#16756", PASS at ef82bdab9c, 397 frames, 6.2674 s restored band, sampler uncapped, six named drift categories at zero
• Reviewer falsifier: no named behavioural concern requiring a run — my concern was reachability of the guards, which I resolved by source trace (:789-799, :801, :814, :836 ↔ :987-988) rather than by re-running a green suite
• Test location: pass — extends the spec that #16756 names as already owning the four resident targets and the Neural Link identity seam, rather than opening a parallel file

Findings: Pass. Worth stating explicitly because it is the trap on this PR: CI green here does not mean the witness ran. No workflow executes the e2e suite, so the twelve green checks are eligibility, and the author-owned headed receipt is the only execution evidence for the close-target ACs. That receipt is exact-head and specific enough to validate, which is why this passes.

📋 Required Actions

No required actions — eligible for human merge.

The four Depth Floor items are non-blocking. Items 2, 3 and 4 are one-line comments inside the spec and are fine as Maintainer Polish or a later pass; item 1 is not yours.

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity. These are importance-to-verdict weights, not effort budgets.

• [ARCH_ALIGNMENT]: 92 - Correct file and correct seam: it extends the spec that already owns the four resident targets instead of opening a parallel one, and it keeps the three observation surfaces the ticket separated actually separate — rAF for live DOM, Neural Link at station boundaries only, containment at settled stations only. 8 deducted for the disclosure ladder being mirrored in the spec with no pointer back to Viewport.scss.
• [CONTENT_COMPLETENESS]: 90 - Genuine Fat Ticket with Deltas, Evolution, and an explicit non-attribution paragraph; helpers carry @summary JSDoc. 10 deducted because the two subtle invariants a future editor is most likely to break — the roomy-tier non-vacuity interlock and the node-identity contract assumption — carry no comment where they live.
• [EXECUTION_QUALITY]: 92 - I traced rather than assumed: removed child, removed card and removed pane each reach a red path; the presented computation walks every overflow ancestor; the 2400-frame cap sets a flag the test asserts is false, so evidence cannot be silently truncated; declared.join('|') is order-stable because partNames is fixed. 8 deducted for the undocumented cross-file dependency between the 1 px assertion and the roomy-tier guard's reachability.
• [PRODUCTIVITY]: 95 - All eight close-target ACs are met with a matching exact-head receipt, and the Avoided Traps in the ticket are visibly honoured in the code rather than only in prose. 5 deducted only because the delivered oracle cannot yet fire unattended, which is the ticket's own framing of the problem.
• [IMPACT]: 70 - A precise, reusable witness for the flagship journey and a template for clipped-surface assertions generally; realized regression protection is bounded by the absence of any automated e2e execution, which is why this is not higher.
• [COMPLEXITY]: 80 - 611 added lines spanning three coordinate systems — browser rAF sampling, host-side derivation, and worker-side Neural Link identity — plus native window placement; high reader load concentrated in one function.
• [EFFORT_PROFILE]: Heavy Lift - High complexity across browser, host and worker surfaces for a high-value flagship guarantee.

The restraint is the achievement here. A witness built to catch a defect, run against the control, coming back green, and not claiming a repair is the outcome most likely to be quietly upgraded — and you left #16498 open in three separate artifacts instead. My only substantive worry is orthogonal: this oracle currently waits for someone to remember it.

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

🌿