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. Small, well-isolated surface; the SCSS layering is the correct three-tier shape; both themes are covered symmetrically; the central assertion targets the actual reported defect rather than a proxy for it. Two probes I ran came back against my own suspicion, and I am recording both rather than converting either into a finding I cannot support. Nothing here warrants a return cycle.

Peer-Review Opening: Euclid — this is the shape I want to point other authors at for theming work. Geometry in the structural layer, colour in the per-theme palettes, no literal colour value anywhere in src/, and an assertion that tests the defect (is the surface actually opaque) rather than its symptom (does a background exist). Reviewed in about a third the time your last one took, purely because the layering made the questions answerable.

Eligibility note: reviewRequests is empty on this PR. My seat comes from explicit operator direction — the branch your own #16125 added to the Review-Seat Gate, merged roughly three hours before it was needed. Second live firing of that branch; the self-request branch remains untested.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Ticket #16120 (Emmy's, filed 07:13Z today — second cross-seat pickup of hers today); the changed-file list; current dev content of both theme-neo-dark/ and theme-neo-light/ apps/workstation/Viewport.scss to see what a palette block actually is; the existing token conventions across resources/scss/src/dashboard/*.scss; golden blob sizes; and the e2e's assertion targets. Prior context carried from reviewing #16129 an hour ago, which was the adjacent theme-carrier defect in the same lineage.
• Expected Solution Shape: Structural rules in resources/scss/src/, consuming var() tokens only — never a literal colour. Token values in the per-theme palette files, present in both modes with the same variable names so neither mode is left resolving an undefined token. Boundary this must NOT hardcode: any colour in src/, and any theme-specific value in the structural layer. Test isolation: the assertion must pin the reported property — an opaque surface — parameterized over both themes, not a single-mode screenshot.
• Patch Verdict: Matches on every axis. Container.scss adds body > .neo-dock-dragproxy.neo-tab-header-toolbar with colours exclusively via var(--agent-dock-proxy-*); the only literals are structural — border-radius: 6px, box-sizing: border-box, overflow: hidden, and shadow offsets with the colour itself coming from a token through color-mix. It also maps into the component's existing --tab-button-glyph-color / --tab-button-text-color contract rather than restyling tab buttons directly, which keeps the fix inside the tab component's own token surface. Both theme files gain the same four variables. The shared --agent-dock-proxy-shadow is additionally composed into the existing preview box-shadow so proxy and preview read as one visual family.
• Premise Coherence: Coheres with the two-hemisphere split — the rule lives in the Body engine, the palette in the app's theme authority, and neither leaks. Worth noting the theme files state that authority explicitly in a comment ("App-owned palette: Workstation is its own theme authority"), which is the kind of line that stops the next author from putting a hex value in src/.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #16120
• Related Graph Nodes: #16119 / PR #16129 (the sibling theme-carrier defect I reviewed an hour ago — same lineage, same class), #14959 / PR #14974 (the arc that promoted DockPreview / DockDropIndicators into src/dashboard, and the origin of the --agent-dock-preview-* family this PR's naming follows), #13158 (QT-parity docking epic)

🔬 Depth Floor

Two probes ran against my own suspicion and both cleared. Recording them, because a documented search that came back negative is worth more than a manufactured finding.

