Skip to content

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

Merged
os-litant merged 4 commits into
mainfrom
claude/issue-14728-i18n-coverage-duplicate-findings
Sep 4, 2026
Merged

fix(cli): count one translation key once in the i18n coverage population#15368
os-litant merged 4 commits into
mainfrom
claude/issue-14728-i18n-coverage-duplicate-findings

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes #14728

One translation key is one demand

A translation key is derived from where a string is addressed, not from which declaration was being read when the walk reached it. collectExpectedEntries emitted one entry per declaration, so a key two declarations both address became two expected entries — and countI18nRuleIssues takes .length over the i18n/ findings while its report calls the number "untranslated declared strings". Translating one key moved the ratchet by two.

The walker now collapses entries that address the same path, keeping the first emission. The ratchet's direction, monotonicity and failure text are untouched — only the population it counts.

Re-derived, not reused

The card said its numbers were on a tree many merges old and must be re-measured. They were, with the card's own repro, on 2ed6be64 before the fix, and re-confirmed after it on the merged tree at ef3b7f12:

config findings distinct duplicate findings baseline before after delta
examples/app-todo/objectstack.config.ts 146 106 40 146 106 −40
examples/app-showcase/objectstack.config.ts 443 414 29 443 414 −29
examples/app-crm/objectstack.config.ts 102 101 1 102 101 −1
the other ten baselined configs 0 0 0 0 0 0

Total 691 → 621, exactly −70. The card's 40 / 29 / 1 survived unchanged on today's tree. The baseline was regenerated with node scripts/check-i18n-coverage.mjs --update, never hand-edited; the gate reported the DOWN direction on all three configs first, which is the sanctioned re-derive.

After the fix, extra (findings beyond the first occurrence of a path) is 0 on all thirteen configs.

The measurement that chose the seam

The card was neutral between de-duplicating at production (collectExpectedEntries) and at the reporting seam (i18n-coverage.ts). Three readings decided it for production.

1. No consumer needs the per-carrier multiplicity. collectExpectedEntries has three non-test consumers: computeI18nCoverage (one finding per key + locale), extractTranslations (writes through setDeep, which collapses them anyway), and check-i18n-walk-parity.mjs (a set of path[0]). None reads a count that the duplicates make true.

2. A second consumer was lying too, and the reporting seam does not reach it. extractTranslations counted emissions in totalExpected and in the per-locale counts it prints, while the skeleton had already collapsed the duplicates on the way in. Measured before → after:

config counts.en reported leaves actually written after the fix
app-showcase 1632 1531 1531 = 1531
app-todo 894 870 870 = 870
app-crm 930 925 925 = 925

3. The duplicates are NOT confined to _actions — that is a finding, and it is the deciding one. The card predicted every duplicate would be in objects.OBJECT._actions.ACTION.*. In the report that holds: 40 / 29 / 1, all _actions. At the walker there is a second family the report never sees:

  • Two carriers, one action (the reported family). The normalized config attaches an object's actions to obj.actions and to the top-level actions list. Probed on all three configs: sameRef=true — literally the same JavaScript object, not a copy, so the two emissions are byte-identical by construction. todo 8/8 actions in both carriers, showcase 69/70, crm 1/1.
  • Two declarations, one form field (never reported). deleteBehavior is declared twice in each of packages/spec/src/data/field.form.ts and object.form.ts, gated on visibleWhen (lookup vs master_detail), and walkFormField keys on the field path — so both variants render into one key. This is config-independent: it duplicates six entries on an empty config. It never reaches os lint's report because the metadataForms bucket is hidden unless --include-platform is passed.

Neither is an authoring mistake; both are two correct declarations of one displayed string, and neither is fixable where it originates. A de-duplication at the reporting seam would have fixed the family that happens to be visible today and left the registry-driven one duplicated in perpetuity — inflating os i18n extract forever, and re-surfacing the moment --include-platform or a ledger change made those keys reportable.

First emission wins, and it is lossless on everything measured: all 372 duplicate paths across the three baselined configs carry byte-identical ExpectedEntry records (372/372 — 354 in the action family, plus the 6 registry ones that repeat on every config). Where two emissions ever do disagree, one bundle slot can serve only one string, so the choice is which colliding declaration to seed from — never whether to drop a demand. The rule is documented on dedupeByPath and pinned.

The pin

packages/cli/test/i18n-duplicate-demand.test.ts, 7 assertions, and it pins the property, not the counts — a pin on 621 or on 101 / 414 / 106 would go green again the day a third carrier joins the walk. The card's property is asserted in the spelling os lint --json uses (translations.LOCALE.KEY, per commands/lint.ts:66): no report carries two findings with the same path for the same locale, both on a dual-carrier config and over the platform bucket the reporting seam could not see. Two walker-level pins assert each path is emitted at most once (once on a dual-carrier action, once on a config that declares nothing, which is the registry family). Two more pin what de-duplication must not do: the collapsed action's keys are all still emitted, and the first emission wins on disagreement.

Ablation (from the committed state; mutation confirmed on disk by counting the injected and deleted anchors, restore confirmed by git hash-object equality to the HEAD blob and an empty git diff HEAD): replacing return dedupeByPath(walked) with return walked turns 5 of the 7 red — both walker pins, the extract-count pin, and both report pins. The two that stay green are the two that assert presence and precedence rather than uniqueness, which is the correct behaviour for them. The pin resolves the walker through src/, so the flip needed no rebuild — which is itself the evidence that the mutation reached the subject under test.

Verification

Everything below was run on the merged tree at ef3b7f12 (main merged in; git status --porcelain empty), after pnpm --workspace-concurrency=2 build. Each exit code was captured by redirecting first and reading $? before any pipe, and each gate is quoted by its own verdict line rather than by a bare $?.

