diff --git a/apps/web/scripts/check-i18n-keys.mjs b/apps/web/scripts/check-i18n-keys.mjs index b31d9f9ca3..4c9b2f9c31 100644 --- a/apps/web/scripts/check-i18n-keys.mjs +++ b/apps/web/scripts/check-i18n-keys.mjs @@ -10,6 +10,35 @@ * entry -> missing keys are an ERROR (they would render as the raw key) * - every catalog entry is referenced somewhere -> orphans are a WARNING * - `en` and `pseudo` have the same key sets -> drift is an ERROR + * - real locales (anything that is not `en`/`pseudo`, discovered from the + * directory listing rather than named here) -> parity issues are a WARNING + * + * ## Why real locales only warn + * + * `en` and `pseudo` are ours: English is authored in the same PR as the code, + * and `pseudo` is GENERATED from it by `pnpm run i18n:pseudo`. A mismatch there + * is always the author's to fix in the change that caused it, so it gates. + * + * Real-locale catalogs are translated OUT OF BAND, by a third party, on their + * own cadence. Gating on them means an ordinary English-only PR fails CI for + * work nobody in the merge path can do — and that is not hypothetical: #2261 + * added 13 `en` chat keys and left `main` red, because #2243 had introduced + * `zh-cn` and the gate 20 minutes earlier. So these are reported loudly, for + * whoever owns the translation, and do not block a merge. + * + * ## What this check is NOT + * + * The real-locale pass is STRUCTURAL, not translational. It compares shapes, so + * a green run does not mean "translated" — measured against `main`: + * + * caught: missing key, extra key, missing/extra namespace, empty or + * non-string value, dropped `{{placeholder}}`, dropped `` tag + * NOT caught: a value identical to English, a whole catalog copy-pasted from + * `en`, and swapped `_one`/`_other` forms + * + * The identical-to-English tolerance is deliberate — brand nouns and technical + * literals (`Kandev`, `GitHub`, `Webhook URL`) must stay verbatim, so there is + * no sound automatic rule here. Reviewing a translation is a human job. * * Usage: node scripts/check-i18n-keys.mjs [--strict-orphans] */ @@ -130,10 +159,15 @@ if (pseudoMissing.length || pseudoExtra.length) { ); } +// Advisory, never fatal — see the "Why real locales only warn" note at the top. +// Deliberately NOT gated behind --strict-orphans or any flag: the point is that +// no invocation of this script can be made to fail on a translation catalog. if (realLocaleIssues.length) { - failed = true; - console.error(`\n✖ ${realLocaleIssues.length} real-locale catalog parity issue(s):`); - for (const issue of realLocaleIssues) console.error(` ${formatParityIssue(issue)}`); + console.warn( + `\n⚠ ${realLocaleIssues.length} real-locale catalog parity issue(s) ` + + `— advisory, does not fail the build:`, + ); + for (const issue of realLocaleIssues) console.warn(` ${formatParityIssue(issue)}`); } if (orphans.length) { @@ -147,9 +181,16 @@ if (orphans.length) { } if (!failed) { + // Say only what was actually gated. The previous wording claimed "pseudo and + // zh-cn in sync", which was misleading twice over: it implied a real locale + // gates the build, and it read as "translated" for a check that cannot see + // English left inside a translated value. + const advisory = realLocales.length + ? ` ${realLocaleIssues.length} advisory ${realLocales.join(", ")} issue(s).` + : ""; console.log( `✓ i18n keys OK — ${used.size} key(s) referenced, ${en.size} en entr(ies), ` + - `${orphans.length} orphan(s), pseudo and ${realLocales.join(", ") || "no real locales"} in sync.`, + `${orphans.length} orphan(s), pseudo in sync.${advisory}`, ); } process.exit(failed ? 1 : 0); diff --git a/apps/web/scripts/check-i18n-keys.test.ts b/apps/web/scripts/check-i18n-keys.test.ts index ec0a4b64ec..4c97159a7d 100644 --- a/apps/web/scripts/check-i18n-keys.test.ts +++ b/apps/web/scripts/check-i18n-keys.test.ts @@ -60,81 +60,88 @@ describe("real locale catalog parity", () => { expect(result.status).toBe(0); }); - it("rejects a missing namespace", () => { + it("warns about a missing namespace without failing", () => { const fixture = completeFixture(); delete fixture[ZH_CN].settings; const result = runFixture(fixture); - expect(result.status).toBe(1); + expect(result.status).toBe(0); + expect(result.stderr).toContain("advisory"); expect(result.stderr).toContain(ZH_CN); expect(result.stderr).toContain("settings"); expect(result.stderr).toContain("missing namespace"); }); - it("rejects an extra namespace", () => { + it("warns about an extra namespace without failing", () => { const fixture = completeFixture(); fixture[ZH_CN].extra = { surprise: "意外" }; const result = runFixture(fixture); - expect(result.status).toBe(1); + expect(result.status).toBe(0); + expect(result.stderr).toContain("advisory"); expect(result.stderr).toContain(ZH_CN); expect(result.stderr).toContain("extra"); expect(result.stderr).toContain("extra namespace"); }); - it("rejects a missing key", () => { + it("warns about a missing key without failing", () => { const fixture = completeFixture(); delete fixture[ZH_CN].common.second; const result = runFixture(fixture); - expect(result.status).toBe(1); + expect(result.status).toBe(0); + expect(result.stderr).toContain("advisory"); expect(result.stderr).toContain(ZH_CN); expect(result.stderr).toContain("common"); expect(result.stderr).toContain("missing key: second"); }); - it("rejects an extra key", () => { + it("warns about an extra key without failing", () => { const fixture = completeFixture(); fixture[ZH_CN].common.surprise = "意外"; const result = runFixture(fixture); - expect(result.status).toBe(1); + expect(result.status).toBe(0); + expect(result.stderr).toContain("advisory"); expect(result.stderr).toContain(ZH_CN); expect(result.stderr).toContain("common"); expect(result.stderr).toContain("extra key: surprise"); }); - it("rejects a translation that drops an interpolation placeholder", () => { + it("warns about a translation that drops an interpolation placeholder", () => { const fixture = completeFixture(); fixture.en.common.first = "Open {{url}} as {{user}}"; fixture[ZH_CN].common.first = "以 {{user}} 身份打开"; const result = runFixture(fixture); - expect(result.status).toBe(1); + expect(result.status).toBe(0); + expect(result.stderr).toContain("advisory"); expect(result.stderr).toContain(ZH_CN); expect(result.stderr).toContain("common"); expect(result.stderr).toContain("interpolation placeholder mismatch: first"); }); - it("rejects a translation that drops numeric Trans tags", () => { + it("warns about a translation that drops numeric Trans tags", () => { const fixture = completeFixture(); fixture.en.common.first = "Open <1>{{url}}"; fixture[ZH_CN].common.first = "打开 {{url}}"; const result = runFixture(fixture); - expect(result.status).toBe(1); + expect(result.status).toBe(0); + expect(result.stderr).toContain("advisory"); expect(result.stderr).toContain(ZH_CN); expect(result.stderr).toContain("common"); expect(result.stderr).toContain("Trans tag structure mismatch: first"); }); - it("rejects a translation that changes a numeric Trans tag index", () => { + it("warns about a translation that changes a numeric Trans tag index", () => { const fixture = completeFixture(); fixture.en.common.first = "Open <1>{{url}}"; fixture[ZH_CN].common.first = "打开 <2>{{url}}"; const result = runFixture(fixture); - expect(result.status).toBe(1); + expect(result.status).toBe(0); + expect(result.stderr).toContain("advisory"); expect(result.stderr).toContain(ZH_CN); expect(result.stderr).toContain("common"); expect(result.stderr).toContain("Trans tag structure mismatch: first"); @@ -142,37 +149,109 @@ describe("real locale catalog parity", () => { }); describe("translated value validation", () => { - it("rejects an empty translated value", () => { + it("warns about an empty translated value without failing", () => { const fixture = completeFixture(); fixture[ZH_CN].common.first = ""; const result = runFixture(fixture); - expect(result.status).toBe(1); + expect(result.status).toBe(0); + expect(result.stderr).toContain("advisory"); expect(result.stderr).toContain(ZH_CN); expect(result.stderr).toContain("common"); expect(result.stderr).toContain("empty translated value: first"); }); - it("rejects a whitespace-only translated value", () => { + it("warns about a whitespace-only translated value without failing", () => { const fixture = completeFixture(); fixture[ZH_CN].common.first = " \t\n "; const result = runFixture(fixture); - expect(result.status).toBe(1); + expect(result.status).toBe(0); + expect(result.stderr).toContain("advisory"); expect(result.stderr).toContain(ZH_CN); expect(result.stderr).toContain("common"); expect(result.stderr).toContain("empty translated value: first"); }); - it("rejects a non-string translated value", () => { + it("warns about a non-string translated value without failing", () => { const fixture = completeFixture(); fixture[ZH_CN].common.first = 42; const result = runFixture(fixture); - expect(result.status).toBe(1); + expect(result.status).toBe(0); + expect(result.stderr).toContain("advisory"); expect(result.stderr).toContain(ZH_CN); expect(result.stderr).toContain("common"); expect(result.stderr).toContain("non-string translated value: first"); expect(result.stderr).not.toContain("matchAll is not a function"); }); }); + +/** + * `en` <-> `pseudo` is the half that still gates, and after making real locales + * advisory it carries all of the weight — so it needs its own coverage rather + * than being assumed. Both catalogs are ours: English is authored beside the + * code and `pseudo` is generated from it, so a mismatch is always the author's + * to fix in the same change. + */ +describe("pseudo catalog drift still fails the build", () => { + it("rejects a key missing from pseudo", () => { + const fixture = completeFixture(); + delete fixture.pseudo.common.second; + const result = runFixture(fixture); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("pseudo catalog is out of sync"); + }); + + it("rejects a key present only in pseudo", () => { + const fixture = completeFixture(); + fixture.pseudo.common.surprise = "Śũřƥřîśē"; + const result = runFixture(fixture); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("pseudo catalog is out of sync"); + }); + + it("still fails on pseudo drift even when a real locale is also broken", () => { + const fixture = completeFixture(); + delete fixture.pseudo.common.second; + delete fixture[ZH_CN].common.second; + const result = runFixture(fixture); + + expect(result.status).toBe(1); + expect(result.stderr).toContain("pseudo catalog is out of sync"); + expect(result.stderr).toContain("advisory"); + }); +}); + +/** + * The real-locale pass compares SHAPES, not meaning. These pin the limits so a + * green run is never mistaken for "translated" — the assumption that this check + * validates translation quality has already caused two wrong conclusions. + */ +describe("the real-locale pass is structural, not translational", () => { + it("accepts a catalog copy-pasted wholesale from en", () => { + const fixture = completeFixture(); + fixture[ZH_CN] = structuredClone(fixture.en); + const result = runFixture(fixture); + + expect(result.status).toBe(0); + expect(result.stderr).not.toContain("advisory"); + }); + + it("accepts swapped plural forms", () => { + const fixture = completeFixture(); + fixture.en.common = { items_one: "{{count}} item", items_other: "{{count}} items" }; + fixture.pseudo.common = { items_one: "{{count}} îţēḿ", items_other: "{{count}} îţēḿś" }; + fixture[ZH_CN].common = { items_one: "{{count}} 个项目", items_other: "{{count}} 个项目" }; + const swapped = structuredClone(fixture); + swapped[ZH_CN].common = { + items_one: fixture[ZH_CN].common.items_other, + items_other: fixture[ZH_CN].common.items_one, + }; + const result = runFixture(swapped); + + expect(result.status).toBe(0); + }); +}); diff --git a/docs/i18n.md b/docs/i18n.md index f79c549fff..e8f2f2b13c 100644 --- a/docs/i18n.md +++ b/docs/i18n.md @@ -44,8 +44,12 @@ Two enforcement layers, and it matters which is which: `sidebar`, `slack`, `system`, `workflows`, and `workspaces`. Each migrated area can add the namespace it needs (`statusBar`, `kanban`, `task`, …). A string used in two or more areas belongs in `common` so it is translated once. A new -namespace needs no registration: `lib/i18n/index.ts` discovers catalogs by glob, -but every real locale must add the same namespace in the same change. +namespace needs no registration: `lib/i18n/index.ts` discovers catalogs by glob. + +Adding the same namespace to every real locale in the same change is the ideal, +but it is **not required and does not gate** — translation is handled out of +band, so `i18n:check` reports a real-locale gap as a warning. See +[Real locales warn, they do not gate](#real-locales-warn-they-do-not-gate). ### Interpolation @@ -841,9 +845,10 @@ case variants such as `zh-CN`, but cookies and `` use the canonical i tests for canonicalization, cookie and `Accept-Language` negotiation, message lookup, exact catalog parity, and the first shell response. 5. Run `pnpm run i18n:check` from `apps/web`. It discovers every real locale - except `en` and generated `pseudo`, then fails on any missing or extra - namespace or key. Regenerate `pseudo` only from English with - `pnpm run i18n:pseudo`; never copy a human translation into it. + except `en` and generated `pseudo`, and **warns** on any missing or extra + namespace or key — advisory, not a build failure; see below. Regenerate + `pseudo` only from English with `pnpm run i18n:pseudo`; never copy a human + translation into it. 6. In a development or E2E build, select the new language under **Settings > General > Appearance**, reload, and switch back to English. Confirm translated copy, the locale cookie, ``, interpolation, layout, and then use @@ -853,3 +858,46 @@ Kandev's migration remains incremental. A complete locale catalog guarantees that every currently externalized message has a translation; it does not mean that every screen has been externalized. Unmigrated surfaces continue to render English until their directory joins `i18nGuardFiles`. + +### Real locales warn, they do not gate + +`en` and `pseudo` gate. `en` is authored in the same PR as the code and `pseudo` +is **generated** from it, so a mismatch between them is always the author's to +fix in the change that caused it. Real-locale parity is a **warning**. + +That is a deliberate policy, not an oversight. Translation happens out of band, +on a third party's cadence, so gating on it makes an ordinary English-only PR +fail CI for work nobody in the merge path can do. It is not hypothetical: #2243 +introduced `zh-cn` together with the parity gate, #2261 added 13 English `chat` +keys twenty minutes later, and `main` went red — not because either change was +wrong, but because the two could not have been ordered to avoid it. The gate +fired exactly as designed and blocked the wrong people. + +So: **add the keys to every real locale when you can, and never let a missing +one stop you merging.** The warning is addressed to whoever owns the +translation. + +#### It is a structural check, not a translation check + +Worth stating plainly, because assuming otherwise has already produced two wrong +conclusions: a green run does **not** mean "translated". The pass compares +shapes. Measured, one mutation at a time: + +| mutation | result | +|---|---| +| value identical to English | **not caught** | +| whole catalog copy-pasted from `en` | **not caught** | +| swapped `_one` / `_other` | **not caught** | +| empty / whitespace / non-string value | caught | +| missing key, extra key | caught | +| missing namespace, extra namespace | caught | +| dropped `{{placeholder}}` | caught | +| dropped or renumbered `` tag | caught | + +The identical-to-English tolerance is deliberate and cannot be tightened +naively: brand nouns and technical literals must stay verbatim (`automations` +alone has seven — `GitHub`, `Webhook`, `Webhook URL`, …), so `value === en` would +report each of them. Closing it needs a per-key exemption list or a +whole-file proportion threshold. Reviewing a translation is a human job; +`scripts/check-i18n-keys.test.ts` pins both the caught and the uncaught cases so +this stays honest. diff --git a/docs/specs/platform/i18n.md b/docs/specs/platform/i18n.md index bbb738351a..60d937c24b 100644 --- a/docs/specs/platform/i18n.md +++ b/docs/specs/platform/i18n.md @@ -50,10 +50,17 @@ language and the pseudo-locale as the migration QA oracle. migration PR extends the allowlist by the path it converts, so coverage only ever grows and un-migrated code is never a blocker. - CI SHALL verify keys and catalogs do not drift: every `ns:key` referenced in - source must exist in the `en` catalog; every committed real locale must have - exactly the same namespaces and key sets as `en`; and `pseudo` must remain in - strict generated sync with `en`. (Catalogs are NOT regenerated from source — - with key-based ids the English copy exists only in the catalog.) + source must exist in the `en` catalog, and `pseudo` must remain in strict + generated sync with `en`. Both are authored or generated in the same change as + the code, so both SHALL fail the build. (Catalogs are NOT regenerated from + source — with key-based ids the English copy exists only in the catalog.) +- Real-locale parity SHALL be reported as a **warning and SHALL NOT fail the + build**. Translation is performed out of band by a third party, so gating on it + blocks an English-only change for work nobody in the merge path can do. This is + a deliberate reversal of the original requirement: #2243 introduced `zh-cn` + together with a hard parity gate, #2261 added English keys twenty minutes + later, and `main` went red although neither change was wrong and no ordering of + the two could have avoided it. - The full sweep SHALL externalize **every** user-facing string across `apps/web/components/` (1,121 files), `apps/web/app/` (285 files), and the `@kandev/ui` primitives, leaving no user-facing English literal outside the @@ -70,8 +77,8 @@ merge. Sequence: language switcher, CI gates, codemods, the scoped lint guard, and one page (Settings → General → Appearance) migrated end-to-end as the worked example. 2. **First real language**: ship `zh-cn` catalogs for every existing namespace, - register the locale in the frontend and backend, and strengthen catalog - parity checks for every committed real locale. + register the locale in the frontend and backend, and add advisory catalog + parity reporting for every committed real locale. 3. **Per-directory migrations**: each PR externalizes one directory, appends it to `i18nGuardFiles`, and adds its screen to the pseudo-coverage spec. 4. **Close-out**: `@kandev/ui` primitives, the shared-task artifacts, and @@ -83,8 +90,10 @@ i18n adds no server-side persistent entities. It introduces: - **Message catalogs** — checked-in JSON under `apps/web/src/locales//.json` for `en`, `zh-cn`, and `pseudo`. `en` is hand-authored (keys are stable ids; the English copy lives - only here); `zh-cn` is a human-maintained translation whose namespace and key - sets exactly mirror `en`; `pseudo` is generated from `en` by + only here); `zh-cn` is a human-maintained translation, maintained out of band, + whose namespace and key sets aim to mirror `en` but may lag it — a gap warns + rather than failing, and i18next falls back to `en` at runtime; `pseudo` is + generated from `en` by `scripts/generate-pseudo-locale.mjs` and is never hand-edited. Catalogs load at runtime via Vite glob import — no build plugin. - **Locale preference** — the user's chosen locale, one value from the supported @@ -120,9 +129,12 @@ Frontend module contract (new): **THEN** `` is `en`, all text renders from the `en` catalog, and the language switcher shows English selected. - **GIVEN** the Settings language switcher, **WHEN** the user selects - **简体中文**, **THEN** every currently migrated message resolves from the - `zh-cn` catalog, `` becomes `zh-cn`, and the selection survives a - reload through the `kandev_locale=zh-cn` cookie. + **简体中文**, **THEN** every migrated message present in the `zh-cn` catalog + resolves from it, any message not yet translated falls back to its `en` value + rather than rendering the raw key, `` becomes `zh-cn`, and the + selection survives a reload through the `kandev_locale=zh-cn` cookie. (The + catalog may lag `en`, because real-locale parity is advisory — the fallback is + what makes that safe to ship.) - **GIVEN** no locale cookie and `Accept-Language: zh-CN`, **WHEN** the Go shell or a server-rendered browser error is returned, **THEN** locale negotiation selects `zh-cn`, the shell emits ``, and the backend message @@ -156,8 +168,12 @@ Frontend module contract (new): - **GIVEN** a production build, **WHEN** the language switcher renders, **THEN** English and 简体中文 are offered and the pseudo-locale is not. - **GIVEN** an English namespace or key changes, **WHEN** `i18n:check` runs, - **THEN** every real locale missing or adding a namespace or key fails with an - error naming the locale, namespace, and missing or extra entries. + **THEN** every real locale missing or adding a namespace or key is reported as + a warning naming the locale, namespace, and missing or extra entries, **AND** + the command still exits 0. +- **GIVEN** the `pseudo` catalog has drifted from `en`, **WHEN** `i18n:check` + runs, **THEN** it fails, **even when** a real locale is also out of parity — + the advisory path must not mask the gating one. ## Failure modes - **Missing translation for the active locale** → i18next falls back to `en`, @@ -213,10 +229,20 @@ Frontend module contract (new): persisted and document locale. Locale negotiation is case-insensitive, so BCP-47 input such as `zh-CN` resolves to `zh-cn`. The switcher label is the fixed endonym `简体中文`, not a translated message. -- **Real-locale parity**: every committed real locale must contain every English - namespace and exactly the same keys, including plural suffixes and `` - structures. Runtime English fallback remains a safety net, not a substitute - for a complete checked-in catalog. +- **Real-locale parity**: every committed real locale should contain every + English namespace and exactly the same keys, including plural suffixes and + `` structures — but this is **advisory, not enforced**, because + translation happens out of band (see the CI requirement above). Runtime English + fallback is what makes an incomplete catalog safe to merge; it remains a safety + net rather than a substitute for a complete checked-in catalog. +- **The parity check is structural, not translational**: it compares shapes, so a + green run does not mean "translated". It catches a missing/extra key or + namespace, an empty or non-string value, a dropped `{{placeholder}}`, and a + dropped or renumbered `` tag. It does **not** catch a value identical to + English, a catalog copy-pasted wholesale from `en`, or swapped `_one`/`_other` + forms. The identical-to-English tolerance is required — brand nouns and + technical literals must stay verbatim — so tightening it needs a per-key + exemption list or a whole-file proportion threshold, not a `value === en` rule. - **Library**: react-i18next (superseding the initial Lingui implementation) — chosen so the English copy lives in one place and components stay free of inline source text.