PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: §9.0 Premise Pre-Flight run against all seven triggers — none fires. The fix moves authority upstream to a value that already exists rather than inventing a second source, both halves of the decision are pinned to the same input matrix, and the mutation controls are real. Two non-blocking notes below, neither of which is debt this PR creates. I went looking for a blocking finding and the suite defeated it — recorded honestly in the Depth Floor, because the near-miss is more useful to you than a clean-looking checklist.

Peer-Review Opening: The two things that usually go wrong here both went right. Object.hasOwn(schemes, requestedTheme) rather than in or a truthiness check means ?theme=toString and ?theme=constructor fall to the default instead of resolving a prototype key — and the value that actually reaches the DOM is schemes[theme], read out of a frozen map, never the URL string. Two independent barriers on a URL-supplied value, and the second one holds even if the first were bypassed. Separately: f1653ce41f is a commit that implements a premise in order to falsify it, and the Evolution section says so plainly. "Not white" was weaker than theme parity, and you found that by building the weak version and measuring it. That is worth more than the diff.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #16092 (labels, state, body — checked for a Contract Ledger and residual annotations); the changed-file list; the full source diff of index.html, app.mjs, Workspace.mjs at the head; apps/workstation/neo-config.json themes; MainNativeWindowRoute.spec.mjs:416-486 read in full before asserting anything about guard coverage; a diff-wide grep for style= / .style. / <style / <link / .css / cssText to test the app-work styling mandate.
• Expected Solution Shape: The child must know the parent's active theme before its first paint, which means the value has to arrive through something that runs ahead of the module graph. Carrier must be allowlist-validated because it comes from a URL; the written value must not be the input; no CSS-in-JS and no raw stylesheet may appear in an app; and the prepaint half and the app half must not be able to disagree.
• Patch Verdict: Matches. Workspace.me.theme is an existing selected value rather than a new authority, resolveBootstrapTheme derives its admitted set from config.themes instead of hardcoding it, and it is exported so the resolution is provable without booting the app. One structural asymmetry between the two halves is real and is addressed below.
• Premise Coherence: Coheres with verify-before-assert in its most expensive form: the original premise was falsified by its own implementation, on measured evidence, and the claim was then strengthened rather than defended. That is friction→gold operating inside a single PR.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16092
• Related Graph Nodes: #15252 (film readiness — this was the last product-pixel gate), #15573 (same-origin route authority paths), Workspace.setWorkspaceTheme, apps/workstation/neo-config.json

🔬 Depth Floor

Challenge (non-blocking) — and I was about to file it as blocking, so the reasoning matters more than the note. The two halves of one decision take their allowlist from different places: resolveBootstrapTheme derives it from config.themes, while index.html's inline script hardcodes {'neo-theme-neo-dark': 'dark', 'neo-theme-neo-light': 'light'} and hardcodes the 'neo-theme-neo-dark' fallback. My intended finding was that adding a third theme to neo-config.json would silently reintroduce this PR's own defect for that theme: the module half would admit it, the prepaint half would refuse it and paint dark, and nothing would fail.

That is wrong, and checking rather than asserting is what caught it. MainNativeWindowRoute.spec.mjs:427 pins config.themes to a literal, so a third theme turns that test red immediately, in the same file that owns every index.html bootstrap assertion — the author cannot reach the divergence without being sent to the second edit site. The drift is guarded; it is just guarded indirectly. What remains is diagnostic wording: the failure will read as a toEqual mismatch on config.themes, which says "the config changed," not "extend the prepaint scheme map too." Since inspectWorkstationBootstrap already parses the script, asserting set equality between the parsed scheme-map keys and config.themes would make the guard self-explaining and would additionally pin themes[0] against the hardcoded fallback, which is the other coupling nobody would think to check. Worth a line whenever this file is next open; not worth a cycle now.

Also noting why the inline script cannot simply derive: it has to be synchronous and parser-blocking, ahead of any module, so it has no access to neo-config.json. The duplication is forced by load order, not chosen — which is precisely the situation adapterWitness.spec.mjs's "the forced CJS/ESM duplication cannot drift" test was written for. Same shape, and that precedent is the argument for making this guard direct.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches the diff, including the honest downgrade of the first head's claim. The Evolution section describes a real falsification, not a narrative.
• Anchor & Echo: both new JSDoc blocks use precise terms; @returns {String|undefined} The carried configured theme, or the configured default is accurate including the undefined case when themes is empty.
• [RETROSPECTIVE] tag: N/A.
• Linked anchors: verified — #15573's route paths are genuinely exercised by the same spec file, so citing them is not borrowed authority.

Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The durable lesson is about claim strength, not theming. The first head satisfied the ticket as written — make the white interval not-white — and its own opposing-theme measurement showed that "not white" is a weaker property than "matches the parent's actual theme," because a fixed dark canvas makes an explicitly light Workstation flash dark. Satisfying the literal AC would have shipped a second flash for half the users. When an AC names a symptom ("white flash"), check whether the fix's property is the symptom's negation or the actual invariant; the negation is usually weaker, and the opposing case is where you find out. The generalisable instrument is the one used here: measure the two opposing configurations, not just the reported one.

🎯 Close-Target Audit

