Skip to content

chore(i18n): retire unreferenced catalog keys and gate the scan - #19801

Closed
smwbev wants to merge 55 commits into
stablyai:mainfrom
smwbev:chore/retire-orphaned-catalog-keys
Closed

smwbev wants to merge 55 commits into
stablyai:mainfrom
smwbev:chore/retire-orphaned-catalog-keys

Conversation

@smwbev

@smwbev smwbev commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #22056, which adds the detector this PR runs. Until that lands, the diff here includes the script; once it merges this collapses to the catalog deletions and the gate.

Summary

Retires the catalog keys that no source references, and makes the existing extraction check fail on them so they cannot silently re-accumulate.

en.json drops from 17,395 entries to 15,754 — 1,641 keys, found in one pass. Every locale sheds the same set; two of the keys exist only in English and pt.json never had a third, so pt.json sheds 1,638 and the other five 1,639. en.json itself goes from 1,108,731 to 1,012,031 bytes.

The gate

Reworked in the 10 Oct refresh. The earlier revision added a verify:localization-orphans entry to the lint chain and a matching step to pr.yml, which meant a second i18next extraction on every PR that touches src/. main has since put real effort into running that AST pass once (#26806), so the gate now rides on it instead of repeating it:

  • verify-localization-extraction.mjs hands the keys its own pass extracted to the detector. The detector adds the source-literal scan and the check fails on any key neither can reach.
  • No new workflow step and no new lint entry — pr.yml and the lint chain are byte-identical to main. The step keeps its existing change-scope routing.
  • Editing the detector re-runs the check without a rule of its own. Since 14 Oct the script is verify-localization-orphans.mjs (chore(i18n): gate unreferenced catalog keys in the extraction check and retire 1,644 of them #22056 renamed it so CODEOWNERS covers it), and main's change-scope rule for config/scripts/verify-localization-* already treats it as an input. The line this PR used to add to localization-extraction-change-scope.mjs is gone; a test case pins that the detector file stays in scope.

Cost, measured locally on the 10 Oct tree: the literal scan alone takes 0.7 s over 17,682 source files. Three runs of the whole check on the same tree gave a median of 21.8 s with main's script and 20.6 s with the gate, so the addition sits inside run-to-run noise.

This is the half that makes the prune stick. verify:localization-extraction already computed an unreferenced set but only failed on keys missing from English, so a reverted feature stranded its keys and nothing noticed. A failure names the keys and says what to do:

Catalog keys no source references (delete them from every locale, then run `pnpm run sync:localization-runtime-catalog`):
  auto.__plantedOrphanProbe.strandedCopy
  auto.__plantedOrphanProbe.items_one
  auto.__plantedOrphanProbe.items_other

pnpm run verify:localization-orphans from #22056 stays as the standalone way to list them.

Collateral

Five tests asserted values of keys no screen renders, and main has since deleted all five, so this branch no longer touches a test outside the detector's: #28669 deleted zh-technical-literal-mistranslations.test.ts, technical-literal-catalog-values.test.ts and ko-ui-semantic-mistranslations.test.ts, #34136 deleted catalog-nested-settings.test.ts (the branch only took settings.agents.loadingHost off its list), and #34411 deleted native-chat-shared-copy-matches-catalog.test.ts.

One question from that last file still stands for whoever owns native chat. Five NATIVE_CHAT_TOOL_ACTIVITY_COPY entries (runningPreview, runningCommand, runningNamedPreview, runningNamed, moreCalls) have no desktop call site left: mobile formats the running labels from the constant, desktop only uses them to match labels in native-chat-turn-activity.ts, and moreCalls has no reader at all, so their components.native-chat.tool.* keys are in this prune. If any of the five is about to get a desktop call site, tell me and I will keep its key.

en-runtime-required.json is regenerated with sync:localization-runtime-catalog: 691 of 15,754 entries must ship in the boot bundle, down from 2,339 of 17,395 on main.

Left alone on purpose: the translation-policy override tables under config/scripts/locale-*-key-overrides.* still name 284 of the retired keys, 259 of them in locale-ko-key-overrides.json. Their tests pass unchanged. I kept them out to hold this diff to the catalogs and the gate; say the word and I will trim them here or in a follow-up.

A note on staleness

The catalog moves daily: the 13 Oct refresh was back in conflict within two hours, and none of the seventeen before this one stayed mergeable for a day. When only the generated en-runtime-required.json conflicts, the seven locale files merge cleanly and the earlier prune carries over and is checked. When a locale file conflicts, the refresh takes all seven from upstream and recomputes the prune from scratch rather than replaying the old deletions. This one was the second case again: 381 commits landed on main between the previous refresh (fd25eb0872, 26 Oct) and dc735050b3 (27 Oct), and all seven locale files conflicted along with en-runtime-required.json. All seven were taken from main and the prune was recomputed in one pass. It came out as the previous 1,637 keys plus 4, and none left the set. #30449 dropped the readers of RepositorySourceControlAiActionRows.7a3a8e431d and SourceControlAiActionRecipeDefaults.2cb4bb7e5d, and it added settings.sourceControl.actionModel.permissionsDefaultHint with no reader: only intermediate commits of that branch read it. #33622 dropped the reader of auto.hooks.ipc.events.browserStateIpcBridge.docPreviewLinkFailed. Each catalog differs from main by exactly the retired keys it has, and the detector finds no unreferenced key on the merged tree. The two test files this branch used to edit were deleted on main (#34136, #34411); the refresh takes the deletions.

With all five tests gone from main, the branch differs from it in 13 files: the seven catalogs, en-runtime-required.json, four scripts and tests under config/scripts, and package.json. The 14 Oct round brought the detector changes from the review threads on #22056: it reports a stranded plural pair in the same pass as its base key, its literal scan has no length floor, it stops when a source root cannot be read, and it has a new file name.

If this sits in review, say the word before merging and I will merge main and recompute against whatever it is then.

Screenshots

No visual change — no English string is added, removed, or reworded. Only entries that no component reads are deleted.

Testing

Run on the merged tree — main at dc735050b3 plus this branch.

  • pnpm lint
  • pnpm typecheck
  • pnpm test — replaced by the Node run below
  • pnpm build — not run for this refresh
  • Added or updated high-quality tests that would catch regressions

pnpm lint includes the gate, since it now lives in verify:localization-extraction. verify:localization-catalogs, verify:localization-catalog, verify:localization-runtime-catalog, verify:localization-coverage and the standalone verify:localization-orphans pass as well. This install skipped Bun's postinstall, so pnpm lint's runner ran with the local Bun 1.3.14 first on PATH. Its flaky-test ratchet scopes itself by origin/main, which is a fork here, so that gate ran the way CI runs it: on a merge commit with main as first parent.

vitest run --config config/vitest.config.ts src/renderer src/shared config/scripts under Node: 6,673 files, 66,718 tests, six files failed with 11 tests between them. None is touched by this change, and the same 11 tests in the same six files fail on a clean checkout of main at dc735050b3 on this machine, run on their own. The run also logged 13 unhandled teardown errors (a module loaded after its test environment was torn down) from eight store, hook and worktree test files. Run together, the same eight files log 10 of them on clean main and 12 on this branch; run on its own, a file logs one or two on some runs and none on others, on main and here alike.

That run was on the final tree, bd7c98ab81.

I did not use the pnpm test wrapper as evidence: it runs Vitest on Bun, and this machine has Bun 1.3.14 rather than the pinned 1.4.2, which fails files unrelated to this change.

New tests: the detector judging a fixture catalog against a supplied extraction without extracting again, and the change-scope case for the detector file under its new name.

CI on this PR has not executed yet — the runs are waiting for "Approve and run".

AI Review Report

Reviewed for the one risk that matters — deleting a key something still reads.

  • Independent of the detector: I checked the 1,476 keys of the original prune against every text file in the repository, 34,905 of them. Outside the catalogs they are named only by five translation-policy tests, which use them as sample inputs and pass unchanged, and by the override tables above — no runtime source mentions one in any syntax. Separately, for keys built at run time: among template literals and concatenations with a static key-shaped part, six can match a retired key by shape, and all six build something else — a storage key, a systemd unit name, a git config key, a log file name, two RPC method names. A template made only of variables matches anything by shape, so for those I checked the call side instead: of the 83 translate() calls whose key is not a plain string literal, 9 pass a template literal and none of them can produce a retired key. The rest pass a key held in a variable, and such a key is spelled out as a literal where the variable is assigned, which is what the detector's literal scan keeps. Each later refresh repeats the search over the files the new commits touch, 4,705 files for the 381 commits of this one. Outside the override tables none of them names a retired key, and all 307 translate() calls they add outside tests pass a string-literal key.
  • Plural variants: the detector resolves the base key first, so a _one/_other pair survives as long as some source reaches its base. AutomationsPage.runCount_one / _other are in the set because nothing reaches runCount; until 14 Oct they surfaced only on a second run, after the base key was pruned.
  • Verified: the detector reports 0 after the prune and --check exits 0. Planting an unreachable key with a plural pair makes verify:localization-extraction exit 1 with all three named; removing it restores exit 0. The 197 keys extraction cannot see statically but a source literal spells out are all retained. One family is built from a namespace constant (${NS}.statusConfigured in token-source-control-status.ts); extraction resolves it and all six of its keys are kept.
  • Formatting: keys are removed line by line, so untouched entries keep their exact bytes, including the empty and one-line objects fix(i18n): translate recent UI copy #32125 left in es.json and fr.json; each file is then checked against a structural prune of the parsed JSON.
  • Cross-platform: data-only edits to catalogs, plus Node scripts that use node:fs/node:path only. No shell invocations or Electron APIs.

Security Audit

  • Attack surface: deletions only. No key that any component resolves is removed — that is the property the detector's two conditions establish and the independent scan confirms.
  • Input handling / command execution / secrets / IPC / dependencies: untouched. The gate adds no process spawn of its own: it reuses the extraction the check already runs. No new dependencies.

A key can leave the code without leaving the catalog: a reverted feature, a
setting rewritten in place, a deleted component. Nothing fails, so the entries
accumulate in every locale and translators spend effort on strings no screen
renders.

Extraction alone cannot call a key dead — i18next-cli does not see a key held
in a data structure or built from a namespace variable. The detector reports a
key only when extraction misses it AND the exact string appears in no source
literal under src/ or mobile/.
Every locale carried them: en 1,380, fr 1,333, zh 1,187, ko 1,186, es and ja
1,184 each. The catalog drops from 14,211 keys to 12,831.

Found with config/scripts/find-orphaned-catalog-keys.mjs, run twice: removing a
base key can orphan its plural variants, so the scan is repeated until it
reports none. Extraction alone would have flagged 1,550 — the extra keys are
held in data structures or built from a namespace variable, and the literal
scan keeps them.

Two test anchors went with the keys they guarded:
- three zh assertions for /goal, origin and 内存, whose keys are gone;
- the whole "Open <Brand> usage details" case — all six of its keys are gone.

en-runtime-required.json is regenerated from the pruned catalog: 300 of 12,831
entries must ship in the boot bundle, down from 2,435.
@smwbev
smwbev requested a review from brennanb2025 as a code owner September 9, 2026 22:45
@coderabbitai

coderabbitai Bot commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
Walkthrough

This change adds localization orphan detection and integrates it into localization verification. The check combines extraction with source-literal scanning, supports plural variants, excludes tests and non-runtime paths, and can fail in check mode. Tests cover scanning, file discovery, plural handling, and integration behavior. The English runtime catalog and supported locale files remove and revise entries and add strings. A native-chat catalog test now permits five shared-copy keys without English catalog entries.

Priority: Normal

Merge Risk: Low · up to 0d536

A comment-only key can escape the orphan check, and users selecting Chinese or Spanish can see an English one-server label. These are bounded issues suitable for owner follow-up.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description is thorough and covers the template's core content: a clear summary of the key-retirement change and gate, the mechanism, testing results, and a security audit. The ELI5, Linked Issue,… Add an ELI5 section in plain language. Link an actual issue in the 'Linked Issue' section using 'Fixes #', or confirm the maintainer exemption applies. Fill in the AI Disclosure section if an AI model was used to produce the change.…
Passed Passed checks: 3 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the main changes: removing unreferenced localization keys and enforcing the orphan scan.
Full details: Description check

Explanation

The description is thorough and covers the template's core content: a clear summary of the key-retirement change and gate, the mechanism, testing results, and a security audit. The ELI5, Linked Issue, and Visual Proof sections are partly off-template: the ELI5 section is missing, and the linked issue is not a 'Fixes #' reference but a stacked PR (#22056). The 'Visual Proof' section is handled with N/A and a reason, which the template allows. The 'Review' and 'Agent skill upstream boundary' sections are not addressed. The AI Disclosure is not filled in. The checklist is not ticked, and 'pnpm test' and 'pnpm build' were not run as the template expects. The 'Linked Issue' requirement for outside contributors is not satisfied by a stacked PR reference, so this is a gap for review.

Resolution

Add an ELI5 section in plain language. Link an actual issue in the 'Linked Issue' section using 'Fixes #<number>', or confirm the maintainer exemption applies. Fill in the AI Disclosure section if an AI model was used to produce the change. Complete the 'Agent skill upstream boundary' checkbox or state why it does not apply. Complete the Checklist items, or state which ones do not apply. Run or explain the missing 'pnpm test' and 'pnpm build' checks.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: f239806a-ac80-4375-b615-2884bf44ec99

📥 Commits

Reviewing files that changed from the base of the PR and between aac38d6 and a559ec7.

📒 Files selected for processing (9)
  • config/scripts/find-orphaned-catalog-keys.mjs
  • src/renderer/src/i18n/en-runtime-required.json
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json
  • src/renderer/src/i18n/zh-technical-literal-mistranslations.test.ts
💤 Files with no reviewable changes (1)
  • src/renderer/src/i18n/zh-technical-literal-mistranslations.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — the prune is sound and well-guarded. One suggestion on preventing regrowth, plus a nit.

Reviewed changes

  • Orphaned-key detector — new config/scripts/find-orphaned-catalog-keys.mjs flags catalog keys that i18next-cli extraction misses and that appear nowhere as a source literal, resolving plural variants to their base key and iterating until it reports zero.
  • Catalog prune — retires 1,380 orphaned keys across en/es/fr/ja/ko/zh (en 14,211 → 12,831) and regenerates en-runtime-required.json down to 300 entries.
  • Test updates — removes three zh assertions and the "Open <Brand> usage details" case whose keys were retired.

I verified the methodology independently: the detector reports 0 unreferenced on the pruned tree, verify:localization-runtime-catalog and verify:localization-extraction both pass, and all six locale files parse as valid JSON. The only template-literal key construction in the repo (token-source-control-status.ts's ${NS}.status…) is protected by integration-card-status-localization.test.ts, which spells each full key literally so the detector's literal scan keeps them.

ℹ️ Detector isn't wired into CI, so orphans will silently re-accumulate

verify:localization-extraction already computes the unreferenced set (it prints 170 existing English entries are not statically referenced) but only exits nonzero for missing-from-English and placeholder-drift keys, so orphaned entries never fail CI. The new detector is a standalone script with no package.json entry, so after this lands nothing will flag the next feature revert that strands keys — the 1,380 will regrow unobserved.

Technical details
# Make the orphan scan a ratchet

## Affected sites
- `config/scripts/find-orphaned-catalog-keys.mjs` — new, not registered anywhere.
- `config/scripts/verify-localization-extraction.mjs:113-119` — reports `orphans` but never fails on them.
- `package.json` — no script entry for the detector.

## Required outcome
- CI fails when a catalog key is both extraction-missed and absent from every source literal
  (the detector's own definition of "orphan"), so the prune can't silently regrow.

## Suggested approach
- Fold the detector's literal-scan pass into `verify:localization-extraction` (or add a new
  `verify:localization-orphans` script and wire it into `lint`). The detector already reports a
  clean `0`, so it can serve as a hard gate directly; the 170 extraction-missed-but-live keys are
  exactly the ones the literal scan preserves.

ℹ️ Nitpicks

  • The PR description says en-runtime-required.json dropped "from 2,435"; the committed file on main has 1,680 leaf entries (already reduced by #19495). The actual reduction in this PR is 1,680 → 300.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

Two review points from stablyai#19801.

The detector scanned test files, so an obsolete assertion could pin a key no
screen renders. It now mirrors the ignore list in config/i18next.config.ts —
*.test.*, *.spec.*, __tests__, __snapshots__, assets. That exposed 11 more
unreachable keys, all of them technical values rather than copy: CSS class
strings, `gh auth login`, `glab auth login`, and identifiers like `upstream`
and `pr-view`. They are retired with the rest; the anchors in
technical-literal-catalog-values.test.ts that named them go too, leaving the
17 entries whose keys are still reachable.

Nothing gated the scan, so the prune would regrow. `verify:localization-orphans`
runs the detector with --check, joins the lint chain next to the other
localization gates, and has a step in pr.yml — pr-workflow-lint-parity requires
the two to match.
@smwbev

smwbev commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Both points taken — fe9044a addresses them.

Tests no longer count as usage. You were right that this was a real hole, not a theoretical one: applying the config/i18next.config.ts ignore list exposed 11 more unreachable keys that only a test literal was keeping alive. All eleven are technical values rather than copy — CSS class strings like text-amber-700 dark:text-amber-300, the commands gh auth login and glab auth login, and identifiers such as upstream, pr-view and combined-branch.

They are retired with the rest, and the anchors in technical-literal-catalog-values.test.ts that named them go too. That test guarded the #13121 repairs; 17 of its 28 keys are still reachable and keep their assertions, so the protection that matters is intact — only the entries pointing at keys no component reads are gone.

The scan is now a ratchet. verify:localization-orphans runs the detector with --check, sits in the lint chain beside the other localization gates, and has a step in pr.yml. pr-workflow-lint-parity caught me halfway: adding the step to lint alone fails that test until pr.yml matches, which is exactly the guard you were asking for. Verified in both directions — clean tree exits 0, and a planted unreachable key exits 1 with the key named.

On the nit: you are right, and my number was wrong. main carries 1,680 leaf entries, already reduced by #19495; this PR takes it to 289. The description is corrected.

Totals after the follow-up: 1,391 keys retired, en.json 14,211 → 12,820.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Wired the orphan detector into CI — added a verify:localization-orphans script that runs the detector with --check (exit 1 on any orphan) and joined it to both the lint chain and .github/workflows/pr.yml, addressing the prior review's "orphans will silently re-accumulate" concern.
  • Stopped counting tests as usage — the literal scan now mirrors config/i18next.config.ts's ignore list (*.test.*, *.spec.*, __tests__, __snapshots__, assets), so a key named only by a test no longer pins it alive.
  • Retired 11 more test-only technical keys — CSS class strings, gh auth login, glab auth login, and identifiers like upstream / pr-view — and trimmed technical-literal-catalog-values.test.ts to the 21 entries whose keys remain reachable, regenerating en-runtime-required.json to 289 entries.

I verified the delta independently: the detector reports 0 unreferenced and --check exits 0; verify:localization-runtime-catalog passes (289 of 12,820); and the parity/contract/localization tests pr-workflow-lint-parity, localization-package-contract, technical-literal-catalog-values, zh-technical-literal-mistranslations, and integration-card-status-localization all pass. The one prior worry — that excluding test files would orphan a runtime-referenced key — is unfounded: the only template-literal construction (token-source-control-status.ts's `${NS}.status…`) is in fact resolved by extraction, which I confirmed with a fresh extraction into an empty temp dir.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

The catalog moved on: 816 commits, en.json 14,211 -> 14,452. Locale files and
en-runtime-required.json are taken from upstream and the prune is recomputed
from scratch rather than replayed, so the result reflects the current tree:
1,402 keys over two passes, en.json 14,452 -> 13,050.

package.json keeps upstream's lint chain and re-inserts verify:localization-orphans
beside the other localization gates.
Recomputed against the merged tree rather than replayed: en.json 14,452 ->
13,050, and the scan converges in two passes because removing a base key can
strand its plural variants.

en-runtime-required.json is regenerated: 303 of 13,050 entries must ship in the
boot bundle.
en.json 14,459 -> 13,056. The catalog moves daily, so the prune is recomputed
against the merged tree rather than replayed.
A key can leave the code without leaving the catalog: a feature is reverted, a
setting is rewritten in place, a component is deleted. Nothing fails, so the
entries stay. Every locale carries them, translators spend effort on strings no
screen renders, and coverage numbers understate how much is actually translated.

`verify:localization-extraction` already reports an unreferenced set, but it
cannot be deleted as-is: i18next-cli does not see a key held in a data structure
(`{ key: '…' }`) or passed as a variable, and 18 files do exactly that. So this
requires two independent conditions — extraction misses the key AND the exact
string appears in no source literal under src/ or mobile/.

Two properties matter and both were found the hard way:

- Plural variants are judged by their base key. i18next appends the CLDR
  category at runtime, so `…_one` never appears in a source file even though the
  base does. Without this the scan eats every plural in the catalog.
- The scan is iterative. Removing a base key can strand the variants that
  survived on its account, so it must be re-run until it reports none.

Registered as `verify:localization-orphans` but deliberately left out of the
lint chain: it reports 1,401 keys on current main, so gating now would fail CI.
The prune and the gate come together in a follow-up.
@smwbev smwbev changed the title chore(i18n): retire 1,380 catalog keys no source references chore(i18n): retire unreferenced catalog keys and gate the scan Sep 21, 2026
@smwbev

smwbev commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Split this in two, since the prune goes stale faster than review can keep up — 816 commits landed between opening it and the last refresh, and 53 more during the refresh itself.

Recomputing the prune is one command and about three minutes, so staleness here is cheap to fix: say the word before merging and I will refresh against whatever main is at that moment. Refreshes take the locale files from upstream and re-run the scan from scratch rather than replaying old deletions, so the result always reflects the current tree.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This run re-reviewed the delta since the prior pullfrog review (fe9044a): the upstream/main merge and the follow-up re-prune.

  • Merged upstream/main — pulled in upstream catalog keys (OpenCode Go, native-chat composer, browser user-agent option, repository GitHub account section) and upstream's expanded lint chain, keeping verify:localization-orphans in both package.json and .github/workflows/pr.yml.
  • Re-ran the orphan prune — retired 1,402 keys (en.json 14,452 → 13,050 after the merge), iterating past the plural variants of AutomationsPage.runCount, and regenerated en-runtime-required.json to 303 entries.
  • Trimmed the technical-literal test anchors — technical-literal-catalog-values.test.ts down to the 21 keys still reachable, and dropped the retired zh assertions.

I verified the delta independently: the detector reports 0 orphans and --check exits 0; verify:localization-extraction and verify:localization-runtime-catalog pass; the affected i18n tests and pr-workflow-lint-parity pass. Reconstructing the deleted set from the pre-prune catalog shows it equals the detector's iterative orphan set — no key referenced by extraction or by a source literal was removed.

ℹ️ Title and body counts lag the merged tree

The prune numbers in the PR description predate the upstream/main merge, so a reader checking the headline claims against the tree will find them off.

Technical details
# Stale PR description numbers

## Affected sites
- Title / body — "1,380 keys" vs the 1,402 actually retired at `95c1da9f`.
- Body — `en.json` "14,211 → 12,820" vs the post-merge 14,452 → 13,050.
- Body — `en-runtime-required.json` "289 of 12,820" vs the post-merge 303 of 13,050.
- Body — "extraction now reports 0 unreferenced entries": `verify:localization-extraction` reports 169 (the live dynamic keys); the new *detector* is the one reporting 0.

## Required outcome
- Description matches the post-merge tree so a merger is not misled about the prune's size or about which gate guarantees what.

## Suggested approach
- Refresh the counts, or fold the refresh into the next `main` sync.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

This run re-reviewed the delta since the prior pullfrog review (95c1da9f): a second upstream/main merge with the prune recomputed from upstream's catalogs.

  • Merged upstream/main again — brought in upstream keys (hosted-review "Generate / Stop generating", OpenCode Go account fields) and picked up upstream's removal of shownRecent in favor of resumePastSessions.
  • Recomputed the prune from upstream's catalogs — en.json 14,459 → 13,056 (1,403 keys) and en-runtime-required.json regenerated to 303 entries; the detector, the package.json gate, and the workflow step are unchanged.

I verified the new tree independently: the detector reports 0 orphans with --check exiting 0; verify:localization-extraction and verify:localization-runtime-catalog pass. Reconstructing the deleted set from the pre-prune catalog (77dc0390^2:…/en.json, 14,459 keys) shows it equals the detector's iterative orphan set (1,403; the two AutomationsPage.runCount variants are the second pass after their base went) — no key referenced by extraction or by a source literal was removed.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Both rules decide whether a key is live, and both are easy to break without
noticing: the plural rule keeps `…count_one` when only `…count` appears in
source, and the literal scan is the sole evidence for keys extraction cannot
see — held in a data structure or passed as a variable.

Extracting them as pure functions changes no behaviour: the detector still
reports 14,459 keys, 12,887 referenced, 1,401 orphans, and --check still
exits 1.
main gained the anti-slop rule no-shape-in-symbol-names after this branch
was cut, and KEY_SHAPED trips it once the branch is merged with main.
Renamed to CATALOG_KEY_PATTERN; no behaviour change.
Brings in the exported helpers, the unit tests and the constant rename the
detector gained in its own PR (stablyai#22056), so the script here matches the one
this branch is stacked on.
Takes every locale catalog, the runtime-required catalog and the touched
tests from main, so this commit is main plus the detector script; the prune
and the gate are recomputed against it in the next two commits.

The old gate wiring is dropped here on purpose. It added a lint-chain entry
and a pr.yml step, and main has since reworked how the localization checks
run, so the gate returns in a form that fits that.
Recomputed from scratch against main: en.json goes from 15,774 entries to
14,300, in two passes because removing a base key strands its plural
variants (1,472, then 2). Every locale sheds the same set, removed line by
line so untouched entries keep their bytes. en-runtime-required.json is
regenerated: 542 entries, down from 2,016.

Four tests asserted values of keys no screen renders, so those assertions
go with the keys. The native-chat shared-copy test now lists the five tool
activity strings desktop no longer resolves through i18n, and fails if a
catalog entry for one of them comes back.
…rences

The earlier gate was a verify:localization-orphans entry in the lint chain
and a matching pr.yml step, which meant a second i18next extraction on
every PR that touches src/. main now runs that AST pass once, so the gate
rides on it: verify-localization-extraction hands the keys it extracted to
the detector, which adds the source-literal scan, and the check fails on
any key neither can reach. No new workflow step, no new lint entry; the
scan itself takes under a second.

The change-scope helper lists the detector as an input so editing it
re-runs the check.
@smwbev

smwbev commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed against main at 094f589977 (9 Oct). The branch had been in conflict since 3 Oct. What changed since the last revision:

The prune is recomputed from scratch. 1,474 keys; en.json goes from 15,774 entries to 14,300.

The gate no longer costs a CI step. The previous revision added verify:localization-orphans to the lint chain and to pr.yml, which meant a second i18next extraction on every PR that touches src/. Since #26806 made that pass run once, the gate now rides on it: verify-localization-extraction.mjs hands the keys it extracted to the detector and fails on any key that no source literal spells out either. pr.yml and the lint chain are identical to main. The literal scan itself measures 0.7 s locally.

One test outside i18n/ is touched, and it is the part I would most like a second pair of eyes on: native-chat-shared-copy-matches-catalog.test.ts. Five NATIVE_CHAT_TOOL_ACTIVITY_COPY entries have no desktop call site any more, so their catalog keys are among the retired ones. The description has the details. If any of the five is about to be rendered on desktop again, say so and I will keep its key.

#22056 picked up a one-line rename (KEY_SHAPED → CATALOG_KEY_PATTERN): main's no-shape-in-symbol-names rule rejects the old name once that branch meets main.

Checked locally on the merged tree: pnpm lint, pnpm typecheck, and vitest run src/renderer src/shared config/scripts under Node — 6,055 files, 57,065 tests, with three failing files that fail identically on a clean checkout of 094f589977 here. CI on this PR still needs "Approve and run".

As before, the catalog moves daily. If this gets close to merging, tell me and I will merge main and recompute first.

main moved 310 commits since the last refresh (094f589 to
215ee3e) and the six catalogs conflicted again. The locale files and
en-runtime-required.json are taken from main, and the prune is
recomputed against it rather than replayed: 1,475 keys in two passes
(1,473, then 2 plural variants stranded by the first), so en.json goes
from 15,776 entries to 14,301. en-runtime-required.json is regenerated
at 532 of 14,301.

The gate, the detector and the four test edits merge without conflict.
@smwbev

smwbev commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor Author

Refreshed against main at 215ee3e625 (11 Oct). The 10 Oct refresh was back in conflict within a day: 310 commits landed in between, 30 of them touching en.json.

Only the catalogs changed in this round. The gate, the detector and the four test edits merged without conflict.

  • The prune is recomputed, not replayed. 1,475 keys in two passes (1,473, then the runCount plural pair stranded by the first); en.json goes from 15,776 entries to 14,301. en-runtime-required.json is regenerated at 532 of 14,301.
  • The gate still rides on the single extraction pass. main reworked pr.yml and verify-localization-extraction.mjs again in the meantime (@orca/process-host), and the check slots into it unchanged: no workflow step and no lint entry of its own.
  • Checked on the merged tree: pnpm lint, pnpm typecheck, the localization verifiers, and a planted unreachable key with a plural pair, which fails verify:localization-extraction naming all three keys. The Node vitest run over src/renderer, src/shared and config/scripts is 6,114 files; the description has the five failing files and why none of them is this change.

The question about the five components.native-chat.tool.* keys is still open, and so is the offer: say the word before merging and I will merge main and recompute once more. CI here still needs "Approve and run".

288 commits landed on main between 9c113ed and 67fe1e1, and all six catalogs and en-runtime-required.json conflicted. They are taken from main and the prune is recomputed in one pass: 1,475 keys. Four Hermes keys left the set because stablyai#28877 deleted them from main, and two Jira keys joined it after stablyai#28590 and stablyai#28612 stopped reading them. en.json goes from 16,098 entries to 14,623; en-runtime-required.json keeps 558 of them.

stablyai#28669 deleted ko-ui-semantic-mistranslations.test.ts, technical-literal-catalog-values.test.ts and zh-technical-literal-mistranslations.test.ts from main. This branch only removed the entries of pruned keys from them, so the deletion is taken.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ebbc80ca-4fd4-4edf-946a-93ba19759284
📥 Commits

Reviewing files that changed from the base of the PR and between 77dc039 and b747481.

📒 Files selected for processing: 13
  • config/scripts/localization-extraction-change-scope.test.mjs
  • config/scripts/verify-localization-extraction.mjs
  • config/scripts/verify-localization-orphans.mjs
  • config/scripts/verify-localization-orphans.test.mjs
  • package.json
  • src/renderer/src/components/native-chat/native-chat-shared-copy-matches-catalog.test.ts
  • src/renderer/src/i18n/en-runtime-required.json
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Comment thread src/renderer/src/i18n/locales/es.json Resolved
312 commits landed on main between 67fe1e1 and 5db05e8, and the es, ja, ko and zh catalogs and en-runtime-required.json conflicted. All six catalogs are taken from main and the prune is recomputed in one pass: 1,480 keys. Five keys joined the set and none left: four labels of the old disk-usage panels that stablyai#29513 replaced, and workspaceSpace.chart.workspaceSizes, which stablyai#29567 stopped reading and which exists only in en.json. en.json goes from 16,175 entries to 14,695; en-runtime-required.json keeps 567 of them.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e0617793-9de5-4f22-a0de-9ab0e576858b
📥 Commits

Reviewing files that changed from the base of the PR and between b747481 and 0d5365b.

📒 Files selected for processing: 8
  • package.json
  • src/renderer/src/i18n/en-runtime-required.json
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json
🚧 Files skipped from review as they are similar to previous changes: 2
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Comment thread src/renderer/src/i18n/locales/zh.json Resolved
smwbev added 20 commits October 18, 2026 02:02
250 commits landed on main between 5db05e8 and 6fcc183, and the fr catalog and en-runtime-required.json conflicted. All six catalogs are taken from main and the prune is recomputed in one pass: 1,504 keys. 24 keys joined the set and none left: twenty labels and search keywords of the Setup Script Location terminal setting that stablyai#29819 removed, three Space page labels dropped by stablyai#29857, and the tab close-button tooltip removed by stablyai#29851. en.json goes from 16,241 entries to 14,737; en-runtime-required.json keeps 568 of them.
165 commits landed on main between 6fcc183 and 91b696b, and only en-runtime-required.json conflicted; it is taken from main and regenerated. The six catalogs merged cleanly, so the earlier prune carries over. On the merged tree the detector found two more keys, the Hide front matter and Show front matter menu items that stablyai#30129 replaced with a Front Matter checkbox, so the prune is now 1,506 keys and none left the set. en.json goes from 16,309 entries to 14,803; en-runtime-required.json keeps 569 of them.
227 commits landed on main between 91b696b and 1529b0f, and only en-runtime-required.json conflicted; it is taken from main and regenerated. The six catalogs merged cleanly, so the earlier prune carries over. On the merged tree the detector found three more keys: the Checking chat support placeholder and the Could not build launch command message that stablyai#29074 left behind when it reworked the + menu, and the native-chat cannotResume message whose last reader stablyai#29150 removed. The prune is now 1,509 keys and none left the set. en.json goes from 16,395 entries to 14,886; en-runtime-required.json keeps 578 of them.
208 commits landed on main between 1529b0f and ebc8b06, and all six catalogs conflicted along with en-runtime-required.json, so the catalogs and en-runtime-required.json are taken from main and the prune is recomputed in one pass. It is the previous 1,509 keys plus 109: 108 lost their reader when stablyai#31082 folded the six Remote Orca Servers sections into one list and stablyai#31099 retitled the SSH host section, and SshTargetList.selectGroup arrived with stablyai#31082 without a reader. None left the set. en.json goes from 16,704 entries to 15,086; en-runtime-required.json keeps 579 of them.
414 commits landed on main between ebc8b06 and f045046. The six older catalogs conflicted along with en-runtime-required.json, and pt.json, which stablyai#31627 added to main, came in with them, so all seven catalogs and en-runtime-required.json are taken from main and the prune is recomputed in one pass. It is the previous 1,618 keys plus 6: three lost their reader when stablyai#31831 combined untracked files into Changes, two when stablyai#31593 added the PTY inventory to the Resource Manager, and SshStatusSegment.connecting with stablyai#32083. None left the set. en.json goes from 16,929 entries to 15,305; en-runtime-required.json keeps 597 of them.

native-chat-shared-copy-matches-catalog.test.ts conflicted because stablyai#31425 added a test right above the block this branch rewrites. The branch's edit was re-applied on main's version of the file with git apply -C1 ("Context reduced to (1/1) to apply fragment at 45"); the file now differs from main by exactly the branch's added and removed lines.
187 commits landed on main between f045046 and 7cafdb0. All seven catalogs conflicted along with en-runtime-required.json, so they are taken from main and the prune is recomputed in one pass. It is the previous 1,624 keys plus 9: five lost their reader when stablyai#32284 unified the single and multiple PR hover details, two when stablyai#31688 moved skill updates to Orca's own installer, and two when stablyai#28709 reworked where chat agents are offered. None left the set. Since stablyai#32782 filled the missing entries in the other catalogs, all seven now shed the same 1,633 keys. en.json goes from 16,992 entries to 15,359; en-runtime-required.json keeps 615 of them.

native-chat-shared-copy-matches-catalog.test.ts merged cleanly this time and still differs from main by exactly the branch's edit.
248 commits landed on main between 7cafdb0 and dbde821. Only zh.json conflicted this time, but one conflicting catalog is enough, so all seven are taken from main along with en-runtime-required.json and the prune is recomputed in one pass. It came out as exactly the previous 1,633 keys: none joined the set and none left it. All seven catalogs shed the same 1,633 keys. en.json goes from 17,037 entries to 15,404; en-runtime-required.json keeps 617 of them.

native-chat-shared-copy-matches-catalog.test.ts merged cleanly again and still differs from main by exactly the branch's edit.
338 commits landed on main between dbde821 and fd25eb0. All seven catalogs and en-runtime-required.json conflicted, so they are taken from main and the prune is recomputed in one pass: the previous 1,633 keys plus 4, none left the set. stablyai#32971 dropped the readers of settings.agents.loadingHost and permissionChoiceDescription, stablyai#32334 the reader of permissionEachOwnDescription, and stablyai#31859 the reader of auto.hooks.useIpcEvents.56d3ec4203. The two permission descriptions exist only in en.json, so en.json sheds 1,637 and the other six catalogs 1,635. en.json goes from 17,143 entries to 15,506; en-runtime-required.json keeps 643 of them.

catalog-nested-settings.test.ts listed loadingHost among the agent settings it checks across catalogs; with its last reader gone, the key comes off that list and the test checks the other ten.

native-chat-shared-copy-matches-catalog.test.ts merged cleanly again and still differs from main by exactly the branch's edit.
381 commits landed on main between fd25eb0 and dc73505. All seven catalogs and en-runtime-required.json conflicted, so they are taken from main and the prune is recomputed in one pass: the previous 1,637 keys plus 4, none left the set. stablyai#30449 dropped the readers of RepositorySourceControlAiActionRows.7a3a8e431d and SourceControlAiActionRecipeDefaults.2cb4bb7e5d and added settings.sourceControl.actionModel.permissionsDefaultHint without one; stablyai#33622 dropped the reader of auto.hooks.ipc.events.browserStateIpcBridge.docPreviewLinkFailed. Two keys exist only in en.json and pt.json never had permissionsDefaultHint, so en.json sheds 1,641, pt.json 1,638 and the other five catalogs 1,639. en.json goes from 17,395 entries to 15,754; en-runtime-required.json keeps 691 of them.

main deleted both test files this branch edited: catalog-nested-settings.test.ts (stablyai#34136) and native-chat-shared-copy-matches-catalog.test.ts (stablyai#34411). The deletions are taken, so the branch now differs from main in 13 files.
OrcaWin added a commit to smwbev/orca that referenced this pull request Oct 28, 2026
…nd retire 1,644 unreferenced keys

verify-localization-extraction.mjs now filters its own unreferenced set by
source literals (src/ and mobile/), plural base keys and dynamic prefixes
composed inside translate calls, and fails when any key is named nowhere.
That replaces the standalone verify-localization-orphans.mjs script and its
second extraction pass.

The prune is recomputed on current main: 1,644 keys leave all seven locale
catalogs, 285 stale entries leave the key-override tables, and
en-runtime-required.json is regenerated.

Supersedes stablyai#19801.

Co-authored-by: Evgenii <413011+smwbev@users.noreply.github.com>
@OrcaWin

OrcaWin commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

Thanks @smwbev — this landed via #22056, which folds the detector into the extraction check, recomputes the prune on current main (1,644 keys), and gates it in CI. Closing as superseded.

OrcaWin added a commit that referenced this pull request Oct 28, 2026
…nd retire 1,644 of them (#22056)

* chore(i18n): add a detector for catalog keys no source references

A key can leave the code without leaving the catalog: a feature is reverted, a
setting is rewritten in place, a component is deleted. Nothing fails, so the
entries stay. Every locale carries them, translators spend effort on strings no
screen renders, and coverage numbers understate how much is actually translated.

`verify:localization-extraction` already reports an unreferenced set, but it
cannot be deleted as-is: i18next-cli does not see a key held in a data structure
(`{ key: '…' }`) or passed as a variable, and 18 files do exactly that. So this
requires two independent conditions — extraction misses the key AND the exact
string appears in no source literal under src/ or mobile/.

Two properties matter and both were found the hard way:

- Plural variants are judged by their base key. i18next appends the CLDR
  category at runtime, so `…_one` never appears in a source file even though the
  base does. Without this the scan eats every plural in the catalog.
- The scan is iterative. Removing a base key can strand the variants that
  survived on its account, so it must be re-run until it reports none.

Registered as `verify:localization-orphans` but deliberately left out of the
lint chain: it reports 1,401 keys on current main, so gating now would fail CI.
The prune and the gate come together in a follow-up.

* test(i18n): cover the orphan detector's two non-obvious rules

Both rules decide whether a key is live, and both are easy to break without
noticing: the plural rule keeps `…count_one` when only `…count` appears in
source, and the literal scan is the sole evidence for keys extraction cannot
see — held in a data structure or passed as a variable.

Extracting them as pure functions changes no behaviour: the detector still
reports 14,459 keys, 12,887 referenced, 1,401 orphans, and --check still
exits 1.

* chore(i18n): rename the detector's key pattern constant

main gained the anti-slop rule no-shape-in-symbol-names after this branch
was cut, and KEY_SHAPED trips it once the branch is merged with main.
Renamed to CATALOG_KEY_PATTERN; no behaviour change.

* chore(i18n): name the orphan detector like the other localization verifiers

CODEOWNERS routes /config/scripts/*localization*.mjs to the localization owner, and find-orphaned-catalog-keys.mjs matched neither that pattern nor *locale*, so a change to it asked nobody for review. The file now carries the name of the npm script that runs it, verify:localization-orphans.

* fix(i18n): report stranded plural variants in the same pass as their base key

isKeyReferenced kept a plural variant alive whenever en.json still carried its base key, even when no source reached that base key. The base key was reported and the variants were not, so a prune needed a second run to find them. Only a base key that extraction or a source literal references keeps its variants now; the --check message drops the re-run advice and names the override tables, which sit outside the scan roots.

* fix(i18n): let the literal scan accept every key-shaped literal

The scan captured only literals of seven characters or more and then applied CATALOG_KEY_PATTERN, which has no such floor. A live key shorter than that, held in a data structure, would have been reported as an orphan. en.json has no key that short today, so the result is unchanged; the capture now leaves the shape decision to the one pattern.

* fix(i18n): stop the orphan scan when a source root cannot be read

collectSourceFiles swallowed every readdir error, so a directory it could not read contributed no literals and a key named only there would have been reported as safe to delete. Only a missing mobile/ root is skipped now, since a checkout without the mobile app is still a complete desktop scan; any other error stops the scan.

* style(i18n): wrap one line of the orphan detector to the formatter's width

* chore(i18n): fold the orphan-key detector into the extraction check and retire 1,644 unreferenced keys

verify-localization-extraction.mjs now filters its own unreferenced set by
source literals (src/ and mobile/), plural base keys and dynamic prefixes
composed inside translate calls, and fails when any key is named nowhere.
That replaces the standalone verify-localization-orphans.mjs script and its
second extraction pass.

The prune is recomputed on current main: 1,644 keys leave all seven locale
catalogs, 285 stale entries leave the key-override tables, and
en-runtime-required.json is regenerated.

Supersedes #19801.

Co-authored-by: Evgenii <413011+smwbev@users.noreply.github.com>

* chore(i18n): scope the unused-key scan to src/ and require a dotted dynamic prefix

mobile/ edits do not trigger the extraction check, so literals there must not
decide its verdict; a dotless runtime-built prefix would keep unrelated keys alive.

Co-authored-by: Evgenii <413011+smwbev@users.noreply.github.com>

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
Co-authored-by: Evgenii <413011+smwbev@users.noreply.github.com>
@OrcaWin OrcaWin closed this Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants