diff --git a/apps/web/package.json b/apps/web/package.json index 605f9a2a6a..04148b9b10 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -26,7 +26,8 @@ "i18n:pseudo": "node scripts/generate-pseudo-locale.mjs", "i18n:check": "node scripts/check-i18n-keys.mjs && node scripts/check-trans-indices.mjs && node scripts/check-inline-plurals.mjs && node scripts/check-module-scope-t.mjs", "i18n:ratchet": "node scripts/run-i18n-ratchet.mjs", - "i18n:removed": "node scripts/check-removed-literals.mjs" + "i18n:removed": "node scripts/check-removed-literals.mjs", + "i18n:sweep": "node scripts/i18n-sweep.mjs" }, "dependencies": { "@base-ui/react": "^1.0.0", diff --git a/apps/web/scripts/i18n-sweep.mjs b/apps/web/scripts/i18n-sweep.mjs new file mode 100644 index 0000000000..ebe493ef19 --- /dev/null +++ b/apps/web/scripts/i18n-sweep.mjs @@ -0,0 +1,186 @@ +#!/usr/bin/env node +/** + * Find user-facing copy the `i18next/no-literal-string` guard cannot see. + * + * The rule runs in `mode: "jsx-only"`, so it only inspects literals in JSX. Copy + * assigned to a variable, used as a default parameter, held in a module-scope + * config object, or built inside a plain `.ts` helper is invisible to it — and so + * is an English plural morpheme, because `words.exclude` matches a bare "s". + * + * Two sections, deliberately separated: + * + * 1. English plural concatenation — `{n} file{n === 1 ? "" : "s"}`. A defect. + * No gate we own sees it: `no-literal-string` is jsx-only, and + * `check-inline-plurals.mjs` only inspects morphemes passed through `t()`. + * 2. Prose literals in non-JSX positions — for eye review, not auto-flagged. + * + * This is a human tool, not a gate: it always exits 0 and is wired into neither + * CI nor pre-commit. The eye-review half needs judgement, and a check that fires + * on every clean run teaches people to ignore it. + * + * Known limits, inherited from the Python original and kept for parity with it: + * a line is skipped only when it *starts* with a comment, so a trailing + * `// n === 1 ? "" : "s"` would be reported (0 occurrences across `components`, + * `app`, `hooks` and `lib` today — if that changes, masking comments needs a + * quote-aware lexer, since `//` also appears inside strings and URLs), and + * `.d.ts` files are scanned like any other `.ts` (there are none in the + * directories this is pointed at). See also the note on `NOISE_VAL` below. + * + * Usage: node scripts/i18n-sweep.mjs [ ...] + */ +import fs from "node:fs"; +import path from "node:path"; + +/** + * Match EVERY literal, not just long ones: skipping short ones desynchronises + * quote pairing and captures the gap between two strings as if it were copy. + * On `if (size === "xs") return "h-6 w-6…"`, a length filter applied here drops + * `"xs"`, pairs its closing quote with the next string's opening quote, and + * reports `") return "` as copy. Length is filtered after the match, never + * during it. + */ +const LITERAL = /"((?:[^"\\\n]|\\.)*)"/g; +/** Positions that are configuration or markup, never copy. */ +const NOISE_CTX = + /(className|classNames|data-testid|data-legacy-testid|viewBox|xmlns|setAttribute|stroke|d=|style=)/; +/** + * Values that are identifiers/CSS/markup rather than prose. + * + * Note what this actually rejects, because it is broader than it looks and the + * numbers below were validated with it in place: `[a-z0-9-]+` and + * `[A-Z][A-Z0-9_]*` each match a single leading character before `.*$` swallows + * the rest, so **any value starting with a letter or digit is dropped**. What + * survives is prose starting with punctuation — bracketed log prefixes, media + * queries, diff markers, bullet separators. The eye-review section is therefore + * a narrow sample of non-JSX copy, not an inventory of it; a short section is + * not evidence that a directory is clean. The pseudo-locale remains the + * completeness check. Kept as-is deliberately: this is a faithful port, and + * loosening it changes every count the tool has been calibrated against. + */ +const NOISE_VAL = + /^(use (client|strict)|https?:\/\/|[a-z0-9-]+|[A-Z][A-Z0-9_]*|.*(?:rgba?\(|hsl\(|!important|px|rem)).*$/; +const TAILWIND = + /(^|\s)(flex|grid|absolute|relative|h-|w-|p-|m-|text-|bg-|border|rounded|opacity-|transition|group|hover:|min-|max-)/; +/** + * An English plural morpheme concatenated onto a count — forbidden, and invisible + * to check-inline-plurals.mjs, which only inspects morphemes passed through t(). + */ +const PLURAL = /\?\s*"(s|es)"\s*:\s*""|\?\s*""\s*:\s*"(s|es)"/; +/** Already a catalog key (`namespace:key`), so not a literal to migrate. */ +const CATALOG_KEY = /\b\w+:[a-zA-Z]/; +/** Lines that cannot hold rendered copy. */ +const SKIP_LINE = /^(import |\/\/|\*|\/\*)/; + +const MIN_LENGTH = 4; + +/** `str.isupper()` for a single character: has case, and that case is upper. */ +function isUpper(char) { + return char.toLowerCase() !== char.toUpperCase() && char === char.toUpperCase(); +} + +function isProse(value) { + if (!/[A-Za-z]/.test(value)) return false; + if (!(value.includes(" ") || isUpper(value[0]))) return false; + if (TAILWIND.test(value) || NOISE_VAL.test(value)) return false; + return true; +} + +/** + * Python `repr()` for a string, so output reads the same as the original tool: + * single quotes, unless the value contains one and no double quote. + */ +function repr(value) { + const escaped = value.replace(/\\/g, "\\\\"); + if (escaped.includes("'") && !escaped.includes('"')) return `"${escaped}"`; + return `'${escaped.replace(/'/g, "\\'")}'`; +} + +/** + * Walk without following build output. The tool is pointed at source directories, + * but `.` is a plausible mistake and node_modules would take minutes. + */ +function listFiles(dir, out = []) { + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return out; + } + for (const entry of entries.sort((a, b) => a.name.localeCompare(b.name))) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (["node_modules", "dist", ".git"].includes(entry.name)) continue; + listFiles(full, out); + } else if (/\.tsx?$/.test(entry.name) && !entry.name.includes(".test.")) { + out.push(full); + } + } + return out; +} + +function sweep(dirs) { + const plurals = []; + const hits = []; + + for (const dir of dirs) { + for (const file of listFiles(dir)) { + const lines = fs.readFileSync(file, "utf8").split("\n"); + lines.forEach((line, index) => { + const lineNumber = index + 1; + if (SKIP_LINE.test(line.trimStart())) return; + if (PLURAL.test(line)) { + plurals.push(`${file}:${lineNumber}: ${line.trim().slice(0, 100)}`); + } + for (const match of line.matchAll(LITERAL)) { + const value = match[1]; + if (value.length < MIN_LENGTH || !isProse(value)) continue; + if (NOISE_CTX.test(line.slice(0, match.index))) continue; + if (CATALOG_KEY.test(value)) continue; + hits.push(`${file}:${lineNumber}: ${repr(value)}`); + } + }); + } + } + + return { plurals, hits }; +} + +/** + * The prompt-builder exclusion, printed rather than auto-detected. Agent-facing + * text is sent verbatim to a model, so translating it — or removing a plural + * morpheme from it — would be a bug. Both sections need this caveat: the known + * examples in `components/github` surface under *plural concatenation*, not + * eye-review, so a note carried only by the second section would leave the + * defect list telling a maintainer to break working prompts. + */ +const PROMPT_CAVEAT = [ + " Note: prompt builders and other agent-facing text are deliberately English —", + " the 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 known-good", + " examples, and both sit under comments explaining the choice.", +]; + +function report({ plurals, hits }) { + const out = []; + out.push( + `=== ${plurals.length} English plural concatenation(s) — forbidden, undetected by any gate ===`, + ); + if (plurals.length) { + out.push(...PROMPT_CAVEAT, " Every other entry below is a defect."); + } + for (const p of plurals) out.push(` ${p}`); + out.push("", `=== ${hits.length} literal(s) to review by eye ===`); + out.push( + ...PROMPT_CAVEAT, + " Judge each hit; nothing in this section is automatically a defect.", + ); + for (const h of hits) out.push(` ${h}`); + return out.join("\n"); +} + +const DIRS = process.argv.slice(2).filter((a) => !a.startsWith("--")); +if (!DIRS.length) { + console.error("Usage: node scripts/i18n-sweep.mjs [ ...]"); + process.exit(2); +} +console.log(report(sweep(DIRS))); diff --git a/apps/web/scripts/i18n-sweep.test.ts b/apps/web/scripts/i18n-sweep.test.ts new file mode 100644 index 0000000000..f9945469a3 --- /dev/null +++ b/apps/web/scripts/i18n-sweep.test.ts @@ -0,0 +1,266 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { spawnSync } from "node:child_process"; + +import { describe, expect, it } from "vitest"; + +const SCRIPT = path.resolve(import.meta.dirname, "i18n-sweep.mjs"); + +type Fixture = Record; + +interface SweepResult { + status: number | null; + stdout: string; + /** Findings under "English plural concatenation" — defects. */ + plurals: string[]; + /** Findings under "literal(s) to review by eye" — judgement calls. */ + eyeReview: string[]; +} + +/** Split the report on its two headers so a test can assert which half a finding landed in. */ +function parseSections(stdout: string): Pick { + const lines = stdout.split("\n"); + const eyeHeader = lines.findIndex((l) => l.includes("literal(s) to review by eye")); + const isFinding = (l: string) => /^\s+\S+:\d+:/.test(l); + return { + plurals: lines.slice(0, eyeHeader).filter(isFinding), + eyeReview: lines.slice(eyeHeader).filter(isFinding), + }; +} + +function sweep(fixture: Fixture): SweepResult { + const root = mkdtempSync(path.join(tmpdir(), "kandev-i18n-sweep-")); + try { + for (const [file, source] of Object.entries(fixture)) { + const full = path.join(root, file); + mkdirSync(path.dirname(full), { recursive: true }); + writeFileSync(full, source); + } + const result = spawnSync(process.execPath, [SCRIPT, root], { encoding: "utf8" }); + return { status: result.status, stdout: result.stdout, ...parseSections(result.stdout) }; + } finally { + rmSync(root, { recursive: true, force: true }); + } +} + +describe("English plural concatenation", () => { + it("reports a morpheme concatenated onto a count in JSX", () => { + const result = sweep({ + "file-count.tsx": [ + "export function FileCount({ n }: { n: number }) {", + ' return {n} file{n === 1 ? "" : "s"};', + "}", + ].join("\n"), + }); + + expect(result.plurals).toHaveLength(1); + expect(result.plurals[0]).toContain("file-count.tsx:2"); + }); + + it("reports both orderings of the conditional", () => { + const result = sweep({ + "orderings.ts": [ + 'export const a = (n: number) => `${n} item${n !== 1 ? "s" : ""}`;', + 'export const b = (n: number) => `${n} item${n === 1 ? "" : "s"}`;', + ].join("\n"), + }); + + expect(result.plurals).toHaveLength(2); + }); + + it("reports a plural morpheme hoisted into a helper variable", () => { + const result = sweep({ + "helper.ts": [ + "export function summary(count: number) {", + ' const plural = count === 1 ? "" : "s";', + " return `${count} match${plural}`;", + "}", + ].join("\n"), + }); + + expect(result.plurals).toHaveLength(1); + expect(result.plurals[0]).toContain("helper.ts:2"); + }); + + it("does not report a t() call using count with _one/_other keys", () => { + const result = sweep({ + "translated.tsx": [ + 'import { useTranslation } from "react-i18next";', + "", + "export function FileCount({ n }: { n: number }) {", + " const { t } = useTranslation();", + ' return {t("common:fileCount", { count: n })};', + "}", + ].join("\n"), + }); + + expect(result.plurals).toEqual([]); + }); + + it("does not report a ternary that selects whole words rather than a morpheme", () => { + const result = sweep({ + "words.ts": ['export const verb = (n: number) => (n === 1 ? "is" : "are");'].join("\n"), + }); + + expect(result.plurals).toEqual([]); + }); +}); + +/** + * The regression that matters. Filtering literals by length *during* the scan + * desynchronises quote pairing: `"xs"` is skipped, so its closing quote pairs + * with the next string's opening quote and the gap between them (`") return "`) + * is reported as copy. That produced 25 false positives on a directory with one + * real hit, which is exactly the noise that gets a check ignored. + */ +describe("quote pairing", () => { + it("does not turn the gap between two literals into a finding", () => { + const result = sweep({ + "icon-size.ts": [ + "export function iconSize(size: string) {", + ' if (size === "xs") return "h-6 w-6 shrink-0";', + ' return "h-8 w-8 shrink-0";', + "}", + ].join("\n"), + }); + + expect(result.eyeReview).toEqual([]); + expect(result.stdout).not.toContain("return"); + }); + + it("still finds prose that follows a short literal on the same line", () => { + const result = sweep({ + "mixed.ts": [ + "export function label(size: string) {", + ' if (size === "xs") return "[layout] Compact mode selected";', + "}", + ].join("\n"), + }); + + expect(result.eyeReview).toHaveLength(1); + expect(result.eyeReview[0]).toContain("Compact mode selected"); + }); +}); + +describe("eye-review filtering", () => { + it("skips Tailwind class strings, catalog keys and identifiers", () => { + const result = sweep({ + "noise.tsx": [ + 'export const cls = "flex items-center gap-2";', + 'export const key = "settings:deleteExecutor";', + 'export const id = "task-archive-dialog";', + 'export const url = "https://kandev.ai/docs";', + ].join("\n"), + }); + + expect(result.eyeReview).toEqual([]); + }); + + it("skips test files, comments and imports", () => { + const result = sweep({ + "thing.test.tsx": 'export const copy = "Archive this board";', + "commented.ts": [ + 'import { thing } from "./Archive this board";', + '// const copy = "Archive this board";', + ' * "Archive this board"', + ].join("\n"), + }); + + expect(result.eyeReview).toEqual([]); + }); + + it("finds copy in a plain .ts helper the jsx-only lint rule cannot see", () => { + const result = sweep({ + "storage-hooks.ts": [ + "export function warn(error: unknown) {", + ' console.warn("[storage] Could not read the disk usage report:", error);', + "}", + ].join("\n"), + }); + + expect(result.eyeReview).toHaveLength(1); + expect(result.eyeReview[0]).toContain("storage-hooks.ts:2"); + }); + + /** + * Characterization, not endorsement. `NOISE_VAL`'s `[a-z0-9-]+` and + * `[A-Z][A-Z0-9_]*` branches each match a single leading character before + * `.*$` swallows the rest, so every value starting with a letter is dropped — + * a `const ROWS = [{ label: "Disk usage" }]` config table stays invisible. + * Pinned here so the limit is a known property of the tool rather than a + * surprise, and so a future widening has to change this test on purpose. + */ + it("does not surface prose beginning with a letter — a known limit of the filter", () => { + const result = sweep({ + "rows.ts": [ + "export const ROWS = [", + ' { label: "Disk usage" },', + ' { label: "Memory in use" },', + "];", + ].join("\n"), + }); + + expect(result.eyeReview).toEqual([]); + }); +}); + +describe("report shape", () => { + it("always prints both sections and exits 0, even with nothing to report", () => { + const result = sweep({ "clean.ts": "export const n = 1;" }); + + expect(result.status).toBe(0); + expect(result.stdout).toContain("0 English plural concatenation(s)"); + expect(result.stdout).toContain("0 literal(s) to review by eye"); + }); + + it("exits 0 even when defects are found — it is a human tool, not a gate", () => { + const result = sweep({ + "defect.ts": 'export const s = (n: number) => `${n} file${n === 1 ? "" : "s"}`;', + }); + + expect(result.status).toBe(0); + }); + + it("notes the prompt-builder exclusion in the eye-review section", () => { + const result = sweep({ "clean.ts": "export const n = 1;" }); + + expect(result.stdout).toContain("agent-facing"); + }); + + /** + * The known-good prompt builders in `components/github` surface under *plural + * concatenation*, not eye-review — an agent-facing string carrying `{n} + * check{s}` is not a defect, because the text goes verbatim to a model. A + * caveat printed only by the eye-review section would leave the defect list + * telling a maintainer to break working prompts. + */ + it("repeats the prompt-builder exclusion in the plural section when it has findings", () => { + const result = sweep({ + "prompt-builder.ts": [ + "// Agent-facing: sent verbatim to the model, deliberately not translated.", + "export const heading = (failed: string[]) =>", + ' `### ${failed.length} CI Check${failed.length !== 1 ? "s" : ""} Failed`;', + ].join("\n"), + }); + + const pluralSection = result.stdout.slice(0, result.stdout.indexOf("to review by eye")); + expect(result.plurals).toHaveLength(1); + expect(pluralSection).toContain("agent-facing"); + expect(pluralSection).toContain("pr-checks-section.tsx"); + }); + + it("keeps the plural section bare when it has no findings", () => { + const result = sweep({ "clean.ts": "export const n = 1;" }); + + const pluralSection = result.stdout.slice(0, result.stdout.indexOf("to review by eye")); + expect(pluralSection).not.toContain("agent-facing"); + }); + + it("exits non-zero when given no directory", () => { + const result = spawnSync(process.execPath, [SCRIPT], { encoding: "utf8" }); + + expect(result.status).toBe(2); + expect(result.stderr).toContain("Usage:"); + }); +}); diff --git a/docs/i18n.md b/docs/i18n.md index 14a47b0414..4e20ae2f96 100644 --- a/docs/i18n.md +++ b/docs/i18n.md @@ -381,6 +381,38 @@ completeness check: anything still rendering plain English under it was missed. `EXCLUDED` in that script lists paths whose strings are deliberately left alone (`lib/api`, `lib/types`, layout serialization, fixtures) and why. +### Sweeping for what the guards cannot see + +`pnpm run i18n:sweep ` (`scripts/i18n-sweep.mjs`) is a **human** tool for the +blind spots above — deliberately not wired into CI or pre-commit, and it always +exits 0. It prints two separate sections, because mixing "this is broken" with +"look at this" gets a report read as neither: + +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 + 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 + `components/task`**, the largest directory still to migrate, so run the sweep + on every `components/task` batch specifically rather than treating it as a + generic step. +2. **Prose literals in non-JSX positions** — for eye review, never automatically + a defect. Two caveats. **Prompt builders are deliberately English**: their + 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 + 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. + +Because lint is jsx-only, finding counts from `pnpm run lint:i18n` **understate +the real work by roughly 15%** — measured at ~14 extra strings on a 99-string +batch. Budget for that when sizing a directory, and keep the pseudo-locale as +the completeness check. + ## How it's wired - **Runtime**: `apps/web/lib/i18n/` — the i18next instance, `activateLocale()`,