Uh oh!
There was an error while loading. Please reload this page.
fix(design-system): stop the radius contract from mandating invalid CSS - #1514
Merged
Conversation
`search-modal.css` rendered square corners where the concentric formula asks for 4px, and the governance test was the reason. CSS requires whitespace around `+`/`-` inside `calc()`. Without it the declaration is a parse error and the browser drops it, so the box falls back to radius 0. Measured in the renderer: calc(var(--radius-modal)-8px) -> 0px (dropped) calc(var(--radius-modal) - 8px) -> 4px (intended) The contract accepted only the broken spelling. Two rules combined to force it: `findCssOffenders` split corner values on `/\s+/`, which tore the valid form into three nonsense corners (`calc(var(--radius-modal)`, `-`, `8px)`) and rejected it; and `CALC_ALLOW_RE` used `\s*-\s*`, so the unspaced form passed. The only spelling that satisfied both was the one the browser throws away. - `splitCorners()` splits on whitespace at paren depth 0, so a calc() stays one corner and genuine multi-corner values still split. - `CALC_ALLOW_RE` now requires the whitespace CSS requires, making the invalid spelling a contract failure rather than the mandated one. - `search-modal.css:32` moves to the valid form. Verified live: the `.maka-search-modal-input-row` rule now computes 4px. Tailwind arbitrary values keep the unspaced spelling, deliberately — a literal space there would have to be written `_`, and Tailwind emits valid CSS either way. Confirmed against the built bundle: source rounded-[calc(var(--radius-md)-1px)] -> built calc(var(--radius-md) - 1px) normalized source border-radius: calc(var(--radius-modal)-8px) -> built calc(var(--radius-modal)-8px) verbatim So the TSX scanner keeps a lenient `isWhitelistedTailwindCalc` (which also accepts `_`-escaped spaces) while the CSS scanner is strict. Same rule, two languages; the token allowlist and shrink-only constraint apply to both. Found while landing #1511, which had to route around the contract by spelling a shrink as its tier token. Gates: 2858 desktop tests green, biome clean, 0 dead CSS.
Review of this PR found that `radius-nesting-contract.test.ts:63` is a SECOND gate on the same declaration `search-modal.css` shipped broken, and it still matched `\s*-\s*` — so after tightening the converge contract it became the only remaining place that would accept the unspaced spelling CSS drops as a parse error. Two gates on one site are only worth having if they agree. Also corrects that entry's label: the selector lives in `styles/search-modal.css`, not `sidebar.css`. The label is what the failure message points a future reader at, so a stale one sends them to the wrong file. Gates: 2874 desktop tests green, biome clean.
Uh oh!
There was an error while loading. Please reload this page.
jackwener added a commit
that referenced
this pull request
Jul 27, 2026
The CSS governance scanner matched `border-radius` values with the class `[^;}\n]+`. Excluding `\n` meant a declaration whose VALUE wrapped onto a second line matched nothing at all — the scanner skipped it in silence rather than reporting it, so any radius could escape the token contract just by being formatted across two lines: border-radius: 10px 12px; /* never scanned */ Neither prettier nor biome reflows such a value back onto one line, so a long four-corner shorthand or calc() reaches this state on its own. (A newline directly after the colon was always fine — the `\s*` there already spans it. Only a newline *inside* the value was affected, which is narrower than it first looks.) Widening the class to `[^;}]+` cannot make a match outrun its own declaration: `;` and `}` are still excluded, so it stops at the first declaration terminator or the end of the rule body either way. The regression test covering that is asserted alongside the fix. Found while reviewing #1514, which fixed a separate bug in the same contract (it accepted only invalid unspaced calc()). This is the remaining scanner gap.
jackwener added a commit
that referenced
this pull request
Jul 27, 2026
…ues (#1520) * fix(design-system): stop the radius scanner from skipping wrapped values The CSS governance scanner matched `border-radius` values with the class `[^;}\n]+`. Excluding `\n` meant a declaration whose VALUE wrapped onto a second line matched nothing at all — the scanner skipped it in silence rather than reporting it, so any radius could escape the token contract just by being formatted across two lines: border-radius: 10px 12px; /* never scanned */ Neither prettier nor biome reflows such a value back onto one line, so a long four-corner shorthand or calc() reaches this state on its own. (A newline directly after the colon was always fine — the `\s*` there already spans it. Only a newline *inside* the value was affected, which is narrower than it first looks.) Widening the class to `[^;}]+` cannot make a match outrun its own declaration: `;` and `}` are still excluded, so it stops at the first declaration terminator or the end of the rule body either way. The regression test covering that is asserted alongside the fix. Found while reviewing #1514, which fixed a separate bug in the same contract (it accepted only invalid unspaced calc()). This is the remaining scanner gap. * fix(ci): restore green main after #1515 `main` has been red since 17362dc (#1515); 0e80fe1 (#1359) inherited it. All three jobs failed, none of it visible in that PR's own checks because the earliest failure short-circuits the rest of the job. typecheck job - `format:check`: scripts/build-cursor-overlay.mjs was committed unformatted. Reapplied `biome format`; purely mechanical. - `knip`: #1515 added a second overlay entry point pair (permission-overlay{,-preload}.ts) but registered only the cursor overlay in knip.json, so both files read as unused. Registered them beside their cursor-overlay twins — they are loaded by path at runtime (loadFile / preload), never imported. - `knip`: asOsPermissionId was exported and referenced nowhere in the repo. It is an identity function (`return id`), so nothing is lost by dropping it — git history holds it if Stage 2 wants it back. Its sole import (OsPermissionId) went with it. test job - `check-console`: three console.warn sites in permission-overlay-main.ts (locale fallback, missing .app bundle, the controller log sink). These are main-process diagnostics that expose paths, not secrets — the same category as the daily-review and settings-runtime entries — so they are allow-listed with a reason per the script's own documented convention. e2e job - `settings.spec.ts:141` asserted a permission row draws exactly two grant buttons. #1515 added a third (拖拽授权) to precisely the row the fixture aims at — screen_recording is requestable, openable AND drag-grantable — so the `=== 2` filter matched nothing and the guard silently measured no rows. Relaxed to `>= 2`, which is what the contract was always about: a row whose actions track competes with the body. A wider actions track only makes the squeeze worse, so the three-button row is strictly the better subject. Verified locally: lint, format:check, build, typecheck, knip (both workspaces), check-console, and the full test:dist suite.
jackwener added a commit
that referenced
this pull request
Jul 27, 2026
…rns (#1535) An audit of the radius system found the same failure repeated: the rules were not derived from the design language, they were retrofitted to whatever the CSS already said. The suite was 18/18 green — but green because the non-conforming parts were either absent from the tables or had the rule rewritten around them. Two contracts had gone further and were actively holding defects in place. Contracts that were protecting defects -------------------------------------- .providerLogo — PR-UI-13 had pinned a single ~27-28% radius ratio across plate sizes after an audit found the 44px and 32px plates reading as different shapes. The tier convergence rewrote both to --radius-surface, which is 18.2% at 44px and 25% at 32px: the exact drift that PR removed. Both inline comments still claimed 27.3% / 28.1%, now arithmetically false, and SELECTOR_TIER pinned the regression, so restoring the ratio failed CI. Square icon plates are governed by RATIO, not by tier — a fixed px cannot read the same at 32px and 48px. Added --radius-plate: 27%, which is scale-invariant, restores the PR-UI-13 anchor (12px at 44px), and is correct at sizes not yet added. Applied to all six plates, which had drifted to four different ratios (18.2 / 25 / 16.7 / 22.2%). The compact override and the catalog-row override no longer restate a radius at all — with a ratio the size change alone is enough. .maka-search-modal-input-row — pinned by radius-nesting-contract to calc(var(--radius-modal) - 8px) = 4px, described as concentric nesting inside the 12px shell, with a warning that a tier "would read as too round". Measured live: that row is the middle of a 3-row grid, 57px below the shell's top edge, 12px in from its sides, adjacent to no shell corner. The 8px was the vertical margin to the header band — an axis with nothing to do with the corner. Its twin .maka-palette-input-wrap (same primitive, same position, same shell) used the control tier all along, so the two searches differed by 50% in roundness. Now control, and pinned in SELECTOR_TIER so they stay together. The rule itself was the root cause: it said only "with padding between them" and never mentioned adjacency, so the search modal satisfied it literally. Rewritten in maka-tokens.css and the nesting contract to state the precondition — the inner corner must sit in the outer corner, with an equal inset on both edges meeting there. Its worked example was this very bug; it now cites the one genuinely concentric site instead. Rules rewritten to fit the code ------------------------------- TabsList / TabsTrigger were dropped from COMPONENT_RADIUS when #499 moved them to primitives/tabs.tsx, on the stated grounds that they were "governed by primitives-design-contract escape hatches". That file contains no radius assertion at all, leaving the vocabulary's own "tab trigger" and "tab list" as the only named roles governed nowhere. The components had merely changed file; re-pointed. The vocabulary said `checkbox` belongs to pill/999px while every checkbox renders rounded-[var(--radius-control)]. The code is right (a fully round checkbox is a radio); the vocabulary was wrong, and because Checkbox is absent from COMPONENT_RADIUS nothing ever caught it. .enabledEmptyChip was pinned to control by reading its class name — it renders as a full-width two-line card with 16px padding. Surface now, matching its twin .settingsWechatQrState. Dead by construction -------------------- input-group's `**:[textarea_button]:rounded-[calc(var(--radius-sm)-1px)]` compiles to `:is(... *):is(textarea button)` — a <button> inside a <textarea>, which the HTML content model forbids. Not merely unused; unmatchable. Removed. Also refreshed the segmented.css comment, which justified using the tier over the calc form on the grounds that the calc was unspellable — true until #1514 fixed the value splitter. The tier is still right, for the better reason that it survives a token revalue. Not changed, deliberately ------------------------- Toggle / ToggleGroup look dead from the app's side (no JSX call sites) but are design-system primitives with a dedicated story, pinned by storybook-baseline-contract. Their entries stay. The tier VALUES themselves (6/8/12/999) are untouched. The audit raised real questions there — one control token spans 16px Kbd to 36px Input, a 2.25x swing in roundness, and 6-vs-8 is below the perceptual threshold as a global semantic distinction — but changing them moves the whole app's surface and is a design decision, not a defect fix. Verified: lint, format:check, build, typecheck, knip (both workspaces), check-console, check-dead-css, and 2879 main-process tests. Plate geometry was compared visually at all four sizes before and after.
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
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.
search-modal.cssrenders square corners where the concentric formula asks for 4px — and the governance test is why.CSS requires whitespace around
+/-insidecalc(). Without it the declaration is a parse error, the browser drops it, and the box falls back to radius 0. Measured in the renderer:The contract accepted only the broken spelling. Two rules combined to force it:
findCssOffenderssplit corner values on/\s+/, tearing the valid form into three nonsense corners —calc(var(--radius-modal),-,8px)— none of which is an allowed corner, so correct CSS was rejected.CALC_ALLOW_REused\s*-\s*, so the unspaced form passed.The only spelling satisfying both was the one the browser throws away.
Fix
splitCorners()splits on whitespace at paren depth 0, so acalc()stays one corner while genuine multi-corner values still split.CALC_ALLOW_REnow requires the whitespace CSS requires, so the invalid spelling is a contract failure rather than the mandated form.search-modal.css:32moves to the valid form. Verified live — the.maka-search-modal-input-rowrule now computes4px.Tailwind is deliberately exempt
Arbitrary values keep the unspaced spelling: a literal space there would have to be written
_, and Tailwind emits valid CSS either way. Confirmed against the built bundle:So the TSX scanner keeps a lenient
isWhitelistedTailwindCalc(which also accepts_-escaped spaces) while the CSS scanner is strict. Same rule, two languages. The token allowlist and shrink-only constraint apply to both.New cases cover the unspaced CSS rejection, the Tailwind acceptance, and
splitCornersend-to-end through the scanner.Found while landing #1511, which had to route around the contract by spelling a shrink as its tier token.
Gates
2858 desktop tests green · biome clean · 0 dead CSS.