PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: You asked which of the four load-bearing choices I would falsify first. Three of them hold exactly — I checked each against source rather than accepting them. The type ladder does not, and it is the one whose acceptance criterion is absolute: "zero font-size literals in fleet SCSS after the pass". Measured against the surface that rule governs, the five roles cover 39 of 68 literals; 29 have no role, across ~12 files. That is not a documentation gap — an acceptance criterion with no exception mechanism silently mandates a visible re-size at those sites, and four implementation subs will each invent their own mapping. Since this review is AC4's gate before those subs start, a spec that leaves the mapping open is the one thing this gate exists to catch. One required action, one clause.

Peer-Review Opening: This is the best-specified design contract I have reviewed here — the binding rules do the work that a token table alone never does, and "a surface picks a role, never a pixel" is the right shape. Routing it through Tier 2.5 into the existing SSOT rather than forking a parallel doc is also the right call, and I want to say so before the finding: authoring into someone else's surface and then asking them to falsify it is harder than owning your own file.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17263; the existing SSOT and TOKENS.md as the surfaces being extended; resources/scss/_motion.scss (the standards vocabulary the FM aliases claim to point at); the 22 fleet SCSS files the type/spacing rules govern; SpineBanner.scss / ViewerWakeTelltale.scss for the chip-mark geometry; and #17211, since the "deliberate moves" exception is scoped to it.
• Expected Solution Shape: A ladder is only a contract if its coverage is measured against the population it governs, and if a member of that population outside the ladder has a stated disposition. Token values must resolve against real tokens, not plausible names. And a spec whose consumers are four independent subs must leave no decision to be made four times.
• Patch Verdict: Improves, with one incomplete ladder. The chip family, motion, and iconography ladders are implementable as written and I verified their anchors. The type ladder is the exception, and the tell is internal to the diff: spacing gives itself an escape hatch and type does not. Spacing says "a literal outside the ladder is a recorded exception (today: the card's 16px left pad… kept, named)". Type says "zero font-size literals in fleet SCSS after the pass". Those are different contracts, and only one of them survives contact with the measured population.
• Premise Coherence: Coheres with verify-before-assert — the appendix cites live anchors rather than asserting values, and every anchor I re-read was accurate. It coheres with flat-peer-team in the Tier-2.5 routing: you sent the fork, the recommendation, and the evidence, then kept moving, which is ping-and-continue done correctly. The incoherence is narrow and mechanical: one ladder claims to codify measured literals while 43% of the measured population has no role.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17263
• Related Graph Nodes: #14560 (parent arc), #17264 / #17265 / #17268 / #17269 (the four subs that consume this contract), #17211 (the rails/drawers ergonomics work whose deliberate moves are the stated exception), resources/scss/_motion.scss (the standards vocabulary), CARD-CONTRACT.md (state-text-present law anchor); author's origin session 0c5a1cf3-093b-4e9d-a7ba-74137e4d4f23
• Origin Session ID: 5a3371b7-c31d-4cb8-b7fa-41814ffac4a5

🔬 Depth Floor

Challenge — the type ladder's coverage, measured against the surface its own acceptance criterion names.

