From 7b69bff7945de0c3d603714029c85f30d7c843fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Fri, 7 Aug 2026 12:37:11 +0200 Subject: [PATCH] ci(i18n): make the pseudo-locale coverage oracle a hard CI gate (#2360) Co-authored-by: Kandev Agent --- .../e2e/tests/i18n/pseudo-coverage.spec.ts | 53 ++++++++++++++----- docs/i18n.md | 19 ++++++- docs/specs/platform/i18n.md | 11 +++- 3 files changed, 68 insertions(+), 15 deletions(-) diff --git a/apps/web/e2e/tests/i18n/pseudo-coverage.spec.ts b/apps/web/e2e/tests/i18n/pseudo-coverage.spec.ts index f0011d0a2c..0418bd7570 100644 --- a/apps/web/e2e/tests/i18n/pseudo-coverage.spec.ts +++ b/apps/web/e2e/tests/i18n/pseudo-coverage.spec.ts @@ -21,12 +21,48 @@ import { test, expect } from "../../fixtures/test-base"; * only sees plain literals in JSX, so it and this spec cover different halves of * the same question — add to both in the PR that migrates a directory. * - * Gated on `KANDEV_I18N_COVERAGE=1` rather than run in CI: with the migration - * proceeding one directory at a time, a screen's own copy can be clean while - * shared chrome it renders is not yet. It becomes a hard gate when the last - * directory lands and the env guard comes off. + * THIS IS A HARD CI GATE. It ran behind `KANDEV_I18N_COVERAGE=1` only while the + * migration was mid-flight, because a screen's own copy could be clean while + * shared chrome it renders was not yet. The last live directory has landed and + * the env guard is gone: it now runs unconditionally in the `chromium` project + * alongside every other spec. * - * KANDEV_I18N_COVERAGE=1 pnpm e2e -- e2e/tests/i18n/pseudo-coverage.spec.ts + * pnpm e2e -- e2e/tests/i18n/pseudo-coverage.spec.ts + * + * WHAT IT GUARANTEES: on each screen in `SCREENS`, every rendered text node and + * every value of a `COPY_ATTRIBUTES` attribute that the detector below can see + * either came through `t()` / `` (and so renders accented under the + * pseudo-locale), or is on an allowlist that says why it must not be translated. + * + * It is a floor, not a proof of total coverage. Two limits are deliberate and + * both are documented where they are implemented, because relaxing either + * reddens every screen today: + * - `wordlike` requires a 4-letter ASCII run, so a hardcoded string shorter + * than that ("Add", "of") is not reported. Lowering it to 2 was measured: + * 9 of the 10 tests here go red, on real but out-of-scope copy owned by + * `@kandev/ui` and sonner (e.g. the `alt+T` in sonner's toast-region label). + * - the text pass clears a node on ANY accented character, so an un-migrated + * word sharing one text node with migrated copy is missed. The attribute + * pass does NOT have this limit — it strips allowlisted tokens first and + * reports the remainder as `English frame, migrated value`. docs/i18n.md + * covers the text-node case under "…and it is weakest at an interpolated + * value"; it is why the by-hand pseudo walk is still step 9 of a migration. + * + * IF YOUR PR JUST WENT RED HERE, the failure names the exact strings and the + * screen. Read them: each one is copy your change put on screen without routing + * it through `t()`. This spec is the ONLY gate that can see most of them — + * `i18next/no-literal-string` inspects literals in JSX and nothing else, so copy + * in a variable, a default parameter, a `.ts` helper or a SCREAMING_CASE config + * table is invisible to lint and visible only here. Repeated by-hand sweeps + * found 15-60 such strings that every other gate passed. + * + * The fix is to externalize the string: add it to `src/locales/en/.json` and + * call `t(":")`. See docs/i18n.md. Adding an `allow:` entry is NOT the + * fix and is the one failure mode this file has actually shipped twice — an + * entry asserting a path was migrated while it still rendered English. `allow` + * is for text the frontend must not translate but cannot avoid rendering (a + * backend-owned record, a product name), and it must say where the value comes + * from. * * BOTH passes walk `document.body`, deliberately, and NOT the page's `
`. * Scoping to the page under test is the obvious reading of what each test claims, @@ -47,8 +83,6 @@ import { test, expect } from "../../fixtures/test-base"; * chrome, not narrowing the oracle. */ -const COVERAGE_ENABLED = process.env.KANDEV_I18N_COVERAGE === "1"; - /** * Migrated screens whose visible text is overwhelmingly UI chrome, not user data. * @@ -494,11 +528,6 @@ async function findUnlocalizedCopy( } test.describe("i18n pseudo-locale coverage", () => { - test.skip( - !COVERAGE_ENABLED, - "Set KANDEV_I18N_COVERAGE=1 to run the string-externalization oracle (hard gate at task-40).", - ); - for (const screen of SCREENS) { test(`no un-externalized copy on ${screen.name}`, async ({ testPage }) => { await activatePseudo(testPage, screen.url); diff --git a/docs/i18n.md b/docs/i18n.md index 7aba65538d..8f727de4c9 100644 --- a/docs/i18n.md +++ b/docs/i18n.md @@ -316,7 +316,12 @@ pnpm run lint:i18n components/ # 5. clean? step that makes the work stick** — without it the directory drifts straight back. 7. If the area has a screen worth crawling, add it to `SCREENS` in - `e2e/tests/i18n/pseudo-coverage.spec.ts`. + `e2e/tests/i18n/pseudo-coverage.spec.ts`. That spec is a **hard CI gate** — + it runs unconditionally in the E2E `chromium` project — so adding a screen + commits the whole DOM that screen renders, not just your directory. It is a + regression floor rather than a completeness proof: it still cannot see a + hardcoded string under four letters, or one sharing a text node with migrated + copy, which is why step 9 below is still worth doing by hand. 8. `pnpm run i18n:check && pnpm lint --max-warnings 0 && pnpm exec vitest run`. 9. Switch to **Pseudo (QA)** in Settings → General → Appearance and look at the screen. Anything still in plain English was missed — see below for why lint @@ -537,6 +542,18 @@ through `t()` — that is the acceptance check for a migrated screen, and the on one that sees the guard's blind spots. Interpolation placeholders and `` tag markers are preserved by the generator. +The by-hand walk is for a screen you are migrating. For the screens already +migrated, `e2e/tests/i18n/pseudo-coverage.spec.ts` automates most of this check +and **runs as a hard gate in CI** — it is no longer behind an env var. A red run +there names the strings and the screen; externalize them rather than allowlisting +them. + +It does not replace the walk. The spec reports a plain-ASCII run of four letters +or more in a text node with no accented character in it — so a hardcoded string +shorter than that, or one sitting in the same text node as migrated copy, still +reaches you only by eye. Both limits are load-bearing rather than oversights; +the spec's header records what happens when you relax them. + One thing it shows you is **not** a miss: an interpolated API or network payload stays English by design, so it renders unaccented and is still correct. See [the interpolated-value limit](#and-it-is-weakest-at-an-interpolated-value). diff --git a/docs/specs/platform/i18n.md b/docs/specs/platform/i18n.md index 60d937c24b..a346420609 100644 --- a/docs/specs/platform/i18n.md +++ b/docs/specs/platform/i18n.md @@ -81,8 +81,15 @@ merge. Sequence: 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 - removing the env guard on the pseudo-coverage e2e spec once coverage is total. +4. **Close-out**: the env guard on the pseudo-coverage e2e spec is **removed** — + it runs unconditionally in the E2E `chromium` project, so a regression on the + screens in its `SCREENS` list fails CI wherever the detector can see it (a + plain-ASCII run of four letters or more, in a text node or a copy-bearing + attribute). It is a regression floor, not a proof of total coverage; the + spec's header records the two detection limits and what relaxing them costs. + Still outstanding: `@kandev/ui` primitives (a separate workspace package, + deliberately deferred; its hardcoded strings are not reached by any screen + currently in `SCREENS`) and the shared-task artifacts. ## Data model i18n adds no server-side persistent entities. It introduces: