fix(cli): count one translation key once in the i18n coverage population

.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.

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.<o>._actions.<a>.*`.
+ *   - **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<string>();
+  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.<name>.…`) ──────────────────────────

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<string, number>();
+  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<number>(
+        (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);
+  });
+});

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,