From 5ca6afa7893e404fa184cd6290d8c5f26a4d0230 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 12:41:31 +0200 Subject: [PATCH] fix(i18n): the untranslated ratchet sees plural copies and copied phrases Two holes let English through the ratchet unseen. A plural form en has no key for (Russian .few and .many, Arabic .two) was compared with en's value for that same key, which never exists, so a locale could copy en's text into exactly the forms a new plural group needs and the count stayed put. Such a form is now compared with every form en spells out for the group. A form en does declare keeps the exact comparison: Italian "{count} file" is the Italian plural, not a copy of en's "{count} file". The // same-as-en marker excused any value in a Latin-script locale, so one comment waved a whole copied sentence through. It is honoured only on a value holding a single plain word, the case shared/CLAUDE.md already describes. No locale carries the marker today, so nothing that passed before fails now; the counts are unchanged. --- shared/CLAUDE.md | 2 +- shared/scripts/fixtures/i18n-plural/en/a.ts | 6 ++ shared/scripts/fixtures/i18n-plural/ru/a.ts | 8 +++ shared/scripts/fixtures/i18n-words/de/a.ts | 2 + shared/scripts/fixtures/i18n-words/en/a.ts | 1 + shared/scripts/fixtures/i18n-words/fr/a.ts | 1 + shared/scripts/fixtures/i18n-words/ja/a.ts | 1 + shared/scripts/i18n-untranslated.mjs | 66 ++++++++++++++++++--- shared/src/i18n/i18n-untranslated.spec.ts | 12 ++++ 9 files changed, 90 insertions(+), 9 deletions(-) create mode 100644 shared/scripts/fixtures/i18n-plural/en/a.ts create mode 100644 shared/scripts/fixtures/i18n-plural/ru/a.ts diff --git a/shared/CLAUDE.md b/shared/CLAUDE.md index 7fbb6baa1..656f760de 100644 --- a/shared/CLAUDE.md +++ b/shared/CLAUDE.md @@ -49,7 +49,7 @@ The parity rule governs **existing** routes; it is not a license to mint new deb - **`languages.ts`** — `SUPPORTED_LANGUAGES` is the canonical registry and the source of `SupportedLanguageCode`. Adding a language starts here. - **`/`** — one folder per language, one file per UI domain plus an `index.ts` barrel; each file exports a flat map of dot-namespaced keys typed as `TranslationStrings`. The runtime `t(key)` only resolves these top-level keys. - **`en/` is canonical.** Every other locale must have the identical file set and top-level keys — `i18n:parity:strict` enforces it in CI. When you add or rename a key, update every locale. -- **Every locale gets a real translation; an English placeholder is not acceptable.** Write native phrasing with the locale's own punctuation and the same `{placeholders}` as `en`; if you genuinely cannot translate one, say so instead of filling it with English. Parity only checks that the key exists, so `i18n:parity:strict` also runs a ratchet (`scripts/i18n-untranslated.mjs`): per locale and file, the `// en-fallback` markers and the unmarked values identical to `en` may not grow past `scripts/i18n-untranslated-baseline.json`. An identical value is excused only when, after dropping placeholders, markup, URLs/e-mail/paths, the product names in `BRAND_NAMES` (Google Maps, Atlas, Immich) and the unit symbols in `UNITS`, no plain word is left (a word with a capital after its first letter, an acronym or a single letter does not count), so "{count} km", "GPX", "OAuth" and "Atlas" pass while "Settings" and "Source code" count. A single ordinary word that really is the locale's own (German "Status", French "Transport") carries `// same-as-en` on its declaration; the marker is honoured only in Latin-script locales (`LATIN_LOCALES`), because a Latin word in Japanese or Arabic is English whatever it says. Every locale folder must be on `LATIN_LOCALES` or `NON_LATIN_LOCALES`, so a new language is classified there first. Deleting a marker without translating moves the string from one count to the other and still fails. After translating, run `npm run i18n:untranslated:update` to lower the baseline; nothing raises it. +- **Every locale gets a real translation; an English placeholder is not acceptable.** Write native phrasing with the locale's own punctuation and the same `{placeholders}` as `en`; if you genuinely cannot translate one, say so instead of filling it with English. Parity only checks that the key exists, so `i18n:parity:strict` also runs a ratchet (`scripts/i18n-untranslated.mjs`): per locale and file, the `// en-fallback` markers and the unmarked values identical to `en` may not grow past `scripts/i18n-untranslated-baseline.json`. An identical value is excused only when, after dropping placeholders, markup, URLs/e-mail/paths, the product names in `BRAND_NAMES` (Google Maps, Atlas, Immich) and the unit symbols in `UNITS`, no plain word is left (a word with a capital after its first letter, an acronym or a single letter does not count), so "{count} km", "GPX", "OAuth" and "Atlas" pass while "Settings" and "Source code" count. A single ordinary word that really is the locale's own (German "Status", French "Transport") carries `// same-as-en` on its declaration; the marker is honoured only on a value holding one plain word once the excused parts are dropped (on a phrase it is ignored, so it cannot wave a copied sentence through) and only in Latin-script locales (`LATIN_LOCALES`), because a Latin word in Japanese or Arabic is English whatever it says. A plural form en has no key for (Russian `.few`, Arabic `.two`) counts when it equals any form en spells out for that group. Every locale folder must be on `LATIN_LOCALES` or `NON_LATIN_LOCALES`, so a new language is classified there first. Deleting a marker without translating moves the string from one count to the other and still fails. After translating, run `npm run i18n:untranslated:update` to lower the baseline; nothing raises it. - **Count-bearing strings use plural forms, never `count === 1` in the caller.** The key holds the general form (the CLDR "other" category) and `key.zero|one|two|few|many` hold the forms the language's `Intl.PluralRules` selects for whole numbers (`plural.ts`); `t(key, { count })` (or `{ n }`) picks one. Pass the count as a number. Russian needs `.one`, `.few` and `.many`, Arabic all five, Japanese none. `i18n:parity` checks that every locale has exactly the forms its language reaches with whole numbers and none it never selects. A form may spell the number out (Arabic "ملف واحد") only where its category is that one number; French `one` also covers 0 and Russian `one` covers 21, so they keep `{count}`. A string whose wording changes with the number (the old "Copy to trip" for one place, "Copy {count} to trip" for more) is a plural group whose `one` drops the number where the language allows it, not two keys switched in the caller. - **An `en` string carrying `{count}` or `{n}` must be a plural group**, even when the forms read the same (a label such as "Notes: {count}"), so every language can inflect. `i18n:parity` refuses one that is not, unless `NOT_PLURAL` in `scripts/i18n-parity.mjs` lists it with a reason: only for a number that is never a quantity (a day number, a step, a multiplier). An entry that stops matching fails too. - **Further i18n specs run in `npm test`** and fail changes that parity lets through: placeholder parity (every `{placeholder}` in an `en` string must appear in each translation), no `en` string may call TREK "self-hosted" (the same build runs on managed installs; exemptions live in the spec), and a wording regression test for plugin permission descriptions (currently one string; add a case there when a permission's real behavior changes). None of these are part of `i18n:parity`, which checks key sets, plural forms, count strings and the untranslated ratchet. diff --git a/shared/scripts/fixtures/i18n-plural/en/a.ts b/shared/scripts/fixtures/i18n-plural/en/a.ts new file mode 100644 index 000000000..ddf116ee2 --- /dev/null +++ b/shared/scripts/fixtures/i18n-plural/en/a.ts @@ -0,0 +1,6 @@ +// Fixture for the untranslated ratchet's plural rule: en spells out only `one`. +const a = { + 'a.pack': '{count} items to pack', + 'a.pack.one': '{count} item to pack', +}; +export default a; diff --git a/shared/scripts/fixtures/i18n-plural/ru/a.ts b/shared/scripts/fixtures/i18n-plural/ru/a.ts new file mode 100644 index 000000000..6b055caa3 --- /dev/null +++ b/shared/scripts/fixtures/i18n-plural/ru/a.ts @@ -0,0 +1,8 @@ +// Fixture: the general form, `few` and `many` are English copies; en declares no `few` or `many`. +const a = { + 'a.pack': '{count} items to pack', + 'a.pack.one': 'Собрать {count} вещь', + 'a.pack.few': '{count} items to pack', + 'a.pack.many': '{count} item to pack', +}; +export default a; diff --git a/shared/scripts/fixtures/i18n-words/de/a.ts b/shared/scripts/fixtures/i18n-words/de/a.ts index c72fd5550..bfd2a1038 100644 --- a/shared/scripts/fixtures/i18n-words/de/a.ts +++ b/shared/scripts/fixtures/i18n-words/de/a.ts @@ -1,9 +1,11 @@ // Fixture: "Status" is the German word too and says so; the name and the unit need no marker. +// The marker on a copied sentence is not honoured: a.delete still counts. const a = { 'a.settings': 'Einstellungen', 'a.appearance': 'Erscheinungsbild', 'a.status': 'Status', // same-as-en 'a.addon': 'Atlas', 'a.distance': '{count} km', + 'a.delete': 'Delete this trip and all its places permanently', // same-as-en }; export default a; diff --git a/shared/scripts/fixtures/i18n-words/en/a.ts b/shared/scripts/fixtures/i18n-words/en/a.ts index c5953ce60..a24c50e19 100644 --- a/shared/scripts/fixtures/i18n-words/en/a.ts +++ b/shared/scripts/fixtures/i18n-words/en/a.ts @@ -5,5 +5,6 @@ const a = { 'a.status': 'Status', 'a.addon': 'Atlas', 'a.distance': '{count} km', + 'a.delete': 'Delete this trip and all its places permanently', }; export default a; diff --git a/shared/scripts/fixtures/i18n-words/fr/a.ts b/shared/scripts/fixtures/i18n-words/fr/a.ts index 942d761f7..72efd8c78 100644 --- a/shared/scripts/fixtures/i18n-words/fr/a.ts +++ b/shared/scripts/fixtures/i18n-words/fr/a.ts @@ -5,5 +5,6 @@ const a = { 'a.status': 'Status', 'a.addon': 'Atlas', 'a.distance': '{count} km', + 'a.delete': 'Supprimer définitivement ce voyage et tous ses lieux', }; export default a; diff --git a/shared/scripts/fixtures/i18n-words/ja/a.ts b/shared/scripts/fixtures/i18n-words/ja/a.ts index aed8e5af1..e4061cb03 100644 --- a/shared/scripts/fixtures/i18n-words/ja/a.ts +++ b/shared/scripts/fixtures/i18n-words/ja/a.ts @@ -5,5 +5,6 @@ const a = { 'a.status': 'Status', // same-as-en 'a.addon': 'Atlas', 'a.distance': '{count} km', + 'a.delete': 'この旅行とすべての場所を完全に削除', }; export default a; diff --git a/shared/scripts/i18n-untranslated.mjs b/shared/scripts/i18n-untranslated.mjs index 32669713e..14e4ca7bc 100644 --- a/shared/scripts/i18n-untranslated.mjs +++ b/shared/scripts/i18n-untranslated.mjs @@ -10,8 +10,10 @@ * translator leaves on a string they could not translate yet * identical an unmarked value equal to en's, unless the rule in * `isInvariant` says the text reads the same in every language, - * or a translator confirmed it with `// same-as-en` in a - * Latin-script locale (`isExcused`) + * or a translator confirmed a single word with `// same-as-en` + * in a Latin-script locale (`isExcused`). A plural form en has + * no key for (a Russian `.few` or `.many`) is compared with + * every form en spells out for its group (`enReference`). * * Both counts are held at scripts/i18n-untranslated-baseline.json. The check * fails when either grows past its entry (a file without one may hold none), @@ -30,6 +32,7 @@ * files, or a value the catalogue reader cannot parse is an error, never a * pass. */ +import { pluralFormOf, pluralGroups } from '../src/i18n/plural.ts'; import { asPath, I18N_ROOT, listDomainFiles, listLocales, readCatalog } from './i18n-catalog.mjs'; import { existsSync, readFileSync, realpathSync, writeFileSync } from 'node:fs'; @@ -134,8 +137,13 @@ export const isPlainWord = (word) => word.length >= 2 && !/\p{Lu}/u.test(word.sl * and that is for a translator to confirm, not for the rule to assume. */ export function isInvariant(value) { + return plainWords(value).length === 0; +} + +/** The plain words `value` holds once placeholders, markup, addresses, names and units are dropped. */ +export function plainWords(value) { const text = value.replace(PLACEHOLDER_RE, ' ').replace(TAG_RE, ' ').replace(ADDRESS_RE, ' ').replace(NAME_RE, ' '); - return !(text.match(WORD_RE) ?? []).some(isPlainWord); + return (text.match(WORD_RE) ?? []).filter(isPlainWord); } /** Whether `locale` is written in Latin script. Throws for a locale on neither list. */ @@ -149,9 +157,46 @@ export function isLatinLocale(locale) { /** * Whether an unmarked value equal to en's is excused: it is invariant, or a - * translator marked it `// same-as-en` in a Latin-script locale. + * translator marked it `// same-as-en` in a Latin-script locale and it holds + * one plain word. The marker is for a word that really is the locale's own + * (German "Status"); a phrase that reads the same as en's is a copy, and the + * marker on it is ignored rather than trusted. */ -export const isExcused = (value, same, locale) => isInvariant(value) || (same && isLatinLocale(locale)); +export const isExcused = (value, same, locale) => { + const words = plainWords(value); + return words.length === 0 || (same && isLatinLocale(locale) && words.length === 1); +}; + +/** + * Every en value a locale's `key` may not repeat. A key en declares has its + * en value. A form of a plural group that en does not declare (Russian + * `.few`, Arabic `.two`) has every form en spells out for the group, since + * it has no en counterpart of its own and a copy of any of them is English. + * A form en declares keeps the exact comparison: Italian "{count} file" for + * en's general "{count} files" is the Italian plural, not en's `.one`. + */ +export function enReference(en, groups) { + const forms = new Map(); + for (const [key, value] of en) { + const base = groupOf(key, groups); + if (base !== null) { + if (!forms.has(base)) forms.set(base, new Set()); + forms.get(base).add(value); + } + } + return (key) => { + if (en.has(key)) return new Set([en.get(key)]); + const base = groupOf(key, groups); + return base === null ? new Set() : (forms.get(base) ?? new Set()); + }; +} + +/** The plural group `key` belongs to (its base, `key.other` or a category form), else null. */ +function groupOf(key, groups) { + if (groups.has(key)) return key; + if (key.endsWith('.other') && groups.has(key.slice(0, -'.other'.length))) return key.slice(0, -'.other'.length); + return pluralFormOf(key, groups)?.base ?? null; +} /** * Today's counts: `{ [locale]: { [file]: { marked, identical } } }`, only @@ -162,7 +207,12 @@ export function countUntranslated(root = I18N_ROOT) { const locales = listLocales(root); if (!locales.includes('en')) throw new Error(`${asPath(root)}/en is required as the reference locale`); const enFiles = listDomainFiles('en', root); - const enValues = new Map(enFiles.map((f) => [f, new Map(readCatalog('en', f, root).map((e) => [e.key, e.value]))])); + const enValues = new Map( + enFiles.map((f) => { + const en = new Map(readCatalog('en', f, root).map((e) => [e.key, e.value])); + return [f, enReference(en, pluralGroups(en.keys()))]; + }), + ); const counts = {}; for (const locale of locales) { // Classify every folder first: an unlisted locale is an error, not a pass. @@ -171,12 +221,12 @@ export function countUntranslated(root = I18N_ROOT) { const files = new Set(listDomainFiles(locale, root)); for (const file of enFiles) { if (!files.has(file)) throw new Error(`${locale}/${file} is missing; run i18n-parity for the file report`); - const en = enValues.get(file); + const enValuesOf = enValues.get(file); let marked = 0; let identical = 0; for (const { key, value, marked: isMarked, same } of readCatalog(locale, file, root)) { if (isMarked) marked++; - else if (en.get(key) === value && !isExcused(value, same, locale)) identical++; + else if (enValuesOf(key).has(value) && !isExcused(value, same, locale)) identical++; } if (marked || identical) (counts[locale] ??= {})[file] = { marked, identical }; } diff --git a/shared/src/i18n/i18n-untranslated.spec.ts b/shared/src/i18n/i18n-untranslated.spec.ts index e9889b062..da82c21d7 100644 --- a/shared/src/i18n/i18n-untranslated.spec.ts +++ b/shared/src/i18n/i18n-untranslated.spec.ts @@ -61,6 +61,9 @@ describe('i18n untranslated ratchet', () => { it('honours // same-as-en in a Latin-script locale only, and refuses an unlisted locale', () => { expect(isExcused('Status', true, 'de')).toBe(true); + expect(isExcused('Status: {state}', true, 'de')).toBe(true); + expect(isExcused('Delete this trip and all its places permanently', true, 'de')).toBe(false); + expect(isExcused('Source code', true, 'de')).toBe(false); expect(isExcused('Status', false, 'de')).toBe(false); expect(isExcused('Status', true, 'ja')).toBe(false); expect(isExcused('GPX', false, 'ja')).toBe(true); @@ -69,11 +72,20 @@ describe('i18n untranslated ratchet', () => { it('counts a one-word English copy: Settings in fr, Appearance in ja', () => { expect(countUntranslated(new URL('i18n-words/', FIXTURES))).toEqual({ + // The copied sentence carries // same-as-en, which only a single word earns. + de: { 'a.ts': { marked: 0, identical: 1 } }, fr: { 'a.ts': { marked: 0, identical: 2 } }, ja: { 'a.ts': { marked: 0, identical: 2 } }, }); }); + it('counts a plural form en has no key for when it holds any of en’s forms', () => { + // ru copies en's general form into the base and `few`, and en's `one` into `many`. + expect(countUntranslated(new URL('i18n-plural/', FIXTURES))).toEqual({ + ru: { 'a.ts': { marked: 0, identical: 3 } }, + }); + }); + it('fails closed on a locale folder on neither script list', () => { expect(() => countUntranslated(new URL('i18n-unlisted/', FIXTURES))).toThrow(/locale xx is on neither/); });