fix(i18n): make real-locale catalog parity advisory, not a gate (#2272)

Co-authored-by: Kandev Agent <agent@kandev.dev>
2026-08-05 11:22:36 +02:00
parent c3fd434a24
commit a2db334adc
4 changed files with 240 additions and 46 deletions
@@ -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 `<n>` 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);
@@ -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}}</1>";
    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}}</1>";
    fixture[ZH_CN].common.first = "打开 <2>{{url}}</2>";
    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);
+  });
+});
@@ -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 `<html lang>` 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, `<html lang>`, 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 `<n>` 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.
@@ -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/<locale>/<namespace>.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** `<html lang>` 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, `<html lang>` 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, `<html lang>` 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 `<html lang="zh-cn">`, 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 `<Trans>`
-  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
+  `<Trans>` 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 `<Trans>` 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.