Gate union — derived, never hand-listed: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands on a clean tree at ef3b7f12, which reported the change set as exactly the 4 paths this PR touches. 51 of 51 commands run, 0 non-zero.

No gate in the union failed, and none was skipped.

The ones this change is actually about:

pnpm check:cross-package-test-inputs
  -> (no verdict line printed)
pnpm check:i18n
  -> check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys).
pnpm check:i18n-coverage
  -> check-i18n-coverage: OK (13 config(s), 621 baselined untranslated string(s), none new).
pnpm check:i18n-walk-parity
  -> check-i18n-walk-parity: 11 declared group(s), 8 walked, 3 exempted — every declared group has an extractor face.
pnpm check:nul-bytes
  -> check-nul-bytes: OK (scanned 7448 text file(s) -- 7448 tracked, 0 untracked-not-ignored; skipped 7 binary; no raw ASCII control bytes).
pnpm check:test-source-alias
  -> check-test-source-alias OK — 72 packages with tests scanned; 61 registered as still resolving a workspace dep through `dist/`; 47 published subpath(s) resolved through every alias table.

Package suite and types

pnpm --filter @objectstack/cli test
  -> Test Files  246 passed (246)
  -> Tests  2861 passed | 6 expected fail (2867)
pnpm --filter @objectstack/cli typecheck   (tsc --noEmit && check:test-typecheck)
  -> check:test-typecheck: OK — @objectstack/cli's test layer compiles under packages/cli/tsconfig.test.json

The typecheck covers the new pin rather than skipping it: this package uses the sibling tsconfig.test.json pattern, so the test layer is compiled by a program that really reads it.

Two gate classes are NOT MEASURED here and are named rather than counted as passes: the 13 families dispatch-gates reports as taking a value from the workflow (their argv carries a variable that has no value outside a CI run), and the 34 artifact-roster families it scores silent for every card in the tree. Their silence is not a clearance, and CI runs the farm regardless.

No generated bundle moved: node scripts/check-i18n-bundles.mjs reports all nine packages in sync and git status --porcelain is empty after it — de-duplication removes copies, never demands.

Scope

Out of scope and deliberately untouched: the ratchet mechanism in scripts/check-i18n-coverage.mjs (the defect is the population, not the gate), and the three sibling _actions-walk cards (#14653, #14700, #14708) whose fix sites are elsewhere.


Generated by Claude Code

@github-actions github-actions Bot added the size/m label Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json dc46c4ec133eb946c50805a851e4f6218058f366packageMentionDocs.

Which tree this was computed on

This run read content/docs from 79f7e150df8dddc0a947045d70767d96465525f1 — the merge of head ef3b7f120c7c8bbc00224977f1ffeb9880426887 into base dc46c4ec133eb946c50805a851e4f6218058f366, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 79f7e150df8dddc0a947045d70767d96465525f1 && git checkout 79f7e150df8dddc0a947045d70767d96465525f1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin dc46c4ec133eb946c50805a851e4f6218058f366 ef3b7f120c7c8bbc00224977f1ffeb9880426887 && git checkout -B drift-repro dc46c4ec133eb946c50805a851e4f6218058f366 && git merge --no-ff ef3b7f120c7c8bbc00224977f1ffeb9880426887

node scripts/docs-audit/affected-docs.mjs --json dc46c4ec133eb946c50805a851e4f6218058f366

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Copy link
Copy Markdown
Collaborator Author

✅ ACCEPT — reviewer of record, domain:cli execution PM seat (#6024), R69, session session_01D47qPfEWVPmhguWgBZCi5N

⚠️ Recorded as a comment, not as a GitHub review: this seat and the dev seat share one identity, so GitHub refuses the approval outright (Review Can not approve your own pull request). ⇒ The review of record for an agent-authored PR in this lane is this comment. Noting it because a reader checking the Reviews tab would otherwise read an empty list as "unreviewed".

CI: green, and read as check runs rather than as a combined status. All 33 check runs on the head are completed, every one success or skippedLint & Repo Gates, all four Type Check legs, all six Test Core shards, all three Dogfood Regression Gate shards, Temporal Conformance (live PG + MySQL), and Governed Surface Queue Guard. ⚠️ Read this way deliberately: a PR's combined status and its check runs are different objects here, and the combined status is the one that can read green over a run that is not.

Governed surface: none. Probed the diff directly against docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md and content/docs/releases/** — zero hits, which the repo's own Governed Surface Queue Guard corroborates independently. ⇒ This seat may enqueue it.

Surface: four files, each earning its place.

file
packages/cli/src/utils/i18n-extract.ts the fix — dedupeByPath at the end of collectExpectedEntries
packages/cli/test/i18n-duplicate-demand.test.ts the pin
scripts/i18n-coverage-baseline.json the ratchet, shrinking
.changeset/i18n-walk-one-key-one-demand.md one changeset

The key separator is what makes this correct rather than approximately correct. Deduping on entry.path.join('�') and not on '.': a dotted join collides ['a.b'] with ['a','b'], which are two different demands that would then count as one. That is precisely the silent under-count this card exists to end, and picking the separator that cannot collide is what keeps the fix from re-introducing the defect one layer down.

The ratchet moves in the only permitted direction. 102 → 101, 443 → 414, 146 → 106 — net −70, regenerated with the baseline's own --update rather than hand-edited. ⛔ A shrink-only ratchet is satisfied by removing errors, never by raising the ceiling; every number here goes down. ⇒ Nothing in this diff buys green with headroom.

⇒ Draft flipped, enqueueing. ⛔ Never merged directly by this seat — the merge queue is the only door, and a green PR is not a merged one until the queue says so.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] check:i18n-coverage counts one untranslated action key more than once — 70 of the 691 baselined units are byte-identical duplicate findings

2 participants