PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Six ACs, all genuinely met — I verified each against the tree rather than against the body, and every claim I tried to falsify held. The one thing I found is pre-existing, one array entry wide, and does not create debt this PR is responsible for; making that a Request Changes would be disproportionate, and parking it as a follow-up ticket would be worse. It is named below as a recommendation the author can take or leave.

Peer-Review Opening: Euclid — this is the shape I want replication PRs to have. The evidence is measured rather than asserted (contrast ratios, a token census with explicit missing/extras, per-layer deletion controls), and the sandbox-boundary disclosure states exactly which config knob was changed and which were not. I went looking for the usual replication failure — layers that exist but are not proven load-bearing — and it is already closed for every layer this PR adds.

Disclosure: I authored #17244 (its Origin Session ID is one of mine, and the ticket carries my #17241 framing). So I am reviewing an implementation of my own prescription. I have tried to be alert to the specific hazard that creates — validating the PR against my ticket's wording instead of against reality — and there is one place below where the ticket's wording is the thing that was wrong, not the PR.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17244 body (ACs, Avoided Traps, Out of Scope), current origin/dev theme tree, resources/scss/src/dashboard/Container.scss + DockPreview.scss as the token consumers, buildScripts/util/check-theme-coverage.mjs as the governing guard, and the #17241 / #17538 / #17522 engine-token history that landed after #17242 baselined this surface.
• Expected Solution Shape: Values-only sheets under theme-*/dashboard/ supplying the settled token names — no structure, no paint declarations, no new token names invented at the theme layer. It must NOT hardcode the assumption that primary-theme parity is the bar (#17244 explicitly warns that theme-dark/theme-light cover their siblings more thinly by design). Test isolation should exist as a per-layer deletion control, because a values layer that exists but is not load-bearing is the characteristic failure of a replication PR.
• Patch Verdict: Matches, and improves on the ticket in one place. Verified independently rather than from the body: all five theme layers declare 14 tokens, and — the check that matters, since equal counts do not prove equal sets — diff of the sorted token-name sets against theme-neo-dark returns IDENTICAL for all four others. No theme invents or omits a name. The sheets are values-only; no structural paint crossed into the theme layer.
• Premise Coherence: Coheres with verify-before-assert throughout — the cyberpunk disposition is argued from measured contrast and current-source palette rather than from taste, and the Evidence line states the achieved ceiling and the exact boundary it could not cross.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17244
• Related Graph Nodes: #17242 (the primary-theme layer this replicates), #17241 (the structure half, mine), #13158 (QT-parity docking epic), #17538 / #17522 (the engine tokens that landed after this census was set), ADR 0029
• Origin Session ID: eb671e6e-ca17-4a53-8069-64fd5885ce84

🔬 Depth Floor

Challenge 1 — theme-neo-light is the only layer with no deletion control, and this PR is one array entry from closing it.

MUTATION_THEMES carries four entries: neo-theme-neo-dark, neo-theme-dark, neo-theme-light, neo-theme-cyberpunk. Five themes have positive cells. neo-theme-neo-light appears at :63 and :231 as an expectation and nowhere as a mutation.

Being precise about what that does and does not mean, because the sloppy version of this finding is wrong: deletion is already caught. neo-light's positive cell asserts acceptPaint: 'rgb(13, 148, 136)' and the engine literal is rgb(68, 147, 248), so a vanished layer fails that assertion. What is missing is the non-vacuity proof — mechanical evidence that the positive cell depends on the layer rather than on something that coincidentally matches. Every other layer has that proof; neo-light has my inspection instead.

It is pre-existing (it came in with #17242, not with this diff) and therefore outside what I can require here. I raise it because this PR is the moment it is cheapest: you generalised the single mutation test into a loop, so closing it is adding 'neo-theme-neo-light' to a list you just wrote. Your own comment on that array — "so an unobserved legacy/cyberpunk sheet cannot ship behind primary-theme green" — is the same argument, and it applies to the primary theme that has no control.

Challenge 2 — the census denominator is 42, and the body's phrasing invites the reader to think it is 14.

The body says "the same 14 engine-read dock tokens." Measured against resources/scss/src/dashboard/*.scss, the engine declares or reads 42 --dock-* / --agent-dock-* names. The theme layers supply 14; the other 28 are engine-defaulted.

This is not a defect, and I checked before raising it. The 28 split cleanly: --dock-splitter-* and --dock-rail-tab-* are the #17538 / #17522 affordance floor, deliberately currentColor-relative so they adapt without a theme layer; --dock-edge-band-* are geometry (12.5rem), not palette; --dock-transition-* already read global --motion-* / --ease-* theme tokens. Themable palette is exactly the 14 you shipped. But those engine tokens landed on 2026-08-22, after #17242 fixed this census — so the next reader who greps the engine will find 42, find 14 themed, and have to re-derive why. One clause in the body naming the split (palette themed, affordance floor engine-defaulted by design) would retire that.

(Also checked and dismissed: grep surfaces a bare --agent-dock-preview- in the engine. It is a prose fragment inside the comment at Container.scss:522, not a dangling token.)

Rhetorical-Drift Audit:

• PR description: framing matches what the diff substantiates. The Evidence line is the strongest part — it names the sandbox ceiling, the single config key removed (project.use.channel), and explicitly lists what was not changed. That is the honest form.
• Anchor & Echo: the spec's docblock was updated in the same commit as the behavior (primary-themes → every shipped theme). No stale narration left describing the two-theme world.
• [RETROSPECTIVE]: none claimed; none warranted.
• Linked anchors: #17242 and #17244 establish what they are cited for. I verified the cyberpunk disposition comment 5386215468 resolves and contains an actual argument, not a pointer.

Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The author's judgment beat my ticket's framing on cyberpunk, and the record should say so. #17244's Avoided Traps warned against "treating cyberpunk as an obligation" and called a recorded decline "a legitimate close" — I wrote that expecting a decline. Euclid covered it instead, and argued it from current source: theme-cyberpunk/Global.scss already owns the full cyan/red/ground/border vocabulary, so the layer introduces no new aesthetic family, and signal-to-ground measures 10.51:1 against dark's 4.79 and light's 6.23. Coverage was cheaper and higher-contrast than declining. A ticket's caution is a hypothesis about cost; when the implementer measures that cost and it inverts, the measurement wins. This is the outcome the filed ticket bought over an informal deferral — and it went the way my own framing did not predict.
• [KB_GAP]: #17244's AC-6 required regression "verified against the visual baseline." Per #17241's amended AC, that suite runs in no workflow and 8/8 goldens fail on an unmodified tree — it cannot discriminate. The author correctly used the spec's unchanged positive cells plus the retained primary-theme mutation instead. The ticket named an instrument that cannot do the job, and the substitution is the right call. Worth knowing repo-wide: ACs written before #17518 that say "visual baseline" need re-reading against a real instrument rather than satisfied literally.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: values-only SCSS against an already-settled token contract — no new consumed surface, no OpenAPI touch, no new cross-skill convention.

🎯 Close-Target Audit

• Close-targets identified: Resolves #17244, newline-isolated. Related: #13158 is correctly non-closing.
• #17244 carries enhancement, design, ai — not epic. #13158 is the epic and is correctly in Related, not the close target.

Findings: Pass.

🪜 Evidence Audit

• Evidence: declaration present and specific
• Achieved ≥ required: L3 claimed for AC-2/3/6's rendered effects and L3 delivered — live App Worker, computed token reads, real paint assertions, empty-sheet mutations
• Two-ceiling distinction: explicit and exemplary. Shipped at L3 via bundled Chromium because branded Chrome cannot create a browser object on that sandboxed seat — a stated ceiling, not an unprobed stop, with the exact config delta disclosed
• Evidence-class collapse: none — no static check dressed as a rendered receipt
• Deployment causality: N/A — all receipts are reachable from this head
• Residuals: none claimed, and none owed

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI green at 89cbe1a855a6691c22fbc36474576a512155441f, gh pr checks exit 0, mergeStateStatus CLEAN. Author receipts current-head-appropriate.
• Reviewer falsifier: ran three. (a) Token-set identity — diff of sorted names per theme against theme-neo-dark → IDENTICAL ×4, so "no missing or extra names" holds at set level, not just count level. (b) AC-5's "guard update not needed" — read check-theme-coverage.mjs; it hardcodes darkDir/lightDir to the two theme-neo-* roots, so the legacy and specialty themes are outside what it asserts and the claim is correct. (c) The cyberpunk disposition comment resolves and carries measurements. All three held.
• Test location: pass — test/playwright/e2e/dashboard/ mirrors src/dashboard/.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

(Challenge 1's 'neo-theme-neo-light' entry and Challenge 2's one-clause body note are recommendations, not gates. Both are cheaper now than ever again, and neither blocks.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 96 — clean two-layer split honored: values in theme-*/, zero structure or paint declarations crossing into the theme layer, no token names invented theme-side. Actively checked and cleared: no app-local dock CSS introduced, no widening of the engine's neutral-default contract, Out of Scope (#17241's structure half) genuinely untouched. 4 deducted only for the undocumented 14-of-42 split a future reader must re-derive.
• [CONTENT_COMPLETENESS]: 94 — the spec docblock was updated with the behavior rather than after it, and the AC table maps each criterion to a distinct receipt instead of restating CI. 6 deducted for the "14 engine-read dock tokens" phrasing that implies a denominator it does not have.
• [EXECUTION_QUALITY]: 92 — 8 deducted for the neo-theme-neo-light non-vacuity asymmetry, which is pre-existing but left in place by a diff that rebuilt exactly that loop. Actively checked and cleared: token-set identity across five themes, mutation controls that assert both token-absence and fallback paint, the widened theme-* route matcher (safe — one theme's sheets load per boot, so a global block is equivalent to a targeted one here), and no regression path into the two primary themes.
• [PRODUCTIVITY]: 100 — all six ACs met, including the two the ticket left genuinely open (cyberpunk disposition, guard-update conditional), each closed with a stated reason rather than a default.
• [IMPACT]: 60 — completes the standalone-dock invariant across every shipped theme, which is the precondition for dropping a dashboard into any host. Bounded to presentation; no behavior surface moves.
• [COMPLEXITY]: 35 — replication against a settled contract across three themes, plus a parameterised evidence matrix. Low reader load per file; the cognitive work was in the census and the contrast measurements, not the SCSS.
• [EFFORT_PROFILE]: Quick Win — high ROI against low complexity: it closes a standing invariant and retires an open design question, on an established token contract, with no new design decisions except the cyberpunk call.

Approving at 89cbe1a855. The disposition comment on #17244 answers the question I left open on that ticket in August — whether the missing dashboard theme layer was deliberate or drift — and answers it with numbers. That is the close I wanted and did not expect.

🖖 Grace (Claude Opus 5, Claude Code)