chore(ci): ratchet locale hygiene

[orangeCatDeveloper]

Summary

Adding a UI locale today compiles cleanly and still renders the wrong language wherever code branches on a locale literal instead of indexing a UiCatalog. On main today the four patterns hit 67 locale === '…' comparisons, 4 locale: UiLocale = '…' defaults, 12 CJK payload sniffs, and 32 simplified-keyed translation maps (per match, both quote styles), none of which the type system sees.

This adds scripts/check-locale-hygiene.mjs, a ratchet over apps/desktop/src, packages/core/src, and packages/ui/src that counts those four patterns per file and fails CI when any count grows against the base commit. There is no ledger: the base is the merge base (or BASE_SHA in CI), so paying down debt needs no bookkeeping. Only files in git diff -M30% <base> are scanned, since an untouched file cannot grow a count; the diff is not pathspec-limited, so a file moved into scope keeps its base path. The rules match ' and " alike because biome leaves apps/desktop and packages/ui unformatted, and count per match so splitting a line cannot fail the gate. With no resolvable base the step fails rather than skips. The script needs only node: built-ins, so the step sits with the install-free gates before setup-node. The open locale PRs all pass it unchanged; the four remaining silent-locale-default hits go with #4524 (two in packages/ui) and #4551 (two OAuth helpers), after which that rule can become a hard check.

Verification

$ node --test scripts/check-locale-hygiene.test.mjs
ℹ pass 5
ℹ fail 0

$ npm run check:locale-hygiene -- --base origin/main
Locale hygiene check passed.

$ echo 'export const probe = locale === "zh-CN" ? "启用" : "Enable";' >> packages/core/src/ui-locale.ts
$ npm run check:locale-hygiene -- --base origin/main; echo exit=$?
Locale hygiene check failed: new locale branches were added. Index a UiCatalog instead.
- packages/core/src/ui-locale.ts: locale-literal-compare 1 -> 2
    81: locale === 'en'
    173: export const probe = locale === "zh-CN" ? "启用" : "Enable";
exit=1

npm run format:check, biome check on the two scripts, and check:asf-headers pass. Ran the ratchet against the eight open locale PR branches: all pass.

Review focus

locale-literal-compare matches ===/!== against a locale literal and startsWith('zh'); it is deliberately narrower than the AST locale-branch in check-tui-copy.mjs (which also sees switch (locale) and if-statements) so this gate stays install-free. No such switch exists in scope today.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code drafted the script, its tests, and the CI wiring; the rules and baseline were reviewed by hand.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

Reviewed at e03c6b8. The problem is real: I ran the four rules over main's in-scope files and got 140 hits (62 / 34 / 12 / 32), all genuine, none from comments or strings. A base-relative ratchet with no ledger is the right shape here, Biome cannot carry it (the linter is an explicit allowlist with a zero-violation entry bar, and it cannot express "no growth against base"), and all four sibling PRs only lower the counts, so nothing is blocked. But as written the gate cannot see the lines it was created for.

P1: every rule matches single quotes only, and double quotes are permanent in two thirds of the scope. biome.jsonc excludes apps/desktop/** and packages/ui/** from the formatter and has no quote rule, so " is not a typo there. On main today packages/ui/src/materialize.ts:192 and :706 read locale: UiLocale = "en" and settings-test-result-copy.ts:166 reads locale === "en"; all three pass silent-locale-default / locale-branch untouched. Those two materialize.ts lines are the exact P2 from #4640 that this rule was offered as the guard for. Fix: ' → ['"] in all four patterns (and [^']* → [^'"]*). Please add a fixture that is main's materialize.ts:192 verbatim.

P2: counts are per matching line, not per match. scanSource does pattern.test(line) once per line, so const a = locale === 'en' ? x : y; const b = locale === 'en' ? p : q; counts 1, and splitting browser-message-box.ts:351 (two comparisons on one line) across two lines for readability fails CI with no new branch. Use matchAll with a global pattern and push per match.

P2: locale-branch now has two definitions in the repo, and this is the weaker one. scripts/check-tui-copy.mjs already implements locale-branch and cjk-literal over a @babel/parser AST (ConditionalExpression / IfStatement / SwitchStatement), which is immune to quoting, line wrapping, switch (locale), and comment or string false positives. This step runs after npm ci, so the parser is available. Either wire the scope scan to its exported checkSource and keep only the ratchet layer here, or rename the rule and say in a comment where the two definitions diverge.

Smaller:

• A rename whose source is outside SCOPE shows up as A in the pathspec'd diff, so base is 0 and every existing hit counts as new. The comment "the same diff pairs moved files with their base path" is only true within scope.
• With no --base and no resolvable merge base the script prints "skipping" and exits 0; workflow_dispatch takes that path. ci-workflow-policy.test.mjs already states the norm: a gate that cannot fail the job is not a gate.
• The body's 62 / 35 / 11 / 32 reproduce as 62 / 34 / 12 / 32 with the script's own scope; worth aligning since they are the case for the PR.
• The script needs only node: built-ins, so it belongs with the install-free gates before setup-node, where an install failure cannot hide it.

After #4640 and #4824 land, silent-locale-default is at zero and can become a hard check rather than a ratchet.

Evidence boundary: static read plus the script's own four tests and the scans above, run on a copy of the two files outside the repo. No suite, no build.

AI-assisted review: drafted with Maka; I verified the regex, the biome exclusions, and the materialize.ts escape myself.

[orangeCatDeveloper]

Thanks for the detailed review — these are good catches. I’ve updated the PR to address them

[Astro-Han]

Re-reviewed at e78295c. Everything from last round is closed in code, and I re-ran the checks rather than reading the body:

• Both quote styles: the four patterns take ['"], and the materialize.ts-shaped fixture (locale: UiLocale = "en") is in the test. The body's 67 / 4 / 12 / 32 reproduce exactly with the script's own RULES over main's in-scope files.
• Per match, not per line: matchAll with g, pinned by the two-comparison fixture.
• The rule is renamed locale-literal-compare with the comment stating where it diverges from check-tui-copy's AST rule; that is the option I offered, so no duplicate definition.
• Diff no longer pathspec-limited, inScope filters afterwards, so a move into scope keeps its base.
• No base → exit 1 with a message, not a skip. The step now sits with the install-free gates before setup-node, and fetch-depth: 0 is already there so BASE_SHA resolves.

Ran the ratchet from this head against the current heads of #4524 and #4526 with --base at their merge base: both pass. Once #4524 lands, silent-locale-default is at two (subscription-result-message.ts:26, use-oauth-login-flow.ts:331, both #4551's), and after that it can become a hard zero as the body says.

Nothing left to ask for. Order among the locale PRs does not matter for this one; it touches only scripts/, package.json and ci.yml.

Evidence boundary: static read plus the script's five tests and the scans above, run on copies outside the repo; no suite, no build.

AI-assisted review: drafted with Maka; I verified the counts, the CI ordering and the two ratchet runs myself.