diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 8bcc8baadd..eb4ca4ab1e 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -74,8 +74,11 @@ jobs: working-directory: apps run: pnpm install --frozen-lockfile + # `build-web-e2e`, not `build-web`: the production web build no longer + # bundles the pseudo QA catalog, and tests/i18n/pseudo-coverage.spec.ts + # needs it. See apps/web/lib/i18n/bundling.ts. - name: Build backend and web - run: make build-backend build-web + run: make build-backend build-web-e2e # Fixture plugin package consumed by the plugin E2E specs. Built here # (the build job has the Go toolchain) and shipped to the shards as its diff --git a/Makefile b/Makefile index e72a853a89..55027730b2 100644 --- a/Makefile +++ b/Makefile @@ -399,6 +399,16 @@ build-web: @printf "$(CYAN)Building web app...$(RESET)\n" @cd $(APPS_DIR) && VITE_KANDEV_API_PORT= VITE_KANDEV_DEBUG= $(PNPM) --filter @kandev/web build +## Web build for the E2E harness. Identical to build-web except that it keeps the +## pseudo QA catalog, which a production build drops (see +## apps/web/lib/i18n/bundling.ts). e2e/tests/i18n/pseudo-coverage.spec.ts is the +## only oracle for copy the jsx-only eslint guard cannot see, and it needs the +## catalog present in the artifact it runs against. +.PHONY: build-web-e2e +build-web-e2e: + @printf "$(CYAN)Building web app (with the pseudo QA locale)...$(RESET)\n" + @cd $(APPS_DIR) && VITE_KANDEV_API_PORT= VITE_KANDEV_DEBUG= $(PNPM) --filter @kandev/web build:e2e + .PHONY: build-web-quiet build-web-quiet: @printf " $(DIM)Web app$(RESET)\n" @@ -508,17 +518,17 @@ test-scripts: @node --test scripts/validate-public-docs.test.mjs .PHONY: test-e2e -test-e2e: build-backend build-web build-e2e-plugin-package +test-e2e: build-backend build-web-e2e build-e2e-plugin-package @printf "$(CYAN)Running E2E tests (headless, parallel)...$(RESET)\n" @cd $(APPS_DIR) && $(PNPM) --filter @kandev/web e2e .PHONY: test-e2e-headed -test-e2e-headed: build-backend build-web build-e2e-plugin-package +test-e2e-headed: build-backend build-web-e2e build-e2e-plugin-package @printf "$(CYAN)Running E2E tests (headed)...$(RESET)\n" @cd $(APPS_DIR) && $(PNPM) --filter @kandev/web e2e:headed .PHONY: test-e2e-ui -test-e2e-ui: build-backend build-web build-e2e-plugin-package +test-e2e-ui: build-backend build-web-e2e build-e2e-plugin-package @printf "$(CYAN)Opening Playwright UI mode...$(RESET)\n" @cd $(APPS_DIR) && $(PNPM) --filter @kandev/web e2e:ui diff --git a/apps/web/e2e/global-setup.ts b/apps/web/e2e/global-setup.ts index 2d44d36294..a07920c5f3 100644 --- a/apps/web/e2e/global-setup.ts +++ b/apps/web/e2e/global-setup.ts @@ -19,6 +19,8 @@ export default function globalSetup() { throw new Error(`Vite web build not found: ${spaIndex}\nRun "make build-web" first.`); } + assertPseudoCatalogBundled(); + // tests/plugins/plugins.spec.ts installs this package through the real // upload UI. Like the binaries above, this only checks existence — not // freshness — so rebuild after touching cmd/plugin-fixture (see @@ -30,3 +32,65 @@ export default function globalSetup() { ); } } + +/** + * Fail fast when the coverage oracle is asked to run against a bundle that has + * no pseudo catalog. + * + * A production `vite build` deliberately drops it (see lib/i18n/bundling.ts). + * Run tests/i18n/pseudo-coverage.spec.ts against such a bundle and every screen + * falls back to `en` — so all ten tests fail with a wall of findings that reads + * exactly like a mass externalization regression, when the real cause is that + * the artifact was built with `make build-web` instead of `make build-web-e2e`. + * Only checked under KANDEV_I18N_COVERAGE=1; nothing else needs the catalog. + * + * The markers are read from the catalogs themselves rather than hardcoded, so + * this cannot drift as `pnpm run i18n:pseudo` regenerates them — and the whole + * SOURCE directory is scanned rather than one named file, so adding, renaming or + * splitting a namespace cannot quietly turn the check into a no-op. Every path + * out of here either throws or has actually compared markers against `dist/`: a + * guard that can only ever pass is worth less than no guard, because it also + * reports clean. + */ +function assertPseudoCatalogBundled() { + if (process.env.KANDEV_I18N_COVERAGE !== "1") return; + + const catalogDir = path.join(WEB_DIR, "src", "locales", "pseudo"); + const catalogs = fs.existsSync(catalogDir) + ? fs.readdirSync(catalogDir).filter((name) => name.endsWith(".json")) + : []; + const markers = catalogs + .flatMap((name) => + Object.values( + JSON.parse(fs.readFileSync(path.join(catalogDir, name), "utf8")) as Record, + ), + ) + .filter((value): value is string => typeof value === "string" && value.length >= 20) + .slice(0, 50); + + if (markers.length === 0) { + throw new Error( + `KANDEV_I18N_COVERAGE=1 but no usable pseudo source catalog was found in ${catalogDir}.\n` + + 'The coverage oracle cannot mean anything without it — run "pnpm run i18n:pseudo" ' + + "to regenerate it. See docs/i18n.md.", + ); + } + + const assets = path.join(WEB_DIR, "dist", "assets"); + const bundled = fs + .readdirSync(assets) + .filter((name) => name.endsWith(".js")) + .some((name) => { + const source = fs.readFileSync(path.join(assets, name), "utf8"); + return markers.some((marker) => source.includes(marker)); + }); + + if (!bundled) { + throw new Error( + "KANDEV_I18N_COVERAGE=1 but dist/ contains no pseudo catalog, so the coverage " + + "oracle would report every screen as un-externalized.\n" + + 'Rebuild the SPA with the QA locale: "make build-web-e2e" ' + + '(or "pnpm --filter @kandev/web build:e2e"). See docs/i18n.md.', + ); + } +} diff --git a/apps/web/e2e/scripts/run-e2e.sh b/apps/web/e2e/scripts/run-e2e.sh index f6a70b5d9d..7a9b3372a8 100755 --- a/apps/web/e2e/scripts/run-e2e.sh +++ b/apps/web/e2e/scripts/run-e2e.sh @@ -92,9 +92,14 @@ restore_pr_asset_ownership() { } } +# `build:e2e`, not `build`: a plain `vite build` drops the pseudo catalog from +# the bundle (it is the largest locale and no production user can select it), and +# tests/i18n/pseudo-coverage.spec.ts is the oracle that needs it. See +# apps/web/lib/i18n/bundling.ts. Mirror any change here in the CI e2e build job +# (.github/workflows/e2e-tests.yml -> make build-web-e2e). build_fe() { - log "building Vite web assets" - ( cd "$REPO_ROOT/apps" && pnpm --filter @kandev/web build ) \ + log "building Vite web assets (with the pseudo QA locale)" + ( cd "$REPO_ROOT/apps" && pnpm --filter @kandev/web build:e2e ) \ || die "FE build failed" } diff --git a/apps/web/lib/i18n/bundling.test.ts b/apps/web/lib/i18n/bundling.test.ts new file mode 100644 index 0000000000..a929397285 --- /dev/null +++ b/apps/web/lib/i18n/bundling.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from "vitest"; + +import { PSEUDO_BUNDLE_ENV_VAR, shouldBundlePseudoLocale } from "./bundling"; + +/** + * Both directions, deliberately. + * + * This switch is only exercised for real by a `vite build`, so nothing in the + * unit or e2e suites would notice it silently stuck on "include" — which is the + * state it is replacing. Asserting only the e2e/dev side would leave the whole + * point of the change untested. + */ +describe("shouldBundlePseudoLocale", () => { + it("excludes pseudo from a plain production build", () => { + expect(shouldBundlePseudoLocale({ command: "build", env: {} })).toBe(false); + }); + + it("keeps pseudo when a build opts in", () => { + for (const value of ["1", "true", "yes", "TRUE", " 1 "]) { + expect( + shouldBundlePseudoLocale({ command: "build", env: { [PSEUDO_BUNDLE_ENV_VAR]: value } }), + `${PSEUDO_BUNDLE_ENV_VAR}=${JSON.stringify(value)} should opt in`, + ).toBe(true); + } + }); + + it("does not treat an arbitrary non-empty value as opting in", () => { + for (const value of ["", "0", "false", "no", "off"]) { + expect( + shouldBundlePseudoLocale({ command: "build", env: { [PSEUDO_BUNDLE_ENV_VAR]: value } }), + `${PSEUDO_BUNDLE_ENV_VAR}=${JSON.stringify(value)} should not opt in`, + ).toBe(false); + } + }); + + it("always keeps pseudo in serve mode, which covers `pnpm dev` and vitest", () => { + expect(shouldBundlePseudoLocale({ command: "serve", env: {} })).toBe(true); + expect(shouldBundlePseudoLocale({ command: "serve" })).toBe(true); + }); +}); diff --git a/apps/web/lib/i18n/bundling.ts b/apps/web/lib/i18n/bundling.ts new file mode 100644 index 0000000000..ae49d230d2 --- /dev/null +++ b/apps/web/lib/i18n/bundling.ts @@ -0,0 +1,42 @@ +/** + * Build-time decision: does this bundle compile in the `pseudo` QA catalog? + * + * `pseudo` is the largest catalog we ship (accented characters are multi-byte) + * and no production user can ever select it — `selectableLocales` has always + * filtered it out of the switcher. Until this switch existed it was still + * *bundled*, so every user downloaded it. Filtering at runtime cannot fix that; + * the entries have to be dropped before rolldown builds the module graph. + * + * `import.meta.env.PROD` is the wrong signal, and the reason is the whole + * difficulty of this change: the e2e harness serves a **production** bundle, so + * that constant is true there too. e2e is exactly where `pseudo` must survive — + * `e2e/tests/i18n/pseudo-coverage.spec.ts` is the only oracle that sees copy the + * jsx-only eslint guard structurally cannot (strings in variables, default + * parameters, `.ts` helpers, SCREAMING_CASE config tables). So the decision is + * taken from the *build invocation* instead: `vite dev` and vitest always keep + * it, and a `vite build` keeps it only when asked to. + * + * Kept as a pure function, separate from `vite.config.ts`, so both directions + * are unit-tested. A bundling switch is only ever exercised by a real + * production build, and one that is silently stuck on "include" is + * indistinguishable from one that works. + */ + +/** Environment variable that opts a `vite build` into shipping `pseudo`. */ +export const PSEUDO_BUNDLE_ENV_VAR = "KANDEV_I18N_BUNDLE_PSEUDO"; + +/** The same truthy set the Makefile uses (`$(filter 1 true yes,...)`). */ +const TRUTHY = new Set(["1", "true", "yes"]); + +export function shouldBundlePseudoLocale(options: { + /** Vite's `ConfigEnv.command`. */ + command: string; + /** Usually `process.env`. */ + env?: Record; +}): boolean { + // `serve` covers `pnpm dev` and vitest, which resolves config in serve mode. + // Neither produces an artifact a user downloads, and both want the oracle. + if (options.command === "serve") return true; + const raw = options.env?.[PSEUDO_BUNDLE_ENV_VAR]; + return TRUTHY.has((raw ?? "").trim().toLowerCase()); +} diff --git a/apps/web/lib/i18n/index.test.ts b/apps/web/lib/i18n/index.test.ts index 76abd3ffb1..7b931368ff 100644 --- a/apps/web/lib/i18n/index.test.ts +++ b/apps/web/lib/i18n/index.test.ts @@ -6,6 +6,7 @@ import { i18n, isSupportedLocale, normalizeLocale, + PSEUDO_LOCALE_BUNDLED, selectableLocales, SUPPORTED_LOCALES, } from "./index"; @@ -50,10 +51,30 @@ describe("locale predicates", () => { expect([...SUPPORTED_LOCALES]).toEqual(["en", "pt-pt", "zh-cn", "pseudo"]); }); - it("hides the pseudo locale in production builds only", () => { + // Only the `isProd` half of the contract. `selectableLocales` also requires + // `PSEUDO_LOCALE_BUNDLED`, and that half cannot be exercised here: the constant + // is fixed to `true` for the whole vitest run (config resolves in serve mode). + // `bundling.test.ts` covers the decision that produces it, in both directions. + it("hides the pseudo locale from production builds", () => { expect(selectableLocales(false)).toEqual(["en", "pt-pt", "zh-cn", "pseudo"]); expect(selectableLocales(true)).toEqual(["en", "pt-pt", "zh-cn"]); }); + + /** + * The include half of the bundling switch, asserted end to end. + * + * `shouldBundlePseudoLocale` is unit-tested in `bundling.test.ts`, but that + * only covers the decision — not that `vite.config.ts` actually defines + * `__KANDEV_PSEUDO_LOCALE_BUNDLED__` and that the constant reaches this module. + * If that wiring breaks in the "exclude" direction, dev and e2e lose the + * pseudo oracle silently: every screen simply falls back to `en` and reads as + * un-externalized. This pins it, and "resolves real catalog messages for the + * active locale" below is what proves the catalog itself came with it. + */ + it("bundles the pseudo catalog outside a production build", () => { + expect(PSEUDO_LOCALE_BUNDLED).toBe(true); + expect(selectableLocales(false)).toContain("pseudo"); + }); }); describe("activateLocale", () => { diff --git a/apps/web/lib/i18n/index.ts b/apps/web/lib/i18n/index.ts index d687d2b12e..be9229bd67 100644 --- a/apps/web/lib/i18n/index.ts +++ b/apps/web/lib/i18n/index.ts @@ -9,8 +9,8 @@ import { writeLocaleCookie } from "./cookie"; * `en` is the source locale; `pseudo` is a QA-only accented/padded locale used * to visually prove every string was externalized (any plain-ASCII text on * screen under `pseudo` is a literal that was never routed through `t`). - * `pseudo` is filtered out of the language switcher in production builds, but - * the runtime can still activate it if a `kandev_locale=pseudo` cookie is set. + * `pseudo` is filtered out of the language switcher in production builds, and + * its catalog is not compiled into them at all — see `PSEUDO_LOCALE_BUNDLED`. * * Catalogs live at `src/locales//.json` and are loaded * eagerly per locale via Vite's glob import, so a locale switch needs no @@ -55,6 +55,16 @@ export function normalizeLocale(value: unknown): SupportedLocale { : DEFAULT_LOCALE; } +/** + * Whether this bundle contains the pseudo catalog at all. + * + * `pseudo` stays in `SUPPORTED_LOCALES` unconditionally — that type drives the + * `en` ↔ `pseudo` parity gate in `i18n:check` and the generator — so this is the + * only thing that distinguishes "shipped" from "supported". `false` in a plain + * `vite build`; see `./bundling.ts`. + */ +export const PSEUDO_LOCALE_BUNDLED: boolean = __KANDEV_PSEUDO_LOCALE_BUNDLED__; + /** * Locales offered by the switcher. The pseudo QA locale is hidden in production. * @@ -62,18 +72,42 @@ export function normalizeLocale(value: unknown): SupportedLocale { * `import.meta.env.PROD`: the e2e harness serves a production bundle, so that * constant is true there too and would hide the locale the QA tests need. The * boot payload's `nonProduction` flag is the authoritative signal. + * + * `PSEUDO_LOCALE_BUNDLED` is checked as well, because those two signals can now + * disagree: a released binary started with `KANDEV_DEBUG_DEV_MODE=true` reports + * `nonProduction` while serving a bundle that has no pseudo catalog. Offering it + * there would switch the user to a locale that resolves entirely through the + * `en` fallback — plain English under a "Pseudo (QA)" label, which reads exactly + * like a total externalization failure. */ export function selectableLocales(isProd: boolean): SupportedLocale[] { - return SUPPORTED_LOCALES.filter((locale) => locale !== "pseudo" || !isProd); + return SUPPORTED_LOCALES.filter( + (locale) => locale !== "pseudo" || (!isProd && PSEUDO_LOCALE_BUNDLED), + ); } type CatalogModule = Record; -// Vite resolves this at build time; each entry is `../../src/locales//.json`. -const catalogModules = import.meta.glob("../../src/locales/*/*.json", { - eager: true, - import: "default", -}); +// Vite resolves these at build time; each entry is `../../src/locales//.json`. +// +// `pseudo` is globbed separately, behind the build-time constant, so a production +// build folds the branch away and rolldown never pulls those JSON modules into +// the graph. Filtering the merged object at runtime instead would still ship +// every byte — and the bytes are the entire problem: pseudo is the LARGEST +// catalog we have (accented characters are multi-byte) and no production user +// can select it. +const catalogModules: Record = { + ...import.meta.glob( + ["../../src/locales/*/*.json", "!../../src/locales/pseudo/*.json"], + { eager: true, import: "default" }, + ), + ...(__KANDEV_PSEUDO_LOCALE_BUNDLED__ + ? import.meta.glob("../../src/locales/pseudo/*.json", { + eager: true, + import: "default", + }) + : {}), +}; function localeResources(locale: SupportedLocale): Record { const resources: Record = {}; diff --git a/apps/web/package.json b/apps/web/package.json index 04148b9b10..e1cc413756 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -8,6 +8,7 @@ "prebuild": "node scripts/generate-release-notes.mjs && node scripts/generate-changelog.mjs", "build": "vite build", "build:vite": "vite build", + "build:e2e": "cross-env KANDEV_I18N_BUNDLE_PSEUDO=1 pnpm run build", "start": "vite preview --host 0.0.0.0", "lint": "eslint --max-warnings 0", "lint:i18n": "eslint --config eslint.i18n.config.mjs", diff --git a/apps/web/vite-env.d.ts b/apps/web/vite-env.d.ts index 11f02fe2a0..2d1c6a6e4e 100644 --- a/apps/web/vite-env.d.ts +++ b/apps/web/vite-env.d.ts @@ -1 +1,8 @@ /// + +/** + * Injected by the `kandev:i18n-pseudo-locale-bundling` plugin in `vite.config.ts`. + * `false` in a plain `vite build`, `true` under `vite dev`, vitest, and a build + * run with `KANDEV_I18N_BUNDLE_PSEUDO=1` (the e2e artifact). + */ +declare const __KANDEV_PSEUDO_LOCALE_BUNDLED__: boolean; diff --git a/apps/web/vite.config.ts b/apps/web/vite.config.ts index 80991ab41d..aecf74d526 100644 --- a/apps/web/vite.config.ts +++ b/apps/web/vite.config.ts @@ -1,9 +1,11 @@ import path from "node:path"; import react from "@vitejs/plugin-react"; -import { defineConfig } from "vite"; +import { defineConfig, type Plugin } from "vite"; + +import { shouldBundlePseudoLocale } from "./lib/i18n/bundling"; export default defineConfig({ - plugins: [react()], + plugins: [react(), pseudoLocaleBundling()], server: { port: readPort(process.env.PORT), strictPort: Boolean(process.env.PORT), @@ -26,6 +28,30 @@ export default defineConfig({ }, }); +/** + * Defines `__KANDEV_PSEUDO_LOCALE_BUNDLED__`, which gates the `pseudo` catalog's + * `import.meta.glob` in `lib/i18n/index.ts`. See `lib/i18n/bundling.ts` for why + * the build invocation — not `import.meta.env.PROD` — decides this. + * + * A plugin rather than a top-level `define` because only the `config` hook is + * handed `ConfigEnv.command`, and keeping the default export an object literal + * leaves `vitest.config.ts`'s `mergeConfig(viteConfig, …)` working unchanged. + */ +function pseudoLocaleBundling(): Plugin { + return { + name: "kandev:i18n-pseudo-locale-bundling", + config(_userConfig, env) { + return { + define: { + __KANDEV_PSEUDO_LOCALE_BUNDLED__: JSON.stringify( + shouldBundlePseudoLocale({ command: env.command, env: process.env }), + ), + }, + }; + }, + }; +} + function readPort(value: string | undefined): number | undefined { if (!value) return undefined; const port = Number(value); diff --git a/docs/i18n.md b/docs/i18n.md index 8f727de4c9..da8de61cd5 100644 --- a/docs/i18n.md +++ b/docs/i18n.md @@ -56,32 +56,38 @@ band, so `i18n:check` reports a real-locale gap as a warning. See ```json { "deletedItems": "Deleted {{count}} items" } ``` + ```tsx -t("common:deletedItems", { count }) +t("common:deletedItems", { count }); ``` ### Plurals i18next uses suffixed keys; the call site passes `count`: + ```json { "sessions_one": "{{count}} session", "sessions_other": "{{count}} sessions" } ``` + ```tsx -t("kanban:sessions", { count: sessionCount }) +t("kanban:sessions", { count: sessionCount }); ``` ### Copy containing markup Use react-i18next's ``. Tag indices count **every** child (text nodes included), and the tag body is the element's text: + ```tsx Type {phrase} to continue. ``` + ```json { "typeToConfirm": "Type <1>{{phrase}} to continue." } ``` + Pass `values` explicitly whenever the message interpolates. Because the indices are positional, **the message and the JSX are coupled**: @@ -99,10 +105,12 @@ drifted indices onto the actual element positions. Never put a `t()` call inside `.ts` helpers and non-component functions cannot use the hook — import the module-level `t`, which binds to the shared instance: + ```ts import { t } from "@/lib/i18n"; export const label = () => t("common:saved"); ``` + Note this resolves at call time; a value captured at module load will not update on a locale switch. @@ -122,7 +130,7 @@ rule ends up at the call site, so a language with three or six forms has nowhere to express them, and a translator editing the catalog cannot fix it. `scripts/check-inline-plurals.mjs` (part of `i18n:check`) rejects that shape, and `scripts/fix-inline-plurals.mjs --write` converts existing ones. Add `_one`/`_other` -to *every* message that interpolates `{{count}}`, even where English does not +to _every_ message that interpolates `{{count}}`, even where English does not inflect (`{{count}} selected`) — other languages agree the participle with the number, and identical forms cost nothing. @@ -145,8 +153,8 @@ token in a constant and interpolate it, so the shown and compared values cannot drift: ```tsx -const CONFIRM_TOKEN = "RESET"; // sentinel, never translated -t("settings:typeToConfirm", { token: CONFIRM_TOKEN }) +const CONFIRM_TOKEN = "RESET"; // sentinel, never translated +t("settings:typeToConfirm", { token: CONFIRM_TOKEN }); ``` The guard's `words.exclude` carries a multi-word SCREAMING-CASE pattern for this @@ -182,7 +190,7 @@ directory you are about to migrate; it never gates a merge. The real gate is Unlike the eslint guard, `i18n:check` is **not** scoped to the allowlist — its four checks scan `components`, `app`, `hooks` and `lib` repo-wide, and it runs in -CI (Frontend Tests → *Run i18n checks*). That is deliberate: each one pins a bug +CI (Frontend Tests → _Run i18n checks_). That is deliberate: each one pins a bug that fails **silently at runtime** rather than merely leaving a string in English, so there is no reason to let it into unmigrated code either. Unmigrated files pass trivially, because a file that never calls `t()` cannot break any of the four. @@ -198,9 +206,43 @@ scope. Module scope matters because the module-level `t` resolves when the file is imported: `const ROWS = [{ label: t("k") }]` pins that copy to the boot locale, so it renders correctly and simply never changes on a switch. The pseudo-locale -cannot see it — the text *is* translated. Store the key and resolve it at render, +cannot see it — the text _is_ translated. Store the key and resolve it at render, or make the value a component. +### Pseudo is not bundled in production + +`pseudo` is the **largest** catalog we have — accented characters are multi-byte — +and no production user can ever select it, because `selectableLocales` filters it +out of the switcher. So a plain `vite build` does not compile it in at all: +`lib/i18n/index.ts` globs it separately behind `__KANDEV_PSEUDO_LOCALE_BUNDLED__`, +which `vite.config.ts` defines from `shouldBundlePseudoLocale` in +`lib/i18n/bundling.ts`. The branch folds away and rolldown never pulls those JSON +modules into the graph. Filtering the merged catalog at runtime instead would +still ship every byte, which is the entire problem. + +| invocation | pseudo? | +| ----------------------------------------------------------------------- | ------- | +| `pnpm dev`, `vitest` (Vite `serve`) | yes | +| `pnpm build` / `make build-web` / release / desktop | **no** | +| `pnpm build:e2e` / `make build-web-e2e` (`KANDEV_I18N_BUNDLE_PSEUDO=1`) | yes | + +`import.meta.env.PROD` cannot make this decision: the e2e harness serves a +**production** bundle, so it is true there too — and e2e is exactly where pseudo +has to survive, because `e2e/tests/i18n/pseudo-coverage.spec.ts` is the only +oracle that sees copy the jsx-only guard structurally cannot. That is why the e2e +runner (`e2e/scripts/run-e2e.sh`) and the CI e2e build job both use the `:e2e` +target. **If you add another build path that e2e runs against, use that target** — +the coverage spec fails loudly rather than silently when pseudo is absent (every +screen falls back to `en` and reads as un-externalized), but it fails as a wall of +findings that look like a migration regression rather than a build misconfiguration. + +`SUPPORTED_LOCALES` still lists `pseudo` unconditionally, and must: that type +drives the `en` ↔ `pseudo` parity gate in `i18n:check` and `i18n:pseudo`. The +exclusion is a bundling decision only. + +To eyeball pseudo in a production-mode local instance (rather than `pnpm dev`), +build with `make build-web-e2e` instead of `make build-web`. + ## The guard is scoped, on purpose `i18next/no-literal-string` is an **error**, but only for the paths listed in @@ -231,12 +273,12 @@ exists — globs included, resolved with `fs.globSync`. So **never remove an ent to make a build pass, and equally, never tidy several file globs into one `dir/**`. Both produce the same diff to the array, the check cannot tell them apart, and refusing to guess is the point: one of them silently un-protects -migrated copy. A broader glob added *alongside* the existing entries passes; +migrated copy. A broader glob added _alongside_ the existing entries passes; swapping them for it does not. The same check rejects a path listed **twice**. An exact duplicate changes no behaviour, so nothing else looks for one — the eslint rule is indifferent to a -repeated pattern, and the removal check above only inspects entries that *left* +repeated pattern, and the removal check above only inspects entries that _left_ the array. That is how #2214 landed two past every gate the repo then had. What a duplicate costs is the record: it claims an earlier migration's coverage as the new PR's. Entries that a broader glob already covers are deliberate and stay — @@ -245,7 +287,7 @@ check, not a redundancy check. #### An entry can be born dead, and only a glob expansion shows it -Both checks above ask what happened to an entry *between* two states. Neither +Both checks above ask what happened to an entry _between_ two states. Neither asks the prior question: **does this entry match any file at all?** An entry that matched nothing on the day it was added is indistinguishable from a healthy one — it never leaves the array, it is never a duplicate, and `pnpm lint` passes @@ -267,13 +309,13 @@ it. So when an entry contains `[`, escape it as `[[]…[]]`. particular class cannot recur — the three checks it runs are deliberately separate because each is blind to the others' failure: -| check | sees | -|---|---| -| removal | an entry that **left** the array while its path still exists | -| duplicate | an entry listed **twice** | +| check | sees | +| --------- | --------------------------------------------------------------------------- | +| removal | an entry that **left** the array while its path still exists | +| duplicate | an entry listed **twice** | | unmatched | an entry that **never matched anything**, including on the day it was added | -The first two compare two *states* of the array, which is why neither can see a +The first two compare two _states_ of the array, which is why neither can see a born-dead entry. If you add a check here, add the test that shows the existing ones miss what it catches — `scripts/lib/guard-allowlist.test.ts` pins that for all three. @@ -284,10 +326,10 @@ Scoping the whole-file guard leaves a hole: a brand-new component under an un-migrated directory would be unchecked. `check-new-code-i18n.mjs` closes it by judging the **change** rather than the file: -| | Judged on | -|---|---| -| A file the change **added** | the whole file — it carries no legacy debt | -| A file the change **modified** | only the lines it touched | +| | Judged on | +| ------------------------------ | ------------------------------------------ | +| A file the change **added** | the whole file — it carries no legacy debt | +| A file the change **modified** | only the lines it touched | Existing literals elsewhere in a modified file are somebody else's migration, so this can be repo-wide without recreating the treadmill. It is the same contract @@ -325,12 +367,14 @@ pnpm run lint:i18n components/ # 5. clean? 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 - cannot tell you this. + cannot tell you this. Use `pnpm dev`; a production build does not bundle the + pseudo catalog and will not offer the option (see _Pseudo is not bundled in + production_). ### The guard has blind spots `i18next/no-literal-string` only inspects **literals in JSX**. Template literals -*are* now checked (`should-validate-template: true` — measured at +2 to +5 +_are_ now checked (`should-validate-template: true` — measured at +2 to +5 findings per un-migrated directory, all of them real copy), but it still cannot see a `confirm()` argument or copy returned from a plain `.ts` helper. @@ -338,9 +382,9 @@ It has one more blind spot worth knowing, because it is the shape config tables take: **a literal assigned to a SCREAMING_CASE identifier is skipped entirely.** ```tsx -const HEADER = Archive board; // NOT flagged -const header = Archive board; // flagged -const ROWS = [{ label: "Disk usage" }]; // NOT flagged +const HEADER = Archive board; // NOT flagged +const header = Archive board; // flagged +const ROWS = [{ label: "Disk usage" }]; // NOT flagged ``` That is why module-scope config objects survived the sweep, and why the @@ -372,7 +416,7 @@ export function ActionButtonContent({ }: Props) { ``` -These are evaluated in the parameter list, which runs *before* the component +These are evaluated in the parameter list, which runs _before_ the component body, so a `const { t } = useTranslation()` in the body is not in scope for them — you cannot fix this in place. Resolve the fallback inside the body instead (`const pending = pendingLabel ?? t("system:jobRunning")`). Prop defaults on a @@ -396,7 +440,7 @@ exits 0. It prints two separate sections, because mixing "this is broken" with 1. **English plural concatenation** — `{n} file{n === 1 ? "" : "s"}`, and the hoisted `const plural = count === 1 ? "" : "s"` form. These are defects, and **no gate we own sees them**: `no-literal-string` is jsx-only, and - `check-inline-plurals.mjs` only inspects morphemes passed *through* `t()`. A + `check-inline-plurals.mjs` only inspects morphemes passed _through_ `t()`. A repo-wide sweep found 44 of them across 36 files. One helper had 2 of its 11 call sites visible to lint — migrating the visible two would have left nine broken while the directory reported clean. **30 of those 44 are under @@ -408,7 +452,7 @@ exits 0. It prints two separate sections, because mixing "this is broken" with content is sent verbatim to a model, so translating it would be a bug (`components/github/pr-checks-section.tsx` and `pr-ci-popover.tsx` are the known-good examples, and the tool prints this caveat itself). And the section - is a *narrow sample*, not an inventory — its noise filter drops any literal + is a _narrow sample_, not an inventory — its noise filter drops any literal starting with a letter, so it surfaces bracketed log prefixes and media queries but not a `{ label: "Disk usage" }` config table. A short section is not evidence a directory is clean. @@ -435,7 +479,7 @@ the completeness check. these are localized at the **call site**, not in the package. Check the shape before inventing a seam, because the two halves differ: - **A label in an ATTRIBUTE needs nothing.** Every one of them sits on an - element that spreads `{...props}` *after* it, so the overridable string prop + element that spreads `{...props}` _after_ it, so the overridable string prop the spec asks for already exists and the English literal is only its default. `Breadcrumb`'s `aria-label="breadcrumb"` was this shape: the fix was `aria-label={t("common:breadcrumb")}` at its three consumers (`page-topbar`, @@ -455,8 +499,9 @@ the completeness check. A third-party primitive is the same call: sonner names its toast region `Notifications alt+T` and accepts `containerAriaLabel`, which `AppToaster` in `src/app-shell.tsx` passes. That has to be its own component — `` - is mounted *by* `AppShell`, so a `t()` call in `AppShell`'s own render sits + is mounted _by_ `AppShell`, so a `t()` call in `AppShell`'s own render sits outside the provider and would not re-render on a locale switch. + - **Tests**: `vitest.setup.ts` calls `initI18nForTests()`. react-i18next falls back to the default instance when no provider is present, so no render wrapper is needed and every test's render tree is unchanged. @@ -530,7 +575,7 @@ the share builders mix all of them into the same lines: - **Emoji**, which stay in the Go format string rather than the catalog. **Contract for new user-facing backend output:** prefer returning a stable error -*code* the frontend translates. Only use `i18n.T` when Go itself renders the bytes +_code_ the frontend translates. Only use `i18n.T` when Go itself renders the bytes a user reads. ## Pseudo-locale QA (the completeness oracle) @@ -560,7 +605,7 @@ stays English by design, so it renders unaccented and is still correct. See Two things it cannot show you at all — the second is detailed below: -1. **Module-scope `t()`.** That copy *is* translated, just frozen at the boot +1. **Module-scope `t()`.** That copy _is_ translated, just frozen at the boot locale, so it renders accented and looks correct while never responding to a switch. `check-module-scope-t.mjs` is what catches it. 2. **An un-migrated value interpolated into a migrated frame** — it renders @@ -568,7 +613,7 @@ Two things it cannot show you at all — the second is detailed below: A third used to belong on that list — copy that never becomes a text node — and the spec now reads it directly. That pass is described next, because what it does -*not* check still matters when you migrate. +_not_ check still matters when you migrate. ### It reads attribute copy, which nothing on screen shows you @@ -592,7 +637,7 @@ aria-label on button#language-select: "Display language (pseudo)" renders as `Mōŕē ĩńfōŕmàţĩōń`, which has no 4-letter ASCII run and so is dropped at the `wordlike` gate before the accented test ever sees it. Soundness is fine — a real miss is pure ASCII — but it means a working pass and a pass whose selector -matched *nothing* produce the same empty output. One agent scanning by hand +matched _nothing_ produce the same empty output. One agent scanning by hand concluded their scan was broken on exactly this. The spec therefore asserts `inspectedAttributes > 0` per screen, prints how many rendered fully accented, and pins accent detection once against `#language-select`'s known-migrated @@ -616,9 +661,9 @@ output: bury every other string on the screen. - **One accented character does not clear an attribute**, unlike the text pass. An `aria-label` built as `` `Collapse ${label}` `` renders `Collapse Ĝēńēŕàĺ` — - an un-migrated English *frame* around a migrated value, which accent-clears-all + an un-migrated English _frame_ around a migrated value, which accent-clears-all would call done. Allowlist stripping is what separates that from the legitimate - case, where the ASCII fragment is allowlisted *data* rather than the frame + case, where the ASCII fragment is allowlisted _data_ rather than the frame (`Àćţĩōńś ƒōŕ Agent` on the layouts screen strips to nothing). Such findings are tagged `— English frame, migrated value`. @@ -657,7 +702,7 @@ grep -rnE 't\("[^"]+", \{ (error|message)' components app hooks | grep -v '\.tes see [Backend](#what-the-backend-deliberately-does-not-translate) for why the ~1,100 server error strings are out of scope. Around 37 catalog keys already interpolate one; none of them is a miss. Where such a call needs a fallback for - a missing payload, the *fallback* is copy and is translated + a missing payload, the _fallback_ is copy and is translated (`t("linear:testFailed", { error: result.error || t("linear:unknownError") })`) while the payload itself is not. - **A client-side literal reaching a translated frame is a miss.** A string @@ -690,9 +735,9 @@ Which screens you walk matters as much as what you look for. A component's copy is verified by walking **every route that mounts it** — not the routes your migration owns. -The natural method fails here. Walking the import closure *from your own pages* +The natural method fails here. Walking the import closure _from your own pages_ traverses the graph in the wrong direction: it finds what your routes render, and -by construction never finds a route that renders *you*. One migration hit it in +by construction never finds a route that renders _you_. One migration hit it in both directions — the cards it converted mount only on a sibling migration's route, and a card that sibling owned mounts on its own. Neither could have been verified from its own pages alone. @@ -718,7 +763,7 @@ rediscovering the question. ### Copy that belongs to no directory -A file shared by *every* area has no natural owner and so gets skipped by all of +A file shared by _every_ area has no natural owner and so gets skipped by all of them. `src/settings-routes.tsx` holds the live title and description for every settings route in one `SETTINGS_ROUTES` map — a SCREAMING_CASE identifier the guard skips entirely, in a 650-line file no single migration's closure claims. @@ -739,15 +784,15 @@ Two surfaces rendering "the same" sentence have usually **drifted**. Settings System had an unreferenced `app/settings/system//page.tsx` twin for every route, and the twins were migrated at a different time by a different PR: -| surface | copy | -|---|---| -| `SETTINGS_ROUTES` — live, what users see | `Create a diagnostic ZIP with frontend and backend logs.` | +| surface | copy | +| --------------------------------------------------------------------------- | ------------------------------------------------------------------------- | +| `SETTINGS_ROUTES` — live, what users see | `Create a diagnostic ZIP with frontend and backend logs.` | | `app/settings/system/logs/page.tsx` — dead → `settings:logsPageDescription` | `Download a bounded diagnostic ZIP containing frontend and backend logs.` | Pointing the live route at the existing key changed the sentence on a shipping page. `pnpm lint`, `pnpm run i18n:check`, the ratchet and 8,000+ unit tests were all green — the key resolved, the catalogs matched, nothing was hardcoded. The -copy was simply *different*. `feature-toggles` had drifted the same way in the +copy was simply _different_. `feature-toggles` had drifted the same way in the same directory. Nothing in the toolchain can catch this, because every check verifies that a key @@ -791,8 +836,8 @@ migration: a value extracted into `{{interpolation}}`, either half of a ` sentence split around an element, and an `_one`/`_other` pair. **The hazard has two shapes, and only one of them is above.** The case in this -section is an *attribute* — `description="…"` on a route shell — which is -quoted, so even a crude scan finds it. The other is *element text*, which is +section is an _attribute_ — `description="…"` on a route shell — which is +quoted, so even a crude scan finds it. The other is _element text_, which is not quoted and is where most migrated copy lives; a reused key that rewrites a button or a link label leaves nothing for a quoted-string search to find. Do not read this section as "route descriptions drift". Any copy rendered from two @@ -800,7 +845,7 @@ places can. It is **advisory and deliberately not in CI**: it is diff-based, so it needs a base ref (`i18n:check` is repo-wide on purpose), and a finding is a prompt to -*classify* rather than proof of a bug — each is a value extraction, a literal +_classify_ rather than proof of a bug — each is a value extraction, a literal hoisted to a constant, or the real thing. It narrows the diff for a human; it does not decide. `--base ` overrides the `origin/main` fork point, and `--all` widens past prose to identifier-shaped strings. @@ -809,7 +854,7 @@ Some benign findings recur and are worth recognising rather than investigating: an array of fragments collapsed into one comma-separated value is accepted because every fragment survives verbatim inside the join — the safe property is that the collapse is **concatenative**, not that it is an array, and a collapse -that also *rewords* is reported, correctly. The shape to be wary of is not +that also _rewords_ is reported, correctly. The shape to be wary of is not "array collapsed to string", it is "collapse that also edits". ### Prove it can fail, before you believe it passed @@ -821,13 +866,13 @@ check that read your diff and found nothing from one that could not have failed. Both print the same tick. That distinction is not hypothetical. Every defect found in this tool during -review had the same signature — *it returned a pass it had not earned* — and not +review had the same signature — _it returned a pass it had not earned_ — and not one was visible to its own unit tests, because those assume a well-formed catalog and a committed tree. A message that was nothing but a placeholder compiled to a match-anything pattern and accepted every string in the repository. The "after" side read `HEAD` while the file list came from the -working tree, so uncommitted edits were invisible and it printed a tick *with -the correct file count*. Each was found by someone running it for real and +working tree, so uncommitted edits were invisible and it printed a tick _with +the correct file count_. Each was found by someone running it for real and declining to believe the result. **The probe must be a rewrite, and it must change the opening words.** Two @@ -848,7 +893,7 @@ A good probe changes the whole sentence: `"What do you want to change?"` → message with too little of it earns no pattern at all — it needs two words of prose. That threshold is not arbitrary. One short word clears any "is there a word?" bar while leaving the pattern almost unbounded: `"Delete {{name}}"` -compiled to `/^Delete [\s\S]+?$/` and accepted *every* sentence beginning +compiled to `/^Delete [\s\S]+?$/` and accepted _every_ sentence beginning "Delete ", which left 19 of 43 strings unprotected in the first file this check was pointed at. @@ -863,7 +908,7 @@ is a separate step, and why "the check was clean" is not on its own an answer to One limit is structural rather than a tuning choice, and is worth knowing before anyone tries to close it: `"{{count}} of {{total}}"` (a legitimate value extraction) and `"{{a}} of {{b}}"` (far too permissive to anchor anything) both -reduce to the residue `"of"` and compile to the *same* regex. Nothing computed +reduce to the residue `"of"` and compile to the _same_ regex. Nothing computed from the message can accept one and reject the other. Both are rejected, so a string like `"3 of 9"` is reported and classified by hand — the right direction for an advisory check, since a false positive costs someone ten seconds and a @@ -943,16 +988,16 @@ 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 | +| 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`