Skip to content

Commit fc3fb7c

Browse files
os-litantclaude
os-litant
and
claude
authored
fix(cli): report os i18n extract key counts off the emitted bytes (#16247)
* fix(cli): report i18n extract key counts off the emitted bytes `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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * test(cli): fix the grammar of the key-count pin's case title Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N * fix(cli): unbreak the pin's typecheck, drop a false symmetry claim, report suppressed modules 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c3b63f7 commit fc3fb7c

5 files changed

Lines changed: 629 additions & 70 deletions

File tree

‎.changeset/i18n-extract-key-count-describes-emitted-bytes.md‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
`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.
6+
7+
`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'`:
8+
9+
```
10+
Skeleton summary
11+
zh-CN 776 key(s) (of 776 expected) + 773 metadataForms key(s)
12+
Wrote OUT/zh-CN.objects.generated.ts (776 keys)
13+
```
14+
15+
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:
16+
17+
```
18+
Skeleton summary
19+
zh-CN 775 of 776 key(s) emitted objects 2 · metadataForms 773
20+
Wrote OUT/zh-CN.objects.generated.ts (2 keys)
21+
Wrote OUT/zh-CN.metadata-forms.generated.ts (773 keys)
22+
```
23+
24+
**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.
25+
26+
**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.
27+
28+
**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 `<locale>.objects.generated.ts` holding `{}` and announced it as 774 keys. The gate is now the module's own leaf count.
29+
30+
**`--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`.
31+
32+
⚠️ 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.
33+
34+
**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.
35+
36+
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.

‎packages/cli/src/commands/i18n/extract.ts‎

Lines changed: 145 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -21,25 +21,16 @@ import {
2121
extractTranslations,
2222
renderTranslationModule,
2323
renderSourceHashModule,
24-
stackAuthoredSubtree,
2524
parseSourceHashModule,
2625
narrowToCommittedSections,
26+
translationModulePayload,
27+
countTranslationLeaves,
2728
type FillStrategy,
29+
type TranslationModuleKind,
2830
} from '../../utils/i18n-extract.js';
2931

3032
const FILL_STRATEGIES: FillStrategy[] = ['empty', 'default', 'todo'];
3133

32-
/** Count string-leaf entries under a nested object — used for reporting. */
33-
function countLeaves(obj: unknown): number {
34-
if (!obj || typeof obj !== 'object') return 0;
35-
let n = 0;
36-
for (const v of Object.values(obj as Record<string, unknown>)) {
37-
if (typeof v === 'string') n += 1;
38-
else if (v && typeof v === 'object') n += countLeaves(v);
39-
}
40-
return n;
41-
}
42-
4334
/**
4435
* `os i18n extract` — scaffold translation skeletons.
4536
*
@@ -184,11 +175,34 @@ export default class I18nExtract extends Command {
184175
const localesEmitted = Object.keys(result.bundles);
185176
const objectsOnly = flags['objects-only'];
186177

187-
// Count metadataForms keys per locale (computed separately so we
188-
// can show users an honest summary even when --objects-only).
178+
/**
179+
* The sub-tree the stack module holds, and the selector that picks it.
180+
*
181+
* ⭐ Every key count this command reports — the summary line, each
182+
* `Wrote … (N keys)` line, the `--json` payload — is a leaf count of one
183+
* of these payloads, taken with the same selector the module is RENDERED
184+
* from. It is never `result.counts`: that is the whole skeleton the
185+
* extractor built for the locale, sections this run does not write
186+
* included, which under the default `--objects-only` is the ~773-key
187+
* metadata-form baseline plus every non-`objects` group the stack
188+
* authors. Reporting it beside a 2-leaf file is #16121.
189+
*
190+
* A third emission mode later (`--apps-only`, say) adds a `kind` and is
191+
* counted correctly without a number here moving, because no number here
192+
* is arithmetic over another one. ⛔ In particular nothing subtracts the
193+
* baseline at a print site: that repairs today's two modes and leaves the
194+
* next one wrong in the same way.
195+
*/
196+
const stackKind: TranslationModuleKind = objectsOnly ? 'objects' : 'stack';
197+
const stackPayload = (locale: string) => translationModulePayload(result.bundles[locale], stackKind);
198+
199+
// Counted for every locale, emitted or not, so the operator can still see
200+
// how big the baseline is when their flags suppress it.
189201
const metadataFormsCounts: Record<string, number> = {};
190202
for (const locale of localesEmitted) {
191-
metadataFormsCounts[locale] = countLeaves(result.bundles[locale]?.metadataForms);
203+
metadataFormsCounts[locale] = countTranslationLeaves(
204+
translationModulePayload(result.bundles[locale], 'metadataForms'),
205+
);
192206
}
193207
const anyMetadataForms = Object.values(metadataFormsCounts).some((n) => n > 0);
194208
// Whether the companion `<locale>.metadata-forms.generated.ts` file is
@@ -212,6 +226,65 @@ export default class I18nExtract extends Command {
212226
const emitsMetadataForms = (locale: string): boolean =>
213227
flags['metadata-forms'] && (metadataFormsCounts[locale] ?? 0) > 0;
214228

229+
/** One module this run's flags CONSIDER for one locale. */
230+
interface CandidateModule {
231+
/** Written to `<locale>.<suffix>` when {@link CandidateModule.emitted}. */
232+
suffix: string;
233+
/** Sub-tree selector — picks the payload AND the rendered module's type. */
234+
kind: TranslationModuleKind;
235+
/** How this module is named in a `--dry-run` heading and in the summary. */
236+
label: string;
237+
/** Leaves this module holds. The ONE number reported for it, anywhere. */
238+
keys: number;
239+
/** Whether this run writes it. A candidate a flag SUPPRESSED is still
240+
* reported — how big the thing they switched off is, is a reading the
241+
* operator needs, and 8 of this repo's 9 extract configs are on that
242+
* path (`--no-metadata-forms`). */
243+
emitted: boolean;
244+
}
245+
246+
/**
247+
* Every module one locale's run considers, in file order — the single
248+
* list the summary, `--dry-run`, `--check` and the write loop all read,
249+
* so no two of them can disagree about what this run produces. The last
250+
* three take the `emitted` ones; the summary reports all of them and adds
251+
* up only the `emitted` ones.
252+
*
253+
* A module with no leaves is not a candidate at all. The write gate used
254+
* to be `result.counts[locale] > 0`, which is a property of the SKELETON:
255+
* on a stack whose only surface is apps, the default `--objects-only`
256+
* wrote an `<locale>.objects.generated.ts` holding `{}` and announced it
257+
* as 774 keys. Measured on this repair's fixture at `f5aec38a6af`.
258+
*/
259+
const candidatesFor = (locale: string): CandidateModule[] => {
260+
const mods: CandidateModule[] = [];
261+
const stackKeys = countTranslationLeaves(stackPayload(locale));
262+
if (stackKeys > 0) {
263+
mods.push({
264+
suffix: 'objects.generated.ts',
265+
kind: stackKind,
266+
label: 'objects',
267+
keys: stackKeys,
268+
emitted: true,
269+
});
270+
}
271+
if ((metadataFormsCounts[locale] ?? 0) > 0) {
272+
mods.push({
273+
suffix: 'metadata-forms.generated.ts',
274+
kind: 'metadataForms',
275+
label: 'metadataForms',
276+
keys: metadataFormsCounts[locale] ?? 0,
277+
emitted: emitsMetadataForms(locale),
278+
});
279+
}
280+
return mods;
281+
};
282+
const candidates: Record<string, CandidateModule[]> = {};
283+
for (const locale of localesEmitted) candidates[locale] = candidatesFor(locale);
284+
/** The candidates this run actually writes — what every file face iterates. */
285+
const emittedModules = (locale: string): CandidateModule[] =>
286+
candidates[locale].filter((m) => m.emitted);
287+
215288
/**
216289
* The provenance table for one locale, narrowed to the sections this run
217290
* actually COMMITS (#12559).
@@ -241,9 +314,8 @@ export default class I18nExtract extends Command {
241314
* coupling ADR-0029 D8 and each package's `bundle-ownership.test.ts` exist
242315
* to keep out of its committed bundles.
243316
*
244-
* So the section list is decided by the SAME predicates that decide the
245-
* bundle files, never by a second rule: `result.counts` for `objects` and
246-
* {@link emitsMetadataForms} for `metadataForms`. A set that commits both —
317+
* So the section list is decided by the SAME list that decides the
318+
* bundle files — {@link emittedModules} — never by a second rule. A set that commits both —
247319
* `platform-objects` is the one today — keeps every record it had. The
248320
* narrowing itself is `narrowToCommittedSections`, a pure function in the
249321
* extractor's utils so it can be pinned without driving oclif; this layer
@@ -253,25 +325,40 @@ export default class I18nExtract extends Command {
253325
const table = result.sourceHashes[locale];
254326
if (!table) return undefined;
255327
const committed: string[] = [];
256-
if ((result.counts[locale] ?? 0) > 0) committed.push('objects');
257-
if (emitsMetadataForms(locale)) committed.push('metadataForms');
328+
if (emittedModules(locale).some((m) => m.kind !== 'metadataForms')) committed.push('objects');
329+
if (emittedModules(locale).some((m) => m.kind === 'metadataForms')) committed.push('metadataForms');
258330
return narrowToCommittedSections(table, committed);
259331
};
260332

261333
if (flags.json) {
262334
await emitJson({
263335
totalExpected: result.totalExpected,
264-
counts: result.counts,
336+
// Leaves of the `bundles` payload below, locale by locale, so this
337+
// count describes the tree printed beside it.
338+
//
339+
// It used to forward `result.counts`, the extractor's per-locale
340+
// SKELETON size, while `bundles` carried only the sub-tree this run
341+
// emits: on a one-object stack under the default `--objects-only`
342+
// that was 776 against a 2-leaf `bundles` payload (#16121). The
343+
// skeleton total is still here — it is `totalExpected`.
344+
//
345+
// ⚠️ This is NOT the relationship `metadataFormsCounts` has to
346+
// `metadataForms`, and an earlier revision of this comment claimed it
347+
// was. `metadataFormsCounts` reports the baseline's size whether or
348+
// not the baseline is emitted — under `--no-metadata-forms` the
349+
// payload carries `metadataFormsCounts: { 'zh-CN': 773 }` beside
350+
// `metadataForms: {}`, deliberately, and a sibling pin holds it there
351+
// so an operator can still see how big the thing they switched off
352+
// is. So this payload carries TWO count semantics: `counts` is what
353+
// was emitted, `metadataFormsCounts` is what was built. Whether it
354+
// SHOULD is a question for the maintainer; this change neither
355+
// settles it nor moves either face.
356+
counts: Object.fromEntries(localesEmitted.map((l) => [l, countTranslationLeaves(stackPayload(l))])),
265357
metadataFormsCounts,
266358
// `--json` is documented as "output JSON instead of writing files",
267359
// so this payload mirrors the FILE SET: `bundles` is the stack
268360
// module, `metadataForms` below is the companion (#14894).
269-
bundles: Object.fromEntries(
270-
localesEmitted.map((l) => [
271-
l,
272-
objectsOnly ? (result.bundles[l].objects ?? {}) : stackAuthoredSubtree(result.bundles[l]),
273-
]),
274-
),
361+
bundles: Object.fromEntries(localesEmitted.map((l) => [l, stackPayload(l)])),
275362
// The baseline's JSON home, gated by {@link emitsMetadataForms} —
276363
// the SAME predicate that decides the companion file, deliberately
277364
// not a second one.
@@ -308,13 +395,31 @@ export default class I18nExtract extends Command {
308395
console.log(chalk.bold(' Skeleton summary'));
309396
const nameWidth = Math.max(8, ...localesEmitted.map((l) => l.length));
310397
for (const locale of localesEmitted) {
311-
const n = result.counts[locale];
312-
const tone = n === 0 ? chalk.green : chalk.yellow;
313-
const mfN = metadataFormsCounts[locale] ?? 0;
314-
const mfTail = mfN > 0 ? chalk.dim(` + ${mfN} metadataForms key(s)`) : '';
398+
const mods = candidates[locale];
399+
// The modules are disjoint sub-trees of the skeleton, so this line is a
400+
// partition of it: how many of the locale's keys reach a module, out of
401+
// how many were built, and which module holds which. The old line added
402+
// the baseline to a number that already contained it and read as 1549
403+
// of 776 (#16121).
404+
//
405+
// A candidate a flag SUPPRESSED is named too, with its size and the
406+
// words that keep it out of the sum. Dropping it was an information
407+
// regression on the commonest path: `--no-metadata-forms` is what 8 of
408+
// this repo's 9 extract configs pass, and the old line at least told
409+
// those runs how big the baseline they switched off was.
410+
const emittedKeys = mods.filter((m) => m.emitted).reduce((n, m) => n + m.keys, 0);
411+
const skeleton = result.counts[locale] ?? 0;
412+
// Green means there is nothing to translate for this locale, which is a
413+
// property of the SKELETON. `0 of 774 emitted` is not that: it is a run
414+
// whose flags excluded everything built, and reading green there is the
415+
// same conflation this card is about.
416+
const tone = skeleton === 0 ? chalk.green : chalk.yellow;
417+
const breakdown = mods.length > 1 || mods.some((m) => !m.emitted)
418+
? chalk.dim(` ${mods.map((m) => `${m.label} ${m.keys}${m.emitted ? '' : ' not emitted'}`).join(' · ')}`)
419+
: '';
315420
console.log(
316-
` ${locale.padEnd(nameWidth)} ${tone(String(n).padStart(5))} key(s)` +
317-
chalk.dim(` (of ${result.totalExpected} expected)`) + mfTail,
421+
` ${locale.padEnd(nameWidth)} ${tone(String(emittedKeys).padStart(5))}` +
422+
chalk.dim(` of ${skeleton} key(s) emitted`) + breakdown,
318423
);
319424
}
320425
console.log('');
@@ -325,18 +430,9 @@ export default class I18nExtract extends Command {
325430

326431
if (flags['dry-run'] || !flags.out) {
327432
for (const locale of localesEmitted) {
328-
if (result.counts[locale] === 0 && metadataFormsCounts[locale] === 0) continue;
329-
console.log(chalk.dim(`── ${locale} (objects) ──`));
330-
console.log(renderTranslationModule(result.bundles[locale], {
331-
locale,
332-
objectsOnly,
333-
}));
334-
if (emitsMetadataForms(locale)) {
335-
console.log(chalk.dim(`── ${locale} (metadataForms) ──`));
336-
console.log(renderTranslationModule(result.bundles[locale], {
337-
locale,
338-
kind: 'metadataForms',
339-
}));
433+
for (const mod of emittedModules(locale)) {
434+
console.log(chalk.dim(`── ${locale} (${mod.label}) ──`));
435+
console.log(renderTranslationModule(result.bundles[locale], { locale, kind: mod.kind }));
340436
}
341437
}
342438
printInfo('Dry run — no files written (pass --out=<dir> to write).');
@@ -351,18 +447,11 @@ export default class I18nExtract extends Command {
351447
// what a real extract writes.
352448
const emitted: Array<{ file: string; content: string; keys: number }> = [];
353449
for (const locale of localesEmitted) {
354-
if (result.counts[locale] > 0) {
355-
emitted.push({
356-
file: path.join(resolvedOutDir, `${locale}.objects.generated.ts`),
357-
content: renderTranslationModule(result.bundles[locale], { locale, objectsOnly }),
358-
keys: result.counts[locale],
359-
});
360-
}
361-
if (emitsMetadataForms(locale)) {
450+
for (const mod of emittedModules(locale)) {
362451
emitted.push({
363-
file: path.join(resolvedOutDir, `${locale}.metadata-forms.generated.ts`),
364-
content: renderTranslationModule(result.bundles[locale], { locale, kind: 'metadataForms' }),
365-
keys: metadataFormsCounts[locale],
452+
file: path.join(resolvedOutDir, `${locale}.${mod.suffix}`),
453+
content: renderTranslationModule(result.bundles[locale], { locale, kind: mod.kind }),
454+
keys: mod.keys,
366455
});
367456
}
368457
// The provenance companion rides in the SAME list, so `--check` compares

0 commit comments

Comments
 (0)