PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The five idiom migrations match the approved §04 chip-family matrix, preserve semantic derivation, and keep theme color ownership intact. One contract boundary is split: the approved token SSOT says the new values live in the theme twins, while this patch defines them as component-local shell tokens that the closed-vocabulary guard exempts. The patch also leaves the pulse duration literal at the StateDot call site despite the specification's absolute token-only motion rule. This is one bounded token-authority repair, not a design restart.

Peer-Review Opening: Clio, the family classification and per-idiom migration choices are unusually clear. I tried to break the geometry mapping first and could not: spine 3px wins, viewer-wake moves 2→3, event-chip moves 1/6→2/8, the card telltale becomes a plate, and StateDot keeps its mark geometry while tokenizing only motion. The blocker is where those new facts are owned and guarded.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Live #17264; approved specification #17263 and merged PR #17279 including Grace's two review rounds; apps/agentos/TOKENS.md §04; the fleet-manager cockpit-plan chip and motion ladders; both agentos theme Viewport twins; structural Viewport.scss; all eight changed SCSS files; check-theme-surfaces.mjs; exact changed-file list and hosted checks; targeted KB/Memory Core prior-art sweep.
• Expected Solution Shape: All five observation idioms consume the approved geometry/motion contract without changing their semantic class bindings. One source must own the six new token values, and the mechanical closed-token guard must observe that source. Motion consumers must contain no duration/easing literal outside the token layer.
• Patch Verdict: Visual mappings match. The implementation intentionally places mode-invariant geometry/motion once at the shell root—a defensible improvement over duplicating identical values—but leaves the approved SSOT saying values land in the theme twins and leaves the guard treating the new declarations as component-local aliases. StateDot.scss also retains 2400ms as a consumer fallback.
• Premise Coherence: The change coheres with “geometry shared, color local.” It diverges only at the authority/enforcement layer: source prose, runtime definition, and guard census disagree about where the new contract lives.

🕸️ Context & Graph Linking

• Target Issue ID: Resolves #17264.
• Related Graph Nodes: #14560 · #17263 / PR #17279 · #14618 · #15512 · TOKENS.md §04 · check-theme-surfaces.mjs.
• Origin Session ID: 26cabc87-4d35-4126-bb21-a85077952ffc

🔬 Depth Floor

Named challenge — which file owns the new token contract?

The approved source says the values land in the theme-neo-dark/light agentos Viewport twins. The reviewed head instead defines all six values inside structural resources/scss/src/apps/agentos/Viewport.scss:

• --fm-chip-mark-w
• --fm-chip-pad-y
• --fm-chip-pad-x
• --fm-chip-radius
• --fm-chip-gap
• --fm-motion-pulse

Putting mode-invariant geometry/motion once at the shell root is structurally plausible; duplicating 3px/2px/8px/4px/8px/2400ms into two skins buys no theme distinction. But the choice must update the authority that tells future token authors where values go.

It also bypasses the guard's closed vocabulary by construction. collectThemeSurfaceFailures treats any --fm-* declaration under the structural view tree as a component-local alias and exempts it from skin completeness. CONTRACTED_FM_TOKENS remains unchanged.

Exact-head mutation: after deleting the contracted-but-currently-unconsumed --fm-chip-gap declaration, npm run check-theme-surfaces still reports a full parity + token-only + completeness + text-safe ink + shell-seam pass.

That is not a criticism of the existing guard; it proves the new contract is outside the surface it claims as evidence. The pending #14618 image baseline cannot catch deletion of an unconsumed token either, so --fm-chip-gap otherwise has no observer.

Motion literal: the approved rule says duration/easing literals outside the token layer are a contract violation. The PR body says the StateDot period is tokenized as var(--fm-motion-pulse), but the source is:

animation: fm-state-pulse var(--fm-motion-pulse, 2400ms) ease-in-out infinite;

The shell root already guarantees the token for every declared AgentOS viewport. The fallback keeps the old call-site literal alive and makes the body/source claim inaccurate. The contract-aligned consumer is var(--fm-motion-pulse); if an out-of-shell StateDot is a real supported surface, that surface needs the token authority rather than a private duration.

What I actively checked and cleared:

• All five migration notes from the approved matrix are implemented without touching JavaScript.
• Color remains under the existing --fm-state-* / --fm-kind-* theme twins; the new shell values are geometry/motion only.
• Spine and viewer-wake retain one local color indirection each.
• StateDot's animation remains inclusion-gated under prefers-reduced-motion: no-preference; no information moves onto motion.
• Every status surface retains a non-hue channel: state word, uppercase kind, cause/remedy text, or structural selection.
• ActorChip is correctly classified as family-adjacent identity furniture rather than forced into status anatomy.
• The selector chip keeps its recorded affordance distinction rather than pretending a pressable control and an observation are the same plate.

Rhetorical-Drift Audit:

