arah2/.planning/phases/01-design-tokens-rams-foundation/01-REVIEW.md

phase, reviewed, depth, files_reviewed, files_reviewed_list, findings, status

phase	reviewed	depth	files_reviewed	files_reviewed_list	findings	status
01-design-tokens-rams-foundation	2026-10-22T01:08:55Z	standard	13	package.json theme/package.json theme/src/tokens.css theme/src/theme.css theme/src/components.css theme/src/sheet.html theme/basemap/build.mjs theme/basemap/liberty-rams.json theme/basemap/index.html theme/scripts/audit.mjs theme/scripts/audit.test.mjs theme/scripts/screenshot.mjs theme/gallery.html	critical warning info total 1 5 7 13	issues_found

Phase 1: Code Review Report

Reviewed: 2026-10-22T01:08:55Z Depth: standard (with empirical verification: controlled Tailwind builds, live audit runs, test-suite execution, machine verification of the 6,089-line generated liberty-rams.json) Files Reviewed: 13 Status: issues_found

Summary

The phase delivers a genuinely well-engineered token package: npm run audit exits 0 on the real build (354 declarations checked), the TDD suite passes 16/16, all 7 self-hosted woff2 files exist and are wired correctly (no CDN fonts), the compiled dist/rams.css emits the --color-*/--spacing-*/--type-* tokens plus the D-04 shell slots with the dark-scheme media query, and liberty-rams.json was machine-verified clean (no accent hex/rgb forms, no line-gradient, no icon-color, no non-zero halo blur, only opacity ramps preserved, font substitutions honestly recorded in metadata).

However, the adversarial pass found one blocker in the foundation deliverable itself, plus a cluster of audit-gate holes proven by end-to-end evasion tests:

1. BLOCKER (CR-01): --radius-DEFAULT: 0px is a dead token in Tailwind v4.3.3 — the bare rounded utility is driven by the deprecated --radius key, not --radius-DEFAULT. The shipped theme/dist/rams.css therefore contains .rounded{border-radius:.25rem} — a non-zero radius value owned by no token, violating the binding zero-roundness constraint, D-07 ("the token package owns all numeric values"), and the tokens.css comment that asserts "every token-driven rounded-* utility compiles to 0".
2. Vocabulary leakage (WR-01): .shadow{…} and .blur{…} (Rams-forbidden constructs, with off-token values #0000001a / blur(8px) baked in) compile into the shipped stylesheet from prose, not from any mockup class attribute — so audit adjudication 1's rationale ("utilities the mockups wear") does not even cover them.
3. Audit-gate holes (WR-02/03/04): proven evasions — font: 700 14px/18px Arial passes unflagged (logic bug in familiesFromShorthand), as do color: red, oklch(...), color(display-p3 …), non-hex text-shadow, and all spacing/type-scale numerics (padding: 13px, font-size: 17px), contradicting the tool's own coverage claims and D-13's stated contract.

None of these are caught by the current green gates, which is exactly why they need fixing before later phases consume the package.

Critical Issues

CR-01: Dead --radius-DEFAULT token — bare rounded compiles to 0.25rem, violating the binding zero-roundness constraint (BLOCKER)

File: theme/src/tokens.css:98-109 (token), theme/src/theme.css:13-15 (build entry); evidence in compiled theme/dist/rams.css (minified, line 2): .rounded{border-radius:.25rem} Issue: The radius block sets --radius-DEFAULT: 0px intending to zero the bare rounded utility (v3-style DEFAULT key), but Tailwind v4.3.3 does not read --radius-DEFAULT for that utility — the bare rounded is driven by the deprecated --radius key (see node_modules/tailwindcss/theme.css:508, @theme default inline reference { --radius: 0.25rem; }). Empirically verified with controlled builds against the repo's own Tailwind 4.3.3:

• @theme { --radius-DEFAULT: 0px; } → .rounded { border-radius: 0.25rem; } (token ignored)
• @theme { --radius: 0px; } → .rounded { border-radius: var(--radius); } (correct override)

Consequences:

• The shipped dist/rams.css contains .rounded{border-radius:.25rem} — a numeric value owned by no token, in the phase deliverable whose thesis is D-07 ("the token package owns all numeric values") and the "zero off-token values" evidence claim.
• The tokens.css comment (lines 98-101) is factually wrong: it carves out only rounded-none/rounded-full as literal built-ins and asserts every other token-driven rounded-* compiles to 0. Bare rounded compiles to 0.25rem with the current token set.
• Any later phase wearing rounded (a Tailwind-default habit; the class is in the compiled vocabulary) silently gets 4px corners. The audit does flag it once worn (verified with a fixture page: theme/dist/rams.css:2: .rounded { border-radius: .25rem } — non-zero border-radius), but the package is supposed to prevent, not be caught by, its own gate.
• The audit's radius drift-guard (theme/scripts/audit.mjs:173, /--radius-[A-Za-z0-9-]*\s*:\s*(…)/) only matches --radius-* with a dash, so a future bare --radius override escapes the drift guard entirely; radiusAllowed() (audit.mjs:409) also passes any var() unconditionally.

Fix:

/* theme/src/tokens.css — @theme static radius block */
@theme static {
  --radius: 0px;      /* v4's deprecated bare key — THE key that drives the bare `rounded` utility */
  --radius-xs: 0px;  /* …rest unchanged… */
}

Delete --radius-DEFAULT: 0px; (inert in v4) or keep it only with a comment marking it inert; correct the block comment to state that rounded/rounded-none/rounded-full are literal built-ins and that --radius must be overridden explicitly. Rebuild and confirm .rounded{border-radius:0px} in dist/rams.css. Extend the audit drift-guard:

/* audit.mjs — cover the deprecated bare key too */
for (const m of text.matchAll(/--radius(?:-[A-Za-z0-9-]*)?\s*:\s*([^;}]+?)\s*;/g)) { … }

