--- phase: 01-design-tokens-rams-foundation reviewed: 2026-10-22T01:08:55Z depth: standard files_reviewed: 13 files_reviewed_list: - 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 findings: critical: 1 warning: 5 info: 7 total: 13 status: 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:** ```css /* 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: ```js /* 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: ```css @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: ```js 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: ```js 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:** ```js 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 `