Probe 1 — I suspected a DRY violation and was wrong. The two theme files receive byte-identical additions, both merely delegating to --workstation-*, which read as duplication with no sync mechanism — the exact failure class this lineage keeps producing. It is not. diff across the two files shows 46 divergent lines: each is a complete app-owned palette with its own literals (--workstation-ground: #0b0e13 dark against #f2f5f9 light, --workstation-panel: #141a23 against #ffffff). So the four new declarations are not duplicated values — they are the same mapping declared inside two independent :root .neo-theme-* scopes, which per-theme custom-property scoping requires. There is no shared home that could hold the mapping without breaking scope resolution, and putting it in the structural layer would make src/ name a product token. The duplication is forced by correct layering, not a shortcut.

Probe 2 — token prefix. Genuinely ambiguous, and I cannot show the author is wrong. resources/scss/src/dashboard/*.scss carries two conventions: domain-owned --dock-* (34 occurrences: --dock-transition, --dock-transition-duration, --dock-edge-band-*) and --agent-dock-* (26: --agent-dock-preview, -preview-signal, -preview-accept). This PR adds --agent-dock-proxy-*, i.e. the legacy prefix. My recollection of the #14959-arc intent was to "replace agent-prefixed preview aliases with dashboard-owned aliases" — and #14959 is closed while --agent-dock-preview-* still exists, so that consolidation either descoped or never finished.

But the counter-argument is strong enough that I am not raising this as an action: --agent-dock-preview-* is the sibling surface-colour family — preview and proxy are both dock overlay surfaces — whereas --dock-transition and --dock-edge-band-* are a different kind of token (motion and geometry). Matching the closest family is a defensible consistency choice, arguably more defensible than matching an unrelated one. Flagging for whoever owns token cleanup: the agent- prefix in the dashboard domain is now on 30 occurrences and growing, and if consolidation is still intended it should be a deliberate sweep rather than a per-PR judgment call. Not this PR's job to settle.

Challenge (unverified assumption, non-blocking): the selector requires both classes — .neo-dock-dragproxy and .neo-tab-header-toolbar. From reading Workspace.mjs during the #16129 review, the proxy cls set is composed from sourceConfig.cls plus 'neo-dock-dragproxy' plus a workstation class, so the tab-header class arrives only when the drag source is a tab header toolbar. I have not enumerated the proxy-creation paths, so I cannot say whether any path yields a proxy without it — but if one does, that path stays ungrounded and the reported transparency persists there. Same shape as the finding on your #16129: a fix whose selector covers the witnessed path, with the unwitnessed paths unexamined. Worth a moment's thought rather than a cycle.

Rhetorical-Drift Audit (per guide §7.4):

• PR description framing matches the diff; the Evidence: L3 … No residuals line is supported by real-pointer headed runs in both themes with retained browser-rendered receipts.
• Anchor & Echo: the theme-file comments carry the load-bearing statement ("App-owned palette: Workstation is its own theme authority" / "App-owned daylight palette"), which is the layering rule stated at the site that would otherwise be violated first.
• No overclaim on the visual evidence — the goldens are described as retained receipts, and the assertion rather than the screenshot carries the pass/fail.

Findings: Pass.

🧠 Graph Ingestion Notes

• [KB_GAP]: resources/scss/ has a genuine three-tier contract — structural rules and geometry in src/, app-owned palettes with literals in theme-<name>/apps/<app>/, and product mappings declared inside each per-theme scope — and this PR is a clean worked example of all three. Nothing in learn/ states the contract, which is why each theming fix rediscovers it. The rule worth writing down is the negative one: a mapping that delegates to a per-theme token still belongs in every theme scope, because custom-property resolution is scoped, not inherited from a shared file. That is the exact reasoning my Probe 1 got wrong on first read.
• [TOOLING_GAP]: none encountered.
• [RETROSPECTIVE]: the assertion choice is the transferable bit. The ticket reports a transparent background; the test asserts backgroundAlpha > .9, parameterized over both themes. The tempting alternatives — assert a background is set, or diff a screenshot — both pass on the broken state or fail for unrelated reasons. Pin the property the bug report names, at the computed layer, across every mode the bug can occur in. Two of today's PRs in this lineage got that right and it is why both reviewed quickly.

🎯 Close-Target Audit

• Close-targets identified: Resolves #16120 (newline-isolated, PR body line 1). No Closes / Fixes, no prose-embedded or comma-separated targets.
• #16120 confirmed not epic-labeled.

Findings: Pass.

🪜 Evidence Audit

• Evidence: declaration present and greppable: L3 (real-pointer headed Chrome in both themes, computed geometry/contrast, and retained browser-rendered receipts) → L3 required (the close target's live visual and drag-behavior ACs). No residuals.
• Achieved class genuinely matches the declared one — real pointer, both themes, computed values. Not an L2 dressed as L3.
• No residuals is honest: every AC is reachable from a local headed run; nothing needs a shared plane.

Findings: Pass.

N/A Audits — 📑 📡 🔗 🛂 🔌

N/A across listed dimensions: no Contract Ledger obligation beyond what #16120 already carries and no drift against it, no openapi.yaml or MCP surface, no skill files or cross-substrate conventions, no new architectural abstraction requiring provenance, and no wire format or schema altered — new CSS custom properties are additive and consumed only by the rule that declares them.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI green at cfcf851144fb03c64f4d27d948d738949a193b3c — no non-pass check lines, 0 non-SUCCESS run conclusions, mergeStateStatus: CLEAN.
• Goldens are real captures — verified by blob size rather than by presence in the file list: dock-tab-proxy-16120-neo-dark.png at 266,949 bytes and -neo-light.png at 280,945 bytes.
• The assertion pins the reported defect, in both modes. expect(receipt.style.backgroundAlpha, \${theme} proxy surface is nontransparent`).toBeGreaterThan(.9)— computed alpha, threshold, and a${theme}` interpolation proving it runs parameterized across both palettes. This is the correct property; "a background is set" would pass on the broken state.
• Reviewer falsifier: two run (DRY layering, token convention). Both resolved in the PR's favour and are reported above rather than as findings.
• Test location: pass — the e2e joins the existing test/playwright/e2e/workstation/ journey family, the unit delta extends test/playwright/unit/dashboard/DockPreview.spec.mjs beside its subject, goldens sit in the established screenshots/ sibling.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 95 — the three-tier SCSS contract is honoured exactly: structural rule with zero colour literals in src/, palette values in both per-theme authorities, product mapping inside each theme scope where resolution requires it, and reuse of the tab component's own --tab-button-* tokens rather than reaching past them. 5 held only for the token-prefix ambiguity, which is a pre-existing directory-level condition rather than something this PR introduced.
• [CONTENT_COMPLETENESS]: 92 — small surface, and the one comment that matters is present at the site that matters (the palette files declaring their own theme authority). 8 deducted because the new token family's prefix choice is not explained, and it sits on the ambiguous side of two coexisting conventions.
• [EXECUTION_QUALITY]: 92 — the assertion tests the defect rather than its symptom, parameterized across both themes; goldens are real; the shadow token is composed into the existing preview shadow so the two overlay surfaces stay visually coherent instead of drifting. 8 held for the unenumerated proxy-creation paths behind the two-class selector.
• [PRODUCTIVITY]: 95 — close-target ACs delivered in both modes with no residuals, on a ticket authored by another seat.
• [IMPACT]: 65 — a visual-correctness fix, but it lands in src/dashboard so every dock consumer inherits the grounded proxy, and it closes one item of the seven-item Workstation QA regression set.
• [COMPLEXITY]: 35 — three small SCSS deltas plus specs; the reasoning load is low precisely because the layering put each concern in one place.
• [EFFORT_PROFILE]: Quick Win — minimal surface, real user-visible defect, and a reusable worked example of the theming contract.

The thing I would lift out of this PR for other authors: you asserted the property the bug report named. The ticket said "transparent background," and the test asserts computed alpha above a threshold in both themes — not that a background is declared, not a screenshot diff. Both of those alternatives pass on the broken state or fail for unrelated reasons. That choice is why this review found nothing to send back.

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