refactor(tokens): migrate production letterspacing onto the named scale - #1669
Merged
Conversation
Completes the work #1663 set up. #1655 and #1658 have landed, so `ui-primitives.tsx` and the answer surface are free to touch. 68 arbitrary `tracking-[…]` call sites across 40 production files fold onto the five named roles. That is every statically resolvable site the contract can see; the ratchet now reports `arbitrary tracking 0`, down from 68. Fold applied, as committed in globals.css: -0.008 / -0.01 / -0.035 / -0.04 -> tracking-display (6 sites) 0.01 / 0.02 -> tracking-normal (2) 0.04 / 0.05 / 0.06 -> tracking-label (47) 0.08 / 0.09 -> tracking-eyebrow (8) 0.1 / 0.11 / 0.12 -> tracking-kicker (7) Largest single move is 0.04em; almost all are 0.01-0.02em. Baseline tightened from 68 to 0 with the per-path map cleared, so the family cannot regrow at all rather than regrowing up to the old ceiling. Every other metric in the baseline was left untouched by construction — the edit sets only `arbitraryTracking`. `*-mockups.tsx` and `*-mockups/` directories are deliberately untouched: design scratch, gate-exempt, and migrating them would triple the diff for no enforcement gain. `tracking-[var(--quantity-unit-tracking)]` is the sanctioned token form and stays. Verification: check:design-system-contract exit 0 (`arbitrary tracking 0`) · typecheck 0 · lint 0 · full unit suite 5 473 passed, 14 skipped. Three test files fail, none caused by this diff: - tests/installed-lock-parity.test.ts — playwright 1.62.0 vs locked 1.62.1, environmental, issue #149. - tests/pr-handoff-stop.test.ts and tests/mode-nav-addon-slot.dom.test.tsx — both arrived on main after this session's earlier clean full run, from #1649 and #1647. They fail for Windows-environment reasons: simulated `jq` absence, and an assertion comparing repo-relative POSIX paths against absolute Windows paths. The mode-nav assertion compares path FORMAT over an identical two-file set, so it is content-independent and a class rename cannot produce it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
📝 WalkthroughWalkthroughThe PR replaces arbitrary letter-spacing utilities with shared design-system tracking tokens across application surfaces. It also updates the design-system baseline from 68 violations to zero. ChangesTracking token standardization
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
BigSimmo
enabled auto-merge (squash)
August 7, 2026 06:28
Owner
Author
Cursor review-and-fix summary — PR 1669Tip: Snapshot / blockers cleared
Review findings
Local gates (decisive lines)
Push / ledger
Residual risks
Merge left to you. |
BigSimmo
added a commit
that referenced
this pull request
Aug 7, 2026
… used it (#1673) The session handed this file found four defects in it and one hazard it omitted. All five verified against origin/main before being written back. Corrections: - #1669 was listed "open at handover"; it merged as b557a26. Any status in a handover is stale the moment it is written, so §1 now says re-measure rather than quote — the staleness figure itself drifted 111 -> 129 commits in two days. - "two source sites already carry workaround comments" was an undercount. There are five comments across four files; a literal grep for "tailwind-merge" finds only three, because two say "plain join" and "concatenates". The fifth, mode-home-template.tsx:106, does not merely comment — it strips stray `justify-*` tokens, so twMerge would make that workaround redundant or double-handle it. - "~200 importing files" understated the blast radius: `cn(` is called 1 888 times across 188 source files, and 210 files reference ui-primitives. - Records that this needs tailwind-merge v3.x (v2 targets Tailwind 3; the repo is on tailwindcss ^4.3.1) and that neither tailwind-merge nor clsx is a dependency. The omitted hazard, now the headline of A1: stock tailwind-merge classifies an unknown `text-<x>` as a text COLOUR, and this repo's @theme defines scales it has never seen (text-2xs, text-3xs, the tracking-* roles, min-h-tap, size-icon-*, thirteen shadow-*, custom radius-*/leading-*). `cn(eyebrowText)` would therefore have either its size or its colour silently deleted — a regression across the kit with no type or lint signal. The deliverable is the extendTailwindMerge config, not the `cn()` body. Also records the two hazard classes to audit first and the instruction to baseline the known Windows failures before touching anything. Prompt fix (§9): that session stalled before step 0 asking whether `git fetch` fell inside the provider-confirmation boundary. It read the earlier wording correctly — "any provider-backed workflow" plainly covers a fetch. The boundary exists for spend, mutation and production risk, none of which a read-only fetch carries. §9 now pre-authorises fetch, worktree creation and a single-package npm install by name, keeps the real gates, and records why the paragraph exists so it is not trimmed away later. docs:check-links exit 0. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes what #1663 set up. #1655 and #1658 have landed, so
ui-primitives.tsxand the answer surface are free to touch.68 arbitrary
tracking-[…]call sites across 40 production files fold onto the five named roles. That is every statically resolvable site the contract can see. The ratchet now reportsarbitrary tracking 0, down from 68.-0.008-0.01-0.035-0.04tracking-display0.010.02tracking-normal0.040.050.06tracking-label0.080.09tracking-eyebrow0.10.110.12tracking-kickerLargest single move is 0.04em; almost all are 0.01–0.02em.
Baseline tightened from 68 to 0, per-path map cleared, so the family cannot regrow at all rather than regrowing up to the old ceiling. Every other metric was left untouched by construction — the edit sets only
arbitraryTracking.*-mockups.tsxand*-mockups/directories are deliberately untouched. Design scratch, gate-exempt; migrating them would triple the diff for no enforcement gain.tracking-[var(--quantity-unit-tracking)]is the sanctioned token form and stays.Verification
npm run check:design-system-contract— exit 0,arbitrary tracking 0npm run typecheck— exit 0npm run lint— exit 0npm run test— 5 473 passed, 14 skipped (author report on original tip)npm run verify:pr-local— to be re-confirmed after main sync on the review-and-fix tipf275e9b2; a local Chromium screenshot pass would not meaningfully adjudicate sub-pixel tracking.Risk and rollout
patient-safety-plan.tsxis touched only for twotracking-[0.06em]→tracking-labelclass renames (same computed role).Clinical Governance Preflight
Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
Clinical-risk classification is triggered solely by the path name
src/components/patient-safety-plan.tsx(matches thepatientpresentation-surface pattern). Diff is letterspacing token migration only.RAG impact: no retrieval behaviour change — this diff renames CSS utility classes and adjusts a contract baseline. No file under
src/lib/rag/**, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness, or the golden fixture is modified.