Skip to content

fix(lint): compare the prose skip against a posix path - #5982

Merged
senamakel merged 1 commit into
tinyhumansai:mainfrom
ntdatt812:fix/lint-scales-windows-sep
Sep 11, 2026
Merged

senamakel merged 1 commit into
tinyhumansai:mainfrom
ntdatt812:fix/lint-scales-windows-sep

Conversation

@ntdatt812

@ntdatt812 ntdatt812 commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

The defect

On a Windows checkout pnpm lint:ui-tokens fails with 15 violations, and every one of them is English prose out of a translation catalogue:

lint:ui-tokens: 15 Tailwind utility/utilities name a scale or token that the Tailwind v4 theme does not define.
These emit NO CSS and render uncoloured:

  src\lib\i18n\en.ts:237:  text-to-speech  (to-speech is not a token this theme defines)
  src\lib\i18n\en.ts:690:  to-dos          (dos is not a token this theme defines)
  src\lib\i18n\en.ts:1135: text-based      (based is not a token this theme defines)
  src\lib\i18n\it.ts:818:  to-do           (do is not a token this theme defines)
  … 11 more, all in en/fr/id/it

scripts/lint-undefined-scales.mjs already has a guard for exactly this, and its doc comment names these exact strings:

- prose. The shadeless pass skips src/lib/i18n/and test files, where "text-to-speech", "to-do" and fixture strings likebg-noise are English and test data, not class lists.

The guard never fires on Windows:

function holdsProseNotClasses(relative) {
  return (
    relative.startsWith('src/lib/i18n/') ||   // relative is 'src\lib\i18n\en.ts'
    relative.includes('/__tests__/') ||       // and this one too
    /\.(test|spec)\.(ts|tsx|js|jsx)$/.test(relative)
  );
}

relative comes from path.relative, which yields the platform separator. Both /-spelled comparisons miss, the catalogues are scanned as class lists, and text-to-speech parses as the text- utility plus an undefined token to-speech. Only the extension regex, which has no separator in it, still works.

Why it matters beyond a noisy lint

The gate runs from .husky/pre-push, so on Windows it blocks every push, on a failure the author cannot have caused and cannot act on — the remedy the script prints ("choose a defined semantic token — do NOT add a scale only to silence this lint") does not apply to a to-do in a sentence. The only ways past are --no-verify or editing someone else's translations.

It also hides real findings: the 15 false positives are the entire output, so a genuine undefined scale introduced on a Windows machine is indistinguishable from the noise.

The fix

Normalise to / once, then compare. Nothing else in the script is separator-sensitive — the same walk() hands platform paths to every other check, and those are separator-blind.

Verification

On this host (Windows 11, Git Bash), against main:

node scripts/lint-undefined-scales.mjs
before 15 violations, all in src\lib\i18n\
after no undefined colour scales or tokens (30 shade-bearing scales, 68 shadeless colours, 18 shadows)
after, with the two comparisons reverted to relative the same 15 come back

The reverted-comparison run is the part worth stating: it pins the cause on the separator rather than on anything else that moved.

I did not add a unit test — lint-undefined-scales.mjs executes its scan at import time against a hard-coded appRoot/src, so testing holdsProseNotClasses would mean extracting it, which is a larger change than the bug warrants. Happy to do that separately if you would rather have the seam.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed prose-file detection on Windows by ensuring path checks work consistently across operating systems.
    • Prevented unintended lint warnings for matching internationalization test files.

`lint:ui-tokens` fails on a Windows checkout with 15 violations, every one of
them English out of a translation catalogue:

    src\lib\i18n\en.ts:237: text-to-speech  (to-speech is not a token this theme defines)
    src\lib\i18n\en.ts:690: to-dos          (dos is not a token this theme defines)
    src\lib\i18n\it.ts:818: to-do           (do is not a token this theme defines)

`holdsProseNotClasses` exists precisely to prevent this, and its own doc
comment names these exact strings. It never fires on Windows: `path.relative`
returns the platform separator, so the value it tests is `src\lib\i18n\en.ts`,
and `startsWith('src/lib/i18n/')` and `includes('/__tests__/')` both miss.
The catalogues are then read as class lists, where "text-to-speech" parses as
`text-` plus an undefined token.

The gate is in `.husky/pre-push`, so on Windows it blocks every push with a
failure the author cannot have caused and cannot silence — the suggested
remedy ("choose a defined semantic token") does not apply to prose.

Normalising to `/` before the comparisons is the whole change; the same walk
already yields platform paths to every other check, which are separator-blind.

Verified on this host: `node scripts/lint-undefined-scales.mjs` goes from
15 violations to `no undefined colour scales or tokens (30 shade-bearing
scales, 68 shadeless colours, 18 shadows)`, and reverting the two-line change
brings all 15 back.
@ntdatt812
ntdatt812 requested a review from a team September 2, 2026 16:00

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

$0.0000 · 0 in / 0 out

@tinysweeper

tinysweeper Bot commented Sep 2, 2026

Copy link
Copy Markdown

How this change flows

0 changed behaviours across 4 relationships. 5 surrounding behaviours are shown (60 graph nodes walked). 52 further behaviours left out to keep the diagram readable.

flowchart LR
  n0["join"]:::impacted
  n1["relative"]:::impacted
  n2["shadeResolver"]:::impacted
  n3["absorbThemeCss"]:::impacted
  n4["appRoot"]:::impacted
  n1 -->|uses| n4
  n2 -->|calls| n0
  n2 -->|calls| n3
  n2 -->|uses| n4
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading

Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge.

tinysweeper 0.1.0

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Sep 2, 2026
@coderabbitai

coderabbitai Bot commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 241bfb7f-a668-434b-98c4-cd21686860d4

📥 Commits

Reviewing files that changed from the base of the PR and between 8e65c40 and 1425632.

📒 Files selected for processing (1)
  • app/scripts/lint-undefined-scales.mjs

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The lint helper now normalizes relative paths to POSIX-style separators before applying prose and test-file checks. This makes the skip logic work correctly on Windows.

Changes

Cross-platform lint path handling

Layer / File(s) Summary
Normalize relative paths
app/scripts/lint-undefined-scales.mjs
holdsProseNotClasses converts relative paths to forward-slash form before matching i18n, test-directory, and test-file patterns.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 14256

This localized lint fix normalizes Windows paths so prose translation files are skipped correctly; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: senamakel

Poem

A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: comparing the prose-skip check against a POSIX path. It is concise and specific.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@senamakel
senamakel merged commit 1010b19 into tinyhumansai:main Sep 11, 2026
33 of 35 checks passed
senamakel added a commit to HDZTony/openhuman that referenced this pull request Sep 11, 2026
…indows-sep\n\nfix(lint): compare the prose skip against a posix path\n
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants