From 8f96d3e67683645ba7236b1ded8f3761e8b2ef8c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 12:29:41 +0000 Subject: [PATCH 1/3] fix(cli): collapse duplicate expected translation entries in the i18n walk Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- packages/cli/src/utils/i18n-extract.ts | 65 +++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/utils/i18n-extract.ts b/packages/cli/src/utils/i18n-extract.ts index 13dc74a6d4..d3c457d8f9 100644 --- a/packages/cli/src/utils/i18n-extract.ts +++ b/packages/cli/src/utils/i18n-extract.ts @@ -1188,8 +1188,69 @@ export function collectExpectedEntries( walkMetadataForms(out); const warnedGroups = opts.warnedGroups ?? authorWarnedTranslationGroups(); - if (warnedGroups.size === 0) return out; - return out.filter((entry) => !warnedGroups.has(entry.path[0])); + const walked = warnedGroups.size === 0 ? out : out.filter((entry) => !warnedGroups.has(entry.path[0])); + return dedupeByPath(walked); +} + +/** + * Collapse entries that address the same key to one. + * + * **One path is one demand.** A translation bundle has exactly one slot per + * path, so an author owes exactly one string for it no matter how many places + * in the walk arrived at that slot. The walk does not have that property on its + * own, because a translation key is derived from *where the string is + * addressed*, not from *which declaration was being read* when it was reached + * — and two declarations can address one slot: + * + * - **Two carriers, one action.** The normalized config attaches an object's + * actions to the object (`obj.actions`) *and* to the top-level `actions` + * list — measured to be the SAME object reference, not a copy — so the two + * action branches above both emit `objects.._actions..*`. + * - **Two declarations, one form field.** `field.form.ts` and + * `object.form.ts` each declare `deleteBehavior` twice, gated on + * `visibleWhen` (`lookup` vs `master_detail`). Both variants render into + * the same key, because {@link walkFormField} keys on the field path. + * Config-independent: it duplicates six entries on *every* config, + * including an empty one. + * + * Neither is an authoring mistake and neither is fixable where it originates — + * they are two correct declarations of one displayed string. So the walker owns + * the collapse. + * + * **De-duplicating HERE rather than in the report** is what makes both + * consumers honest with one change. `computeI18nCoverage` counted the same + * 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 — + * 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 + * and left the second, and would have left the registry-driven family + * duplicated in perpetuity — it never reaches `os lint`'s report, which hides + * the `metadataForms` bucket unless `--include-platform` is passed. + * + * **First emission wins.** Walk order is deterministic, so the rule is + * deterministic. It is also lossless on everything measured: all 372 duplicate + * paths across the three baselined example configs, and all 6 registry ones, + * carry byte-identical {@link ExpectedEntry} records — for the action family + * necessarily so, since both carriers hold one reference. Where two emissions + * ever *do* disagree, the disagreement is already unresolvable downstream: one + * bundle slot can serve only one string, so the choice is which of two + * colliding declarations to seed from, not whether to drop a demand. + */ +function dedupeByPath(entries: ExpectedEntry[]): ExpectedEntry[] { + const seen = new Set(); + const out: ExpectedEntry[] = []; + for (const entry of entries) { + // `\u0000` cannot occur in a path segment, so joining on it cannot make two + // different paths collide the way a `.` join would (`['a.b']` vs `['a','b']`). + const key = entry.path.join('\u0000'); + if (seen.has(key)) continue; + seen.add(key); + out.push(entry); + } + return out; } // ─── Analytics datasets (`datasets..…`) ────────────────────────── From 578a5dcd2c30bd9f2d1b88b4c3012bf022ee6046 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 12:34:13 +0000 Subject: [PATCH 2/3] test(cli): pin one-key-one-demand across the walk and the coverage report Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- .../cli/test/i18n-duplicate-demand.test.ts | 149 ++++++++++++++++++ scripts/i18n-coverage-baseline.json | 6 +- 2 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 packages/cli/test/i18n-duplicate-demand.test.ts diff --git a/packages/cli/test/i18n-duplicate-demand.test.ts b/packages/cli/test/i18n-duplicate-demand.test.ts new file mode 100644 index 0000000000..fe78bb0ca4 --- /dev/null +++ b/packages/cli/test/i18n-duplicate-demand.test.ts @@ -0,0 +1,149 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * One translation key is ONE demand — the property, not the counts. + * + * `pnpm check:i18n-coverage` ratchets `countI18nRuleIssues`, which is `.length` + * over the `i18n/`-prefixed findings, while its report calls that number + * "untranslated declared strings". Those are the same number only if the + * population holds each key once. It did not: two places in the walk addressed + * one bundle slot, so 70 of 691 baselined units were one string counted twice + * and translating ONE key moved the ratchet by TWO. + * + * These pins deliberately assert NO COUNT. A pin on 621, or on the per-config + * 101 / 414 / 106, goes green again the day a third carrier is added to the + * walk — the exact regression it would exist to catch. The property is what + * cannot regress silently, so the property is what is pinned, at both seams the + * defect was visible from: + * + * 1. `collectExpectedEntries` emits each path at most once (production), and + * 2. no report carries two findings with the same `path` for one locale + * (reporting — `os lint` spells that path `translations.LOCALE.KEY`, in + * `commands/lint.ts`). + * + * Both measured duplicate families are exercised below, because they have + * different causes and only one of them is visible in a report at all: + * + * - Two carriers, one action. The normalizer attaches an object's actions to + * `obj.actions` AND to top-level `config.actions` — the same object + * reference, measured on all three baselined example configs — so both + * action branches emit `objects.OBJECT._actions.ACTION.*`. + * - Two declarations, one form field. `deleteBehavior` is declared twice in + * each of `field.form.ts` / `object.form.ts`, gated on `visibleWhen`; both + * render into one key. Config-independent — it duplicates six entries on an + * EMPTY config, and it never reaches `os lint`'s report, which hides the + * `metadataForms` bucket unless `--include-platform` is passed. A + * de-duplication at the reporting seam would have left this family + * duplicated in perpetuity, which is why the fix lives in the walker. + */ + +import { describe, it, expect } from 'vitest'; +import { collectExpectedEntries, extractTranslations } from '../src/utils/i18n-extract.js'; +import { computeI18nCoverage } from '../src/utils/i18n-coverage.js'; + +/** Repeated paths in a walk, as `[path, occurrences]`, occurrences > 1 only. */ +function repeatedPaths(entries: ReadonlyArray<{ path: string[] }>): Array<[string, number]> { + const counts = new Map(); + for (const entry of entries) { + const key = entry.path.join('.'); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + return [...counts].filter(([, n]) => n > 1); +} + +/** + * A config shaped the way the normalizer really emits one: the action object is + * carried by BOTH the object and the top-level list, by reference. Sharing the + * reference is the point — a copy would not reproduce the defect faithfully, + * and a shared reference is what was measured on the real configs. + */ +function dualCarrierConfig(): any { + const action = { + name: 'convert_lead', + label: 'Convert Lead', + objectName: 'lead', + confirmText: 'Convert?', + successMessage: 'Converted.', + params: [{ name: 'owner', label: 'New Owner' }], + }; + return { + i18n: { defaultLocale: 'en', supportedLocales: ['en', 'zh-CN'] }, + objects: [{ name: 'lead', label: 'Lead', fields: { name: { label: 'Name' } }, actions: [action] }], + actions: [action], + translations: [{ en: { objects: { lead: { label: 'Lead' } } } }], + }; +} + +describe('one key is one demand: the i18n walk', () => { + it('emits each expected path at most once, for a dual-carrier action', () => { + expect(repeatedPaths(collectExpectedEntries(dualCarrierConfig()))).toEqual([]); + }); + + it('emits each expected path at most once on a config that declares nothing', () => { + // Guards the registry-driven family (`metadataForms.*.fields.deleteBehavior.*`), + // which is reached with no author metadata at all. + expect(repeatedPaths(collectExpectedEntries({}))).toEqual([]); + }); + + it('still emits the action keys it collapsed: de-duplication drops copies, never demands', () => { + const paths = collectExpectedEntries(dualCarrierConfig()).map((e) => e.path.join('.')); + for (const key of [ + 'objects.lead._actions.convert_lead.label', + 'objects.lead._actions.convert_lead.confirmText', + 'objects.lead._actions.convert_lead.successMessage', + 'objects.lead._actions.convert_lead.params.owner.label', + ]) { + expect(paths).toContain(key); + } + }); + + it('keeps the FIRST emission when two declarations disagree about one path', () => { + // Not reachable through the normalizer today (both carriers hold one + // reference), so this pins the documented rule rather than a measurement. + const config: any = { + objects: [{ name: 'lead', label: 'Lead', actions: [{ name: 'act', label: 'From the object' }] }], + actions: [{ name: 'act', objectName: 'lead', label: 'From the top level' }], + }; + const entry = collectExpectedEntries(config).find( + (e) => e.path.join('.') === 'objects.lead._actions.act.label', + ); + expect(entry?.sourceValue).toBe('From the object'); + }); + + it('reports the number of keys it actually wrote into the skeleton', () => { + // The second consumer the duplicates lied to: `setDeep` collapsed them on + // the way into the bundle while `counts` kept counting emissions, so + // `os i18n extract` over-reported (measured: 1632 claimed against 1531 + // written, app-showcase). + const result = extractTranslations(dualCarrierConfig(), { locales: ['en', 'zh-CN'] }); + const leaves = (node: any): number => + Object.values(node ?? {}).reduce( + (n, v) => n + (v !== null && typeof v === 'object' ? leaves(v) : 1), + 0, + ); + expect(result.counts.en).toBe(leaves(result.bundles.en)); + expect(result.totalExpected).toBe(leaves(result.bundles.en)); + }); +}); + +describe('one key is one demand: the coverage report', () => { + it('carries no two findings with the same path for the same locale', () => { + const report = computeI18nCoverage(dualCarrierConfig()); + // `os lint --json` spells a finding's path exactly this way. + const paths = report.issues.map((i) => `translations.${i.locale}.${i.key}`); + expect(paths.length).toBeGreaterThan(0); + expect(new Set(paths).size).toBe(paths.length); + }); + + it('carries no duplicate path per locale over the platform bucket either', () => { + // The `metadataForms` family is only reportable with `--include-platform`, + // and it is the family a reporting-seam de-duplication could not see. + const report = computeI18nCoverage( + { i18n: { defaultLocale: 'en', supportedLocales: ['en', 'zh-CN'] } }, + { locales: ['zh-CN'] }, + ); + const paths = report.issues.map((i) => `translations.${i.locale}.${i.key}`); + expect(paths.length).toBeGreaterThan(0); + expect(new Set(paths).size).toBe(paths.length); + }); +}); diff --git a/scripts/i18n-coverage-baseline.json b/scripts/i18n-coverage-baseline.json index 6365dbb7ea..ad3d313a33 100644 --- a/scripts/i18n-coverage-baseline.json +++ b/scripts/i18n-coverage-baseline.json @@ -1,8 +1,8 @@ { - "examples/app-crm/objectstack.config.ts": 102, + "examples/app-crm/objectstack.config.ts": 101, "examples/app-multi-package/objectstack.config.ts": 0, - "examples/app-showcase/objectstack.config.ts": 443, - "examples/app-todo/objectstack.config.ts": 146, + "examples/app-showcase/objectstack.config.ts": 414, + "examples/app-todo/objectstack.config.ts": 106, "packages/platform-objects/scripts/i18n-extract.config.ts": 0, "packages/plugins/plugin-approvals/scripts/i18n-extract.config.ts": 0, "packages/plugins/plugin-audit/scripts/i18n-extract.config.ts": 0, From c47c13cbbd38719c33462a2acdf594530b8adc0e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 12:45:54 +0000 Subject: [PATCH 3/3] chore(changeset): one key is one demand in the i18n walk Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N --- .changeset/i18n-walk-one-key-one-demand.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 .changeset/i18n-walk-one-key-one-demand.md diff --git a/.changeset/i18n-walk-one-key-one-demand.md b/.changeset/i18n-walk-one-key-one-demand.md new file mode 100644 index 0000000000..6255ad9acc --- /dev/null +++ b/.changeset/i18n-walk-one-key-one-demand.md @@ -0,0 +1,19 @@ +--- +"@objectstack/cli": patch +--- + +`os lint` and `os i18n extract` no longer count one translation key twice. + +A translation key is derived from *where a string is addressed*, not from *which declaration was being read* when the walk reached it — and two declarations can address one bundle slot. `collectExpectedEntries` emitted one entry per declaration, so a key reachable twice became two expected entries. Two families were measured, with different causes: + +- **Two carriers, one action.** The normalized config attaches an object's actions to `obj.actions` *and* to the top-level `actions` list — the same object reference, not a copy — so both action branches emitted `objects.OBJECT._actions.ACTION.*`. This is the family the coverage report shows: 70 of 691 baselined units across `app-todo` (40), `app-showcase` (29) and `app-crm` (1). +- **Two declarations, one form field.** `deleteBehavior` is declared twice in each of the `field` and `object` metadata forms, gated on `visibleWhen` (`lookup` vs `master_detail`); both render into one key. Config-independent — it duplicated six entries on every config, including an empty one. + +Neither is an authoring mistake, and neither is fixable where it originates: both are two correct declarations of one displayed string. So the walker now collapses entries that address the same path, keeping the first emission. + +What that corrects, in both directions: + +- **`os lint`'s i18n findings.** The same missing key was reported twice, byte-identically. `pnpm check:i18n-coverage` ratchets the finding *count* while its report calls the number "untranslated declared strings", so translating one key moved the ratchet by two and the frozen debt was ~11% larger than the work it described. The three coverage baselines are regenerated in this change and fall by exactly 70 (691 to 621): `app-crm` 102 to 101, `app-showcase` 443 to 414, `app-todo` 146 to 106. The ratchet's direction, monotonicity and failure text are unchanged — only the population it counts. +- **`os i18n extract`'s reported counts.** `totalExpected` and the per-locale `counts` counted emissions while the skeleton itself had already collapsed the duplicates on the way in, so extract over-reported what it wrote — 1632 claimed against 1531 keys written on `app-showcase`, 894 against 870 on `app-todo`, 930 against 925 on `app-crm`. Those numbers now match the skeleton. + +No generated bundle changes: every duplicate pair measured carries a byte-identical record, so de-duplication removes copies and never a demand. All nine `translations/*.generated.ts` packages stay in sync.