From 16fb27d1bc4c6986e1ca2440b08e538e7e260132 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 06:52:43 +0000 Subject: [PATCH 1/3] fix(cli): report i18n extract key counts off the emitted bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `extractTranslations` returned `counts[locale]` as a walk counter and the command spent it as the size of the file it had just written. Under the default `--objects-only` those are different numbers: on a one-object, one-app stack the run announced `Wrote …objects.generated.ts (776 keys)` for a file holding 2 leaves, and summarised it as `776 key(s) (of 776 expected) + 773 metadataForms key(s)` — appending a number the 776 already contained. `counts` is now a leaf count of the locale's skeleton, taken off the tree rather than off the walk, and documented as not being any file's size. Every count the command reports is `countTranslationLeaves` of that module's own payload, selected with the new `translationModulePayload` — the same function the renderer renders from, so a count and its bytes cannot drift apart, including for a sub-tree mode added later. The summary is a partition of the skeleton (`E of S key(s) emitted` plus a per-module breakdown), never a sum over it, and nothing subtracts one count from another at a print site. Two consequences of the same conflation go with it: the emit gate is now the module's own leaf count, so a stack with no objects no longer writes an empty module under `--objects-only`; and `--json`'s `counts` now counts the `bundles` payload beside it, as `metadataFormsCounts` already counted `metadataForms`. The pin spawns the real CLI in four flag states and compares each printed number against a structural leaf count of the module parsed back off disk — the comparison the defect precluded. Fixes #16121 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- ...tract-key-count-describes-emitted-bytes.md | 34 ++ packages/cli/src/commands/i18n/extract.ts | 163 ++++++---- packages/cli/src/utils/i18n-extract.ts | 87 ++++- .../test/i18n-extract-key-count.e2e.test.ts | 303 ++++++++++++++++++ 4 files changed, 519 insertions(+), 68 deletions(-) create mode 100644 .changeset/i18n-extract-key-count-describes-emitted-bytes.md create mode 100644 packages/cli/test/i18n-extract-key-count.e2e.test.ts diff --git a/.changeset/i18n-extract-key-count-describes-emitted-bytes.md b/.changeset/i18n-extract-key-count-describes-emitted-bytes.md new file mode 100644 index 0000000000..3753ce45df --- /dev/null +++ b/.changeset/i18n-extract-key-count-describes-emitted-bytes.md @@ -0,0 +1,34 @@ +--- +"@objectstack/cli": patch +--- + +`os i18n extract` reports key counts that describe the bytes it emitted, and its summary is a partition of the skeleton rather than a sum over it. + +`extractTranslations` returned `counts[locale]` as a WALK counter — `count += 1` once per expected entry, unconditionally — and the command spent it as the number of keys in the file it had just written. Under the default `--objects-only` the module holds only the `objects` sub-tree, so the two are different numbers. Driven on a one-object, one-app stack with `i18n.defaultLocale: 'zh-CN'`: + +``` + Skeleton summary + zh-CN 776 key(s) (of 776 expected) + 773 metadataForms key(s) + Wrote OUT/zh-CN.objects.generated.ts (776 keys) +``` + +The file that run wrote holds **2** leaves. The true split of the 776 is 2 objects + 1 app + 773 metadata-form baseline, so the summary appended a number the 776 already contained and read as 1549 out of 776 — an operator could not derive the truth from it, and the `(776 keys)` described no file the run produced. Both lines now read off the emitted tree: + +``` + Skeleton summary + zh-CN 775 of 776 key(s) emitted objects 2 · metadataForms 773 + Wrote OUT/zh-CN.objects.generated.ts (2 keys) + Wrote OUT/zh-CN.metadata-forms.generated.ts (773 keys) +``` + +**What each number now means.** `ExtractResult.counts[locale]` is a leaf count of `bundles[locale]` — the whole skeleton built for that locale, taken off the tree instead of off the walk that built it. It is explicitly not the size of any one file: which sections of the skeleton become committed modules is the caller's decision. The command therefore takes every count it reports off that module's own payload, selected with `translationModulePayload` — the same function `renderTranslationModule` renders from, so the number and the bytes cannot drift apart, including for a sub-tree mode added later. Nothing subtracts one count from another at a print site: that would repair today's two modes and leave the third wrong in the same way. + +**The summary line's shape changed** from `N key(s) (of N expected) + M metadataForms key(s)` to `E of S key(s) emitted` with a per-module breakdown when more than one module carries keys. `E` is what this run's modules hold together and `S` is what the locale's skeleton holds, so `E ≤ S` always and the gap is exactly the keys a flag excluded — one app label under the default `--objects-only`, and nothing at all under `--no-objects-only`. + +**A module with no leaves is no longer written.** The emit gate was `counts[locale] > 0`, a property of the skeleton: on a stack whose only surface is apps, the default `--objects-only` wrote a `.objects.generated.ts` holding `{}` and announced it as 774 keys. The gate is now the module's own leaf count. + +**`--json`**: `counts` is now the leaf count of the `bundles` payload printed beside it — the relationship `metadataFormsCounts` already had to `metadataForms` — instead of the extractor's skeleton size. The skeleton total is unchanged and still reported, under its own name, as `totalExpected`. + +**No committed bundle moves.** All nine extract configs in this repository run under the default `--objects-only` on stacks that do author objects, and every emitted module is byte-for-byte unchanged; `pnpm check:i18n` stays green on the committed tree. What changed is stdout, the `--json` counts, and the emission of a module that would have been empty. + +The regression pin spawns the real CLI in four flag states and compares each printed count against a structural leaf count of the module it wrote, parsed back off disk. That comparison is the thing the defect precluded: a walk counter cannot disagree with the walk, so no assertion over `ExtractResult` could have failed while the printed number was wrong by two orders of magnitude. diff --git a/packages/cli/src/commands/i18n/extract.ts b/packages/cli/src/commands/i18n/extract.ts index 5c5e3c50c0..29afc3ea11 100644 --- a/packages/cli/src/commands/i18n/extract.ts +++ b/packages/cli/src/commands/i18n/extract.ts @@ -21,25 +21,16 @@ import { extractTranslations, renderTranslationModule, renderSourceHashModule, - stackAuthoredSubtree, parseSourceHashModule, narrowToCommittedSections, + translationModulePayload, + countTranslationLeaves, type FillStrategy, + type TranslationModuleKind, } from '../../utils/i18n-extract.js'; const FILL_STRATEGIES: FillStrategy[] = ['empty', 'default', 'todo']; -/** Count string-leaf entries under a nested object — used for reporting. */ -function countLeaves(obj: unknown): number { - if (!obj || typeof obj !== 'object') return 0; - let n = 0; - for (const v of Object.values(obj as Record)) { - if (typeof v === 'string') n += 1; - else if (v && typeof v === 'object') n += countLeaves(v); - } - return n; -} - /** * `os i18n extract` — scaffold translation skeletons. * @@ -184,11 +175,34 @@ export default class I18nExtract extends Command { const localesEmitted = Object.keys(result.bundles); const objectsOnly = flags['objects-only']; - // Count metadataForms keys per locale (computed separately so we - // can show users an honest summary even when --objects-only). + /** + * The sub-tree the stack module holds, and the selector that picks it. + * + * ⭐ Every key count this command reports — the summary line, each + * `Wrote … (N keys)` line, the `--json` payload — is a leaf count of one + * of these payloads, taken with the same selector the module is RENDERED + * from. It is never `result.counts`: that is the whole skeleton the + * extractor built for the locale, sections this run does not write + * included, which under the default `--objects-only` is the ~773-key + * metadata-form baseline plus every non-`objects` group the stack + * authors. Reporting it beside a 2-leaf file is #16121. + * + * A third emission mode later (`--apps-only`, say) adds a `kind` and is + * counted correctly without a number here moving, because no number here + * is arithmetic over another one. ⛔ In particular nothing subtracts the + * baseline at a print site: that repairs today's two modes and leaves the + * next one wrong in the same way. + */ + const stackKind: TranslationModuleKind = objectsOnly ? 'objects' : 'stack'; + const stackPayload = (locale: string) => translationModulePayload(result.bundles[locale], stackKind); + + // Counted for every locale, emitted or not, so the operator can still see + // how big the baseline is when their flags suppress it. const metadataFormsCounts: Record = {}; for (const locale of localesEmitted) { - metadataFormsCounts[locale] = countLeaves(result.bundles[locale]?.metadataForms); + metadataFormsCounts[locale] = countTranslationLeaves( + translationModulePayload(result.bundles[locale], 'metadataForms'), + ); } const anyMetadataForms = Object.values(metadataFormsCounts).some((n) => n > 0); // Whether the companion `.metadata-forms.generated.ts` file is @@ -212,6 +226,48 @@ export default class I18nExtract extends Command { const emitsMetadataForms = (locale: string): boolean => flags['metadata-forms'] && (metadataFormsCounts[locale] ?? 0) > 0; + /** One module this run emits for one locale. */ + interface EmittedModule { + /** Written to `.`. */ + suffix: string; + /** Sub-tree selector — picks the payload AND the rendered module's type. */ + kind: TranslationModuleKind; + /** How this module is named in a `--dry-run` heading. */ + label: string; + /** Leaves this module holds. The ONE number reported for it, anywhere. */ + keys: number; + } + + /** + * The modules one locale's run emits, in file order — the single list the + * summary, `--dry-run`, `--check` and the write loop all read, so no two + * of them can disagree about what this run produces. + * + * A module with no leaves is not emitted. The gate used to be + * `result.counts[locale] > 0`, which is a property of the SKELETON: on a + * stack whose only surface is apps, the default `--objects-only` wrote an + * `.objects.generated.ts` holding `{}` and announced it as 774 + * keys. Measured on this repair's fixture at `f5aec38a6af`. + */ + const modulesFor = (locale: string): EmittedModule[] => { + const mods: EmittedModule[] = []; + const stackKeys = countTranslationLeaves(stackPayload(locale)); + if (stackKeys > 0) { + mods.push({ suffix: 'objects.generated.ts', kind: stackKind, label: 'objects', keys: stackKeys }); + } + if (emitsMetadataForms(locale)) { + mods.push({ + suffix: 'metadata-forms.generated.ts', + kind: 'metadataForms', + label: 'metadataForms', + keys: metadataFormsCounts[locale] ?? 0, + }); + } + return mods; + }; + const modules: Record = {}; + for (const locale of localesEmitted) modules[locale] = modulesFor(locale); + /** * The provenance table for one locale, narrowed to the sections this run * actually COMMITS (#12559). @@ -241,9 +297,8 @@ export default class I18nExtract extends Command { * coupling ADR-0029 D8 and each package's `bundle-ownership.test.ts` exist * to keep out of its committed bundles. * - * So the section list is decided by the SAME predicates that decide the - * bundle files, never by a second rule: `result.counts` for `objects` and - * {@link emitsMetadataForms} for `metadataForms`. A set that commits both — + * So the section list is decided by the SAME list that decides the + * bundle files — {@link modules} — never by a second rule. A set that commits both — * `platform-objects` is the one today — keeps every record it had. The * narrowing itself is `narrowToCommittedSections`, a pure function in the * extractor's utils so it can be pinned without driving oclif; this layer @@ -253,25 +308,29 @@ export default class I18nExtract extends Command { const table = result.sourceHashes[locale]; if (!table) return undefined; const committed: string[] = []; - if ((result.counts[locale] ?? 0) > 0) committed.push('objects'); - if (emitsMetadataForms(locale)) committed.push('metadataForms'); + if (modules[locale].some((m) => m.kind !== 'metadataForms')) committed.push('objects'); + if (modules[locale].some((m) => m.kind === 'metadataForms')) committed.push('metadataForms'); return narrowToCommittedSections(table, committed); }; if (flags.json) { await emitJson({ totalExpected: result.totalExpected, - counts: result.counts, + // Leaves of the `bundles` payload below, locale by locale — the same + // relationship `metadataFormsCounts` has to `metadataForms`, so every + // count in this payload describes the tree printed beside it. + // + // It used to forward `result.counts`, the extractor's per-locale + // SKELETON size, while `bundles` carried only the sub-tree this run + // emits: on a one-object stack under the default `--objects-only` + // that was 776 against a 2-leaf `bundles` payload (#16121). The + // skeleton total is still here — it is `totalExpected`. + counts: Object.fromEntries(localesEmitted.map((l) => [l, countTranslationLeaves(stackPayload(l))])), metadataFormsCounts, // `--json` is documented as "output JSON instead of writing files", // so this payload mirrors the FILE SET: `bundles` is the stack // module, `metadataForms` below is the companion (#14894). - bundles: Object.fromEntries( - localesEmitted.map((l) => [ - l, - objectsOnly ? (result.bundles[l].objects ?? {}) : stackAuthoredSubtree(result.bundles[l]), - ]), - ), + bundles: Object.fromEntries(localesEmitted.map((l) => [l, stackPayload(l)])), // The baseline's JSON home, gated by {@link emitsMetadataForms} — // the SAME predicate that decides the companion file, deliberately // not a second one. @@ -308,13 +367,21 @@ export default class I18nExtract extends Command { console.log(chalk.bold(' Skeleton summary')); const nameWidth = Math.max(8, ...localesEmitted.map((l) => l.length)); for (const locale of localesEmitted) { - const n = result.counts[locale]; - const tone = n === 0 ? chalk.green : chalk.yellow; - const mfN = metadataFormsCounts[locale] ?? 0; - const mfTail = mfN > 0 ? chalk.dim(` + ${mfN} metadataForms key(s)`) : ''; + const mods = modules[locale]; + // The modules are disjoint sub-trees of the skeleton, so this line is a + // partition of it: how many of the locale's keys reach a module, out of + // how many were built, and — when more than one module carries some — + // which module holds which. The old line added the baseline to a number + // that already contained it and read as 1549 of 776 (#16121). + const emittedKeys = mods.reduce((n, m) => n + m.keys, 0); + const skeleton = result.counts[locale] ?? 0; + const tone = emittedKeys === 0 ? chalk.green : chalk.yellow; + const breakdown = mods.length > 1 + ? chalk.dim(` ${mods.map((m) => `${m.label} ${m.keys}`).join(' · ')}`) + : ''; console.log( - ` ${locale.padEnd(nameWidth)} ${tone(String(n).padStart(5))} key(s)` + - chalk.dim(` (of ${result.totalExpected} expected)`) + mfTail, + ` ${locale.padEnd(nameWidth)} ${tone(String(emittedKeys).padStart(5))}` + + chalk.dim(` of ${skeleton} key(s) emitted`) + breakdown, ); } console.log(''); @@ -325,18 +392,9 @@ export default class I18nExtract extends Command { if (flags['dry-run'] || !flags.out) { for (const locale of localesEmitted) { - if (result.counts[locale] === 0 && metadataFormsCounts[locale] === 0) continue; - console.log(chalk.dim(`── ${locale} (objects) ──`)); - console.log(renderTranslationModule(result.bundles[locale], { - locale, - objectsOnly, - })); - if (emitsMetadataForms(locale)) { - console.log(chalk.dim(`── ${locale} (metadataForms) ──`)); - console.log(renderTranslationModule(result.bundles[locale], { - locale, - kind: 'metadataForms', - })); + for (const mod of modules[locale]) { + console.log(chalk.dim(`── ${locale} (${mod.label}) ──`)); + console.log(renderTranslationModule(result.bundles[locale], { locale, kind: mod.kind })); } } printInfo('Dry run — no files written (pass --out= to write).'); @@ -351,18 +409,11 @@ export default class I18nExtract extends Command { // what a real extract writes. const emitted: Array<{ file: string; content: string; keys: number }> = []; for (const locale of localesEmitted) { - if (result.counts[locale] > 0) { - emitted.push({ - file: path.join(resolvedOutDir, `${locale}.objects.generated.ts`), - content: renderTranslationModule(result.bundles[locale], { locale, objectsOnly }), - keys: result.counts[locale], - }); - } - if (emitsMetadataForms(locale)) { + for (const mod of modules[locale]) { emitted.push({ - file: path.join(resolvedOutDir, `${locale}.metadata-forms.generated.ts`), - content: renderTranslationModule(result.bundles[locale], { locale, kind: 'metadataForms' }), - keys: metadataFormsCounts[locale], + file: path.join(resolvedOutDir, `${locale}.${mod.suffix}`), + content: renderTranslationModule(result.bundles[locale], { locale, kind: mod.kind }), + keys: mod.keys, }); } // The provenance companion rides in the SAME list, so `--check` compares diff --git a/packages/cli/src/utils/i18n-extract.ts b/packages/cli/src/utils/i18n-extract.ts index 1a44765627..2bb6acd745 100644 --- a/packages/cli/src/utils/i18n-extract.ts +++ b/packages/cli/src/utils/i18n-extract.ts @@ -257,7 +257,26 @@ export interface ExtractOptions extends ExpectedEntryOptions { export interface ExtractResult { /** Locale → TranslationData skeleton (only the entries we emitted). */ bundles: Record; - /** Locale → number of keys emitted. */ + /** + * Locale → the number of leaves in `bundles[locale]`: the whole skeleton + * built for that locale, every section of it. + * + * ⛔ NOT the number of keys any one emitted FILE holds. Which sections of the + * skeleton become committed modules is the caller's decision — `os i18n + * extract` makes it from `--objects-only` and `--metadata-forms` — so under + * the default `--objects-only` this number is far larger than the module the + * command writes: the registry-driven `metadataForms` baseline alone is ~773 + * keys against a one-object stack's 2 (#16121). A caller reporting a count + * for a file takes it off that file's own payload + * ({@link translationModulePayload}), never off this field. + * + * Counted off the built tree, never off the walk that built it. The walk + * counter this replaces could not disagree with anything: it incremented once + * per entry, unconditionally, so it equalled {@link ExtractResult.totalExpected} + * for every locale on every config and no observation could have shown it + * wrong. A leaf count of the bundle CAN fail — it moves the moment `setDeep` + * collapses two entries onto one path. + */ counts: Record; /** Total expected entries before per-locale merge filtering. */ totalExpected: number; @@ -1402,7 +1421,7 @@ export function collectExpectedEntries( * missing key twice, so translating one string moved * `pnpm check:i18n-coverage`'s ratchet by two while its report called the * number "untranslated declared strings". `extractTranslations` counted them - * twice too, in `totalExpected` and in the per-locale `counts` it prints — + * twice too, in `totalExpected` and in the per-locale `counts` it reports — * over-reporting by 101 keys on app-showcase against the 1531 leaves it * actually wrote, because `setDeep` had already collapsed them on the way into * the bundle. A de-duplication at the reporting seam would have fixed the first @@ -1780,7 +1799,6 @@ export function extractTranslations(config: any, opts: ExtractOptions = {}): Ext for (const locale of locales) { const data: TranslationData = {}; - let count = 0; for (const entry of entries) { // Guaranteed by the seed-less filter above; narrows for the branches below. const seed = entry.sourceValue ?? ''; @@ -1816,10 +1834,11 @@ export function extractTranslations(config: any, opts: ExtractOptions = {}): Ext } } setDeep(data, entry.path, value); - count += 1; } bundles[locale] = data; - counts[locale] = count; + // Off the tree that was just built, never off the walk that built it — see + // {@link ExtractResult.counts} (#16121). + counts[locale] = countTranslationLeaves(data); } const sourceHashes: Record> = {}; @@ -1854,6 +1873,55 @@ export function stackAuthoredSubtree(data: TranslationData): Omit + | NonNullable + | Omit { + return kind === 'metadataForms' + ? (data.metadataForms ?? {}) + : kind === 'objects' + ? (data.objects ?? {}) + : stackAuthoredSubtree(data); +} + +/** + * String leaves under a nested translation tree. + * + * The structural measure every key count in this mechanism is taken with — the + * extractor's per-locale {@link ExtractResult.counts}, and each number + * `os i18n extract` prints beside a module. One implementation so that two + * counts of the same tree can never be two different numbers. + */ +export function countTranslationLeaves(node: unknown): number { + if (!node || typeof node !== 'object') return 0; + let n = 0; + for (const value of Object.values(node as Record)) { + if (typeof value === 'string') n += 1; + else if (value && typeof value === 'object') n += countTranslationLeaves(value); + } + return n; +} + /** * Render a TranslationData skeleton as a TypeScript module body. * @@ -1916,7 +1984,7 @@ export function renderTranslationModule( header?: string[]; }, ): string { - const kind: 'objects' | 'metadataForms' | 'stack' = + const kind: TranslationModuleKind = options.kind ?? (options.objectsOnly === false ? 'stack' : 'objects'); const defaultExport = kind === 'metadataForms' @@ -1925,12 +1993,7 @@ export function renderTranslationModule( ? `${camelize(options.locale)}Translations` : `${camelize(options.locale)}Objects`; const exportName = options.exportName ?? defaultExport; - const payload = - kind === 'metadataForms' - ? (data.metadataForms ?? {}) - : kind === 'objects' - ? (data.objects ?? {}) - : stackAuthoredSubtree(data); + const payload = translationModulePayload(data, kind); const typeSig = kind === 'metadataForms' ? "NonNullable" diff --git a/packages/cli/test/i18n-extract-key-count.e2e.test.ts b/packages/cli/test/i18n-extract-key-count.e2e.test.ts new file mode 100644 index 0000000000..72e5aabd94 --- /dev/null +++ b/packages/cli/test/i18n-extract-key-count.e2e.test.ts @@ -0,0 +1,303 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Every key count `os i18n extract` prints is a leaf count of the bytes that + * run actually emitted (#16121). + * + * ## What was wrong + * + * `ExtractResult.counts[locale]` was a WALK counter — `count += 1` once per + * expected entry, unconditionally — and the command used it as the number of + * keys in the file it had just written. Under the default `--objects-only` the + * module holds only the `objects` sub-tree, so on the fixture below the command + * announced: + * + * Skeleton summary + * zh-CN 776 key(s) (of 776 expected) + 773 metadataForms key(s) + * Wrote OUT/zh-CN.objects.generated.ts (776 keys) + * + * against a file holding **2** leaves. The summary's `+ 773` was the same 773 + * already inside the 776, so the line read as 1549 out of 776, and an operator + * could not derive the true split (2 objects + 1 app + 773 baseline) from it. + * Driven on `f5aec38a6af`, the commit this branch forked from — i.e. AFTER + * #14894 / #16120 moved which keys are emitted. + * + * ## Why this shape + * + * The defect's whole nature is that the printed number was never compared with + * the file: a walk counter cannot disagree with the walk, so no unit assertion + * over `ExtractResult` could have failed. Every case here therefore takes its + * second number off the BYTES — the emitted module parsed back — and drives the + * real CLI to get them. A pin that re-implemented the emit rule would inherit + * exactly the blind spot being closed. + * + * Each case names the observation that would have made it come out the other + * way; that is the property the case buys. + * + * ## Fixture placement + * + * The stack config goes under this package's git-ignored `tmp/` and the `--out` + * root in the system temp dir. Only one of the two has to RESOLVE anything: the + * config calls `defineStack`, and `bundle-require` writes its bundled module + * next to the config, so Node resolves the bare `@objectstack/spec` specifier + * from THAT directory — under `packages/cli/tmp/` the lookup walks up into this + * package's real `node_modules`, from the system temp dir it does not. It is + * `tmp/` specifically, not a directory named for this test: `dispatch-gates + * --self-test` requires a tracked ignore rule, and `git check-ignore -v + * packages/cli/tmp/x` answers `.gitignore:55:tmp/`. `afterAll` removes only + * this suite's own `mkdtemp` directories — five suites share that root and run + * concurrently. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFileSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync, readdirSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { basename, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); +const CLI_PACKAGE_ROOT = resolve(HERE, '..'); + +/** One object (2 leaves) + one app (1 leaf); the registry baseline rides along. */ +const STACK_CONFIG = [ + "import { defineStack } from '@objectstack/spec';", + '', + 'export default defineStack({', + " i18n: { defaultLocale: 'zh-CN', supportedLocales: ['zh-CN'] },", + " objects: [{ name: 'kpi_metric', label: 'Metric', fields: { name: { type: 'text', label: 'Name' } } }],", + " apps: [{ name: 'kpi', label: 'KPI Console' }],", + '});', + '', +].join('\n'); + +/** No `objects` at all — the stack module's sub-tree is empty under the default. */ +const APPS_ONLY_CONFIG = [ + "import { defineStack } from '@objectstack/spec';", + '', + 'export default defineStack({', + " i18n: { defaultLocale: 'zh-CN', supportedLocales: ['zh-CN'] },", + " apps: [{ name: 'kpi', label: 'KPI Console' }],", + '});', + '', +].join('\n'); + +let fixtureRoot: string; +let outRoot: string; +let CONFIG: string; +let APPS_ONLY: string; + +beforeAll(() => { + const sharedRoot = join(CLI_PACKAGE_ROOT, 'tmp'); + mkdirSync(sharedRoot, { recursive: true }); + fixtureRoot = mkdtempSync(join(sharedRoot, 'os-i18n-16121-fixture-')); + CONFIG = join(fixtureRoot, 'stack.config.ts'); + APPS_ONLY = join(fixtureRoot, 'apps-only.config.ts'); + writeFileSync(CONFIG, STACK_CONFIG, 'utf8'); + writeFileSync(APPS_ONLY, APPS_ONLY_CONFIG, 'utf8'); + outRoot = mkdtempSync(join(tmpdir(), 'os-i18n-16121-')); +}); + +afterAll(() => { + // This suite's own directories only. Never the shared `tmp/` root. + rmSync(fixtureRoot, { recursive: true, force: true }); + rmSync(outRoot, { recursive: true, force: true }); +}); + +/** stdout with SGR sequences removed — chalk is off through a pipe, belt and braces. */ +function plain(text: string): string { + // The escape byte is SPELLED, never embedded: a raw control byte in a source + // file renders as nothing and is findable by neither spelling. + return text.replace(/\u001b\[[0-9;]*m/g, ''); +} + +function runExtract(name: string, config: string, flags: string[]): { stdout: string; dir: string; files: string[] } { + const dir = join(outRoot, name); + const stdout = execFileSync(TSX, [CLI, 'i18n', 'extract', config, '--locales=zh-CN', ...flags, `--out=${dir}`], { + encoding: 'utf8', + env: childEnv(), + timeout: 180_000, + }); + return { stdout: plain(stdout), dir, files: readdirSync(dir).sort() }; +} + +/** + * Leaves in an emitted module, counted off its BYTES. + * + * `stringifyTs` writes one key per line, so a leaf is a line whose own start is + * a key followed by a string literal. Anchoring at the line start is what makes + * this a measurement rather than a substring search: a `: "` occurring INSIDE a + * translated value cannot begin a line. + */ +function leavesOnDisk(dir: string, file: string): number { + const src = readFileSync(join(dir, file), 'utf8'); + return [...src.matchAll(/^[ \t]+(?:"[^"]*"|[A-Za-z_$][\w$]*): "/gm)].length; +} + +/** The `Wrote (N keys)` lines, as basename to the count the command printed. */ +function printedFileCounts(stdout: string): Record { + const out: Record = {}; + for (const m of stdout.matchAll(/Wrote\s+(\S+)\s+\((\d+) keys\)/g)) out[basename(m[1])] = Number(m[2]); + return out; +} + +/** The one summary row: locale, emitted, skeleton, and the per-module breakdown. */ +function summaryRow(stdout: string): { locale: string; emitted: number; skeleton: number; breakdown: string } { + const m = stdout.match(/^ {4}(\S+) +(\d+) of (\d+) key\(s\) emitted(.*)$/m); + if (!m) throw new Error(`no summary row in:\n${stdout}`); + return { locale: m[1], emitted: Number(m[2]), skeleton: Number(m[3]), breakdown: m[4].trim() }; +} + +function countJsonLeaves(value: unknown): number { + if (!value || typeof value !== 'object') return 0; + let n = 0; + for (const v of Object.values(value as Record)) { + if (typeof v === 'string') n += 1; + else if (v && typeof v === 'object') n += countJsonLeaves(v); + } + return n; +} + +describe('os i18n extract — the number printed is the number written (#16121)', () => { + /** + * The four flag states are the point: a count taken off the emitted sub-tree + * is right in all of them, and one taken off the walk is right in none. + * Falsifier — restoring `keys: result.counts[locale]` makes the two + * `--objects-only` rows print 776 against 2 leaves on disk, and the two + * `--no-objects-only` rows 776 against 3. + */ + it.each([ + ['default', [] as string[]], + ['no-metadata-forms', ['--no-metadata-forms']], + ['no-objects-only', ['--no-objects-only']], + ['neither', ['--no-objects-only', '--no-metadata-forms']], + ])('every printed file count is that file own leaf count (%s)', (name, flags) => { + const run = runExtract(`counts-${name}`, CONFIG, flags); + const printed = printedFileCounts(run.stdout); + + // Every file written is announced, and nothing is announced that was not + // written — two faces of one list. + expect(Object.keys(printed).sort()).toEqual(run.files); + expect(run.files.length).toBeGreaterThan(0); + + for (const file of run.files) { + expect({ file, keys: printed[file] }).toEqual({ file, keys: leavesOnDisk(run.dir, file) }); + } + }); + + /** + * The summary is a PARTITION of the skeleton, never a sum over it. The old + * line appended the baseline to a number that already contained it and read + * `776 key(s) (of 776 expected) + 773 metadataForms key(s)` — 1549 claimed + * against 776 built. Falsifier: any re-appearance of that shape makes + * `emitted` exceed `skeleton` here. + */ + it('summarises the emitted keys as a partition of the skeleton, in both sub-tree modes', () => { + for (const [name, flags] of [ + ['objects-only', [] as string[]], + ['stack', ['--no-objects-only']], + ] as const) { + const run = runExtract(`partition-${name}`, CONFIG, flags); + const row = summaryRow(run.stdout); + const onDisk = run.files.reduce((n, f) => n + leavesOnDisk(run.dir, f), 0); + + expect(row.locale).toBe('zh-CN'); + expect(run.files.length).toBe(2); + // What the summary claims was emitted is what the files hold, together. + expect(row.emitted).toBe(onDisk); + // ...and it is a selection OF the skeleton, so it can never be more. + expect(row.emitted).toBeLessThanOrEqual(row.skeleton); + // Both modules carry keys here, so the breakdown names them and its parts + // add up to the whole — the reading the `+ N metadataForms` tail denied. + const parts = [...row.breakdown.matchAll(/([A-Za-z]+) (\d+)/g)].map((m) => Number(m[2])); + expect(parts.length).toBe(2); + expect(parts.reduce((a, b) => a + b, 0)).toBe(row.emitted); + } + + // Under `--no-objects-only` the two modules together ARE the skeleton, so + // the partition closes exactly: nothing dropped, nothing double-counted. + const wholeRow = summaryRow(runExtract('partition-whole', CONFIG, ['--no-objects-only']).stdout); + expect(wholeRow.emitted).toBe(wholeRow.skeleton); + // ...and under the default one app label sits outside the `objects` + // sub-tree, so exactly one key is built and not emitted. A summary that + // could not tell those two runs apart is the defect. + const narrowed = summaryRow(runExtract('partition-narrow', CONFIG, []).stdout); + expect(narrowed.skeleton).toBe(wholeRow.skeleton); + expect(wholeRow.emitted - narrowed.emitted).toBe(1); + }); + + /** + * The `--json` face carries the same pair. `counts` used to forward the + * extractor's skeleton size while `bundles` beside it held the emitted + * sub-tree — 776 against a 2-leaf payload. Falsifier: that arrangement fails + * the first expectation under `--objects-only`, which is why the narrowed + * state is driven and not only the wide one. + */ + it('--json counts describe the payload printed beside them, in both sub-tree modes', () => { + const runJson = (flags: string[]) => { + const stdout = execFileSync(TSX, [CLI, 'i18n', 'extract', CONFIG, '--locales=zh-CN', '--json', ...flags], { + encoding: 'utf8', + env: childEnv(), + timeout: 180_000, + }); + return JSON.parse(stdout) as { + totalExpected: number; + counts: Record; + metadataFormsCounts: Record; + bundles: Record; + metadataForms: Record; + }; + }; + + const narrow = runJson([]); + const whole = runJson(['--no-objects-only']); + + for (const payload of [narrow, whole]) { + expect(payload.counts['zh-CN']).toBe(countJsonLeaves(payload.bundles['zh-CN'])); + expect(payload.metadataFormsCounts['zh-CN']).toBe(countJsonLeaves(payload.metadataForms['zh-CN'])); + // The skeleton total is still reported — under its own name. + expect(payload.totalExpected).toBeGreaterThan(payload.counts['zh-CN']); + } + + // The sub-tree flag MOVES the count, which is what makes the assertion + // above an axis rather than a coincidence: the app label is in one payload + // and not the other. + expect(whole.counts['zh-CN'] - narrow.counts['zh-CN']).toBe(1); + }); + + /** + * A module with no leaves is not a file. The emit gate was + * `result.counts[locale] > 0`, a property of the skeleton rather than of the + * module: on a stack whose only surface is apps, the default `--objects-only` + * wrote a `zh-CN.objects.generated.ts` holding `{}` and announced it as `774 + * keys` (measured at `f5aec38a6af`). Falsifier: restoring that gate writes + * one file here, and `printedFileCounts` reports 774 for a file with 0 + * leaves. + */ + it('writes no module for an empty sub-tree, and announces none', () => { + const run = runExtract('empty-subtree', APPS_ONLY, ['--no-metadata-forms']); + + expect(run.files).toEqual([]); + expect(printedFileCounts(run.stdout)).toEqual({}); + const row = summaryRow(run.stdout); + expect(row.emitted).toBe(0); + // The keys ARE there — they are simply outside the sub-tree the flags + // selected, and the line says so instead of claiming them as written. + expect(row.skeleton).toBeGreaterThan(100); + + // The same stack under `--no-objects-only` does have a module, so the empty + // result above is the sub-tree selection and not an inert fixture. + const wide = runExtract('empty-subtree-wide', APPS_ONLY, ['--no-objects-only', '--no-metadata-forms']); + expect(wide.files).toEqual(['zh-CN.objects.generated.ts']); + expect(printedFileCounts(wide.stdout)['zh-CN.objects.generated.ts']).toBe( + leavesOnDisk(wide.dir, 'zh-CN.objects.generated.ts'), + ); + expect(summaryRow(wide.stdout).emitted).toBe(1); + }); + // Each case spawns the CLI through `tsx`; measured at ~6 s per run on a + // shared box, well over vitest's 5 s default. Same instrument and the same + // generous ceiling as the sibling CLI-spawning pins in this directory. +}, 900_000); From e528ee122a53d9c270a2c9a50f3fc959f64e59ac Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 07:35:47 +0000 Subject: [PATCH 2/3] test(cli): fix the grammar of the key-count pin's case title Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- packages/cli/test/i18n-extract-key-count.e2e.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/test/i18n-extract-key-count.e2e.test.ts b/packages/cli/test/i18n-extract-key-count.e2e.test.ts index 72e5aabd94..9ebe0d6f74 100644 --- a/packages/cli/test/i18n-extract-key-count.e2e.test.ts +++ b/packages/cli/test/i18n-extract-key-count.e2e.test.ts @@ -174,7 +174,7 @@ describe('os i18n extract — the number printed is the number written (#16121)' ['no-metadata-forms', ['--no-metadata-forms']], ['no-objects-only', ['--no-objects-only']], ['neither', ['--no-objects-only', '--no-metadata-forms']], - ])('every printed file count is that file own leaf count (%s)', (name, flags) => { + ])("every printed file count is that file's own leaf count (%s)", (name, flags) => { const run = runExtract(`counts-${name}`, CONFIG, flags); const printed = printedFileCounts(run.stdout); From b7afc733b30de3c6a5f149d6d6293239462503f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 08:20:10 +0000 Subject: [PATCH 3/3] fix(cli): unbreak the pin's typecheck, drop a false symmetry claim, report suppressed modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four repairs from contract review, none of them a rethink of the count design. 1. `runExtract`/`runJson` took `flags: string[]` while the partition table is `as const`, so the call site handed them a readonly tuple: TS2345 at test/i18n-extract-key-count.e2e.test.ts(203,59). The parameters only ever read, so they are `readonly string[]` now — the table keeps its literal types. Present since the first commit, and green under every gate that was re-run at that head because each either skips the test layer or is type-blind. 2. The changeset, the PR body and the `--json` comment all claimed the new `counts`/`bundles` relationship was "the relationship `metadataFormsCounts` already had to `metadataForms`". It is not: `metadataFormsCounts` reports the baseline as BUILT whether or not it is emitted, so under `--no-metadata-forms` the payload carries a positive count beside an empty `metadataForms` map. The claim is corrected in all three places and nothing about either face moves; whether `--json` SHOULD carry two count semantics is left to the maintainer. The `--json` case now drives `--metadata-forms` in both states, which is what would have caught the claim. 3. The summary dropped an operator reading on the commonest path: `--no-metadata-forms` is what 8 of this repo's 9 extract configs pass, and there the row named nothing at all. A module a flag suppressed is now a CANDIDATE that is reported but not written — named with its size and the words `not emitted`, so it stays out of the total: zh-CN 2 of 776 key(s) emitted objects 2 · metadataForms 773 not emitted That is mode-agnostic: a later sub-tree mode is a candidate like any other. The tone also read green on `0 of 774 emitted`; green means there is nothing to translate, which is a property of the skeleton, so it now reads that. 4. The emitted-files mirror judged its `--no-objects-only` arm on the whole bundle while the command judges it on the stack-authored subtree. They diverge on a bundle with no authored surface; the mirror subtracts the baseline too, and a case drives that input class. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- ...tract-key-count-describes-emitted-bytes.md | 8 +- packages/cli/src/commands/i18n/extract.ts | 106 ++++++++++++------ .../test/i18n-extract-emitted-files.test.ts | 34 +++++- .../test/i18n-extract-key-count.e2e.test.ts | 46 +++++++- 4 files changed, 151 insertions(+), 43 deletions(-) diff --git a/.changeset/i18n-extract-key-count-describes-emitted-bytes.md b/.changeset/i18n-extract-key-count-describes-emitted-bytes.md index 3753ce45df..8f3d3cb6a8 100644 --- a/.changeset/i18n-extract-key-count-describes-emitted-bytes.md +++ b/.changeset/i18n-extract-key-count-describes-emitted-bytes.md @@ -23,12 +23,14 @@ The file that run wrote holds **2** leaves. The true split of the 776 is 2 objec **What each number now means.** `ExtractResult.counts[locale]` is a leaf count of `bundles[locale]` — the whole skeleton built for that locale, taken off the tree instead of off the walk that built it. It is explicitly not the size of any one file: which sections of the skeleton become committed modules is the caller's decision. The command therefore takes every count it reports off that module's own payload, selected with `translationModulePayload` — the same function `renderTranslationModule` renders from, so the number and the bytes cannot drift apart, including for a sub-tree mode added later. Nothing subtracts one count from another at a print site: that would repair today's two modes and leave the third wrong in the same way. -**The summary line's shape changed** from `N key(s) (of N expected) + M metadataForms key(s)` to `E of S key(s) emitted` with a per-module breakdown when more than one module carries keys. `E` is what this run's modules hold together and `S` is what the locale's skeleton holds, so `E ≤ S` always and the gap is exactly the keys a flag excluded — one app label under the default `--objects-only`, and nothing at all under `--no-objects-only`. +**The summary line's shape changed** from `N key(s) (of N expected) + M metadataForms key(s)` to `E of S key(s) emitted` with a per-module breakdown. `E` is what this run's modules hold together and `S` is what the locale's skeleton holds, so `E ≤ S` always and the gap is exactly the keys a flag excluded — one app label under the default `--objects-only`, and nothing at all under `--no-objects-only`. A module a flag SUPPRESSED is named in the breakdown too, with its size and the words `not emitted` that keep it out of `E`: under `--no-metadata-forms` the row reads `2 of 776 key(s) emitted objects 2 · metadataForms 773 not emitted`, so the operator still sees how big the baseline they switched off is — which the old, double-counting line did tell them. **A module with no leaves is no longer written.** The emit gate was `counts[locale] > 0`, a property of the skeleton: on a stack whose only surface is apps, the default `--objects-only` wrote a `.objects.generated.ts` holding `{}` and announced it as 774 keys. The gate is now the module's own leaf count. -**`--json`**: `counts` is now the leaf count of the `bundles` payload printed beside it — the relationship `metadataFormsCounts` already had to `metadataForms` — instead of the extractor's skeleton size. The skeleton total is unchanged and still reported, under its own name, as `totalExpected`. +**`--json`**: `counts` is now the leaf count of the `bundles` payload printed beside it, instead of the extractor's skeleton size. The skeleton total is unchanged and still reported, under its own name, as `totalExpected`. + +⚠️ That is **not** the relationship `metadataFormsCounts` has to `metadataForms`, and nothing here changes the latter. `metadataFormsCounts` reports the baseline as BUILT, emitted or not: under `--no-metadata-forms` the payload carries `metadataFormsCounts: { 'zh-CN': 773 }` beside an empty `metadataForms`, deliberately, and a pin holds it there. So the payload carries two count semantics — `counts` is what was emitted, `metadataFormsCounts` is what was built. Both faces are unchanged by this note; it exists because an earlier draft of it claimed a symmetry that does not hold. **No committed bundle moves.** All nine extract configs in this repository run under the default `--objects-only` on stacks that do author objects, and every emitted module is byte-for-byte unchanged; `pnpm check:i18n` stays green on the committed tree. What changed is stdout, the `--json` counts, and the emission of a module that would have been empty. -The regression pin spawns the real CLI in four flag states and compares each printed count against a structural leaf count of the module it wrote, parsed back off disk. That comparison is the thing the defect precluded: a walk counter cannot disagree with the walk, so no assertion over `ExtractResult` could have failed while the printed number was wrong by two orders of magnitude. +The regression pin spawns the real CLI in four flag states and compares each printed count against a structural leaf count of the module it wrote, parsed back off disk. That comparison is the thing the defect precluded: a walk counter cannot disagree with the walk, so no assertion over `ExtractResult` could have failed while the printed number was wrong by two orders of magnitude. Its `--json` case drives `--metadata-forms` in both states, because a case that drives one state of a flag cannot see what that flag does — driving it ON only is exactly how the symmetry claim above survived unmeasured into a first draft. diff --git a/packages/cli/src/commands/i18n/extract.ts b/packages/cli/src/commands/i18n/extract.ts index 29afc3ea11..db15be8983 100644 --- a/packages/cli/src/commands/i18n/extract.ts +++ b/packages/cli/src/commands/i18n/extract.ts @@ -226,47 +226,64 @@ export default class I18nExtract extends Command { const emitsMetadataForms = (locale: string): boolean => flags['metadata-forms'] && (metadataFormsCounts[locale] ?? 0) > 0; - /** One module this run emits for one locale. */ - interface EmittedModule { - /** Written to `.`. */ + /** One module this run's flags CONSIDER for one locale. */ + interface CandidateModule { + /** Written to `.` when {@link CandidateModule.emitted}. */ suffix: string; /** Sub-tree selector — picks the payload AND the rendered module's type. */ kind: TranslationModuleKind; - /** How this module is named in a `--dry-run` heading. */ + /** How this module is named in a `--dry-run` heading and in the summary. */ label: string; /** Leaves this module holds. The ONE number reported for it, anywhere. */ keys: number; + /** Whether this run writes it. A candidate a flag SUPPRESSED is still + * reported — how big the thing they switched off is, is a reading the + * operator needs, and 8 of this repo's 9 extract configs are on that + * path (`--no-metadata-forms`). */ + emitted: boolean; } /** - * The modules one locale's run emits, in file order — the single list the - * summary, `--dry-run`, `--check` and the write loop all read, so no two - * of them can disagree about what this run produces. + * Every module one locale's run considers, in file order — the single + * list the summary, `--dry-run`, `--check` and the write loop all read, + * so no two of them can disagree about what this run produces. The last + * three take the `emitted` ones; the summary reports all of them and adds + * up only the `emitted` ones. * - * A module with no leaves is not emitted. The gate used to be - * `result.counts[locale] > 0`, which is a property of the SKELETON: on a - * stack whose only surface is apps, the default `--objects-only` wrote an - * `.objects.generated.ts` holding `{}` and announced it as 774 - * keys. Measured on this repair's fixture at `f5aec38a6af`. + * A module with no leaves is not a candidate at all. The write gate used + * to be `result.counts[locale] > 0`, which is a property of the SKELETON: + * on a stack whose only surface is apps, the default `--objects-only` + * wrote an `.objects.generated.ts` holding `{}` and announced it + * as 774 keys. Measured on this repair's fixture at `f5aec38a6af`. */ - const modulesFor = (locale: string): EmittedModule[] => { - const mods: EmittedModule[] = []; + const candidatesFor = (locale: string): CandidateModule[] => { + const mods: CandidateModule[] = []; const stackKeys = countTranslationLeaves(stackPayload(locale)); if (stackKeys > 0) { - mods.push({ suffix: 'objects.generated.ts', kind: stackKind, label: 'objects', keys: stackKeys }); + mods.push({ + suffix: 'objects.generated.ts', + kind: stackKind, + label: 'objects', + keys: stackKeys, + emitted: true, + }); } - if (emitsMetadataForms(locale)) { + if ((metadataFormsCounts[locale] ?? 0) > 0) { mods.push({ suffix: 'metadata-forms.generated.ts', kind: 'metadataForms', label: 'metadataForms', keys: metadataFormsCounts[locale] ?? 0, + emitted: emitsMetadataForms(locale), }); } return mods; }; - const modules: Record = {}; - for (const locale of localesEmitted) modules[locale] = modulesFor(locale); + const candidates: Record = {}; + for (const locale of localesEmitted) candidates[locale] = candidatesFor(locale); + /** The candidates this run actually writes — what every file face iterates. */ + const emittedModules = (locale: string): CandidateModule[] => + candidates[locale].filter((m) => m.emitted); /** * The provenance table for one locale, narrowed to the sections this run @@ -298,7 +315,7 @@ export default class I18nExtract extends Command { * to keep out of its committed bundles. * * So the section list is decided by the SAME list that decides the - * bundle files — {@link modules} — never by a second rule. A set that commits both — + * bundle files — {@link emittedModules} — never by a second rule. A set that commits both — * `platform-objects` is the one today — keeps every record it had. The * narrowing itself is `narrowToCommittedSections`, a pure function in the * extractor's utils so it can be pinned without driving oclif; this layer @@ -308,23 +325,34 @@ export default class I18nExtract extends Command { const table = result.sourceHashes[locale]; if (!table) return undefined; const committed: string[] = []; - if (modules[locale].some((m) => m.kind !== 'metadataForms')) committed.push('objects'); - if (modules[locale].some((m) => m.kind === 'metadataForms')) committed.push('metadataForms'); + if (emittedModules(locale).some((m) => m.kind !== 'metadataForms')) committed.push('objects'); + if (emittedModules(locale).some((m) => m.kind === 'metadataForms')) committed.push('metadataForms'); return narrowToCommittedSections(table, committed); }; if (flags.json) { await emitJson({ totalExpected: result.totalExpected, - // Leaves of the `bundles` payload below, locale by locale — the same - // relationship `metadataFormsCounts` has to `metadataForms`, so every - // count in this payload describes the tree printed beside it. + // Leaves of the `bundles` payload below, locale by locale, so this + // count describes the tree printed beside it. // // It used to forward `result.counts`, the extractor's per-locale // SKELETON size, while `bundles` carried only the sub-tree this run // emits: on a one-object stack under the default `--objects-only` // that was 776 against a 2-leaf `bundles` payload (#16121). The // skeleton total is still here — it is `totalExpected`. + // + // ⚠️ This is NOT the relationship `metadataFormsCounts` has to + // `metadataForms`, and an earlier revision of this comment claimed it + // was. `metadataFormsCounts` reports the baseline's size whether or + // not the baseline is emitted — under `--no-metadata-forms` the + // payload carries `metadataFormsCounts: { 'zh-CN': 773 }` beside + // `metadataForms: {}`, deliberately, and a sibling pin holds it there + // so an operator can still see how big the thing they switched off + // is. So this payload carries TWO count semantics: `counts` is what + // was emitted, `metadataFormsCounts` is what was built. Whether it + // SHOULD is a question for the maintainer; this change neither + // settles it nor moves either face. counts: Object.fromEntries(localesEmitted.map((l) => [l, countTranslationLeaves(stackPayload(l))])), metadataFormsCounts, // `--json` is documented as "output JSON instead of writing files", @@ -367,17 +395,27 @@ export default class I18nExtract extends Command { console.log(chalk.bold(' Skeleton summary')); const nameWidth = Math.max(8, ...localesEmitted.map((l) => l.length)); for (const locale of localesEmitted) { - const mods = modules[locale]; + const mods = candidates[locale]; // The modules are disjoint sub-trees of the skeleton, so this line is a // partition of it: how many of the locale's keys reach a module, out of - // how many were built, and — when more than one module carries some — - // which module holds which. The old line added the baseline to a number - // that already contained it and read as 1549 of 776 (#16121). - const emittedKeys = mods.reduce((n, m) => n + m.keys, 0); + // how many were built, and which module holds which. The old line added + // the baseline to a number that already contained it and read as 1549 + // of 776 (#16121). + // + // A candidate a flag SUPPRESSED is named too, with its size and the + // words that keep it out of the sum. Dropping it was an information + // regression on the commonest path: `--no-metadata-forms` is what 8 of + // this repo's 9 extract configs pass, and the old line at least told + // those runs how big the baseline they switched off was. + const emittedKeys = mods.filter((m) => m.emitted).reduce((n, m) => n + m.keys, 0); const skeleton = result.counts[locale] ?? 0; - const tone = emittedKeys === 0 ? chalk.green : chalk.yellow; - const breakdown = mods.length > 1 - ? chalk.dim(` ${mods.map((m) => `${m.label} ${m.keys}`).join(' · ')}`) + // Green means there is nothing to translate for this locale, which is a + // property of the SKELETON. `0 of 774 emitted` is not that: it is a run + // whose flags excluded everything built, and reading green there is the + // same conflation this card is about. + const tone = skeleton === 0 ? chalk.green : chalk.yellow; + const breakdown = mods.length > 1 || mods.some((m) => !m.emitted) + ? chalk.dim(` ${mods.map((m) => `${m.label} ${m.keys}${m.emitted ? '' : ' not emitted'}`).join(' · ')}`) : ''; console.log( ` ${locale.padEnd(nameWidth)} ${tone(String(emittedKeys).padStart(5))}` + @@ -392,7 +430,7 @@ export default class I18nExtract extends Command { if (flags['dry-run'] || !flags.out) { for (const locale of localesEmitted) { - for (const mod of modules[locale]) { + for (const mod of emittedModules(locale)) { console.log(chalk.dim(`── ${locale} (${mod.label}) ──`)); console.log(renderTranslationModule(result.bundles[locale], { locale, kind: mod.kind })); } @@ -409,7 +447,7 @@ export default class I18nExtract extends Command { // what a real extract writes. const emitted: Array<{ file: string; content: string; keys: number }> = []; for (const locale of localesEmitted) { - for (const mod of modules[locale]) { + for (const mod of emittedModules(locale)) { emitted.push({ file: path.join(resolvedOutDir, `${locale}.${mod.suffix}`), content: renderTranslationModule(result.bundles[locale], { locale, kind: mod.kind }), diff --git a/packages/cli/test/i18n-extract-emitted-files.test.ts b/packages/cli/test/i18n-extract-emitted-files.test.ts index 7314a430fb..29bbe1038e 100644 --- a/packages/cli/test/i18n-extract-emitted-files.test.ts +++ b/packages/cli/test/i18n-extract-emitted-files.test.ts @@ -16,14 +16,27 @@ import { describe, it, expect } from 'vitest'; import { extractTranslations } from '../src/utils/i18n-extract'; -/** Mirror of the emit rule in `src/commands/i18n/extract.ts`. */ +/** + * Mirror of the emit rule in `src/commands/i18n/extract.ts`. + * + * ⚠️ The `--no-objects-only` arm reads the stack module's payload, which is + * everything the stack authors MINUS the registry baseline — the command spells + * it `stackAuthoredSubtree` / `translationModulePayload(data, 'stack')` since + * #14894. This mirror said `data`, baseline included, and the two verdicts + * diverge on exactly one input class: a bundle with no authored surface at all, + * where the baseline alone made this predicate positive and the command writes + * nothing (#16121). The mirror stays a re-implementation rather than an import — + * a mirror that calls the thing it mirrors cannot disagree with it — so the + * subtraction is spelled out here too. + */ function emittedFiles( bundles: Record, flags: { objectsOnly: boolean; metadataForms: boolean }, ): string[] { const files: string[] = []; for (const [locale, data] of Object.entries(bundles)) { - if (countLeaves(flags.objectsOnly ? data.objects : data) > 0) { + const { metadataForms: _registryBaseline, ...authored } = data; + if (countLeaves(flags.objectsOnly ? data.objects : authored) > 0) { files.push(`${locale}.objects.generated.ts`); } if (flags.metadataForms && countLeaves(data.metadataForms) > 0) { @@ -79,4 +92,21 @@ describe('os i18n extract emit set', () => { 'en.metadata-forms.generated.ts', ); }); + + /** + * The input class the fixture above cannot reach: a bundle whose only content + * is the registry baseline. Under `--no-objects-only` the stack module's + * payload is then empty, so no stack module is written — and a mirror that + * counted the WHOLE bundle said one was. Driven on a synthetic bundle because + * the config fixture always authors an object. + */ + it('writes no stack module for a bundle with no authored surface', () => { + const baselineOnly = { en: { metadataForms: { form: { field: { label: 'Name' } } } } }; + + expect(emittedFiles(baselineOnly, { objectsOnly: false, metadataForms: true })).toEqual([ + 'en.metadata-forms.generated.ts', + ]); + expect(emittedFiles(baselineOnly, { objectsOnly: false, metadataForms: false })).toEqual([]); + expect(emittedFiles(baselineOnly, { objectsOnly: true, metadataForms: false })).toEqual([]); + }); }); diff --git a/packages/cli/test/i18n-extract-key-count.e2e.test.ts b/packages/cli/test/i18n-extract-key-count.e2e.test.ts index 9ebe0d6f74..e1114128af 100644 --- a/packages/cli/test/i18n-extract-key-count.e2e.test.ts +++ b/packages/cli/test/i18n-extract-key-count.e2e.test.ts @@ -114,7 +114,7 @@ function plain(text: string): string { return text.replace(/\u001b\[[0-9;]*m/g, ''); } -function runExtract(name: string, config: string, flags: string[]): { stdout: string; dir: string; files: string[] } { +function runExtract(name: string, config: string, flags: readonly string[]): { stdout: string; dir: string; files: string[] } { const dir = join(outRoot, name); const stdout = execFileSync(TSX, [CLI, 'i18n', 'extract', config, '--locales=zh-CN', ...flags, `--out=${dir}`], { encoding: 'utf8', @@ -237,7 +237,7 @@ describe('os i18n extract — the number printed is the number written (#16121)' * state is driven and not only the wide one. */ it('--json counts describe the payload printed beside them, in both sub-tree modes', () => { - const runJson = (flags: string[]) => { + const runJson = (flags: readonly string[]) => { const stdout = execFileSync(TSX, [CLI, 'i18n', 'extract', CONFIG, '--locales=zh-CN', '--json', ...flags], { encoding: 'utf8', env: childEnv(), @@ -254,10 +254,10 @@ describe('os i18n extract — the number printed is the number written (#16121)' const narrow = runJson([]); const whole = runJson(['--no-objects-only']); + const suppressed = runJson(['--no-metadata-forms']); - for (const payload of [narrow, whole]) { + for (const payload of [narrow, whole, suppressed]) { expect(payload.counts['zh-CN']).toBe(countJsonLeaves(payload.bundles['zh-CN'])); - expect(payload.metadataFormsCounts['zh-CN']).toBe(countJsonLeaves(payload.metadataForms['zh-CN'])); // The skeleton total is still reported — under its own name. expect(payload.totalExpected).toBeGreaterThan(payload.counts['zh-CN']); } @@ -266,6 +266,43 @@ describe('os i18n extract — the number printed is the number written (#16121)' // above an axis rather than a coincidence: the app label is in one payload // and not the other. expect(whole.counts['zh-CN'] - narrow.counts['zh-CN']).toBe(1); + + // ⚠️ `metadataFormsCounts` is NOT the same relationship, and driving the + // flag ON only is what let that claim stand unmeasured through a review. + // It reports the baseline as BUILT, whether or not the run emits it: with + // the flag off the payload carries a positive count beside an empty + // `metadataForms` map. So the two counts in this payload mean two different + // things — deliberately, and pinned here so a change to either is seen. + expect(narrow.metadataFormsCounts['zh-CN']).toBe(countJsonLeaves(narrow.metadataForms['zh-CN'])); + expect(suppressed.metadataFormsCounts['zh-CN']).toBeGreaterThan(100); + expect(suppressed.metadataForms['zh-CN']).toBeUndefined(); + expect(countJsonLeaves(suppressed.metadataForms['zh-CN'])).toBe(0); + expect(suppressed.metadataFormsCounts['zh-CN']).toBe(narrow.metadataFormsCounts['zh-CN']); + }); + + /** + * The commonest path in this repository is `--no-metadata-forms` — 8 of the 9 + * extract configs pass it — and there the run emits ONE module. The first cut + * of this repair printed a bare `2 of 776 key(s) emitted` for it: correct, and + * a regression on what the old (double-counting) line at least told the + * operator, which is how big the baseline they switched off is. + * + * Falsifier: dropping the suppressed candidate from the summary leaves the + * breakdown empty here, and adding it into the total instead of labelling it + * makes the first expectation read 775. + */ + it('names a flag-suppressed module and its size, without adding it in', () => { + const run = runExtract('suppressed-baseline', CONFIG, ['--no-metadata-forms']); + + expect(run.files).toEqual(['zh-CN.objects.generated.ts']); + const row = summaryRow(run.stdout); + // The emitted total is the one file's leaves — the baseline is NOT in it. + expect(row.emitted).toBe(leavesOnDisk(run.dir, 'zh-CN.objects.generated.ts')); + // ...and it is named, with its size, and with the words that keep it out. + expect(row.breakdown).toMatch(/metadataForms \d+ not emitted/); + const suppressed = Number(row.breakdown.match(/metadataForms (\d+) not emitted/)![1]); + expect(suppressed).toBeGreaterThan(100); + expect(row.emitted + suppressed).toBeLessThanOrEqual(row.skeleton); }); /** @@ -287,6 +324,7 @@ describe('os i18n extract — the number printed is the number written (#16121)' // The keys ARE there — they are simply outside the sub-tree the flags // selected, and the line says so instead of claiming them as written. expect(row.skeleton).toBeGreaterThan(100); + expect(row.breakdown).toMatch(/metadataForms \d+ not emitted/); // The same stack under `--no-objects-only` does have a module, so the empty // result above is the sub-tree selection and not an inert fixture.