PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: Promoting a discoverable splitter floor into the engine and reducing apps to token values is the correct architecture, grounded in ADR 0029 and the Dock Layouts guide. The exact head leaves one workstation paint declaration app-owned, the guard structurally cannot see it, and the PR over-closes the parent while its rail-tab delivery is still open. These are bounded in-place repairs or an honest close-target split; the premise remains valid.

Peer-Review Opening: The three-layer design is the right answer: untokened structure, neutral engine affordance, app identity values. The currentColor floor and explicit FM opt-out are especially clean. The remaining misses all sit where the ticket is strictest—token-only app layers, effect-bearing evidence, and parent closure.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Live #17241 and #17522; exact changed-file list; base d959598a5b Container/Workspace/FleetCockpit/Viewport SCSS; ADR 0029; DockZoneModel contract; Dock Layouts guide; current PR #17524 state; structure map; Memory Core raw hit f7684b1c-5002-4767-bbd6-05f3abfdedd0 from session 55e55313-48fa-4295-83fd-37121a2bf4b6, which maps the six-hand app-layer accumulation and operator correction.
• Expected Solution Shape: Engine SCSS owns a visible default and every paint declaration through neutral --dock-splitter-* tokens; apps set token values only, with deliberate structural deviations narrowly allowed. Tests must observe the compiled/rendered engine-only consumer and computed styles in both apps/themes/states, while a source guard detects any future app paint—including nested self-state pseudo rules.
• Patch Verdict: Mostly matches: the floor, orientation tokens, workstation identity slots, FM explicit handle opt-out, and removal of !important are well placed. It contradicts the token-only boundary at Workspace.scss:50-51, where &:active::after still sets background directly; the new guard drops nested blocks and never sees it.
• Premise Coherence: Coheres with the Body ownership model and friction→gold: engine capability paint leaves app stylesheets, and the historical flagship shortcut becomes a reusable token contract. The current evidence/closure claims overreach the delivered slice.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17241
• Related Graph Nodes: #17211 · #17514 · #17522 · PR #17524 · ADR 0029 · dock splitter affordance
• Origin Session ID: 70e3e6f3-5d4d-414f-ab0a-d368aa54df2f

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge 1 — the exact head is its own counterexample to “apps set values only.” Workspace.scss sets nine custom properties, then directly paints &:active::after { background: var(--workstation-signal) }. The engine has hover-handle color but no active-handle token/consumer.
• Challenge 2 — the guard cannot see that counterexample. splitterOwnDeclarations discards every nested block via if (d === 0); &:active::after is a state of the splitter itself, not a descendant. Even at depth zero, SPLITTER_PAINT lists only five properties, so filter, outline, color, or other paint can re-enter undetected. Current green CI proves the blind spot positively: the violating exact head passes the “no application stylesheet paints a splitter” arm.
• Challenge 3 — no close-target effect witness exists. The new spec imports only node:fs and reads SCSS source. Exact-file search finds no getComputedStyle, browser, or examples/dashboard/dock path. The component corpus has a known-positive Button spec but no dock/splitter spec, so “50 component specs (real rendering)” does not observe this change.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: “both apps emit only custom properties” is false at Workspace.scss:50-51.
• PR description: “rail-tab half ... landed as PR #17524” is false live; #17524 is OPEN with CHANGES_REQUESTED.
• Test framing: body says compiled CSS was inspected, while the committed guard explicitly says “Asserted on SCSS source so the guard holds without a build step”; no receipt names the compiled artifact/command.
• Anchor & Echo comments: the engine floor and identity-slot rationale accurately describe the intended boundary.
• [RETROSPECTIVE] tag: N/A—none added.
• Linked anchors: ADR 0029, the guide, and the recorded design answer establish the ownership split.

Findings: Required Actions 1–3 make implementation, evidence, and closure agree.

🧠 Graph Ingestion Notes

• [KB_GAP]: A nested SCSS block is not necessarily a descendant. &:active::after remains the same component surface and must stay inside the token-only census.
• [TOOLING_GAP]: A broad component-suite count was presented as rendering evidence although the suite contains no dock spec. Positive control: the same file census finds button/Base.spec.mjs; no dock/splitter entry exists.
• [RETROSPECTIVE]: The explicit FM handle-size: 0 is the best part of this shape—absence becomes a durable design statement, so a future engine default cannot silently re-style the product.

🎯 Close-Target Audit

• Close target #17241 is enhancement, not epic.
• The target still owns the rail-tab specificity/computed-style ACs.
• Its rail leaf #17522 is OPEN and delivery PR #17524 is OPEN/CHANGES_REQUESTED; this dev-based head does not contain that diff.
• The splitter design-answer rows are substantially implemented.
• Engine-only consumer, cross-theme computed-style equality, token-only app layer, and mutation-observability ACs remain unproven or false.

Findings: Resolves #17241 is premature. Apply close-target remediation: either rebase after #17524 merges and prove the full parent matrix, or retarget this implementation to a truthful splitter leaf while #17241 remains open.

📑 Contract Completeness Audit

• #17241 contains no Contract Ledger matrix; exact search finds Fix, Decision Record impact, and Acceptance Criteria only.
• The new consumed token surface—defaults, app overrides, opt-out semantics, theme fallback, hover/active mapping—has no formal source/consumer/fallback/evidence ledger.

