feat(design-system): give cn() tailwind-merge (#218) - #1678
Merged
Conversation
`cn()` was a plain `classes.filter(Boolean).join(" ")`, so a later class could
never override an earlier one — every size or colour override had to be worked
around at the call site, and the winner was decided by Tailwind's stylesheet
emission order rather than by intent. Five comments across four files documented
that constraint.
The substance is not the `cn()` body, which is a one-line change keeping its
exact previous signature and falsy filtering. It is `src/lib/tailwind-merge.ts`:
stock tailwind-merge classifies any unrecognised `text-<x>` as a text COLOUR,
and this repo's `@theme` defines scales it has never seen. `text-sm-minus`,
`text-base-minus`, `text-lg-minus`, `text-2xl-minus`, `text-2xl-compact`,
`text-3xl-minus` and `text-hero` were each measured being deleted when they met
`text-[color:var(--text-muted)]` in the same call — the `eyebrowText` recipe is
exactly that pair. `size-icon-*`, `tracking-*`, `leading-*`, `ease-*`,
`animate-*` and the `pt-safe` family were not recognised at all, so they never
merged. All of that is silent: no type error, no lint error.
`--spacing-tap` is deliberately NOT declared. Tailwind emits `.min-h-tap` after
every numeric `.min-h-*`, so the 48px tap token wins today at the 22 call sites
that pair them; declaring it would hand the win to the later class and drop 18
production targets to 32/36/40/42px, which AGENTS.md forbids. The omission is
documented in the config and pinned by a test.
Every other family was measured across all 1 409 `cn()` call sites and
introduces zero new class deletions. The 339 sites where twMerge drops
`focus-visible:outline` beside `focus-visible:outline-2` are CSS-identical:
compiling Tailwind shows both utilities emit the same two properties, with
`--tw-outline-style` defaulting to `solid`.
The five workaround comments now record that the constraint is lifted. The
dodges themselves are kept — reverting `max-sm:hidden` or collapsing the
three-branch chip changes which utilities render, and belongs in a change whose
Chromium job is being read, not in a dependency swap. One of the five is only
half lifted: tailwind-merge scores bare `border` and `border-t` as separate
groups, so `document-search-results.tsx` still needs its `border-0`.
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
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 36 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
Comment |
Merged
4 tasks
BigSimmo
enabled auto-merge (squash)
August 7, 2026 12:15
This was referenced Aug 7, 2026
BigSimmo
added a commit
that referenced
this pull request
Aug 9, 2026
…lection (#262 parts 2 and 3) (#1780) * docs(issues): close #218 and #270, both shipped before this session Both rows were still open in docs/outstanding-issues.md while their work was already live on main, which had scoped a third session from them. #218 (cn() lacks tailwind-merge) shipped in PR #1678, aeba5a2. src/components/ui-primitives.tsx:37 is twMergeClinical(...) rather than a plain join, package.json carries tailwind-merge ^3.6.0, and src/lib/tailwind-merge.ts declares the repo's @theme scales to twMerge. #270 (declare the tap spacing token) shipped in PR #1738, 80cf781, an ancestor of origin/main. "tap" is present in CLINICAL_TWMERGE_THEME.spacing, and tests/tailwind-merge-config.test.ts was inverted rather than deleted so the merge behaviour is now asserted rather than pinned out. Verified in source at origin/main 7aaf934, not inferred from the handover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(design-system): ratchet raw padding, radius and line-height literals (#262 part 3) The design-system contract ratcheted colour, shadow, tap and tracking but not spacing, radius or line-height, so a value could bypass the scale as a bare literal in either a class or a stylesheet and nothing objected. Adds three per-path ratchets, covering both halves the way the colour and legacy-shadow metrics already do: rawPaddingLiterals 67 (17 CSS declarations, 50 class utilities) rawRadiusLiterals 24 (22 CSS declarations, 2 class utilities) rawLineHeightLiterals 3 (3 CSS declarations) The exemption is deliberately "contains no CSS function", not the narrower `(?!var\()` the tracking rule uses. Padding is not only ever a token or a literal: production ships pb-[env(safe-area-inset-bottom)], pt-[max(0.75rem,var(--safe-area-top))], pt-[clamp(1.5rem,5vh,3rem)] and pb-[calc(7rem+env(safe-area-inset-bottom))]. Those are computed from the viewport or the safe-area inset, cannot be spelled as a scale step, and a `var(`-only lookahead would have flagged every one of them. On the CSS side, zero in any unit, the CSS-wide keywords and custom-property declarations (the token definitions themselves) are exempt for the same reason. Every one of the 94 baseline entries was verified present at its cited line before pinning, and the baseline change is additive: all fifteen pre-existing metrics and every pre-existing debtByPath entry are byte-identical. Mutation-tested rather than assumed. Class side, in a file with no prior debt: - rawPaddingLiterals increased from 67 to 68 - rawPaddingLiterals at src/components/ui-primitives.tsx increased from 0 to 1 - rawRadiusLiterals increased from 24 to 25 - rawRadiusLiterals at src/components/ui-primitives.tsx increased from 0 to 1 - rawLineHeightLiterals increased from 3 to 4 - rawLineHeightLiterals at src/components/ui-primitives.tsx increased from 0 to 1 The CSS half fails the same way. Both probes also carried the sanctioned computed forms, and each count rose by exactly one, so the exemptions are proved by the same runs rather than argued. No new npm script: the metrics live inside check:design-system-contract, so docs:check-inventory and check:gate-manifest are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(design-system): gate type-step selection on the decidable half (#262 part 2) check:type-scale blocks arbitrary text-[12px] values. Nothing has stopped the scale itself growing a step no surface ever picks, which is the drift that makes a wrong selection possible in the first place. Whether a heading should have chosen text-sm over text-sm-minus is not mechanically decidable, and this does not pretend otherwise. A step that is declared and consumed by nobody is decidable, and there is one today: --text-2xl-compact (globals.css:112) has zero consumers -- no utility use, no var() use -- while the next-rarest step, text-hero, has one real consumer. The analyzer reports every bare text-<name> it sees and does not decide which names are steps; the checker intersects that against the @theme block it parses from globals.css. So the scale is never written down twice, and a step added to globals.css is covered without touching this gate. Retiring the dead step edits @theme, so it gets its own revertible PR rather than riding along here: it is carried in UNUSED_TYPE_STEP_EXEMPTIONS and tracked as docs/outstanding-issues.md #295. The exemption cannot rot silently -- the gate also fails if an exempted step stops being declared or gains a consumer. Mutation-tested, three ways: - type steps are declared in globals.css @theme but no production surface selects them: --text-2xl-compact (text-2xl-compact). Retire the step or use it; do not leave the scale carrying a step nobody picks. - (a newly added --text-probe-step fails identically, so this catches future drift rather than only today's known case) - --text-2xl-compact is exempted as unused but production now selects text-2xl-compact -- drop the exemption Measurement note, since three different figures were in circulation for this row: the "1318 sites" is a repo-wide grep INCLUDING mockups, which the gate excludes (1360 at this HEAD). Production consumers of the nine non-standard steps total 705 -- text-2xs 421, sm-minus 160, base-minus 57, 3xs 42, 2xl-minus 9, 3xl-minus 9, lg-minus 6, hero 1, 2xl-compact 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(design-system): correct GATES.md for the two new scale gates GATES.md's own §1 is the list of what actually runs, and this series' recurring failure is that list lagging the code: four of #264's six prohibitions were already gated while it said "planned". Records the padding/radius/line-height ratchets and the type-step selection rule in the contract row, and rewrites the type-scale callout, which claimed a step-selection lint "does not exist". The decidable half now ships; the half that asks whether text-sm-minus was the right pick over text-sm still does not, and cannot. Also corrects the "1 318 call sites" figure quoted there. It was a repo-wide grep including src/app/mockups/**, which every one of these gates excludes (1 360 at 7aaf934). Production consumers total 705, and there are nine non-standard steps, not eight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record the #262 parts 2/3 gate work (PR #1780) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(design-system): close scale-ratchet and unused-step review gaps Cover Tailwind arbitrary-property forms and modern CSS zero units in the raw scale ratchets, and validate unused-step exemptions against the same class-or-CSS consumer predicate used for ordinary steps. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.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
cn()tailwind-merge (ledger#218, Track A1 ofdocs/design-system/HANDOVER-2026-08-07.md).cn()wasclasses.filter(Boolean).join(" "), so a later class could never override an earlier one; the winner was decided by Tailwind's stylesheet emission order rather than by intent. Its signature and falsy filtering are unchanged, so no call site changes.src/lib/tailwind-merge.ts, theextendTailwindMergeconfig. This is the actual deliverable. Stock tailwind-merge classifies any unrecognisedtext-<x>as a text colour, and this repo's@themedefines scales it has never seen.text-sm-minus,text-base-minus,text-lg-minus,text-2xl-minus,text-2xl-compact,text-3xl-minusandtext-herowere each measured being deleted when they mettext-[color:var(--text-muted)]in the same call — theeyebrowTextrecipe is exactly that pair. Separately,size-icon-*,tracking-*,leading-*,ease-*,animate-*and thept-safefamily were not recognised at all, so they never merged. Both failure modes are silent: no type error, no lint error. Every group is commented against itsglobals.csssource of truth.--spacing-tapis deliberately not declared. Tailwind emits.min-h-tapafter every numeric.min-h-*and.h-tapafter.h-4/.h-10.5, so at equal specificity the 48px tap token wins today wherever a call site pairs the two. Declaring it would hand the win to the later class — measured across 22 call sites, 18 of which would drop from 48px to 32/36/40/42px. AGENTS.md and SPEC §4.10 are explicit that no production target is ever reduced. The omission is documented in the config and pinned by a test, and the 22 sites are captured as follow-up work.tests/tailwind-merge-config.test.ts(33 tests): falsy filtering unchanged, real conflicts resolve last-wins, every custom step survives beside a colour, custom families do not collide with one another, the tap token stays unmerged, and the config is compared field-by-field against the@themeblock inglobals.cssso a token added to one and not the other fails here. Mutation-checked: removing a single token from the config fails 3 of these tests.page-header.tsx,document-search-results.tsx×2,master-search-header.tsx,mode-home-template.tsx) to record that the constraint is lifted. The dodges themselves are kept — revertingmax-sm:hiddenor collapsing the three-branch chip changes which utilities render, which is a visual change and belongs in a change whose Chromium job is being read. One of the five is only half lifted and says so: tailwind-merge scores bareborderandborder-tas separate groups, sodocument-search-results.tsxstill needs itsborder-0.docs/codebase-index.md.Measured blast radius:
cn(is called 1 409 times; 620 call sites resolve statically to two or more arguments. Stock tailwind-merge changes the output at 387 of them, but 339 arefocus-visible:outlinedropped besidefocus-visible:outline-2— compiling Tailwind shows.outlineand.outline-2emit the same two properties, with--tw-outline-styledefaulting tosolid, so those are CSS-identical. Declaring the custom families adds new deletions at exactly one family,--spacing-tap, which is why it is held back. Every other family: zero new deletions.RAG impact: no retrieval behaviour change — this changes only how Tailwind class strings are composed in the presentation layer; no file under
src/lib/rag/**, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness or the golden fixture is touched.Verification
Run locally on this exact head, off
origin/mainat460026814:npm run check:design-system-contract— exit 0. "Design-system contract passed (672 production files; raw colors 2; literal shadows 0; legacy tap classes 0; edge conflicts 28; 1px shadow spreads 2)."npm run typecheck— exit 0npm run lint— exit 0npm run test—Tests 6 failed | 5509 passed | 14 skipped (5529),Test Files 2 failed | 518 passed (520)npx vitest run tests/ui-primitives.dom.test.tsx tests/ui-v2-components.dom.test.tsx tests/ui-v2-answer-safety.dom.test.tsx tests/tailwind-merge-config.test.ts— exit 0,Tests 162 passed (162)npx prettier --check .— exit 0, "All matched files use Prettier code style!"The 6 failures are the three known-on-Windows suites from HANDOVER §7, baselined before any edit on the untouched base (
dfcbe2039):Tests 6 failed | 17 passed (23)acrosspr-handoff-stop.test.ts(5) andmode-nav-addon-slot.dom.test.tsx(1). Same six test names, same two files, after the change and on the newer base — zero delta.installed-lock-parity(issue#149) passes in a freshly installed worktree, so only two of the three §7 files reproduce here. This diff touches no mode-nav path and no hook script.Verification not run:
npm run verify:pr-local— it fails closed atcheck:installed-lock-paritybefore running anything (issue#149), so its constituent gates were run directly and are listed above. UI verification not run: Chromium under contention on this box produces false reds rather than signal (HANDOVER §7); CI's Chromium job is the authority, and this change ships no visual delta by design.Risk and rollout
cn(), but it alters class resolution for all 1 409 call sites, and the failure mode is silent — a misclassified custom token is deleted with no type or lint signal. That risk is why the config was derived fromglobals.cssrather than written from memory, why every family was measured against the full call-site set before being declared, and why--spacing-tapis excluded. Residual risk sits in the 789 call sites that could not be resolved statically (a runtimeclassNameprop, a ternary), where a caller could be relying on an earlier class winning; by construction those are exactly the overrides that were previously broken.cn()body and the comment updates are one commit, and no call site was changed, so the revert restores the previous join semantics exactly.npm install tailwind-merge; the lockfile delta is that package alone, with zero transitive drift.Notes
tailwindcss ^4.3.1). Installed 3.6.0.shadow-*utilities to declare.--shadow-*are plain custom properties consumed asshadow-[var(--shadow-tight)]; noshadow-<name>utility is generated and stock tailwind-merge already groups the arbitrary form correctly.#118holds), nocodex/v2-*branch merged, no outlined P1/P2 component built, no lint duplicatingcheck:type-scale, and no production tap target lowered.🤖 Generated with Claude Code