From 1e3ee2ef4ac6fdc317ee88431d3c34240fd6cdb1 Mon Sep 17 00:00:00 2001 From: dchaudhari7177 <111210939+dchaudhari7177@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:16:31 +0530 Subject: [PATCH] fix: restore missing common i18n keys and guard them with a test common.unknown and common.heartbeatDataIncomplete were added in ff1ada1 and removed again in 8c1be9c, in a hunk unrelated to that PR's subject. Four call sites still reference them, and LanguageContext.t() returns the key path when the leaf is not renderable, so the dashboard rendered the literal text common.heartbeatDataIncomplete in both locales -- exactly when the backend is degraded and a readable message matters most. Restore both keys in both locales with their original wording, recovered from the deleting commit rather than rewritten. Add homepage/src/__tests__/i18nKeyCompleteness.test.ts, which scans every t('...') literal in the homepage source and asserts it resolves to renderable text in every locale. Renderable means a string or a plural form carrying a string 'other' branch, mirroring what t() can actually return -- without that, the intentional dashboard.modulesListed plural object reports as missing. The suite also asserts it found call sites at all, so a scan that stops matching fails loudly instead of passing vacuously, and that the locales stay structurally in step. Lesson recorded in docs/11-risks-and-technical-debt per CLAUDE.md. Refs #238 --- docs/11-risks-and-technical-debt/README.md | 32 ++++++ .../src/__tests__/i18nKeyCompleteness.test.ts | 107 ++++++++++++++++++ homepage/src/i18n/translations.ts | 6 + 3 files changed, 145 insertions(+) create mode 100644 homepage/src/__tests__/i18nKeyCompleteness.test.ts diff --git a/docs/11-risks-and-technical-debt/README.md b/docs/11-risks-and-technical-debt/README.md index 1b5653f2..065230f2 100644 --- a/docs/11-risks-and-technical-debt/README.md +++ b/docs/11-risks-and-technical-debt/README.md @@ -145,6 +145,38 @@ write the lesson here so the next contributor doesn't repeat it. Format: short title + **What happened** + **Why it happened** + **How to avoid it next time**. +### A rebase dropped two i18n keys, and `t()` renders the key path instead of failing — the degraded-mode banner read `common.heartbeatDataIncomplete` in production (2026-08 audit, #238) + +**What happened:** `common.unknown` and `common.heartbeatDataIncomplete` +were added in `ff1ada1` (#50) and removed again in `8c1be9c` (#52) in a +hunk unrelated to that PR's subject — an accidental merge/rebase +regression. Four call sites kept referencing them. Because +`LanguageContext.t()` returns the key path when the leaf is not +renderable, the dashboard rendered the literal string +`common.heartbeatDataIncomplete` to users, in both locales, precisely +when the backend was degraded and a readable message mattered most. +Nothing failed: not the type checker, not the 191-test homepage suite, +not the build. + +**Why:** `t()` takes a `string` path, so a key that does not exist is +indistinguishable at compile time from one that does — there is no +type-level link between a call site and the translations object. The +graceful fallback that makes `t()` safe to call is exactly what makes a +missing key invisible: it degrades to something renderable instead of +throwing, so the failure only ever shows up on screen. + +**How to avoid it next time:** a graceful fallback needs a test that +notices when it fires. `homepage/src/__tests__/i18nKeyCompleteness.test.ts` +scans every `t('...')` literal in the homepage source and asserts each +resolves to renderable text in **every** locale, and that the locales +stay structurally in step. Two things it has to get right, both learned +the hard way while writing it: it must accept plural forms +(`{ one, other }`) as renderable, or it reports the intentional +`dashboard.modulesListed` as missing; and it must assert it found call +sites at all, or a scan that silently matches nothing passes every other +assertion vacuously — which is what happened on the first run, when +`__dirname` was undefined under vitest's ESM loader. + ### The delete path trusted a client filename the read path didn't — and fleet filenames were silently clobbering each other (2026-07 audit, #202) **What happened:** `image-service`'s `delete_image` joined the diff --git a/homepage/src/__tests__/i18nKeyCompleteness.test.ts b/homepage/src/__tests__/i18nKeyCompleteness.test.ts new file mode 100644 index 00000000..8e2f4329 --- /dev/null +++ b/homepage/src/__tests__/i18nKeyCompleteness.test.ts @@ -0,0 +1,107 @@ +import { describe, it, expect } from 'vitest'; +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { join, resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import translations from '../i18n/translations'; + +/** + * Guards the failure in #238: a `t('...')` call site whose key does not exist + * in translations.ts. LanguageContext.t() returns the key path when the leaf + * is not renderable, so the UI shows the literal `common.unknown` instead of + * text, and nothing failed. This scans the source for `t('...')` literals and + * resolves each against every locale. + */ + +const SRC = resolve(dirname(fileURLToPath(import.meta.url)), '..'); + +function sourceFiles(dir: string): string[] { + return readdirSync(dir).flatMap((entry) => { + const full = join(dir, entry); + if (statSync(full).isDirectory()) { + // The tests themselves may reference deliberately-absent keys. + return entry === '__tests__' ? [] : sourceFiles(full); + } + return /\.tsx?$/.test(entry) ? [full] : []; + }); +} + +/** + * Only single-quoted string literals. A template literal or a variable is not + * statically resolvable, and is out of scope for this check. + */ +function keysIn(source: string): string[] { + return [...source.matchAll(/\bt\(\s*'([^']+)'/g)].map((match) => match[1]); +} + +function resolveKey(locale: Record, key: string): unknown { + return key + .split('.') + .reduce( + (node, part) => + node !== null && typeof node === 'object' + ? (node as Record)[part] + : undefined, + locale + ); +} + +/** + * Mirrors what LanguageContext.t() can actually render: a string, or a plural + * form — an object carrying a string `other` branch, which t() selects with + * Intl.PluralRules. Anything else (an array such as setup.stepLabels, or a + * nested group) collapses to the key path on screen, which is the bug being + * guarded. Those are read with useTranslationRaw(), not t(). + */ +function isRenderable(value: unknown): boolean { + if (typeof value === 'string') return true; + return ( + value !== null && + typeof value === 'object' && + typeof (value as Record).other === 'string' + ); +} + +const locales = Object.keys(translations) as (keyof typeof translations)[]; + +const callSites = sourceFiles(SRC).flatMap((file) => + keysIn(readFileSync(file, 'utf8')).map((key) => ({ file, key })) +); + +describe('i18n key completeness', () => { + it('finds t() call sites to check', () => { + // Guards the scan itself: a regex or a directory walk that silently stops + // matching would otherwise make every assertion below vacuously true. + expect(callSites.length).toBeGreaterThan(20); + expect(locales.length).toBeGreaterThan(1); + }); + + it('accepts strings and plural forms, and rejects what t() cannot render', () => { + expect(isRenderable('Unknown')).toBe(true); + expect(isRenderable({ one: '1 module', other: '{count} modules' })).toBe(true); + expect(isRenderable(undefined)).toBe(false); + expect(isRenderable(['a', 'b'])).toBe(false); + expect(isRenderable({ nested: { deeper: 'x' } })).toBe(false); + }); + + it.each(locales)('every t() key resolves to renderable text in %s', (locale) => { + const missing = callSites + .filter(({ key }) => !isRenderable(resolveKey(translations[locale], key))) + .map(({ file, key }) => `${key} (${file.slice(SRC.length + 1)})`); + + expect([...new Set(missing)]).toEqual([]); + }); + + it('keeps every locale structurally in step', () => { + const flatten = (node: unknown, prefix = ''): string[] => + node !== null && typeof node === 'object' + ? Object.entries(node as Record).flatMap(([k, v]) => + flatten(v, prefix ? `${prefix}.${k}` : k) + ) + : [prefix]; + + const [first, ...rest] = locales.map((locale) => flatten(translations[locale]).sort()); + for (const other of rest) { + expect(other).toEqual(first); + } + }); +}); diff --git a/homepage/src/i18n/translations.ts b/homepage/src/i18n/translations.ts index e05d37ab..66268f82 100644 --- a/homepage/src/i18n/translations.ts +++ b/homepage/src/i18n/translations.ts @@ -6,6 +6,9 @@ const translations = { next: 'Next', online: 'Online', offline: 'Offline', + unknown: 'Unknown', + heartbeatDataIncomplete: + 'Heartbeat data unavailable — some module statuses may be incomplete. Refresh in a moment.', loading: 'Loading...', error: 'Error', tryAgain: 'Try Again', @@ -479,6 +482,9 @@ const translations = { next: 'Weiter', online: 'Online', offline: 'Offline', + unknown: 'Unbekannt', + heartbeatDataIncomplete: + 'Heartbeat-Daten nicht verfügbar — einige Modul-Status können unvollständig sein. Bitte gleich neu laden.', loading: 'Laden...', error: 'Fehler', tryAgain: 'Erneut versuchen',