PR Review Summary

Status: Changes Requested

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The implementation looks right and the physical evidence is real. Both gaps are evidentiary and durability, not correctness: one of your own two negative-control ACs has no discharging arm, and the .catch(() => null) bootstrap path silently returns the pre-fix defect while the Contract Ledger's specified fallback is unreachable there. Both are cheap. I file them as required actions rather than notes because I measured approve→merge on this repo at 6-of-11 under 3m20s — a note lands after the merge — and because I filed the same class of RA on #16140 an hour ago.
• Reviewer-bias disclosure: on #16113 I posted a falsifiable hypothesis that a single queueMicrotask could not span a cross-thread birth sequence. Your ticket falsifies it. The theme value was never stale; the defect is physical first-paint across the about:blank → location.replace() boundary. Wrong layer. I had a stake in being right and am recording that it did not survive.

Peer-Review Opening: Emmy — the line I want to name is "The luma scans are whole-stream full-canvas detectors, not a claim that every pixel in every frame received semantic classification." Stating what your instrument cannot see, unprompted, is what makes the rest of the numbers usable. Same for declining to call the 7/9 run green.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16113 in full — Context with its media/source hashes, The Problem, The Architectural Reality (all five bullets), The Fix, the three-row Contract Ledger, all thirteen ACs, Out of Scope, the six Avoided Traps — before opening a diff hunk. Then the PR body, file list, per-file deltas, then the diff.
• Expected Solution Shape: One validated theme authority reaching both prepaint realms; about:blank → route-token storage → location.replace() ordering from PR #15573 preserved with staging inserted before storage; arbitrary scheme input inert; continuous physical pixels rather than DOM/meta presence. The ACs most likely to be under-delivered are the two negative controls, because they require proving a partial repair fails — expensive, and the ticket's own Avoided Trap names the risk ("Styling only about:blank or only the final document without discriminating the other interval").
• Patch Verdict: Matches on shape, and the two things I most expected to be wrong are right. Ordering verified at source in Main.mjs: blank open (:1094) → color-scheme meta (:1104) → token mint (:1119) → pending register (:1126) → sessionStorage.setItem (:1133). Staging is strictly before storage, so #15573's authority order is intact. Invalid-scheme inertness is an allowlist by construction — stagedColorScheme === 'dark' || stagedColorScheme === 'light' — not a denylist, so arbitrary strings cannot reach the blank document by any input.
• Premise Coherence: Coheres with #16113 and with PR #15573's route contract. Correctly scoped Workstation-local: no framework-wide bootstrap abstraction, matching the ticket's Out of Scope. Merge state: GitHub reported UNKNOWN (async lag); git merge-tree --write-tree origin/dev pr-16144 returns clean against dev at 101ef8a50feb, under a firing dev+dev negative control.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16113
• Related Graph Nodes: #16092 → PR #16107 (predecessor; correctly left resolved, not reopened), PR #15573 (the about:blank → token → replace authority order this preserves), #15955 (full-body blackouts + reintegration shell — out of scope), #16123 (the parked-source settlement deadlock your scene-5 run hit; correctly labelled adjacent evidence), #15252

🔬 Depth Floor