• Close-targets identified: #16092
• For each #N: confirmed not epic-labeled — #16092 carries bug, ai, testing, core

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket (or parent epic) contains a Contract Ledger matrix
• Implemented PR diff matches the documented contract (no drift)

Findings: No ledger exists to match — I checked #16092's body directly and it carries no Contract Ledger matrix, and the PR body has no ledger section either. I am marking that unchecked rather than ticking it, and treating it as a note instead of a Required Action, with the reasoning stated so you can overrule it: §5.4 is binding on real drift, and the contract here is genuinely auditable without a matrix. The ?theme carrier, the globalThis.WorkstationBootstrap shape, and the fallback are each specified in prose and pinned behaviourally on both halves against the same five-case matrix — dark, light, invalid, missing, schemeList (spec lines 479-486 for the App Worker half, 448-478 for the prepaint half). A matrix would add auditability that the tests already supply. If you would rather the ledger existed on the ticket regardless, say so and I will not defend the omission.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line — L3 exact-head native macOS, L3 merged-dev replay required
• Achieved evidence ≥ close-target required evidence — L3 is achieved, not aspired to
• Residuals: the declared "Residual: post-merge replay only" is a re-run of already-achieved evidence against the merged base, not an unmet AC — so the [L3-deferred — operator handoff needed] annotation on the close-target is not triggered, and I confirmed #16092 carries none. Worth renaming: three Post-Merge Validation items labelled "Residual" invites a checklist requirement the situation does not actually create.
• Two-ceiling distinction: correct — native macOS capture is the real ceiling and was reached, with no sandbox excuse offered.
• Evidence-class collapse check: no collapse, and the instrument is right. The retained-frame matrix reports luminance from actual captured frames (13.7954 dark, 244.8636 light) across both opposing host-preference combinations, from first retained birth frame through settled viewport, with one navigationStart per row — that measures the thing the bug is about, rather than asserting a DOM attribute and calling it a paint.

Findings: Pass.

📡 MCP-Tool-Description Budget Audit

N/A — no OpenAPI surface in scope.

🧪 Test-Evidence & Location Audit

• Execution evidence: CI green at a5ff089b6c (12/12 SUCCESS); author receipts are per-surface and exact-head — 24/24 bootstrap + route authority, 9/9 five-beat e2e with a stable two-take semantic receipt, 10/10 native frames with a receipt SHA, both preflight modes.
• Reviewer falsifier: run, and it failed against me. Named concern — "the prepaint allowlist is a hardcoded copy of a derived set, so a third theme silently diverges." Method: located every occurrence of the theme identifiers, found MainNativeWindowRoute.spec.mjs already reads neo-config.json, then read lines 416-486 in full instead of asserting absence. Result: concern refuted — the divergence is guarded, indirectly. This is the third time today a "does the guard already exist?" check stopped me from publishing a false severity, and the pattern is always the same: I had a mechanism in hand and was one step from reporting its absence.
• Test location: pass — unit specs mirror their subjects (unit/apps/workstation/, unit/main/), e2e under e2e/workstation/.

Findings: Pass.

🔗 Cross-Skill Integration Audit

N/A — no skill files, conventions, or MCP surfaces touched.

📋 Required Actions

No required actions — eligible for human merge.

App-work mandates checked explicitly, since this is apps/**: no raw CSS and no CSS-in-JS is added anywhere in the diff (grepped for style=, .style., <style, <link, .css, cssText — clean). The <meta name="color-scheme"> is a UA canvas hint that governs the document background before author styles exist, which is the only instrument that can address a pre-stylesheet flash; it is not a stylesheet and is not the pattern the SCSS-only mandate forbids. No data-carrying surface is touched, so the Store-of-Models path is not in scope.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 90 - Authority moved upstream to an already-selected value rather than adding a parallel one; the resolution is exported and pure; the carrier is allowlisted at both ends. 10 deducted for the forced-duplication asymmetry, which is load-order-imposed and guarded but not self-explaining.
• [CONTENT_COMPLETENESS]: 84 - Deltas, Evolution, per-surface receipts, and a five-case matrix pinned on both halves. Deducted for no Contract Ledger on either the ticket or the PR, and for "Residual" labelling what is really Post-Merge Validation.
• [EXECUTION_QUALITY]: 94 - Object.hasOwn over in, frozen map, written value decoupled from input, document.currentScript.before() for exact ordering, four real mutation controls including the dark light delegation trap, and safe behaviour when windowConfigs or themes are absent.
• [PRODUCTIVITY]: 90 - Two commits where the first exists to be falsified, and the second lands the stronger claim with native proof. No scope creep.
• [IMPACT]: 86 - Closes the last product-pixel gate on the #15252 film, and the underlying property — child parity with the parent's active theme at birth — is what every future tear-out inherits.
• [COMPLEXITY]: 72 - Pre-module execution ordering, parser-blocking guarantees, UA colour-scheme semantics, and cross-window config plumbing, none of which are forgiving.
• [EFFORT_PROFILE]: Quick Win - Small source diff, disproportionate durability, and the evidence is the expensive part.

🌿 The first head answered the ticket and the second answered the problem. Building the weak version in order to measure it is how you found the difference — and it is why the fallback is safe rather than merely plausible.