Warnings

WR-01: Prose candidate leakage compiles Rams-forbidden .shadow/.blur utilities into the shipped stylesheet

File: theme/src/theme.css:24 (@source "../../stitch_minimalist_route_navigator"), plus the CLI's implicit auto-scan of theme/ Issue: @source points at the whole mockup tree — including telemetry_rams/DESIGN.md prose — and the CLI auto-scan ingests the toolchain's own text (e.g. scripts/audit.mjs:36 prose "shadow/backdrop-blur/bg-black"). Tailwind extracts class candidates from natural language. Result in dist/rams.css (all verified present, none worn by any mockup class attribute or implementation page):

• .shadow{--tw-shadow:0 1px 3px 0 var(--tw-shadow-color,#0000001a)…box-shadow:…} — bare shadow is worn by no mockup (mockups wear shadow-md/shadow-sm/shadow-[…] only)
• .blur{--tw-blur:blur(8px);…} — bare blur likewise unworn anywhere
• .rounded (see CR-01) entered the vocabulary the same way (e.g. telemetry_rams/DESIGN.md:155 "devoid of rounded ornament")

These carry off-token values (#0000001a, 8px) into the artifact, and audit adjudication 1 ("utilities the mockups wear but the design doc rejects") does not cover them — they are not mockup-worn at all. The audit skips them only because no page harvests those classes, so the "zero off-token values" evidence holds by accident of non-usage. Fix: Scope @source to the four mockup markup files and exclude tooling prose from the auto-scan:

@source "../../stitch_minimalist_route_navigator/active_route_guidance/code.html";
@source "../../stitch_minimalist_route_navigator/navigate_map_overview/code.html";
@source "../../stitch_minimalist_route_navigator/community_incident_reporter/code.html";
@source "../../stitch_minimalist_route_navigator/telegram_live_traffic_feed/code.html";
@source not "../scripts";
@source not "../basemap";

After the fix, .rounded/.shadow/.blur should disappear from dist/rams.css (verify by grep); only mockup-worn classes remain in the compiled vocabulary.

WR-02: familiesFromShorthand logic bug — off-token families in font: shorthands pass the audit unflagged

File: theme/scripts/audit.mjs:449-455 (helper), consumed at audit.mjs:439-441 Issue: The strip regex removes only the first shorthand token: '700 14px/18px Arial' → strips '700 ' → leaves '14px/18px Arial', which the /^[0-9.]/ filter then drops as "numeric" — so the family list comes out empty and no font: shorthand can ever be flagged, regardless of family. Verified end-to-end: .a-font-shorthand { font: 700 14px/18px Arial; } produces zero violations (Arial is off-token and should be flagged per the T2 contract). The test suite only covers the font-family: longhand, so the hole is untested. The package's own convention is font: var(--type-*) everywhere, so nothing currently trips it — but the gate's contract (T2: "off-token font → flagged") is silently incomplete. Fix: Simplest contract-true fix — flag any literal (non-var) font: shorthand outright, matching the package convention:

if (prop === 'font' && !/var\(/i.test(value)) {
  violations.push('literal font: shorthand — use font: var(--type-*) (Rams typography tokens)');
} else if (prop === 'font-family' && !/var\(/i.test(value)) {
  violations.push(...fontViolations(value, tokens));
}

(Alternatively repair familiesFromShorthand to iteratively strip weight and size/line-height components.) Add a T2-style test case for the shorthand form.

WR-03: Color-evasion gaps — named keywords, oklch()/lab()/lch()/hwb()/color(), and text-shadow all pass the audit

File: theme/scripts/audit.mjs:361-394 (literalHexViolations — hex/rgb()/hsl() only), audit.mjs:419-429 (shadowViolations — box-shadow/backdrop-filter/filter only) Issue: Verified evasions (fixture run through the real audit — none flagged): .b { color: red; }, .c { background: oklch(0.6 0.2 30); }, .e { fill: color(display-p3 1 0 0); }, and .h2 { text-shadow: 0 1px 0 red; } (any non-hex text-shadow). Modern CSS Color 4 functions and named keywords are fully unaudited escape hatches, and text-shadow (a shadow Rams forbids) is not in shadowViolations. For a tool whose purpose is "zero off-token values is evidence, not an eyeball claim", these are known-open doors that later phases can walk through accidentally (e.g. color: white, oklch() from a design-tool export). Fix: Extend the literal checks:

const NAMED = new Set(['red','green','blue','white','black','gray','grey','yellow','orange',
  'purple','pink','brown','silver','gold','cyan','magenta','lime','navy','teal','olive',
  'maroon','aqua','fuchsia']);
// in literalHexViolations: flag NAMED keywords on color-ish props
//   (keep currentcolor/inherit/transparent sanctioned);
// flag /\b(?:oklch|oklab|lab|lch|hwb|color|color-mix)\(/ unless the value contains var(.
// in shadowViolations: add text-shadow (any non-none value).

WR-04: D-13 contract shortfall — spacing and typography numerics are not audited at all

File: theme/scripts/audit.mjs:433-446 (checkDeclaration — no spacing/type-size/weight/line-height properties), claims at audit.mjs:8-9, theme/gallery.html:155, .planning/phases/01-design-tokens-rams-foundation/01-CONTEXT.md (D-13) Issue: D-13 states the audit "flags any color/spacing/typography/radius value not in the token set", the audit header says "any color / typography / radius / shadow / blur value", and the gallery tells the approver "colors, typography, radius, shadows, blur all derive from theme/src/tokens.css". Verified: padding: 13px; gap: 7px;, font-size: 17px; font-weight: 650; line-height: 1.7; all pass unflagged. The typography check is font-family only; spacing is entirely absent. The evidence claim is broader than the implementation. Fix: Either implement the missing checks (spacing literals validated against the 8px-grid scale; font-size/font-weight/line-height validated against the --type-* scale) or amend the claim wording in audit.mjs's header and gallery.html to "color families, font families, radius, shadows, blur" so the recorded DSGN-01 evidence matches what actually runs.

WR-05: Static server path guard uses prefix match without separator — sibling theme* paths escape the docroot

File: theme/scripts/screenshot.mjs:68-71 Issue: join(THEME, decodeURIComponent(url.pathname)) then target.startsWith(THEME). A URL-encoded ..%2FthemeX decodes to ../themeX, joins to /…/araharah/themeX, which passes the startsWith('/…/araharah/theme') check while being outside the docroot — the classic incomplete-prefix-traversal idiom. Exploitability is low (bound to 127.0.0.1, ephemeral port, alive only during capture, no CORS headers), but the guard exists precisely to prevent this and fails at its boundary condition. Fix:

import { sep } from 'node:path';
// …
const target = join(THEME, decodeURIComponent(url.pathname));
if (target !== THEME && !target.startsWith(THEME + sep)) {
  res.writeHead(403); res.end('forbidden'); return;
}

Info

IN-01: Dead send script

File: theme/package.json:13 Issue: "send": "mkdir -p dist/fonts && cp -r fonts/geist fonts/space-grotesk dist/fonts/" is referenced by nothing (not delegated from the root, not a dependency of any other script), and its output is unused — dist/rams.css references url(../fonts/…), which resolves from dist/ to the tracked theme/fonts/ directly; nothing serves dist/fonts/. Fix: Remove the script (or document its purpose if one is intended).

IN-02: playwright is a runtime dependency of a CSS package

File: theme/package.json:8-10 Issue: @araharah/theme ships no runtime JS, yet playwright sits under dependencies — any consumer installing the theme package for its CSS pulls the browser toolchain as a prod dependency. Fix: Move playwright to devDependencies (it is only used by scripts/screenshot.mjs, a dev-time capture tool).

IN-03: python3 dependency in an otherwise Node-only toolchain

File: theme/package.json:17 Issue: basemap:preview shells out to python3 -m http.server 8123, introducing a Python runtime requirement into the Node-first toolchain (AGENTS.md free-first/Node-first stack). A tiny Node static server already exists in screenshot.mjs and could be extracted and shared. Fix: Extract the serveTheme() server (with a port argument) into a small shared script, or accept and document the Python requirement.

IN-04: Root scripts miss the workspace's test and basemap entries

File: package.json:8-14 Issue: The root delegates build/sheet/gallery/audit/capture but not test or basemap — npm test at the repo root fails ("Missing script") even though the theme workspace defines a passing 16/16 test suite, and the basemap rebuild documented in theme/basemap/build.mjs:31 must be invoked with the longer --workspace form. Fix: Add "test": "npm run test --workspace theme" and "basemap": "npm run basemap --workspace theme" to the root scripts.

IN-05: Font probing — no fetch timeout; partial substitutions unrecorded

File: theme/basemap/build.mjs:305-315 (probeGlyph), build.mjs:321-345 (resolveFontStacks) Issue: (a) fetch has no AbortSignal.timeout, so a stalled glyphs endpoint hangs the basemap build indefinitely. (b) If some of a role's intended stacks are hosted (e.g. 'Geist Regular' 200 but 'Space Grotesk Regular' 404), served.minor silently becomes the partial list with no rams-font-substitutions record — undermining the "honest fallback provenance" the feature exists to provide. (Current committed output hit the clean all-404 → full-fallback path, so both are latent.) Fix: Add { signal: AbortSignal.timeout(10_000) } to the probes; record a substitution entry whenever served[kind] differs from FONT_INTENT[kind], not only on full fallback.

IN-06: Sheet inputs have no accessible name

File: theme/src/sheet.html:167-168 Issue: Both input-field text inputs are labeled by placeholder only — no <label>/aria-label — leaving them nameless to assistive tech. Fix: Add aria-label="Destination search" / aria-label="Bot command input" (or wrap with visually-hidden label text), consistent with the Rams register.

IN-07: Mockup capture fallback is invisible in the gallery artifact

File: theme/scripts/screenshot.mjs:130-138 Issue: When the network render of a mockup fails, the committed screen.png is copied in as the gallery's "mockup reference render" — indistinguishable from a true render in the final artifact. Only the capture log records the substitution, so a DSGN-02 approver can unwittingly compare the implementation against a fallback capture made under different conditions (offline, no CDN fonts). Fix: Emit a machine-readable marker alongside the capture (e.g. a screenshots/manifest.json with { file, source: "network" | "fallback" }) and/or note fallback occurrences in the phase summary where DSGN-02 is recorded.

---

Verified-clean aspects (for the record): npm run audit exits 0 on the real build (354 declarations, 97 rules, 298 compiled-vocabulary rules out of scope, 49 classes harvested); node scripts/audit.test.mjs passes 16/16; liberty-rams.json machine-verified: 110 layers, no accent hex/rgb forms, no line-gradient, no icon-color paints, no non-zero text-halo-blur, only the 3 documented untouched layers (road_area_pattern, road_one_way_arrow, road_one_way_arrow_opposite), building-3d removed with reason recorded, Noto Sans font substitution honestly recorded in metadata; all 7 self-hosted woff2 present and matched 1:1 by the @font-face URLs; --shell-* slots + dark-scheme media query present in the compiled CSS; the basemap proof page reads the route color from the token package at runtime; the audit's used-class harvesting and utilities-layer scoping behave as documented (verified with fixture pages); the audit does flag .rounded the moment any page wears it.

Reviewed: 2026-10-22T01:08:55Z Reviewer: OpenCode (gsd-code-reviewer) Depth: standard