feat(design-system): gate the dark: colour-override prohibition at zero (#264) - #1702
Conversation
…zero (#264) GATES.md §3 listed "`dark:` colour override" as **planned** with three live instances. A prohibition with no gate row is a suggestion, which is the failure mode that document exists to prevent. This closes it. All three instances were the same shape — `ring-1 ring-<colour>/20 dark:ring-<colour>/10` on a lux surface (`panel` in ui-primitives, the mobile FAB menu in dashboard-nav, the PWA notice card). They are now `--ring-highlight` and `--ring-glass`: per-theme tokens that carry the value the override used to supply, so the utility no longer needs a variant. `legacyPaletteUtilities` (the `ring-white/35` on the same PWA line) went to zero with them. No visual change, and that is measured rather than assumed. Tailwind emits `color-mix(in oklab, <colour> N%, transparent)` for `ring-<colour>/N`, and mixing any colour with `transparent` yields that colour at alpha N regardless of colour space, so the token values are equivalent by construction. `--ring-glass` is deliberately literal white and NOT `--neutral-0`, which inverts to `#060708` in dark and would have turned a white glass ring near-black. Both metrics are pinned at 0 in the contract baseline rather than ratcheted. The check asserts `value <= baseline`, so zero is a hard floor. Mutation-verified: reintroducing one `dark:ring-white/10` fails with `darkColorOverrides increased from 0 to 1` plus the matching per-path assertion. Every other metric was asserted unchanged before the baseline was rewritten, so clearing one number could not silently move another. Deliberately not fixed here, having been triaged and rejected as unsafe for a minimal change: `onePixelShadowSpreads` (2) are `0 0 0 1px … inset` border-substitutes whose conversion changes the box model, and `rawColorLiterals` (2) are a medication accent DATA fallback under a documented colour contract, not markup paint. 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. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:24 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 selected for processing (7)
Comment |
Uh oh!
There was an error while loading. Please reload this page.
Summary
dark:colour-override prohibition (#264, design-system Track A5).GATES.md§3 listed it as planned with three live instances; a prohibition with no gate row is a suggestion, which is the failure mode that document exists to prevent. It is now implemented-blocking.ring-1 ring-<colour>/20 dark:ring-<colour>/10on a lux surface:panelinui-primitives.tsx, the mobile FAB menu indashboard-nav.tsx, and the PWA notice card. They are now--ring-highlightand--ring-glass, per-theme tokens carrying the value the override used to supply, so the utility no longer needs a variant.legacyPaletteUtilities(thering-white/35on the same PWA line) went to zero with them.design-system-contract-baseline.jsonrather than ratcheted. The check assertsvalue <= baseline, so zero is a hard floor.GATES.md§1 and §3, a dated evidence subsection in §4, and aTOKENS.md§5 row.No visual change, measured rather than assumed. Tailwind emits
color-mix(in oklab, <colour> N%, transparent)forring-<colour>/N(verified by compiling Tailwind, not from memory), and mixing any colour withtransparentyields that colour at alpha N regardless of colour space, so the token values are equivalent by construction. One trap worth recording:--ring-glassis deliberately literal white and not--neutral-0, which inverts to#060708in dark — the obvious "use the token" refactor would have turned a white glass ring near-black in dark mode.Deliberately not fixed here, triaged and rejected as unsafe for a minimal change:
onePixelShadowSpreads(2) are0 0 0 1px … insetborder-substitutes whose conversion changes the box model, andrawColorLiterals(2) are a medication accent data fallback under a documented colour contract, not markup paint. Both remain ratcheted at their existing values.RAG impact: no retrieval behaviour change — this touches two CSS custom properties, three class strings and documentation; nothing under
src/lib/rag/**, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness or the golden fixture.Verification
Baselined on the untouched base
da66b283ebefore any edit, then re-run after:npm run check:design-system-contract— exit 0. Before:legacy palette utilities 2; dark color overrides 3. After:legacy palette utilities 0; dark color overrides 0. Every other metric byte-identical, asserted programmatically before the baseline file was rewritten.dark:ring-white/10fails withdarkColorOverrides increased from 0 to 1,legacyPaletteUtilities increased from 0 to 1, plus both per-path assertions. Reverted.npm run typecheck— exit 0npm run lint— exit 0npm run docs:check-links— exit 0, "docs link check passed: 1651 repo path references resolve."npx prettier --check .— exit 0, "All matched files use Prettier code style!"npx vitest runover every suite covering the changed surfaces —ckb-v2-token-contract,design-token-contract,theme,ui-primitives.dom,ui-v2-components.dom,ui-v2-answer-safety.dom,pwa-lifecycle.dom,tailwind-merge-config:Tests 99 passed (99)andTests 139 passed (139), 238 total, zero failures.Tests 6 failed | 17 passed (23)—pr-handoff-stop(5) andmode-nav-addon-slot(1). Unchanged by this diff, which touches neither a hook script nor any mode-nav path.Verification not run: the full
npm run test, and the reason is environmental rather than a pass I am claiming. Three separate infrastructure failures on this box, in order:run-heavyreturned exit 75DATABASE_HEAVY_RUN_ADMISSION_BUSY(another worktree held the lease); a Node upgrade mid-run (24.18.0 → 24.19.0) killed a run and broke Prettier with "Could not determine Node.js install directory"; and the retry died atEPERM: operation not permitted, rename … clinical-kb-heavy-locks\…\owner.jsonbefore a single test executed. The first focused run also lost 4 of 8 files to[vitest-pool]: Timeout waiting for worker to respond; re-run with--no-file-parallelismthey all pass, which is the known false-red-under-contention signature. CI's suite is the authority here. UI verification not run: Chromium under this contention produces false reds, not signal, and this change ships no visual delta by design.Risk and rollout
transparentis colour-space-independent), not a judgement call, and the token contract suites pass.dark:variants exactly.Notes
#261–#270). This closes#264only in part — A5 covers several prohibitions and this PR takes the one that was safely closable without a visual change.GATES.md§3 lists 8 prohibitions asplanned, not six, and several of those already carry a contract ratchet, so they are held rather than ungated. The genuinely zero-gate ones remaining are the colour-boundary lint (numerals painted with a status colour), PDF/diagram invert refusal, and "0 matches" after a failed request.🤖 Generated with Claude Code