Retire the --shadow-tight role alias onto the --e1 elevation tier - #1803
Merged
Conversation
`--shadow-tight` has resolved to exactly `var(--e1)` in every scope since the elevation ladder landed: both theme declarations point at the tier, and the forced-colors block flattens `--e1` alongside the role aliases. GATES.md already forbids reaching for an alias in new code, so every one of these call sites was recorded debt under `legacyShadowAliases` (`#262` part 1). Substituting the tier for the alias is textual and value-preserving — computed `box-shadow` is unchanged in light, dark and forced-colors — so this carries no visual review of its own. - 150 `var(--shadow-tight)` occurrences across 71 files become `var(--e1)`; 90 of those are the gated production sites across 48 files, the rest are design-scratch mockups migrated in the same pass so nothing names the token - repoint the `tailwind-merge` docstring example at the ladder, since the sentence is about which custom properties generate no `shadow-<name>` utility - `check:design-system-contract` legacy shadow aliases 217 -> 127; the three `--shadow-tight` declarations themselves are retired in the next commit Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XDuNkNHU4zFn1cDquUXv5
With every call site on `--e1` the alias has no consumers, and a role that resolves to the same tier is the quietest way for the debt to come back — it looks harmless at the declaration and reads as sanctioned at the call site. Isolated from the migration commit because it edits the theme blocks. The value-preserving claim is not safe from the declarations alone, and the reason is recorded in GATES §6 because it is owed to every remaining alias in `#262`: `ckb-v2-tokens.css` redeclares `--e1` (light `13 40 71 / 5%` against globals' `11 42 56 / 7%`) and never redeclares the roles, and a custom property containing `var()` substitutes on the element it is *declared* on — an alias declared in an outer scope and overridden in a narrower one freezes at the outer value. This migration survives only because both selectors match the same element: `.ckb-v2` is on `<html>` and outspecifies `:root`, so the alias substitutes against the winning v2 tier. Measured in Chromium rather than argued — both spellings compute to `rgba(13, 40, 71, 0.05) 0px 1px 2px 0px`. - delete all three declarations: `:root`, `.dark`, and the forced-colors flattening (`--e1` is already flattened there, so nothing changes) - `design-token-contract.test.ts` drops the alias from the role map and asserts the token is absent from the whole stylesheet rather than per theme block — any scope that redeclares it makes the alias spellable again. Mutation-verified: restoring it to `:root` fails with `--shadow-tight is retired; call sites use --e1` - repoint the one `tailwind-merge` conflict fixture so no test names a token that no longer exists - pin `legacyShadowAliases` to measured: 220 -> 127, 17 paths dropped. The old ceiling also carried 3 aliases of slack across the other six roles (measured 217), the same re-accumulated drift `#264` corrected on 9 August - GATES.md §3 row and a new §6 record; TOKENS.md §6 disposition row; the alias list in design-system.md, the two redesign direction docs and .design-sync/conventions.md; `#262` part 1 closed in the ledger The remaining 127 aliases are `#262`'s other tranches: soft 71, elevated 17, hover 17, card 12, lux 8, lift 2. Count by reading the `var()` call and not the declaration around it — two of the soft hits are the value of `--shadow-focus`, which is `#261` and shares no counter with this metric. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XDuNkNHU4zFn1cDquUXv5
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 selected for processing (82)
Comment |
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XDuNkNHU4zFn1cDquUXv5
BigSimmo
enabled auto-merge (squash)
August 10, 2026 13:52
BigSimmo
commented
Aug 10, 2026
OwnerAuthor
Unblock snapshot (2026-08-10): PR head is now d3f4079. I synced origin/main into this PR (commit d3f4079) to resolve an earlier real merge conflict. Current checks at snapshot: merge-tree clean, mergeable=true, mergeStateStatus=BEHIND, required workflows passing (Gitleaks + PR policy). No required-merge blockers or unresolved blocker threads remain. Required checks are green; pending is only normal behind state vs main while the tip is ahead by one commit from sync. Merge left to you. |
…a duplicate of #296 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XDuNkNHU4zFn1cDquUXv5
…gration-53a8kn' into claude/codex-m4b-shadow-tight-migration-53a8kn
Uh oh!
There was an error while loading. Please reload this page.
BigSimmo pushed a commit
that referenced
this pull request
Aug 13, 2026
Four merge-safe intake requests, no canonical ledger edit: - P1 rec: no gate detects a merged PR whose content is silently reverted by a later merge resolution. Blob sweep over origin/main found seven such PRs since 2026-08-06, six first wiped at one commit in the PR #1815 unblock chain. The reverts took each PR's tests in the same stroke, which is why nothing went red. - P2 task: re-land PR #1800 (fuzzy catalogue search) with the #310 one-edit cap applied in the same commit. - P2 task: re-land PR #1803 (--shadow-tight retirement); 67 files on main still reference the retired alias. - update #310: re-scoped. The defect is not reachable on main because the matcher is absent, not because it was fixed, so the row must not be closed and the fix belongs in the #1800 re-land. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DuYJz8hauCsCdx8r4fXiZU
This was referenced Aug 13, 2026
BigSimmo added a commit
that referenced
this pull request
Aug 14, 2026
…bt rows (#1942) * refactor(tokens): re-land the --shadow-tight retirement onto --e1 PR #1803 retired the --shadow-tight role alias in favour of the --e1 elevation tier across 49 files and squash-merged as 9d8370a on 2026-08-10. The acf78bf merge on 2026-08-11 silently reverted it, along with six other PRs. This re-applies the retirement against current main: 130 call sites across 67 files, plus both declarations. The alias was a pure pass-through -- `--shadow-tight: var(--e1)` in the light and dark role blocks -- so the substitution is value-preserving. Confirmed for forced-colors too rather than assumed: the `@media (forced-colors: active)` block scopes `:root, .dark`, the same `html` element the alias is declared on, so `--shadow-tight` already resolved through the flattened `--e1: none` there. The .ckb-v2 redeclaration hazard does not bite for the same reason -- .ckb-v2 sits on <html> and .ckb-v2.ckb-v2 outspecifies :root, so both spellings substitute against the winning v2 tier. Two comments survived acf78bf while the code they describe did not: the globals.css note that "the resting-hairline role is gone", and the token test's "unlike the --shadow-tight assertion above". Both are accurate again. The token contract test now sweeps the tracked src tree for both spellings (declaration and var() consumer) instead of only asserting the declaration. A declaration-only check would have caught this particular revert, but only because the declarations happened to come back with the call sites; sweeping both makes the gate independent of which half of a bad merge lands. Mutation-verified in both directions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsBPUoxpsvTwvFMJzrNoyz * chore(design-system): re-pin the contract ratchets to their measured values `scripts/design-system-contract-baseline.json` is a ceiling, so paying debt down leaves silent headroom behind. Ledger #302 records that pattern: legacyShadowAliases was pinned at 220 against a measured 193, 27 units of unguarded slack, up from 3 units on 2026-08-10. With the previous commit's --shadow-tight retirement applied the gap is wider still -- 220 pinned against 119 measured -- because the reland pays down the debt the acf78bf revert had re-hidden. Four other ratchets had accumulated slack from unrelated work in the same window. legacyShadowAliases 220 -> 119 edgeOwnershipConflicts 27 -> 25 rawPaddingLiterals 67 -> 63 rawGapLiterals 34 -> 32 layoutTransitionExceptions 12 -> 11 Regenerated with --print-debt-baseline rather than hand-edited, so the per-path debtByPath counts move with the totals -- those are what findDebtPathRegressions compares, and the retirement moved them wholesale. Every metric in the diff decreases; nothing is absorbed upward. This is not the baseline refresh #262 warns against. That stop rule forbids refreshing to hide the movement; this pins the movement in so it cannot silently drift back a second time. Mutation-verified: reintroducing one alias in button.tsx now fails at both the total (119 -> 120) and the per-path level. Under the old 220 ceiling the same addition passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsBPUoxpsvTwvFMJzrNoyz * refactor(tokens): hold the search-band count bubble in a spacing token The active-filter badge sized itself with a raw `h-[1.0625rem] min-w-[1.0625rem]` pair. Ledger #275 tracks that value as leaked debt: it had reached five files, so the fix has always been to tokenise once rather than edit a call site. Re-measured on merged main, the badge role is down to a single call site. #170's convergence landed in the meantime -- document-search- results.tsx now renders the shared control and therapy-compass/ filter-sheet.tsx was deleted outright -- so the leak this row was written about has already been reabsorbed by the extraction. Holding the value in @theme is what stops it leaving again. Two arbitrary values in the same component are deliberately left raw: pr-[0.6875rem] and min-[414px]:max-[429px] -- the repo defines no --breakpoint-* tokens at all, and eight peer sites use the same raw min-[]/max-[] form (359px, 389px, 414px). Naming one window while the peers stay raw is the same drift #275 warns about on another axis, and Tailwind named breakpoints would add variants across the whole utility surface. That belongs in a repo-wide decision, filed separately. The three remaining 1.0625rem hits in mode-nav.tsx and nav-slot-ink.tsx are NOT this token. They size <Icon> glyphs -- a 17px icon against a 12/14/16/20/24 --spacing-icon-* scale -- so folding them under a badge token would merge two roles that only happen to share a number. check:icon-scale deliberately does not flag arbitrary h-[Nrem], so they are a real but separate finding, filed rather than guessed at. The token is also registered in CLINICAL_TWMERGE_THEME.spacing, which tests/tailwind-merge-config.test.ts asserts against the @theme block -- without it `cn()` cannot resolve a conflict on the new utility. Safe by that file's own `tap` reasoning: the single call site is a static string carrying no competing h-*/min-w-* class and never passes through `cn()`, so there is no same-variant pair for declaration to hand to the later class. The entry is protective for future use, not load-bearing today. Value-preserving, and proven rather than inferred: compiling globals.css through @tailwindcss/postcss emits .h-search-band-badge { height: var(--spacing-search-band-badge) } .min-w-search-band-badge { min-width: var(--spacing-search-band-badge) } No ratchet moved, so the ceilings pinned in the previous commit still sit at zero slack. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsBPUoxpsvTwvFMJzrNoyz * docs(design-system): close out DS Track A3 and refresh the stale gate rows Track A3 is `#262`. Its three parts are now all settled, each checked against code rather than against the row that describes it. Part 1 is the --shadow-tight retirement re-landed earlier in this PR. Part 3 shipped in PR #1780 per `#301`: rawPaddingLiterals, rawRadiusLiterals and rawLineHeightLiterals are live baseline keys enforced over both the class and CSS-declaration spellings, plus rawGapLiterals beyond the original ask. Part 2 needs no work, and that had already been adjudicated -- GATES.md section 3 records it, which is why nothing here builds it. The decidable half of step selection shipped on 9 Aug inside check:design-system- contract: a declared @theme step no production surface selects fails the build. The remaining half -- which existing step a component picks -- is documented there as something "nothing mechanical can" gate, being a judgement about the rendered design rather than a property of the source, with a standing instruction not to duplicate the arbitrary-value check check:type-scale already ships. Reading `#262` alone would have sent a session to build it; that is the `#301` failure mode, so the closure record says so explicitly. Section 3's live status rows carried numbers this PR moved. `#301`'s lesson is that a row understating shipped work is a duplicate-work generator, so they are corrected in the same change: legacyShadowAliases 224 -> 119, and the alias is now retired outright rather than "224 left to retire" edgeOwnershipConflicts 27 -> 25 rawPaddingLiterals 67 -> 63 rawGapLiterals 34 -> 32 layoutTransitionExceptions 12 -> 11 Section 5 is left alone deliberately: it is a dated record measured against 8db1e53, not a live status surface, and rewriting its figures would destroy the provenance it exists to hold. Ledger records are queued as immutable inbox requests: `#262`, `#302` and `#275` closed; two carve-outs split out of `#275` filed as their own rows (the repo-wide breakpoint-token decision, and three 17px mode-nav icon glyphs that sit off the --spacing-icon-* scale with no gate covering them). The queued re-land request 210e3db5 is cancelled rather than reconciled -- its headline "67 files on main still use the retired alias" is false as of this branch, so it would open a row wrong on arrival. The request file and the cancellation both survive as provenance for the acf78bf merge loss. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsBPUoxpsvTwvFMJzrNoyz * chore(ledger): record the design-token relands review Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsBPUoxpsvTwvFMJzrNoyz * fix(issues): retarget the reland record after main reconciled it mid-flight CI failed `docs:check-links` on this branch with Error: cancel request 2e791c01... targets missing pending request 210e3db5... `check-docs-links.mjs` replays the inbox batch to resolve link targets, so an unresolvable request fails it. The cause was a race, not a bad record: PR #1936 reconciled 75 queued requests -- 210e3db5 among them -- while this branch was already in flight. Reconciling moves the request file into `docs/outstanding-issues-inbox/applied/` and allocates it a canonical row, so by the time this branch merged main there was no pending request left for the cancellation to name. Cancelling was the right call against a pending request and is the wrong one against a reconciled row. The cancel is dropped and replaced with a `done` against `#319`, the row 210e3db5 became. That is also the better record: the work is finished rather than withdrawn, so the ledger should carry its outcome and its guard, which a cancellation would have thrown away. Also merges origin/main (this branch was 3 behind) and files two findings the PR preflight surfaced, both deliberately not fixed here: - `check:medication-lexicon-report` has been failing on main for every local `verify:pr-local`, and no CI job runs it -- a grep over .github/workflows finds nothing. It is the last step of the local chain, so it fails preflights while CI stays green. The stale file is a clinical-facing generated document; regenerating it inside a CSS-token PR would bundle a clinical-risk artefact with unrelated chores. - Claude Code web containers can ship Node 22 with no node_modules, which fails `npm ci` on engine-strict before any repo script can run. Re-verified after the merge: the tracked tree still holds zero `--shadow-tight` references, and every pinned ratchet still measures exactly its baseline, so the merge moved no metric and the pins stay honest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsBPUoxpsvTwvFMJzrNoyz * docs(design): retire shadow-tight guidance * docs(design): retire shadow-tight guidance * docs(design): retire shadow-tight guidance * docs(design): retire shadow-tight guidance --------- Co-authored-by: Claude <noreply@anthropic.com>
1 task
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
--shadow-tightconsumer onto the--e1elevation tier. 150var(--shadow-tight)occurrences across 71 files becomevar(--e1)— 90 of those are the gated production sites across 48 files thatcheck:design-system-contractrecords underlegacyShadowAliases, and the remaining 60 are design-scratch mockups, migrated in the same pass so that no file names a token that is about to be deleted. GATES.md already forbids reaching for a role alias in new code, so every one of these call sites was recorded debt (#262part 1).--shadow-tightrole alias. All three declarations are deleted::root,.dark, and the forced-colors flattening. Isolated in its own commit because it edits the theme blocks.design-token-contract.test.tsdrops the alias from the role map and asserts the token is absent from the whole stylesheet rather than per theme block, because any scope that redeclares it makes the alias spellable again. The onetailwind-mergeconflict fixture is repointed so no test names a token that no longer exists.legacyShadowAliases220 → 127 with 17 paths dropped and per-path counts made exact. The previous ceiling was also carrying 3 aliases of slack across the other six roles (measured 217 against a 220 ceiling) — the same re-accumulated drift#264corrected on 9 August.#301records: the GATES.md §3 prohibition row, a new GATES.md §6 record of this pass, a TOKENS.md §6 disposition row, and the alias list indocs/design-system.md, both redesign direction docs and.design-sync/conventions.md.#262part 1 is closed in the outstanding-issues ledger.Why this renders identically, and why the obvious argument is not sufficient
--shadow-tightpointed atvar(--e1)in both themes and the forced-colors block already flattened--e1alongside the roles, so the substitution looks trivially safe from the declarations alone. It is not safe on that basis, and the reason is recorded in GATES §6 because it is owed to every remaining alias in#262.ckb-v2-tokens.cssredeclares the tier — light0 1px 2px rgb(13 40 71 / 5%)against globals'0 1px 2px rgb(11 42 56 / 7%)— and never redeclares the roles. A custom property whose value containsvar()substitutes on the element it is declared on, so an alias declared in one scope and overridden in a narrower one freezes at the outer value and the two spellings diverge. This migration survives only because both selectors match the same element:.ckb-v2sits on<html>inlayout.tsxand.ckb-v2.ckb-v2outspecifies:root, so the alias substitutes against the winning v2 tier.That was measured in Chromium rather than argued. A page carrying exactly that cascade computes
var(--shadow-tight)andvar(--e1)both torgba(13, 40, 71, 0.05) 0px 1px 2px 0px— the v2 value, not the:rootvalue. Light is the only theme where the two layers disagree at all; the dark tiers are byte-identical strings and forced-colors isnoneon both sides.What this does not do
Clearing
--shadow-tightdoes not zero the metric. 127 aliases remain across--shadow-soft(71),--shadow-elevated(17),--shadow-hover(17),--shadow-card(12),--shadow-lux(8) and--shadow-lift(2) — the other#262tranches. Count a token by reading thevar()call and not the declaration it sits in: two of the 71--shadow-softhits are the value of the--shadow-focusdeclarations in each theme, which is how an earlier pass mis-read them as a--shadow-focustally.LEGACY_SHADOW_ALIAShas never matchedfocus, so#261shares no counter with this metric.Verification
npm run verify:pr-local— superseded by the broader gate below, which covers it and more.npm run verify:cheapran green through all 30 static gates,lintandtypecheck. Decisive lines:The offline unit suite finished
Test Files 1 failed | 553 passed (554)/Tests 1 failed | 6024 passed | 4 skipped (6029). The single failure istests/pr-handoff-stop.test.ts > emits handoff context only when the marker file exists, and it is pre-existing and environmental, not from this diff: the same test file fails identically on the untouched basea16dd26in a clean worktree. The casechmods the git dir to0o555and expects the marker write to fail; this container runs as root, and root ignores the permission bits, so the write succeeds. It passes on a non-root CI runner.Two targeted proofs beyond the suite:
--shadow-tight: var(--e1);to:rootfails with--shadow-tight is retired; call sites use --e1, and the file was restored afterwards.findDebtPathRegressions.npm run verify:ui— UI verification not run: the installed Playwright browser revision does not match this checkout's expected revision (/opt/pw-browserscarrieschromium-1194while Playwright resolveschromium_headless_shell-1234), which is the known drift tracked as#255. Perdocs/testing.md, browser proof is delegated to the CIProduction UIjob rather than forcing a mismatched Chromium path. The targeted Chromium measurement described above was taken by launching the installed1194binary directly for a single computed-style read, which does not substitute for the journey suite.npm run verify:release— not run; no release or handoff confidence claim is made here, and it is provider-backed.npm run eval:retrieval:quality— not applicable. No retrieval, ranking, selection, chunking or scoring code is touched.npm run eval:rag -- --limit 15+npm run eval:quality -- --rag-only— not applicable. No answer generation, synthesis prompt or answer post-processing is touched.npm run check:production-readiness— not applicable, and provider-backed. No clinical workflow, privacy, environment, Supabase, source-governance or deployment behaviour changes.npm run check:deployment-readiness— not applicable. No deployment startup, hosting or rollout behaviour changes.Risk and rollout
--e1in a scope the alias could not see — was measured in Chromium and is not the case here; that measurement is recorded in GATES §6 as a prerequisite for the remaining#262tranches rather than a one-off. No logic, data, routing, or clinical behaviour is touched. The residual risk is a rendering difference the computed-style read would not catch, which the CIProduction UIjob covers.git reverteither commit independently. The retirement commit reverts on its own and simply restores the alias declarations next to their siblings; the migration commit reverts on its own only if the retirement is reverted first, since restoringvar(--shadow-tight)call sites needs the token to exist.Clinical Governance Preflight
Included because
classifyPullRequestFilesreturnsclinicalRisk: truefor this diff — it edits components under the document-viewer, differentials, DSM, factsheets and clinical-dashboard trees. The edits within those files are confined to shadow token names in class strings, so each item below is satisfied by the change touching nothing in that area.Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
C:\Users\joshs\.claude\projects\...\CODEX-M4B-SHADOW-TIGHT-MIGRATION.md) and was not reachable from the remote container, so the scope was reconstructed from the repository's own record: the branch name,#262part 1, the GATES.md prohibition row, and the#1797precedent for retiring a declared-but-unconsumed token. If the handover asked for something narrower — migrating the call sites without deleting the alias — the second commit is the part to drop.npm run format:checkacross the whole tree, not per changed file.Generated by Claude Code