From b31c2ff1d2b2cb9ff121140c315b550863d50b56 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 10 Oct 2026 13:47:08 +0200 Subject: [PATCH] ci(client): hold the en keys nothing reaches to a baseline that only shrinks lint:i18n-keys only listed unused en keys as information, and it looked for references in client/src alone, so every key the server sends (system notice and notification channel keys in its registries) read as dead. The reference search now also reads server/src and plugin-sdk/src; those references only mark keys as reached and are not held to the client's key rules. The unused keys per en domain file are counted against scripts/i18n-unused-baseline.json, which only shrinks and fails on an entry above its count until --update lowers it. Today's 841 unused keys in 44 files are frozen, none deleted. --- .github/workflows/test.yml | 4 +- CLAUDE.md | 2 +- client/CLAUDE.md | 2 +- client/scripts/i18n-keys.mjs | 165 ++++++++++++++++-- client/scripts/i18n-unused-baseline.json | 46 +++++ client/tests/unit/i18n/i18nKeysScript.test.ts | 62 ++++++- 6 files changed, 258 insertions(+), 23 deletions(-) create mode 100644 client/scripts/i18n-unused-baseline.json diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9b8bbd9b4..1002c96ca 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -454,7 +454,9 @@ jobs: run: cd client && npm run lint:skips - name: i18n keys check - # Every translation key src/ names must exist in shared en. + # Every translation key src/ names must exist in shared en, and the + # en keys nothing in client, server or plugin-sdk reaches may not rise + # above scripts/i18n-unused-baseline.json per en file. run: cd client && npm run lint:i18n-keys - name: Theme tokens check diff --git a/CLAUDE.md b/CLAUDE.md index 0a0ddcda0..d5c184afd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -48,7 +48,7 @@ cd client && npm run lint:format # Prettier outside the baseli cd client && npm run lint:layers # components never import pages/ or mobile/ (CI gate) cd client && npm run lint:offline # views may not import src/api/ beyond the baseline (CI gate) cd client && npm run lint:skips # no .only; skipped tests may only shrink (CI gate) -cd client && npm run lint:i18n-keys # every key the client names exists in en (CI gate) +cd client && npm run lint:i18n-keys # every key the client names exists in en; unused en keys may only shrink (CI gate) cd server && npm run lint:boundaries # no new import cycles or cross-domain reach-ins (CI gate) cd server && npm run lint:dialect # SQLite-only SQL spellings may only shrink (CI gate; rules in server/CLAUDE.md) cd server && npm run probe:pg # dialect helpers + the repository statements it reaches against Postgres (CI job; needs TREK_PG_PROBE_URL) diff --git a/client/CLAUDE.md b/client/CLAUDE.md index 8db441b4b..fc1284c65 100644 --- a/client/CLAUDE.md +++ b/client/CLAUDE.md @@ -20,7 +20,7 @@ npm run lint:offline # no view file (components/mobile/pages/hooks) imports npm run lint:pairs # each desktop/phone pair in scripts/feature-pairs.json imports its shared hooks, and every file under src/mobile/screens is listed there, in a pair or under mobileOnly with a reason (CI gate; --list prints the pairs) npm run lint:dup # copied code: the lines of a file inside a block that repeats elsewhere (100 tokens over 10 lines, SonarCloud's thresholds; .ts and .tsx read as one language) may only shrink against scripts/dup-baseline.json (CI gate; --list shows each block with its other side, --update lowers it) npm run lint:skips # no .only anywhere; skipped/todo/fixme tests per file may only shrink against scripts/skip-baseline.json (CI gate; skipIf/runIf and Playwright's test.skip(condition, 'why') are fine, but skipIf(true) and runIf(false) count as skips) -npm run lint:i18n-keys # every translation key src/ names exists in shared en (CI gate; --unused lists en keys nothing reaches) +npm run lint:i18n-keys # every translation key src/ names exists in shared en, and en keys nothing in src/, server/src or plugin-sdk/src reaches may only shrink per en file against scripts/i18n-unused-baseline.json (CI gate; --unused lists them, --update lowers the baseline) npm run test # vitest run (tests/** + co-located src/**/*.test.{ts,tsx}); also test:unit / test:integration / test:coverage npm run e2e # Playwright (CI runs --project=public --project=app; e2e:report opens the last report) npm run shots # Playwright screenshot project (shots:promote to accept) diff --git a/client/scripts/i18n-keys.mjs b/client/scripts/i18n-keys.mjs index 1d42d6526..a955a3275 100644 --- a/client/scripts/i18n-keys.mjs +++ b/client/scripts/i18n-keys.mjs @@ -42,14 +42,21 @@ * A key held in a variable (`t(opt.label)`) is out of reach unless the table * names it in a *Key property. * + * The en keys nothing reaches are a ratchet. Besides src/, the search for + * references reads server/src (system notice and notification channel keys + * the server sends, *Key properties in its registries) and plugin-sdk/src, so + * a key the server uses does not read as dead; those references only count as + * reached, they are not held to the rules above. The unused keys per en + * domain file may not rise above scripts/i18n-unused-baseline.json, and an + * entry above its count fails until --update lowers it. A key reached only + * through a variable the scan cannot follow is reached by naming it: in a + * *Key property of the table, or as a literal t() where the table is read. + * * npm run lint:i18n-keys check (CI) * npm run lint:i18n-keys -- --unused also list en keys no literal or pattern reaches - * - * The unused list is information, not a gate: keys reached through a - * variable (`t(item.labelKey)` with the table elsewhere, server-sent error - * keys) look unused here. + * npm run lint:i18n-keys -- --update lower the unused-key baseline; it never raises an entry */ -import { existsSync, readdirSync, readFileSync, realpathSync, statSync } from 'node:fs'; +import { existsSync, readdirSync, readFileSync, realpathSync, statSync, writeFileSync } from 'node:fs'; import { dirname, join, relative } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; @@ -58,6 +65,15 @@ import { fileURLToPath, pathToFileURL } from 'node:url'; const HERE = dirname(fileURLToPath(import.meta.url)); const SRC = join(HERE, '..', 'src'); const SHARED_SCRIPTS = join(HERE, '..', '..', 'shared', 'scripts'); +const UNUSED_BASELINE = join(HERE, 'i18n-unused-baseline.json'); + +/** + * Where else a translation key can be named: the server sends keys for the + * client to translate (system notices, notification channels), and the plugin + * SDK names the ones plugin surfaces use. Read only to find which en keys are + * reached. + */ +export const REFERENCE_ROOTS = [join(HERE, '..', '..', 'server', 'src'), join(HERE, '..', '..', 'plugin-sdk', 'src')]; /** How many en keys a template may reach and still pass without an entry below. */ export const MAX_IMPLICIT_MATCHES = 30; @@ -360,13 +376,18 @@ export function scanSource(text, file = '') { return { literal, dynamic }; } +/** en's keys with the domain file each sits in, read with the shared parity tooling's reader. */ +export async function readEnKeyFiles() { + const { listDomainFiles, readCatalog } = await import(pathToFileURL(join(SHARED_SCRIPTS, 'i18n-catalog.mjs')).href); + const files = new Map(); + for (const file of listDomainFiles('en')) for (const { key } of readCatalog('en', file)) files.set(key, file); + if (files.size === 0) throw new Error('shared/src/i18n/en holds no keys: the reader or the path is broken'); + return files; +} + /** en's keys, read from the locale sources with the shared parity tooling's reader. */ export async function readEnKeys() { - const { listDomainFiles, readCatalog } = await import(pathToFileURL(join(SHARED_SCRIPTS, 'i18n-catalog.mjs')).href); - const keys = new Set(); - for (const file of listDomainFiles('en')) for (const { key } of readCatalog('en', file)) keys.add(key); - if (keys.size === 0) throw new Error('shared/src/i18n/en holds no keys: the reader or the path is broken'); - return keys; + return new Set((await readEnKeyFiles()).keys()); } export const hasKey = (enKeys, key) => enKeys.has(key) || enKeys.has(`${key}.other`); @@ -387,9 +408,12 @@ export function countMatches(pattern, enKeys) { * The verdict over a scan: literal keys en lacks, template keys no en key * matches, templates too wide to pass without an allow-list entry, * allow-list entries that no file uses any more or that the check no longer - * needs, and the en keys nothing reaches. + * needs, and the en keys nothing reaches. elsewhere holds the references + * outside src/ (scanElsewhere), which only count as reaching keys. + * + * @param {ReturnType} [elsewhere] */ -export function evaluate(scan, enKeys, allowed = DYNAMIC_ALLOWED) { +export function evaluate(scan, enKeys, allowed = DYNAMIC_ALLOWED, elsewhere = { literal: [], dynamic: [] }) { const missing = scan.literal.filter(({ key }) => !hasKey(enKeys, key)); const site = (file, template) => `${file}\n${template}`; const allowedSites = new Set(allowed.map((a) => site(a.file, a.template))); @@ -414,8 +438,13 @@ export function evaluate(scan, enKeys, allowed = DYNAMIC_ALLOWED) { const n = reach(a.template, templatePattern(a.template)); return n !== null && n <= MAX_IMPLICIT_MATCHES; }); - const reached = new Set([...scan.literal.map((l) => l.key), ...allowed.flatMap((a) => a.resolves ?? [])]); - const patterns = scan.dynamic.map((d) => d.pattern).filter(Boolean); + // References outside src/ (see REFERENCE_ROOTS) only mark keys as reached. + const reached = new Set([ + ...scan.literal.map((l) => l.key), + ...elsewhere.literal.map((l) => l.key), + ...allowed.flatMap((a) => a.resolves ?? []), + ]); + const patterns = [...scan.dynamic, ...elsewhere.dynamic].map((d) => d.pattern).filter(Boolean); const unused = [...enKeys].filter( (k) => !reached.has(k) && !reached.has(base(k)) && !patterns.some((p) => p.test(k) || p.test(base(k))) ); @@ -434,6 +463,74 @@ export function scanTree(root = SRC) { return scan; } +/** + * Every key reference under the reference roots, for the unused count. A root + * that is missing or names no key at all is fine here: these are extra places + * to look, not the code the check is about. + * + * @param {string[]} [roots] + * @returns {{ literal: { file: string, line: number, key: string }[], dynamic: { file: string, line: number, template: string, pattern: RegExp | null, site?: string }[] }} + */ +export function scanElsewhere(roots = REFERENCE_ROOTS) { + const scan = { literal: [], dynamic: [] }; + for (const root of roots) { + if (!existsSync(root)) continue; + for (const path of walk(root)) { + const found = scanSource(readFileSync(path, 'utf8'), relative(root, path).split('\\').join('/')); + scan.literal.push(...found.literal); + scan.dynamic.push(...found.dynamic); + } + } + return scan; +} + +/** The unused keys per en domain file. */ +export function unusedPerFile(unused, keyFiles) { + const counts = {}; + for (const key of unused) { + const file = keyFiles.get(key); + if (file) counts[file] = (counts[file] ?? 0) + 1; + } + return counts; +} + +/** + * The unused counts held against the baseline: the files that gained unused + * keys, the entries above their count (or for a file that is gone), and the + * baseline lowered to today, which --update writes. An entry never rises and + * a file without one holds no unused key. + */ +export function compareUnused(baseline, counts) { + const grown = Object.entries(counts).filter(([file, n]) => n > (baseline[file] ?? 0)); + const lowered = {}; + for (const [file, allowed] of Object.entries(baseline)) { + const now = counts[file] ?? 0; + if (now > 0) lowered[file] = Math.min(allowed, now); + } + const stale = Object.entries(baseline) + .filter(([file, allowed]) => lowered[file] !== allowed) + .map(([file, allowed]) => ({ file, allowed, now: counts[file] ?? 0 })); + return { grown, stale, lowered }; +} + +/** The committed baseline, which fails closed: a missing or malformed file is an error, never empty. */ +export function readUnusedBaseline(path = UNUSED_BASELINE) { + let value; + try { + value = JSON.parse(readFileSync(path, 'utf8')); + } catch (err) { + throw new Error(`scripts/i18n-unused-baseline.json cannot be read (${err.message}); restore it from git`); + } + const ok = + value && + typeof value === 'object' && + !Array.isArray(value) && + Object.values(value).every((n) => Number.isInteger(n) && n > 0); + if (!ok) + throw new Error('scripts/i18n-unused-baseline.json is not an object of file → positive count; restore it from git'); + return value; +} + // Compared by real path, so the check still runs when the script is started through a symlink. const isCli = Boolean(process.argv[1]) && @@ -442,8 +539,17 @@ const isCli = if (isCli) { try { const scan = scanTree(); - const enKeys = await readEnKeys(); - const { missing, unmatched, broad, stale, unused } = evaluate(scan, enKeys); + const keyFiles = await readEnKeyFiles(); + const enKeys = new Set(keyFiles.keys()); + const { missing, unmatched, broad, stale, unused } = evaluate(scan, enKeys, DYNAMIC_ALLOWED, scanElsewhere()); + const unusedCounts = unusedPerFile(unused, keyFiles); + let unusedBaseline = readUnusedBaseline(); + if (process.argv.includes('--update')) { + unusedBaseline = compareUnused(unusedBaseline, unusedCounts).lowered; + const sorted = Object.fromEntries(Object.entries(unusedBaseline).sort(([a], [b]) => (a < b ? -1 : 1))); + writeFileSync(UNUSED_BASELINE, JSON.stringify(sorted, null, 2) + '\n'); + } + const unusedVerdict = compareUnused(unusedBaseline, unusedCounts); for (const { file, line, key } of missing) { console.error(`FAIL ${file}:${line}: '${key}' is not a key in shared/src/i18n/en`); } @@ -462,13 +568,34 @@ if (isCli) { 'lacks, or which is narrow enough to pass without it: remove or fix the entry' ); } + for (const [file, n] of unusedVerdict.grown) { + const keys = unused.filter((k) => keyFiles.get(k) === file).slice(0, 5); + console.error( + `FAIL en/${file}: ${n} key(s) nothing in client, server or plugin-sdk reaches, baseline ` + + `${unusedBaseline[file] ?? 0} (${keys.join(', ')}). Use the key, or delete it from every locale.` + ); + } + for (const { file, allowed, now } of unusedVerdict.stale) { + console.error( + `FAIL en/${file} is held at ${allowed} in scripts/i18n-unused-baseline.json, but ${now} key(s) are unused ` + + 'now. Run npm run lint:i18n-keys -- --update to lower it.' + ); + } if (process.argv.includes('--unused')) for (const key of unused.sort()) console.log(`unused ${key}`); console.log( `i18n keys: ${new Set(scan.literal.map((l) => l.key)).size} literal and ${scan.dynamic.length} template ` + - `reference(s) checked against ${enKeys.size} en keys; ${unused.length} en key(s) reached by neither ` + - '(information, --unused lists them)' + `reference(s) checked against ${enKeys.size} en keys; ${unused.length} en key(s) reached by neither, ` + + `baseline allows ${Object.values(unusedBaseline).reduce((a, b) => a + b, 0)} (--unused lists them)` ); - if (missing.length || unmatched.length || broad.length || stale.length) process.exit(1); + if ( + missing.length || + unmatched.length || + broad.length || + stale.length || + unusedVerdict.grown.length || + unusedVerdict.stale.length + ) + process.exit(1); } catch (err) { console.error(`FAIL ${err.message}`); process.exit(1); diff --git a/client/scripts/i18n-unused-baseline.json b/client/scripts/i18n-unused-baseline.json new file mode 100644 index 000000000..5e9965342 --- /dev/null +++ b/client/scripts/i18n-unused-baseline.json @@ -0,0 +1,46 @@ +{ + "admin.ts": 152, + "atlas.ts": 10, + "backup.ts": 10, + "budget.ts": 40, + "categories.ts": 1, + "collab.ts": 9, + "collection.ts": 35, + "common.ts": 7, + "dashboard.ts": 65, + "dawarich.ts": 16, + "dayplan.ts": 18, + "docsync.ts": 25, + "files.ts": 4, + "help.ts": 3, + "inspector.ts": 4, + "journey.ts": 70, + "login.ts": 1, + "map.ts": 5, + "memories.ts": 8, + "mobileNav.ts": 2, + "mobileVacay.ts": 4, + "nav.ts": 1, + "notif.ts": 7, + "notifications.ts": 18, + "packing.ts": 6, + "pdf.ts": 1, + "perm.ts": 4, + "photos.ts": 22, + "places.ts": 10, + "planner.ts": 53, + "register.ts": 20, + "reservations.ts": 21, + "roadtrip.ts": 38, + "settings.ts": 40, + "share.ts": 7, + "shared.ts": 2, + "stats.ts": 8, + "storage.ts": 16, + "system_notice.ts": 32, + "todo.ts": 4, + "tours.ts": 14, + "trip.ts": 10, + "undo.ts": 1, + "vacay.ts": 17 +} diff --git a/client/tests/unit/i18n/i18nKeysScript.test.ts b/client/tests/unit/i18n/i18nKeysScript.test.ts index 34182ea37..066eee4b1 100644 --- a/client/tests/unit/i18n/i18nKeysScript.test.ts +++ b/client/tests/unit/i18n/i18nKeysScript.test.ts @@ -1,18 +1,22 @@ -import { mkdtempSync, rmSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; import { DYNAMIC_ALLOWED, MAX_IMPLICIT_MATCHES, + compareUnused, countMatches, evaluate, firstArgument, interpolationResults, readEnKeys, + readUnusedBaseline, + scanElsewhere, scanSource, scanTree, templatePattern, + unusedPerFile, } from '../../../scripts/i18n-keys.mjs'; // FE-I18N-KEYS-001 to FE-I18N-KEYS-015: the client key check (scripts/i18n-keys.mjs). @@ -194,4 +198,60 @@ describe('i18n key check', () => { const two = scanSource("t(`wide.${a ?? 'k0'}.${b}`)", 'a.tsx'); expect(two.dynamic.map((d) => d.site)).toEqual([undefined, "wide.${a ?? 'k0'}.${b}"]); }); + + it('FE-I18N-KEYS-016: a key named only by the server counts as reached, and its references are not judged', () => { + const dir = mkdtempSync(join(tmpdir(), 'i18n-elsewhere-')); + try { + mkdirSync(join(dir, 'registry')); + writeFileSync( + join(dir, 'registry', 'notices.ts'), + "export const n = { titleKey: 'budget.title', bodyKey: 'no.such' }" + ); + writeFileSync(join(dir, 'registry', 'notices.test.ts'), "const k = { titleKey: 'places.count' }"); + const elsewhere = scanElsewhere([dir, join(dir, 'missing')]); + expect(elsewhere.literal.map((l) => l.key)).toEqual(['budget.title', 'no.such']); + const verdict = evaluate({ literal: [], dynamic: [] }, EN, [], elsewhere); + expect(verdict.unused).not.toContain('budget.title'); + expect(verdict.unused).toContain('places.count'); + expect(verdict.missing).toEqual([]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('FE-I18N-KEYS-017: counts unused keys per en file and holds them to a baseline that only shrinks', () => { + const files = new Map([ + ['budget.title', 'budget.ts'], + ['places.count', 'places.ts'], + ['places.count.one', 'places.ts'], + ]); + const counts = unusedPerFile(['budget.title', 'places.count', 'places.count.one', 'not.in.en'], files); + expect(counts).toEqual({ 'budget.ts': 1, 'places.ts': 2 }); + expect(compareUnused({ 'budget.ts': 1, 'places.ts': 2 }, counts)).toEqual({ + grown: [], + stale: [], + lowered: counts, + }); + const grown = compareUnused({ 'budget.ts': 1, 'places.ts': 1 }, counts); + expect(grown.grown).toEqual([['places.ts', 2]]); + const stale = compareUnused({ 'budget.ts': 3, 'places.ts': 2, 'gone.ts': 1 }, counts); + expect(stale.stale).toEqual([ + { file: 'budget.ts', allowed: 3, now: 1 }, + { file: 'gone.ts', allowed: 1, now: 0 }, + ]); + expect(stale.lowered).toEqual({ 'budget.ts': 1, 'places.ts': 2 }); + }); + + it('FE-I18N-KEYS-018: a missing or malformed unused baseline stops the check', () => { + const dir = mkdtempSync(join(tmpdir(), 'i18n-baseline-')); + try { + expect(() => readUnusedBaseline(join(dir, 'none.json'))).toThrow(/cannot be read/); + writeFileSync(join(dir, 'bad.json'), '{"a.ts": 0}'); + expect(() => readUnusedBaseline(join(dir, 'bad.json'))).toThrow(/positive count/); + writeFileSync(join(dir, 'ok.json'), '{"a.ts": 2}'); + expect(readUnusedBaseline(join(dir, 'ok.json'))).toEqual({ 'a.ts': 2 }); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); });