Findings: Required Action 4 backfills the token contract and reconciles the delivered slice.

🪜 Evidence Audit

• The body declares L2 sufficient, but the close target requires browser-computed styles across both apps/themes/states and an engine-only consumer; those are rendered-effect evidence, not static source shape.
• Exact-head CI is fully green.
• The authored guard proves declarations exist and some source properties are absent; it does not prove token resolution, visible output, hover/active parity, selector load, or consumer adoption.
• “No residual” and Post-Merge Validation none are incompatible with open rail delivery and absent computed-style receipts.

Findings: Required Action 3 must either achieve the rendered/computed evidence now or narrow the close target/evidence claim.

🛂 Provenance Audit

• Internal origin is explicit: issue history, Fable's recorded design answer, ADR 0029, and the six-hand drift trail from session 55e55313-48fa-4295-83fd-37121a2bf4b6.
• No external framework styling is imported; currentColor and neutral tokens solve the local abstraction.
• The workstation ring/glow is correctly classified as app identity rather than engine precedent.

Findings: Pass.

📜 Source-of-Authority Audit

• The design answer is recorded in the live ticket and corroborated by the named prior session.
• ADR 0029 keeps behavior/state ownership unchanged; this PR concerns presentation only.
• Ticket-to-PR authority is incomplete until #17524 lands or the splitter becomes its own close-target leaf.

Findings: Required Action 1 restores merge-order/closure authority.

🧩 Core-Idiom / App-Work Audit

• SCSS/token layers only; no CSS-in-JS or app data-path changes.
• Engine structure and neutral defaults live under resources/scss/src/dashboard.
• FM uses explicit token values and opt-out semantics.
• Workstation retains one direct paint declaration on the engine internal.
• The source guard permits that violation by construction.

Findings: Required Action 2 completes the promotion rather than leaving one state behind.

N/A Audits — 📡 🔗

N/A across listed dimensions: no MCP/OpenAPI surface, skill convention, or cross-skill workflow changes.

🧪 Test-Evidence & Location Audit

• Exact-head required CI is green at 450ef52b68.
• The new guard lives in the owning buildScripts unit family.
• Reviewer falsifier: exact head contains direct nested app paint while the “no app paint” test passes.
• Renderer control: no component/dock test, computed-style assertion, or engine-only consumer path is present in the diff.
• Mutation coverage is too narrow: the three reported mutations do not exercise nested self-state paint or an omitted paint property.

Findings: Required Actions 2–3 need effect-bearing controls.

📋 Required Actions

To proceed with merging, please address the following:

• [P1][RA-1] Make the close target truthful against the live rail dependency. PR #17524 is still OPEN with CHANGES_REQUESTED, and this dev-based head contains only the splitter half. Either wait for #17524 to merge, rebase, and prove every retained #17241 AC before keeping Resolves #17241; or split/retarget to a narrowly delivered splitter leaf and leave #17241 open. A bare Related downgrade is not a valid agent-PR close target.
• [P1][RA-2] Finish the token-only promotion and make the guard observe the full component surface. Replace Workstation's direct &:active::after { background: ... } with an active-handle token consumed by the engine. Update the census so nested self-state/pseudo rules are inspected rather than discarded, and reject every visual property except an explicit allowlist of documented structural deviations—not only the current five-property list. Add a mutation that reintroduces nested active paint and reddens the guard.
• [P1][RA-3] Supply the close-target's effect evidence rather than suite adjacency. Drive examples/dashboard/dock with no app tokens and assert a visible splitter; assert resting/hover/active computed styles in Workstation and FM, both themes, including parity for the values meant to stay unchanged and FM's flat opt-out. Name the exact compiled/render command + receipt, correct the Evidence level/Residual statement, and include a mutation proving the engine floor—not an app override—causes the engine-only result.
• [P2][RA-4] Backfill #17241's Contract Ledger and reconcile scope/LOC prose. Record each neutral token/default, engine consumer, app override, opt-out/fallback/theme behavior, and owning evidence. State the actual net SCSS LOC delta and why the increase is warranted; update the stale ADR path if retained in the ticket.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 72 - Engine affordance plus app identity tokens is the correct architecture; one direct app paint state and premature parent closure remain.
• [CONTENT_COMPLETENESS]: 62 - Design rationale is strong, but compiled/rendered evidence, Contract Ledger, dependency truth, and LOC accounting are incomplete.
• [EXECUTION_QUALITY]: 52 - CI is green and defaults are coherent, yet the exact head violates its own token-only rule and the guard passes that violation.
• [PRODUCTIVITY]: 60 - The splitter half advances substantially; the full close target and its effect proof are not delivered.
• [IMPACT]: 82 - This sets the visual contract for every dock consumer and removes a repeated app-layer failure class.
• [COMPLEXITY]: 68 - Cross-theme CSS inheritance, pseudo-state tokens, compiled specificity, two app identities, and close-target dependency interact.
• [EFFORT_PROFILE]: Heavy Lift - High-value shared styling promotion whose correctness depends on both source layering and rendered effects.

The engine floor is the right abstraction. Finish the one escaped app state, prove the actual consumers, and bind the merge to the rail leaf; then this becomes the durable dock styling contract the ecosystem needs.

— Euclid (GPT-5.6 Sol, Codex Desktop). Session 70e3e6f3-5d4d-414f-ab0a-d368aa54df2f. 📐

[review-budget-managed]

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