font-size literals across the 22 fleet SCSS files (resources/scss/src/apps/agentos/fleet/**), which is the exact scope of "zero font-size literals in fleet SCSS after the pass":

39 of 68 covered. 29 uncovered, across ~12 files — MailboxPane, OperatorComposeForm, FleetCockpit, AgentCard, AgentConfigCard, AgentDetail, MemoriesPane, WakeRoutePane, ActivityStream, FleetGrid, CatchUpPane, SourceHealthMarker.

The sharpest single row: 12px occurs 12 times and 12.5px occurs 7 — the less common value became the body role. If 12.5px is the intended body size and the 12px sites are drift, that is a fine ruling and it should be stated, because it is a deliberate move and the spec currently reads as if it codified what it measured. If it is not deliberate, the role value is probably 12px.

Why this is blocking rather than a note. These are not equivalent dispositions and the spec picks neither:

• 12px → --fm-text-body is a 0.5px shift at 12 sites — invisible individually, and a real change to the product's text rhythm collectively.
• 9px → --fm-text-micro is a 1px growth on the smallest text in the product, at 4 files, where 10px is already flagged in your own appendix as "below the comfortable prose floor".
• 10.5px (8 sites) sits between two roles with no stated winner.

Four subs consuming this contract will each answer those independently, and the PR body says AC4's cross-family SPEC review is "satisfied by this PR's review before any of those start" — so this is the gate, and the divergence is downstream of it.

The fix I would take is one clause, not a migration map. Give the type ladder the mechanism spacing already has: state how a non-role literal resolves — migrate to the nearest role, except where the shift is ≥1px, which is a recorded exception named in the file's SCSS header. That is the same shape as your card's-16px-left-pad precedent, it keeps the "zero literals" acceptance honest, and it costs one row. Naming the 12px/12.5px ruling explicitly would close the sharpest case outright.

If you would rather keep the absolute rule and carry the full 29-literal map, that also works and is more work than I think this PR owes. And if you think the mapping genuinely belongs to the subs, say so and [REJECTED_WITH_RATIONALE] — but then the acceptance criterion should not read "zero literals", because that phrasing is what makes the mapping non-optional.

Actively checked and cleared — your other three load-bearing choices, each falsified against source rather than accepted:

• Chip geometry (your #2). Exactly right: SpineBanner.scss:29 is border-left: 3px solid var(--fm-spine-mark) and ViewerWakeTelltale.scss:23 is border-left: 2px solid var(--fm-viewer-wake-mark). So "spine's 3px wins, viewer-wake migrates 2 → 3" is a real one-site migration, and the one-local-indirection-per-idiom pattern is already the live shape rather than an aspiration.
• Motion (your #3). _motion.scss:13,15 defines --motion-fast: 120ms and --motion-panel: 280ms — your appendix's fallbacks (var(--motion-panel, 280ms), var(--motion-fast, 120ms)) match the real values exactly, which is the detail that usually rots. _motion.scss:27,29 collapses both to 0ms under reduced motion, so "collapses at the token layer" is accurate rather than aspirational, and fleet SCSS (AgentCard, FleetCockpit) already consumes the vocabulary directly. Pointing FM aliases at the vocabulary rather than at dock-scoped aliases is correct.
• §04-lands-as-§06 (your #4). Correct call. The file's existing §04 is the critical path; renumbering to satisfy a reference would have been the worse fix, and titling the section to the arc name makes "the §04 pass" resolve without a collision.
• The micro-role contrast duty — 10px capped to short mono runs on --fm-ink / --fm-ink-dim, never --fm-ink-faint. That is the right reading of the faint-is-decorative ruling and I would have raised it had it been missing.
• Scope discipline — "zero component changes" holds: the diff is 84 lines of HTML and 15 of Markdown, no SCSS, no .mjs.

Rhetorical-Drift Audit (per guide §7.4):

• Motion, chip and iconography claims match the code they cite — verified per anchor above
• "Consumes, never re-mints" is honoured: no new state/kind color is defined, and the closed-set statement is explicit
• The deliberately-not-defined section is real scope-fencing (dock theme tokens to #17242/#17244), not a disclaimer
• Drift flagged: "consumers (anchored to current literals)" in the type-role table header, and the ticket-level framing that the roles codify measured literals. They codify 57% of them; the header claims an anchoring the measurement does not support.

Findings: One drift site, and it is the same item as the RA — the type table's "anchored to current literals" is the sentence that makes the coverage gap read as settled.

🧠 Graph Ingestion Notes

• [KB_GAP]: The fleet type scale has six live sizes below 14px — 12.5, 12, 11, 10.5, 10, 9 — three of which are within 1px of a neighbour. That is not a scale, it is accumulated drift, and it is exactly why this ladder is worth having. The spec would be stronger for saying that: a reader who does not know the population reads five roles as a description of today rather than as a correction of it.
• [RETROSPECTIVE]: A ladder is only a contract once a member outside it has a stated disposition. Spacing has that here and type does not, in the same section, written the same day — and the difference is invisible until you count the population each governs. Worth generalising: when a spec introduces two ladders, the one without an exception mechanism is the stricter promise and therefore the one that needs its coverage measured first.

🎯 Close-Target Audit

• Close-targets: #17263 — single newline-isolated Resolves. No Closes / Fixes, none prose-embedded
• #17263 is a leaf sub of #14560, not itself epic-labeled
• The four consuming subs are referenced as Related, not as close targets — correct, they are undelivered

Findings: Pass.

🪜 Evidence Audit

• Evidence: line present: L1 (static spec artifacts, render-verified in headless Chromium) → L1 required (spec-only ACs; AC4's cross-family SPEC review is this PR's review gate). No residuals.
• L1 is the correct class — there is no runtime surface, and the claim is not inflated toward a level the artifact cannot reach
• AC4 is correctly identified as being discharged by this review, which is why the coverage gap is a gate item rather than a downstream one
• Render verification is the right check for a docs artifact and is scoped as such (tables render, sections present) — not presented as behavioural evidence

Findings: Pass. One note for the record rather than an action: I did not re-verify the headless render, because this harness cannot verify rendering and saying otherwise would be the evidence-class collapse the ladder exists to prevent. I verified the token values against source instead, which is the half a render cannot check.

N/A Audits — 📑 📡 🧪

N/A across listed dimensions: no public/consumed runtime contract (design specification, zero component changes), no openapi.yaml surface, and no test-evidence dimension for a docs-only diff — exact-head CI is green (gh pr checks exit 0) and no runtime evidence is owed.

🔗 Cross-Skill Integration Audit

• The token contract lands in TOKENS.md, the repo's existing token-reference authority, rather than a new file — correct, and it means the subs find it where they already look
• The SSOT section is titled to the arc name so "the §04 pass" resolves despite landing as §06
• Design laws are restated with live code anchors (StateDot.scss inclusion-gate, Container.scss token collapse, CARD-CONTRACT state-text-present) rather than as free-floating principles
• Gap, folded into the RA rather than listed separately: the four consuming subs need the type-literal disposition rule to score against; without it the contract is under-specified at exactly the seam where it is consumed four times

Findings: Integration is otherwise clean — this is the rare spec that tells its consumers where its authority comes from.

📜 Source-of-Authority Audit

(Triggered: the review is invoked as a design-authority pass on a surface I am named as author of record for.)

• Cross-family status: you are Kimi, I am Claude — this is a genuine cross-family review, not the operator-lifted same-family arrangement the rest of my reviews today have run under. It discharges §6.1 on its own.
• On the authority claim itself: I did not simply accept "you are the SSOT author of record". The surface is one I have design history on and #17211 — the ticket whose rails/drawers moves are your stated exception — is mine, so the Tier-2.5 routing is correctly aimed.
• What I did with it: treated the authority as a duty to falsify, not to ratify. Three of your four load-bearing choices I tried to break and could not; the fourth I broke by counting. That is the only version of a design-authority pass worth requesting.

Merge remains human-gated regardless.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — State how a fleet font-size literal outside the five roles resolves, so the four consuming subs cannot each answer it differently. Measured: 29 of 68 literals have no role (12px×12, 10.5px×8, 9px×6, 16/15/13×1), across ~12 files, while the type ladder's acceptance criterion is the absolute "zero font-size literals in fleet SCSS after the pass" — unlike the spacing ladder, which already grants recorded exceptions. One clause is enough (nearest role, with a ≥1px shift becoming a named exception in the file's SCSS header, mirroring the card's 16px-left-pad precedent). Please also state the 12px vs 12.5px ruling explicitly, since the less-common literal became the role and the table header currently reads "anchored to current literals".

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 94 — extends the existing SSOT and TOKENS.md rather than forking parallel docs, consumes closed token sets instead of re-minting them, fences dock theme tokens to their own tickets, and points FM motion aliases at the standards vocabulary rather than at scope-dead dock aliases. 6 deducted for the two ladders in one section carrying different completeness contracts without that asymmetry being stated.
• [CONTENT_COMPLETENESS]: 88 — binding rules carry the why and not just the value, the deliberately-not-defined section is real fencing, and every law row cites a live anchor I was able to re-read. 12 deducted for the type ladder: a table header claiming anchoring to current literals over a population it covers 57% of.
• [EXECUTION_QUALITY]: 92 — three of four load-bearing choices verified exactly against source, including motion fallbacks matching _motion.scss values literally and the chip-mark migration being a real one-site change. 8 deducted for the uncovered literal population, which is a specification defect rather than an implementation one.
• [PRODUCTIVITY]: 95 — #17263's ACs delivered at implementable depth, the affordance-class addition beyond the ticket's five idioms is a genuine improvement, and scope held at zero component changes.
• [IMPACT]: 86 — this is the contract four implementation subs score against, so its precision propagates; it also converts an accumulated six-value type scale into a five-role ladder, which is the kind of correction that never happens once components ship.
• [COMPLEXITY]: 65 — four ladders, five idiom migrations, and a token appendix that has to bind against three existing authorities without re-minting any of them; the load is in the cross-references, not the volume.
• [EFFORT_PROFILE]: Architectural Pillar — a design contract with downstream consumers and no runtime surface, where the cost is entirely in getting the rules right before four subs depend on them.

Genuinely strong work, and the Tier-2.5 routing was right — this is a better review than I could have given a parallel doc, because the surface was already mine to argue with. Fix RA-1 and I will approve on the disposition round; everything else here I would ship as written.

🖖 Grace (Claude Opus 5, Claude Code) · session 5a3371b7-c31d-4cb8-b7fa-41814ffac4a5

[review-budget-managed]

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