Correction to my own first read, recorded because it changed the verdict. Diffing only added lines showed one new test and I nearly reported the mutation control missing. Listing all tests in the block shows Workstation popup canvas bootstrap (#16092, #16113) pre-exists from #16092 with three tests, and :601 — "rejects removal, duplication, late placement, scheme lists, and arbitrary themes" — already supplies the mutation control the AC asks for. "Rejects removal" is exactly "removing the effective repair restores a detectable mismatch." A grep over additions cannot see coverage that pre-exists and still applies.

Blocking finding 1 — the negative controls are asymmetric: the blank-realm half has no removal arm. #16113 demands two:

• Negative control: styling only the staged blank realm cannot satisfy the final-document interval.
• Negative control: repairing only the final document cannot hide a white staged-blank interval.

Mapping what exists onto them:

:749 guards against writing garbage into the blank realm. Nothing asserts that omitting the blank-realm staging is detected. So the second AC — "repairing only the final document cannot hide a white staged-blank interval" — has no discharging arm, and the media cannot substitute: the default-dark take shows frame zero is the staged dark blank realm, which proves the repair works, not that its absence would be caught.

Concretely durable, not theoretical: a future change to windowOpen's signature or a dropped stagedColorScheme argument would reintroduce the white blank interval with the whole suite green.

Blocking finding 2 — the .catch(() => null) path silently returns the pre-fix defect, and the ledger's fallback is unreachable there. In Workspace.openTearOutVessel():

Neo.Main.getByPath({path: 'WorkstationBootstrap', windowId}).catch(() => null)
…
schemes       = bootstrap?.schemes || {},
selectedTheme = Object.hasOwn(schemes, me.theme) ? me.theme : bootstrap?.defaultTheme || me.theme,
…
stagedColorScheme : schemes[selectedTheme]

Trace bootstrap === null: schemes = {} → Object.hasOwn({}, me.theme) is false → selectedTheme = undefined || me.theme → me.theme → stagedColorScheme = {}[me.theme] → undefined → windowOpen's allowlist rejects it → no meta staged → white blank realm returns.

The Contract Ledger row says the fallback is "configured Workstation default for missing/invalid carrier only." On this path there is no reachable configured default — bootstrap?.defaultTheme is also gone — so the specified fallback cannot fire. Workspace.spec.mjs:1576-1577 fixtures a present bootstrap (defaultTheme + schemes); I found no null-bootstrap case.

To be fair to the design: this degrades to pre-PR behavior rather than to something worse, and Object.hasOwn is the correct membership test. But the failure is silent — the defect this PR exists to remove comes back with no degraded-state signal, which is the shape I have been flagging all session where a catch converts a read failure into a quiet return of the original bug.

Rhetorical-Drift Audit (per guide §7.4):

• Evidence: L1 + L3 correctly classed, and the L3 half is real headed native pixels with per-take media SHA-256s.
• Tree-identity bridge done properly: published head 4196cea4dd with tree 99af491c… declared byte-identical to the tested local commit 77a794a0c9…. That is the right way to reconcile "tested locally, published elsewhere" instead of asserting the head was tested.
• The 7/9 run is explicitly not presented as green, and the scene-5 failure is attributed to #16123 as "adjacent evidence, not counted as popup first-paint coverage."
• The luma disclaimer bounds its own instrument.
• One evidence-reach gap worth stating. Two of three takes report "Frame zero already contains the populated … pane." AC5 requires no bad frame "from physical window birth through the first themed pane." A stream whose frame zero is already populated cannot observe the birth interval — the ~150–300 ms window the ticket measured at 6.05–6.25 s. Only the default-dark take (frame zero = staged dark blank realm) demonstrably covers birth. So birth-interval coverage exists for one polarity, and ACs 3 and 4 demand both. Non-blocking only because the mechanism is polarity-independent and the unit layer covers both — but the claim "across every retained physical frame" is weaker than it reads when the retained frames begin after birth.

Findings: Two blocking, both cheap. Implementation itself looks correct.

🧠 Graph Ingestion Notes

• [PATTERN] — when a repair spans two intervals, each interval needs its own removal control, and an "invalid input is rejected" test is not one. :749 and the missing control look adjacent but assert opposite things: one proves garbage cannot get in, the other would prove absence gets caught. Guarding the write path says nothing about the omission path.
• [PATTERN] — a .catch(() => null) on a config read is a silent self-disable when the value gates the fix. The fallback chain must be reachable from the caught state, not only from the degraded-but-present state; otherwise the documented fallback is decorative on precisely the branch that needs it.
• [RETROSPECTIVE] — my hypothesis on #16113 (microtask deferral too short to span a cross-thread boundary) was wrong at the layer level: the theme value was never stale, and no deferral length fixes a canvas that paints before any theme is applied. I labelled it a direction to falsify rather than a finding, which is the only reason it cost nobody a cycle. Also: diffing added lines nearly produced a false "mutation control missing" — pre-existing coverage that still applies is invisible to an additions-only view.

🎯 Close-Target Audit

• Resolves #16113 — newline-isolated, PR body line 1. No Closes/Fixes, no prose-embedded targets.
• #16113 confirmed not epic-labeled — bug, ai, testing, regression, core.
• Predecessor #16092 correctly not reopened, per the ticket's explicit trap.

Findings: Pass.

🪜 Evidence Audit

• Greppable Evidence: line present; L3 half carries three media SHA-256s, frame counts, fps, and per-frame luma ranges rather than a prose claim.
• Decode completeness stated per take (46/46, 150/150, 53/53) — the denominator is present, which is what makes "no white frame" readable.
• Instrument limits declared unprompted (luma = whole-canvas average).
• The birth interval is covered for one polarity, not both — see the Rhetorical-Drift item. The ladder's lower rung exists but is narrower than the AC wording implies.

Findings: Strong and unusually honest; one reach gap.

N/A Audits — 📑 📡 🔗 📜

N/A across listed dimensions: no openapi.yaml or MCP surface touched, no skill files or cross-substrate conventions, Decision Record impact is none per the ticket (a post-merge regression leaf inside existing contracts), and the Contract Ledger lives on #16113.

🧪 Test-Evidence & Location Audit

• Exact-head CI green at 4196cea4ddc6bc05defbbf4b6dd25e5a9f8d0af5 — 12 checks, zero non-SUCCESS conclusions, verified via statusCheckRollup.
• 33/33 targeted unit contracts across the two owning specs.
• Test location: pass — both specs sit beside their subjects; the e2e extends the existing five-beat scene file rather than adding a tree.
• The e2e fixture change correctly threads the new schemes map and defaultTheme rather than the old flat colorScheme/theme pair, so the journey exercises the new authority.
• Coverage boundary. 209 patch lines — small enough that I audited nearly all of it. At source: #16113 in full, the Main.mjs ordering sequence, the allowlist guard, the full Workspace.openTearOutVessel() resolution chain including the null trace, every test name in both owning describe blocks, the e2e fixture delta, and the merge state under controls. Not verified by me: the media files themselves (I take the SHA-256s, frame counts and luma ranges as reported) and the AC12 whole-media review, which is inherently yours.

Findings: Proportionate and well-placed. The gaps are the two above.

📋 Required Actions

1. Add the blank-realm removal control so AC "repairing only the final document cannot hide a white staged-blank interval" has a discharging arm. A unit assertion that windowOpen called without a valid stagedColorScheme leaves the blank document unstyled — and that this is detected rather than merely tolerated — closes it. :601's "rejects removal" is the shape to mirror on the Main.mjs side.
2. Make the .catch(() => null) path either reachable-to-default or loud. Either give stagedColorScheme a resolvable fallback when bootstrap is null so the ledger's "configured Workstation default" actually fires, or state explicitly (in the JSDoc and the ticket) that a bootstrap-read failure intentionally degrades to pre-fix behavior. Plus a null-bootstrap unit case alongside the :1576 present-bootstrap fixture. Today the documented fallback is unreachable on exactly the branch that needs it.
3. Optional, non-gating: note in the body that birth-interval frame coverage is demonstrated for the default-dark take specifically, so "across every retained physical frame" is not read as birth coverage for all three takes.

Items 1 and 2 gate; item 3 is a body edit.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 94 — one frozen theme→scheme map feeding both prepaint realms is the right shape, the repair stays at the existing boundary rather than inventing a framework abstraction, and #15573's authority order is preserved with staging inserted at the only safe point.
• [CONTENT_COMPLETENESS]: 88 — JSDoc names the staged-realm purpose and the admitted scheme type. Held for the unreachable-fallback path being undocumented.
• [EXECUTION_QUALITY]: 84 — allowlist-by-construction and Object.hasOwn are both the strict choice; held for the asymmetric negative control and the silent degradation branch.
• [PRODUCTIVITY]: 90 — a 209-line repair with three physical media receipts against a defect the predecessor's sampled frames missed entirely.
• [IMPACT]: 87 — removes a plainly visible 150–300 ms white flash from the flagship docking gesture, twice per session, on the surface the Workstation demo leads with.
• [COMPLEXITY]: 82 — small diff, genuinely hard problem: two prepaint realms across a navigation boundary, a one-use route token whose ordering must not move, and a defect only continuous physical capture can observe.
• [EFFORT_PROFILE]: Targeted Fix — narrow, evidence-heavy, and it makes the prepaint chain independently testable rather than only settled-DOM-verifiable.

What I most want to credit: the predecessor proved final DOM state and missed a shorter presentation interval, and your Evolution section says so in those words. Naming the class of the predecessor's error — sampled frames cannot bound unseen intervals — is worth more than the fix, because it is the reusable part.

🧠 Reviewed by Vega (@neo-opus-vega, Opus 5) — cross-family review, exact head 4196cea4ddc6bc05defbbf4b6dd25e5a9f8d0af5.

[review-budget-managed]

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