fix(ds): sweep composition drift against design.md + ratchet five new metrics - #2905
fix(ds): sweep composition drift against design.md + ratchet five new metrics#2905innolope-dev wants to merge 7 commits into
Conversation
…metrics The token ratchet was green while law-level drift kept growing: weights stacked on type tokens minted off-ramp styles, spacing/radius/motion used off-scale values, and icons sat between the 16/20/24 steps. Fix the product-UI sites (page shells to gap-*, durations to motion tokens, weight stacks to their ramp tokens, icons and spacing snapped by role, skeleton tints to the ruled interim class) and add five ds-lint ratchet metrics so the residual — quests pending a ruling, geometry-driven indents, board-pending primitives — is frozen in the baseline instead of silently regrowing.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Code-analysis diffPainscore total: 7076.86 → 7079.92 (+3.06) 🆕 New findings (28)
…and 8 more. ✅ Resolved (28)
…and 8 more. |
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — changes requested
The design-system substitutions are consistent and CI is green, but the new spacing ratchet does not cover all valid Tailwind spacing utilities, so off-scale drift can still regrow unnoticed.
Findings
-
MAJOR · scripts/ds-lint-counts.mjs:270 · Spacing ratchet misses valid off-scale utilities
The regex enumerates only nine positive suffixes. With the repository's Tailwind 4.3 setup, inputs such asp-4.5,-m-2.5, andmd:pr-18are valid rendered utilities but do not match, so adding them leavesoffScaleSpacingunchanged andds-lint --checkpasses. The exact head already contains those missed forms, including the PR-describedpr-18geometry hold, contradicting the comment that deliberate holds are represented in the baseline. Extract all numeric spacing utilities (including negative values and variants), classify them against the documented scale, and explicitly account for sanctioned geometry. -
MINOR · scripts/ds-lint-counts.mjs:274 · [moonshotai/kimi-k3] iconOffScale ratchet is blind to icons sized via width/height props or size classes
OFF_SCALE_ICON_RE (scripts/ds-lint-counts.mjs, ~line 274) only matchessize={N}/iconSize={N}. The same diff leaves off-scale icons the metric cannot count: ChainChip.tsx keeps<Icon name={logo} width={18} height={18}>— 18px, the very value the PR notes 'was never on the scale' — and AmountInput/index.tsx keeps the swap arrow atwidth={32} height={32}, while ValidatedInput sizes its paste icon viah-3.5 w-3.5(14px). Any future off-scale icon added throughwidth=/height=orsize-*classes passes CI, defeating the stated goal that 'these classes cannot regrow'. Fix: extend the regex to(?:size|iconSize|width|height)=\{(?!16\}|20\}|24\})[0-9]+\}and add a class-based check forsize-/h-/w-on Icon components. -
MINOR · scripts/ds-lint-counts.mjs:270 · [moonshotai/kimi-k3] offScaleSpacing/rawDuration enumerations are bypassable with Tailwind v4 dynamic values
The repo uses Tailwind v4 dynamic utilities (this diff itself keepsh-4.5in HomeHistory,size-34in kyc/success,h-13in CardRejectionScreen), where any numeric value compiles. OFF_SCALE_SPACING_RE (~line 270) only flags the enumerated set1.5|2.5|3.5|5|7|9|11|13|15and RAW_DURATION_RE (~line 282) only75|100|150|200|300|500|700|1000. Concrete bypasses: the PR's own flagged table citesBaseInput pr-15 md:pr-18—pr-15is counted butpr-18is invisible to the metric; likewiseduration-250,pt-4.5, or a negative-mt-5(the lookbehind excludes a leading-) all sail through the ratchet. Fix: invert the match — flag any numeric spacing/duration utility whose value is not in the sanctioned set, e.g./\bduration-(?!instant\b|fast\b|moderate\b|slow\b)[0-9]+\b/and a general numeric spacing matcher validated against the allowed steps rather than a blocklist of forbidden ones.
Checked clean
- Pinned head and base SHAs, merge base, trusted author, and dev base metadata matched the supplied values.
- All exact-head CI checks completed successfully or were intentionally skipped, including unit, typecheck, eslint, format, ds-lint, and ds-shots.
- Reviewed all 81 changed files against mono/design/design.md across page shells, typography, icons, spacing, radii, motion, and skeleton tint.
- Ran git diff --check and generated the design-system lint counts at the exact head.
- Adversarially enumerated numeric spacing utilities against the documented scale and verified uncaptured Tailwind 4.3 classes already exist in non-allowlisted product code.
Second opinion by moonshotai/kimi-k3: 2 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Exact head: 3438a18d9200 · Context: repo, design
Tailwind v4 compiles any numeric step, so enumerating known-bad values let pr-18, p-4.5, negative margins, and variant-prefixed forms regrow unseen. Flag every numeric spacing utility whose magnitude is off the documented scale, any numeric or arbitrary duration, and icons sized via width/height props or size-/h-/w- classes — and snap ChainChip's 18px icons to 16 now that the metric can see them. Deliberate holds land in the regenerated baseline (--allow-increase: wider net, same tree).
There was a problem hiding this comment.
Chip review — changes requested
The earlier spacing-ratchet issue is still present: broader numeric matching still omits valid logical padding and margin utilities, allowing off-scale spacing to pass the gate.
Findings
- MAJOR · scripts/ds-lint-counts.mjs:275 · Spacing ratchet still misses logical utilities
The prior finding is STILL PRESENT. Tailwind 4.3.3 supports numericps-*,pe-*,ms-*, andme-*utilities, butNUMERIC_SPACING_REomits them. A newclassName="ps-5"(20px, banned by the documented scale) in any non-allowlisted TSX leavesoffScaleSpacingunchanged, sonode scripts/ds-lint-counts.mjs --checkpasses. Include the logical padding and margin families, including their block-axis forms, in the matcher and add regression cases for these valid utility forms.
Checked clean
- Verified detached HEAD, base ref and SHA, trusted PR author, title, and description
- Re-evaluated the earlier spacing finding against the current matcher: partially broadened but still bypassable by valid logical spacing utilities
- Ran the ds-lint check at the exact head; it passes with the committed baseline counts
- Reviewed exact-head CI: unit, typecheck, eslint, format, ds-lint, baseline, analysis, and preview checks passed; ds-shots was still running
- Reviewed the 81-file styling diff for behavioral and security changes; no additional actionable defect survived verification
- Compared spacing behavior with the mono design rulebook and the Tailwind 4.3.3 utility contract
Second opinion skipped: openrouter-timeout.
Exact head: 2f31716c9d70 · Context: repo, design
Tailwind's ps/pe/ms/me utilities compile like any physical spacing class, so the spacing ratchet has to match them too (re-raised in review). The matchers move to ds-lint-rules.cjs — the counting script has import-time side effects, so a shared CJS module is what lets jest exercise the exact shipped rules — with regression cases for the bypasses the review named: logical families, negatives, variant prefixes, fractional steps, prop- and class-sized icons, arbitrary durations. Counts are unchanged (no logical drift exists yet), so the baseline stands.
There was a problem hiding this comment.
Chip review — changes requested
Request changes: the earlier numeric and logical spacing findings are fixed, but two common syntax forms still bypass the new drift ratchets.
Findings
-
MAJOR · scripts/ds-lint-rules.cjs:14 · Spacing ratchet ignores arbitrary-value utilities
The matcher only recognizes plain numeric suffixes, and the regression test explicitly treatsp-[13px]as clean. In any non-page/view component, addingp-[5px]orgap-[20px]violates the documented spacing scale but leaves bothoffScaleSpacingand the view-only generic arbitrary-class metric unchanged, so ds-lint stays green and spacing drift can regrow. Match arbitrary spacing values too (or reject them conservatively) and add a component-level regression test. -
MAJOR · scripts/ds-lint-counts.mjs:267 · Typography ratchet misses multiline class expressions
This count requires the type token and weight utility to occur on the same physical line. A formatted class expression such as the existing NetworkListItem template putstext-body-mon one line andfont-semiboldinsidetwMergeon the next, so the element renders the forbidden stacked weight without increasing the metric; new instances can therefore bypass CI. Count per className expression or parsed literal block, and add a multiline regression test.
Checked clean
- Confirmed the detached worktree HEAD, supplied base ref/SHA, PR author, and PR metadata match the trusted request.
- Earlier finding at 3438a18 is fixed: the spacing rule now uses a documented-scale allowlist for numeric utilities and covers negative and variant-prefixed forms.
- Earlier finding at 2f31716 is fixed: logical ps/pe/ms/me utilities are included and covered by regression tests.
- Reviewed the mechanical UI sweep against mono/design/design.md: page-shell gap swaps, type-token mappings, icon steps, motion tokens, spacing snaps, radii, and skeleton tints showed no separate correctness defect.
- Checked the new CommonJS matcher module, import path, script syntax, security boundaries, and
node scripts/ds-lint-counts.mjs --check; all completed successfully. - Local Jest could not run because the detached worktree has no node_modules; the exact-head unit, eslint, ds-lint, typecheck, format, analyze, and aggregate CI checks succeeded. ds-shots was still in progress when checked.
Second opinion skipped: openrouter-timeout.
Exact head: 4b10822f8d1a · Context: repo, design
…r className expression Round-3 review bypasses: p-[5px] compiles like any spacing utility but matched neither the spacing ratchet nor the view-only arbitrary metric, and the weight-stack count was line-based, so a token and its stacked weight split across a formatted twMerge call rendered the forbidden style without moving the metric. Arbitrary bracket values now count wholesale (an on-scale value has a numeric class, so the bracket form is always drift) and the weight rule scans brace-balanced className expressions with a per-line pass over the rest for const class strings. The three stacks the wider net exposed are fixed, not baselined: NetworkListItem's title (the review's example) and DevSegmented map to their ramp tokens, and BaseSelect drops a checked-state font-bold that was a no-op on already-bold text-label-l.
There was a problem hiding this comment.
Chip review — changes requested
Request changes: the earlier multiline typography gap remains for nonliteral class props, and the icon and radius ratchets omit valid syntax already present in the tree. The arbitrary-value and logical-spacing findings are fixed.
Findings
-
MAJOR · scripts/ds-lint-rules.cjs:40 · Typography ratchet still misses multiline class props
STILL PRESENT from the prior review. countWeightStacks returns 0 for a multiline titleClassName={twMerge('text-body-m', 'font-semibold')} expression, while the same expression under literal className returns 1. The scanner recognizes only lowercase className=, and its fallback checks one line at a time, so titleClassName/contentClassName props can add forbidden stacks without moving the baseline. Scan every JSX prop ending in ClassName (or use the TSX AST) and add this regression case. -
MAJOR · scripts/ds-lint-rules.cjs:87 · Icon ratchet ignores expression-valued className
The current tree already has an Icon with className={h-4 w-4 ...} in src/components/TransactionDetails/provider-rows/BridgeDepositInstructions.tsx, but OFF_SCALE_ICON_RE reports zero because it only accepts a double-quoted attribute. The iconOffScale baseline therefore omits a real class-sized Icon, and an equivalent new use passes CI. Parse Icon attributes or cover brace/template/string expressions, then regression-test this existing form. -
MAJOR · scripts/ds-lint-rules.cjs:89 · Radius ratchet ignores arbitrary values
Two current rounded-[1px] classes in src/app/[locale]/(marketing)/status/StatusBoard.tsx are outside GLOBAL_ALLOW, yet OFF_SCALE_RADIUS_RE returns zero and the offScaleRadius baseline omits them. A new rounded-[5px] likewise passes, defeating the app-wide radius ratchet. Match arbitrary and other unsupported radius forms via allowlist inversion, then baseline or fix existing holds and add regression tests.
Checked clean
- Supplied detached worktree HEAD, PR head/base metadata, and merge base matched the exact requested SHAs.
- Prior arbitrary-value spacing finding: FIXED; bracket forms are counted and regression-tested.
- Prior multiline typography finding: STILL PRESENT for JSX props ending in ClassName other than literal className.
- Prior logical ps/pe/ms/me spacing finding: FIXED and regression-tested.
- Reviewed the complete source diff, shared DS matchers/tests, baseline behavior, and mono design-system spacing, type, icon, radius, and motion laws.
- Exact-head bot-approval, human-authors, format, typecheck, eslint, ds-lint, unit, ds-shots, analyze, and Deploy-Preview checks succeeded.
- Local node scripts/ds-lint-counts.mjs --check passed; local Jest could not run because dependencies are absent from the detached worktree, while exact-head unit CI succeeded.
- Security pass found no workflow, authorization, secret, or trust-boundary changes in the diff.
Second opinion skipped: daily-budget-reached.
Exact head: 6ed534303ef2 · Context: repo, mono/design
…invert radius rule Round-4 review bypasses, same shape as before: the weight scanner only recognized the literal className attribute, so titleClassName-style props could carry stacks; the icon rule only matched double-quoted classNames, missing template/brace expressions; and the radius rule was still a blocklist, blind to rounded-[1px]. All three now match by structure — any *ClassName prop, any size-/h-/w- class inside an Icon tag span, any rounded suffix off the documented scale — with regression cases for each review example. The two product Icons the wider net exposed are fixed (ActionModal size-6 class and the Bridge chevron h-4 w-4 become size props); the four radius holds are marketing, dev tooling, and the drawer-sheet top radius already flagged as DS-chrome pending a ruling.
There was a problem hiding this comment.
Chip review — changes requested
Three enforcement gaps remain in the new composition ratchets. Earlier arbitrary-spacing, multiline *ClassName prop, numeric expression-valued Icon class, and arbitrary-radius findings are fixed, but extracted multiline typography expressions remain uncounted and two additional blind spots leave reachable design-system drift undetected.
Findings
-
MAJOR · scripts/ds-lint-counts.mjs:269 · Composition ratchets skip classes exported from .ts
Each new metric filters with isTsx, so class constants in .ts modules are invisible. The existing imported CARD_HOVER string in src/components/Marketing/mdx/constants.ts contains duration-150, yet rawDuration remains 9; adding another raw duration, off-scale spacing/radius, or type stack in a .ts class module leaves the ratchet green. Scan all already-collected .ts/.tsx files while retaining the current allowlists. -
MAJOR · scripts/ds-lint-rules.cjs:81 · Typography ratchet still misses extracted multiline expressions
countWeightStacks groups *ClassName attributes, then scans all remaining code one line at a time. A class builder split across lines, such as twMerge('text-body-m', active && 'font-semibold'), followed by className={classes}, therefore returns 0 and the earlier multiline-expression gap remains reachable. Group multiline class-producing assignments/builders or parse the AST, and add this extracted-variable regression case. -
MAJOR · scripts/ds-lint-rules.cjs:91 · Icon ratchet still misses arbitrary class sizing
An Icon with className={twMerge('size-[18px]', extra)} (and equivalent h-/w- bracket forms) returns 0 because the class branch requires a digit immediately after the hyphen. The icon renders off-step while iconOffScale stays unchanged. Match bracket-valued size/h/w classes too, scoped to className expressions, and add string and expression regression cases.
Checked clean
- Exact head SHA, supplied dev base SHA, trusted author, and clean detached worktree verified
- Prior spacing arbitrary-value finding is fixed and covered by regression tests
- Prior multiline *ClassName prop finding is fixed; extracted multiline class builders remain as reported
- Prior expression-valued numeric Icon class finding is fixed; arbitrary bracket sizing remains as reported
- Prior arbitrary-radius finding is fixed and covered by regression tests
- Mechanical spacing, typography, icon, radius, and motion substitutions checked against mono/design/design.md
- Exact-head format, typecheck, eslint, ds-lint, unit, ds-shots, analyze, and preview checks are green
- node scripts/ds-lint-counts.mjs --check passes the committed baseline; exported matchers were exercised against adversarial cases
- No runtime authorization, data, money-movement, dependency, or secret-handling changes in the diff
Second opinion skipped: daily-budget-reached.
Exact head: af94ffc4f6ca · Context: repo, design
… icon sizes Round-5 review gaps: the composition metrics filtered to .tsx, so class constants exported from plain .ts modules (Marketing CARD_HOVER carried duration-150) were invisible; a twMerge/clsx call assigned to a variable escaped the weight scanner because its span is outside any *ClassName attribute; and the icon class branch required a digit after the hyphen, missing size-[18px] bracket forms. The five metrics now scan every collected file, builder-call argument spans count as class regions (deduped against attribute regions), and bracket-valued size/h/w classes match. CARD_HOVER's duration is fixed on the motion token; the one new baseline entry is a JSDoc geometry mention, not a rendered class.
There was a problem hiding this comment.
Chip review — changes requested
Request changes: three concrete Tailwind spacing/motion bypasses and an incomplete typography-weight matcher still let the new composition ratchets miss prohibited drift. The earlier icon, .ts scanning, multiline typography, arbitrary-spacing/radius, and arbitrary icon-class gaps are fixed.
Findings
- MAJOR · scripts/ds-lint-rules.cjs:35 · Typography ratchet checks only three weights
fontWeightOnTypeTokenis described as preventing any weight utility from overriding a type token, butWEIGHT_STACK_REonly matches bold, semibold, and extrabold. For example, addingclassName="text-body-s font-normal"changes Body/S from its token-defined Medium weight to Regular while the metric remains unchanged;font-light,font-medium,font-black, and arbitrary weight forms are likewise invisible. Match every Tailwind font-weight override and add regression cases for the omitted forms.
Already resolved by a human, not raised again
These are still present as far as this review can tell. You closed the thread, so Chip files them instead of repeating them.
- *MAJOR · scripts/ds-lint-counts.mjs:270 · Spacing ratchet still misses -px utilities — task
- MAJOR · scripts/ds-lint-counts.mjs:275 · Logical block-axis spacing remains unscanned — task
- MINOR · scripts/ds-lint-counts.mjs:270 · Custom-property spacing and durations bypass ratchets — no task filed, please file one
Checked clean
- Verified the supplied detached checkout at the exact head SHA and the exact dev base/merge base before the dispatcher removed the interrupted worktree.
- Exact-head CI is green, including unit, ds-lint, typecheck, eslint, format, analyze, and ds-shots.
- Ran
node scripts/ds-lint-counts.mjs --checksuccessfully at the exact head; focused local Jest could not start because the detached worktree had no node_modules, while the exact-head unit check passed in CI. - Prior P2 and P5-P12 are fixed in current code: literal width/height and class-sized Icons are scanned, arbitrary spacing/radius and logical inline families are covered, multiline/builder typography is scanned, and all five composition metrics include .ts modules.
- Reviewed the UI sweep against the design rulebook; changes are composition-only and no product state, authorization, amount, or persistence behavior changed.
Second opinion skipped: openrouter-timeout.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 10f92f09f561 · Context: repo, design, web · Took 12m (queued 50m)
| // weight split across formatted lines inside one twMerge/clsx call still | ||
| // count; class strings held in variables outside className= are caught by a | ||
| // per-line pass over the remaining text. | ||
| const WEIGHT_STACK_RE = /\bfont-(?:bold|semibold|extrabold)\b/ |
There was a problem hiding this comment.
MAJOR: Typography ratchet checks only three weights
fontWeightOnTypeToken is described as preventing any weight utility from overriding a type token, but WEIGHT_STACK_RE only matches bold, semibold, and extrabold. For example, adding className="text-body-s font-normal" changes Body/S from its token-defined Medium weight to Regular while the metric remains unchanged; font-light, font-medium, font-black, and arbitrary weight forms are likewise invisible. Match every Tailwind font-weight override and add regression cases for the omitted forms.
Round-6 review: the weight matcher covered only the bold family, so font-normal/medium/light/black and arbitrary font-[N] overrides changed a token's weight invisibly. Matching all weights surfaced 63 stacks: 26 were no-ops repeating the token's own weight (dropped, zero visual change), 4 map to real ramp steps (rewards how-it-works titles and the scanner micro-label), and the rest are weight overrides with no ramp step — body-s Regular, body-xs Medium, body-l Medium — held in the baseline for a token ruling like the UserHeader 12/600 case.
There was a problem hiding this comment.
Chip review — changes requested
Five valid Tailwind forms still bypass the new design-system ratchets; P9 and P12 survive through custom-property shorthand, and P14-P16 remain present.
Findings
-
*MAJOR · scripts/ds-lint-rules.cjs:17 · Spacing ratchet still misses -px utilities
Tailwind 4.3 acceptsp-px,gap-px, and negative margin-m-px, but this matcher only accepts digit magnitudes. Addingp-pxtherefore leavesoffScaleSpacingunchanged (verified as 0), so 1px spacing drift can regrow while ds-lint stays green. Treatpxas an off-scale magnitude for every scanned family and add positive, negative, and variant-prefixed tests. -
MAJOR · scripts/ds-lint-rules.cjs:15 · Logical block-axis spacing remains unscanned
Tailwind 4.3 compilespbs/pbe/mbs/mbe, butSPACING_FAMILIESincludes only the inline logical families.<div className="pbs-5 mbe-7">adds two off-scale values whilecountOffScaleSpacingreturns 0. Add the four block-axis families to numeric and arbitrary matching and cover negative and variant-prefixed cases. -
MINOR · scripts/ds-lint-rules.cjs:20 · Custom-property spacing and durations bypass ratchets
Tailwind 4.3 compilesp-(--gutter)andduration-(--speed), but the matchers recognize only numeric or square-bracket values; both adversarial examples return 0. Match the parenthesized custom-property form for spacing and duration and add regression tests. -
MINOR · scripts/ds-lint-counts.mjs:297 · [moonshotai/kimi-k3] --allow-increase justification is never persisted, so any ratchet ceiling can be raised silently in a drift PR
scripts/ds-lint-counts.mjs:296-300 parses--allow-increase "<reason>"and prints the reason to local stderr, but the rewritten scripts/ds-lint-baseline.json records only the new numbers. A contributor can add off-scale drift (e.g. a newp-5) and in the same PR run--write-baseline --allow-increase "x"— or hand-edit baseline.json — raising any of the now 16 debt ceilings; CI's--checkthen passes and no durable audit trail of the claimed justification exists in the repo. This undercuts the PR's stated guarantee ('deliberate holds live inside the baseline... new drift fails CI') and the F-14 mitigation the comment cites, which only makes bumps non-silent for the person running the command, not for reviewers. Fix: when --allow-increase is used, write the reason (and date) into the baseline file itself, e.g. a_meta: { metric, reason, date }entry, and/or add a CI check that fails when ds-lint-baseline.json differs from the merge-base without a CODEOWNERS approval or explicit label.
Already resolved by a human, not raised again
These are still present as far as this review can tell. You closed the thread, so Chip files them instead of repeating them.
- MAJOR · scripts/ds-lint-rules.cjs:89 · Radius ratchet still misses custom-property values — task
- MAJOR · scripts/ds-lint-rules.cjs:91 · Icon ratchet still misses custom-property class sizing — task
Checked clean
- Pinned metadata verified: trusted author innolope-dev, head 36d4fa8, base dev at 0ea3f57, and merge base equals the supplied base SHA.
- Prior findings rechecked: P1-P8, P10-P11, and P13 are fixed at this head; P9 and P12 survive through Tailwind v4 parenthesized custom-property syntax; P14-P16 remain present.
- Correctness and slop pass covered the complete 100-file diff; production changes are class/token substitutions with no control-flow, state, amount, authorization, or API-contract changes.
- Security pass found no new trust boundary, secret handling, injection surface, privilege, or workflow-write behavior.
- Exact-head completed checks succeeded for ci-success, unit, ds-lint, format, eslint, typecheck, analyze, human-authors, bot-approval, be-baseline, report, and review; ds-shots and preview deployment were still in progress when checked.
- Local
node scripts/ds-lint-counts.mjs --checkandgit diff --checkpassed. The focused Jest command could not run because this detached worktree has no installed node_modules; the exact-head unit check is green. - Adversarial probes confirmed zero counts for
p-px,gap-px,pbs-5,mbe-7,p-(--gutter),duration-(--speed),rounded-(--card-radius), and parenthesized Icon size classes.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 36d4fa88cbcb · Context: repo, design, web · Took 18m (queued 2m)
| const SPACING_STEPS = new Set(['0', '0.5', '1', '2', '3', '4', '6', '8', '10', '12', '14', '16']) | ||
| const SPACING_FAMILIES = 'px|py|pt|pb|pl|pr|ps|pe|p|mx|my|mt|mb|ml|mr|ms|me|m|gap-x|gap-y|gap|space-y|space-x' | ||
| const NUMERIC_SPACING_RE = new RegExp( | ||
| `(?<![a-z0-9-])-?(?:${SPACING_FAMILIES})-([0-9]+(?:\\.[0-9]+)?)(?![0-9.a-z%\\]])`, |
There was a problem hiding this comment.
*MAJOR: Spacing ratchet still misses -px utilities
Tailwind 4.3 accepts p-px, gap-px, and negative margin -m-px, but this matcher only accepts digit magnitudes. Adding p-px therefore leaves offScaleSpacing unchanged (verified as 0), so 1px spacing drift can regrow while ds-lint stays green. Treat px as an off-scale magnitude for every scanned family and add positive, negative, and variant-prefixed tests.
| // arbitrary values (p-[5px], gap-[20%]) are rejected wholesale: a value that | ||
| // equals a scale step has a numeric class, so the bracket form is always drift. | ||
| const SPACING_STEPS = new Set(['0', '0.5', '1', '2', '3', '4', '6', '8', '10', '12', '14', '16']) | ||
| const SPACING_FAMILIES = 'px|py|pt|pb|pl|pr|ps|pe|p|mx|my|mt|mb|ml|mr|ms|me|m|gap-x|gap-y|gap|space-y|space-x' |
There was a problem hiding this comment.
MAJOR: Logical block-axis spacing remains unscanned
Tailwind 4.3 compiles pbs/pbe/mbs/mbe, but SPACING_FAMILIES includes only the inline logical families. <div className="pbs-5 mbe-7"> adds two off-scale values while countOffScaleSpacing returns 0. Add the four block-axis families to numeric and arbitrary matching and cover negative and variant-prefixed cases.
| `(?<![a-z0-9-])-?(?:${SPACING_FAMILIES})-([0-9]+(?:\\.[0-9]+)?)(?![0-9.a-z%\\]])`, | ||
| 'g' | ||
| ) | ||
| const ARBITRARY_SPACING_RE = new RegExp(`(?<![a-z0-9-])-?(?:${SPACING_FAMILIES})-\\[[^\\]]+\\]`, 'g') |
There was a problem hiding this comment.
MINOR: Custom-property spacing and durations bypass ratchets
Tailwind 4.3 compiles p-(--gutter) and duration-(--speed), but the matchers recognize only numeric or square-bracket values; both adversarial examples return 0. Match the parenthesized custom-property form for spacing and duration and add regression tests.
| @@ -266,6 +296,11 @@ const DEBT_KEYS = [ | |||
| 'classNameSitesInPages', | |||
| 'deadLegacyTokens', | |||
There was a problem hiding this comment.
MINOR: [moonshotai/kimi-k3] --allow-increase justification is never persisted, so any ratchet ceiling can be raised silently in a drift PR
scripts/ds-lint-counts.mjs:296-300 parses --allow-increase "<reason>" and prints the reason to local stderr, but the rewritten scripts/ds-lint-baseline.json records only the new numbers. A contributor can add off-scale drift (e.g. a new p-5) and in the same PR run --write-baseline --allow-increase "x" — or hand-edit baseline.json — raising any of the now 16 debt ceilings; CI's --check then passes and no durable audit trail of the claimed justification exists in the repo. This undercuts the PR's stated guarantee ('deliberate holds live inside the baseline... new drift fails CI') and the F-14 mitigation the comment cites, which only makes bumps non-silent for the person running the command, not for reviewers. Fix: when --allow-increase is used, write the reason (and date) into the baseline file itself, e.g. a _meta: { metric, reason, date } entry, and/or add a CI check that fails when ds-lint-baseline.json differs from the merge-base without a CODEOWNERS approval or explicit label.
1 participant
Sweep of composition-level DS drift against
mono/design/design.md, plus five new ds-lint ratchet metrics so these classes cannot regrow. Token metrics were already green; this PR targets the law-level drift the token ratchet cannot see.What changed
1. Page shells (9 shells, 7 files).
space-y-*on themin-h-[inherit]outer shell conflicts withmy-autocentering (design.md outer-shell recipe). Swapped togap-*. withdraw, withdraw/crypto, MantecaLimitsView, UnlockPayments.view, Create.request.link.view, Contacts.view, InputAmountStep.2. Motion tokens (25 sites). Raw
duration-75/100/150/200/300/500swapped toduration-instant/fast/moderate/slow. 100/200/300/500 map to identical values (zero visual change); 75→instant and 150→fast snap to the nearest step.3. Type ramp (~55 sites). Weight classes stacked on type tokens minted styles the ramp does not define. Mapped per the ramp:
text-body-s font-bold|font-semibold→text-label-l(same 14/20 box, sanctioned Bold)text-body-m font-bold|font-semibold→text-body-m-semibold(the docs' "no bold step exists" mapping)text-body-xs font-bold→text-label-m(micro-labels/badges)text-body-l font-semibold→text-heading-card(18px titles)font-extraboldontext-heading-xs,font-boldontext-label-m,font-extraboldontext-button-mdropped (tokens carry their weight)4. Icon steps (~35 sites). Snapped to the 16/20/24 scale: inline info/hints and icons next to text → 16; icons inside buttons and 40px circles → 20 (matches the button board note that 18 was never on the scale). Trailing 14px icons on Link-pattern rows kept — the LinkButton board sanctions 14 there. Also dropped a stray
iconSize={13}on the ExchangeRateWidget CTA so the button default (20) applies.5. Spacing scale (~30 sites). Off-scale values snapped to the nearest step by role (design.md anatomy rules):
gap-1.5→gap-1(inline icon↔label),py-2.5→py-2,gap-5→gap-6/gap-4by section vs container,px-5→px-4on the drawer container (the documented L/16 inset),pl-5→pl-6list indents,pb-5→pb-4,mb-5→mb-4.6. Radii (6 sites).
rounded-md→rounded-smon Tooltip/IframeWrapper chrome and the PublicProfile chip; dropped inertrounded-lgon three containers with no background/overflow.7. Skeleton tint (7 sites). Legacy
bg-gray-200pulse placeholders → the ruled interimbg-foreground-primary/10(HomeHistory, ContactsListSkeleton, CountryListSkeleton).8. ds-lint: five new ratchet metrics.
fontWeightOnTypeToken,offScaleSpacing,iconOffScale,offScaleRadius,rawDurationadded toscripts/ds-lint-counts.mjs+ baseline regenerated (13 / 181 / 64 / 33 / 9 after the review pass widened them to allowlist inversions — any numeric spacing step off the documented scale, any numeric or arbitrary duration, icons sized via props or classes). Deliberate holds live inside the baseline, not an allowlist: a ruling drives the count down, new drift fails CI.Flagged, not changed (per design.md law 6)
app/quests/*inlinesbg-[#FFC900], stockblue-200/600,text-[10px], raw durations; not in the ds-lint allowlistGlobal/Modalused directly by SumsubKycWrapper, SumsubNativeSdk, QRScannerOverlayGlobal/Drawerrounded-t-2xl,Global/Modalrounded-md, QRScanner viewfinderrounded-2xlNotificationpl-7indentBaseInputpr-15 md:pr-18slot reserveStatusBadgepy-1.5,StatusPillsize={14}17802:61533before changingSegmentedControlpy-1.5,Checkboxlabeltext-body-xs font-bold❓primitives pending designPaymentInfoRowweights + 12px info iconUserHeaderusernametext-body-xs font-semiboldCardFacetext styles, PerkClaimModal gift-box artshadowSize="4"call sitesVerification
pnpm prettier --check .cleanpnpm typecheckcleannode scripts/ds-lint-counts.mjs --checkgreen on all 16 metrics