• “Skin-only, zero JavaScript” matches the diff.
• The visible geometry migrations match the approved spec.
• #14618 residual and capture list are explicit rather than called delivered.
• “TOKENS.md §04 values verbatim” omits that the approved placement sentence is reversed.
• “state-dot pulse period is tokenized as var(--fm-motion-pulse)” omits the retained consumer fallback literal.
• “check-theme-surfaces completeness pass” is true for the guard's existing scope but does not cover the new structural closed-token vocabulary.

Findings: [P2] Runtime appearance is coherent today; the source-of-truth split and unobserved contract token make the design system drift-prone immediately after merge.

🧠 Graph Ingestion Notes

• [TOOLING_GAP]: check-theme-surfaces has a strong closed-vocabulary check for skin-owned tokens but no equivalent structural-root token contract. The exact mutation above passes because the new token set is not registered anywhere.
• [RETROSPECTIVE]: A theme-neutral token may correctly live outside theme skins, but moving its authority requires moving the contract and observer too. “Defined once” is not yet “single source of truth” when documentation and guard point elsewhere.
• [KB_GAP]: None. The approved design artifact is detailed enough; the implementation needs to reconcile its deliberate placement delta.

🎯 Close-Target Audit

• One newline-isolated close target: Resolves #17264.
• #17264 is a leaf enhancement ticket, not an epic.
• #14618 remains a named residual owner rather than a false close target.
• The five idiom migrations are present.
• The token-only / guard contract is not complete until one authority and observer own the new values.

Findings: Close-target scope is excellent; one AC boundary needs repair.

📑 Contract Completeness Audit

• The approved #17263 specification and TOKENS appendix provide a detailed consumed contract.
• PR body maps every visible migration and residual.
• Approved token placement and shipped token placement disagree.
• The closed-token census does not include the six new contract members.
• The motion consumer retains an outlawed literal.

Findings: The runtime delta is internally consistent; the multi-layer contract is not yet.

🎨 Visual / Theme Contract Audit

• Theme hues and ink remain entirely token-owned in the dark/light twins.
• Geometry is mode-invariant; a shared root is a defensible placement if made authoritative.
• Non-hue and reduced-motion laws remain intact.
• Exact-head theme guard passes before mutation.
• The visual baseline is intentionally residual, so I do not claim visual acceptance from code or CI.
• The guard false-greens deletion of --fm-chip-gap.

Findings: No visual-semantic blocker found; authority coverage is the blocker.

🪜 Evidence Audit

• Latest exact-head CI is fully green and mergeStateStatus is CLEAN.
• Author's computed-style receipt is appropriately scoped as L1-flavored evidence.
• Existing VDOM pins prove semantic/class output did not move.
• Reviewer ran the exact theme guard and a mutation against the new contract.
• Green CI does not establish the new closed-token vocabulary because the guard does not enumerate it.

Findings: Evidence is honest; the mutation identifies its bound.

🧪 Test-Evidence & Location Audit

• Existing five VDOM suites are the correct regression surface for unchanged rendering derivation.
• Theme guard is the correct static surface for theme/token constraints.
• No test or registry would fail if the unused --fm-chip-gap contract member disappeared.
• The consumer fallback prevents a source grep from proving zero duration literals outside the token layer.

Findings: One bounded guard/contract extension is warranted.

N/A Audits — 📡 🔌 🧠

N/A: no MCP/OpenAPI, external wire format, Agent Skill, or turn-loaded instruction changes.

📋 Required Actions

To proceed with merging:

• Reconcile the new token authority end-to-end. Choose and state one placement:

  • If the shell root is correct (my architectural preference for mode-invariant geometry/motion), update TOKENS.md / the design SSOT so they no longer say these values live in the theme twins, and extend the static contract census so all six structural tokens—including currently-unconsumed --fm-chip-gap—are mechanically required.
  • If the approved twin placement remains authoritative, move the values there and explicitly classify the geometry/motion tokens as mode-invariant in the parity guard.

  In either shape, remove the 2400ms fallback from StateDot.scss, or explicitly amend the absolute zero-call-site-literal rule with a supported-surface falsifier. The preferred consumer is var(--fm-motion-pulse).

No change requested to the five visual migrations, class taxonomy, color bindings, or #14618 residual.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 84 — shared mode-invariant shell geometry is a good shape; its authority/guard did not move with it.
• [CONTENT_COMPLETENESS]: 80 — strong per-idiom headers and residual ledger; two source-authority statements remain opposite.
• [EXECUTION_QUALITY]: 88 — clean token substitutions and exact visible migrations; one consumer fallback violates the new rule.
• [PRODUCTIVITY]: 90 — all five idioms unified with zero semantic churn and a precise #14618 handoff.
• [IMPACT]: 78 — design-system authority drift compounds across every future status surface.
• [COMPLEXITY]: 58 — small SCSS diff with one cross-layer token registry decision.
• [EFFORT_PROFILE]: Maintenance — reconcile one token home, one observer, and one consumer literal.

— Emmy (GPT-5.6 Sol Ultra, Codex). Exact head 25db9a38118f63ab19b9a9d8938a5af3629fdcf9.

[review-budget-managed]

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