Repository navigation
fix(localization): stop the repair policy rewriting translated generic terms to English - #12192
AnddyAgudelo wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 1149b8bbc2bd3a01c7756d23498a0e36d0af4789 and 8c51730. 📒 Files selected for processing (16)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (15)
📝 WalkthroughWalkthroughThe localization policy now supports canonical translations for generic terms such as Agent, Commit, Continue, Repo, and Terminal. Mistranslation repair preserves valid localized terms and overlapping words. Locale overrides and policy tests update Japanese, Korean, Spanish, and Chinese terminology. Catalog verification detects translated values that would regress to English and reports them as validation failures. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
config/scripts/locale-generic-ui-terms.mjs (1)
69-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGuard against silent term collisions in
RENDERINGS_BY_TERM.
RENDERINGS_BY_TERMis built from a flat list of[term, renderings]pairs and passed tonew Map(...). If a futureGENERIC_TERM_FAMILIESentry reuses a term string already used by another family, theMapconstructor silently keeps only the last renderings for that term.isCanonicalGenericRenderingandoverlapsCanonicalRenderingwould then silently use the wrong family's renderings for that term, with no error to catch the mistake.Add a duplicate check while building the map so that a colliding term fails fast instead of silently overwriting.
Based on learnings, a prior review flagged the same silent first/last-write-wins risk for mapping data built from array entries: "do not allow duplicate
appVersionmapping entries to silently rely on a first-write-wins (??=) behavior ... deterministically deduplicate or validate that each ... maps to exactly one ... (fail/stop generation if ambiguous)."♻️ Proposed guard against duplicate terms
-const RENDERINGS_BY_TERM = new Map( - GENERIC_TERM_FAMILIES.flatMap((family) => family.terms.map((term) => [term, family.renderings])) -) +const RENDERINGS_BY_TERM = new Map() +for (const family of GENERIC_TERM_FAMILIES) { + for (const term of family.terms) { + if (RENDERINGS_BY_TERM.has(term)) { + throw new Error(`Duplicate generic term "${term}" across GENERIC_TERM_FAMILIES entries`) + } + RENDERINGS_BY_TERM.set(term, family.renderings) + } +}Source: Learnings
config/scripts/verify-localization-catalog.test.mjs (1)
129-159: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a true-positive assertion for
collectGenericTermRegressions.The test title states it "flags catalog values the repair policy would rewrite back to English," but none of the three assertions reach the
regressions.push(...)branch ofcollectGenericTermRegressions:
- Case 1 exits early because
repaired === localeValue.- Cases 2 and 3 skip the inner loop because
localeValuenever contains one of the canonical rendering forms.All three assertions return
[], so the actual positive-detection path of this new regression detector is untested here. Add a case where a repaired value strips a canonical rendering back to the bare English term (a genuine regression), and assert thatcollectGenericTermRegressionsreturns a non-empty array containing the expectedkey/form.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e01efa5b-2d35-4f78-89b6-45894b34b225
📥 Commits
Reviewing files that changed from the base of the PR and between 4481b9b and 1149b8bbc2bd3a01c7756d23498a0e36d0af4789.
📒 Files selected for processing (16)
config/scripts/locale-generic-ui-terms.mjsconfig/scripts/locale-generic-ui-terms.test.mjsconfig/scripts/locale-ja-value-overrides.mjsconfig/scripts/locale-ko-key-overrides.jsonconfig/scripts/locale-ko-value-overrides.mjsconfig/scripts/locale-prose-term-exemptions.mjsconfig/scripts/locale-translation-policy-ko-round5.test.mjsconfig/scripts/locale-translation-policy.es-round5.test.mjsconfig/scripts/locale-translation-policy.ja-round5.test.mjsconfig/scripts/locale-translation-policy.mjsconfig/scripts/locale-translation-policy.test.mjsconfig/scripts/locale-translation-policy.zh-round5.test.mjsconfig/scripts/locale-value-overrides.mjsconfig/scripts/locale-zh-value-overrides.mjsconfig/scripts/verify-localization-catalog.mjsconfig/scripts/verify-localization-catalog.test.mjs
💤 Files with no reviewable changes (1)
- config/scripts/locale-prose-term-exemptions.mjs
Greptile SummaryThis PR fixes a bug in the
Confidence Score: 5/5Safe to merge — all changes are build-time localization scripts with no runtime code path, no new dependencies, and no catalog values modified. The fix is tightly scoped: only the repair policy and its CI gate change. The three classes of rewrites (destructive localized→English, desirable English→localized, spacing normalization) were measured separately and only the destructive class is suppressed. Canonical renderings, nonsense forms, overlap guard, and agent-catalog key-pinning are each covered by dedicated tests. The new CI gate closes the feedback loop that made drift invisible. Files Needing Attention: No files require special attention — all changes are within build-time localization scripts under config/scripts/.
|
| Filename | Overview |
|---|---|
| config/scripts/locale-generic-ui-terms.mjs | New module defining generic-term families and canonical locale renderings; exports isCanonicalGenericRendering and overlapsCanonicalRendering guards used by the repair policy. |
| config/scripts/locale-translation-policy.mjs | 17 generic terms removed from NEVER_TRANSLATE_VALUES; isCanonicalGenericRendering guard added in applyBrandMistranslationFixes; overlapsCanonicalRendering guard added in replaceMistranslatedForm to prevent partial-word replacement bugs. |
| config/scripts/verify-localization-catalog.mjs | New collectGenericTermRegressions check added; verify step now fails when repair policy would rewrite a canonical locale rendering back to English, closing the CI gap that made drift invisible. |
| config/scripts/locale-generic-ui-terms.test.mjs | 8 new tests covering: translated generic terms preserved whole-value and in-sentence, real brands still reverted, nonsense forms still reverted, overlap-safe word handling, English→locale fill-in still works, agent-catalog key-based pinning, and invariant that generic terms are absent from NEVER_TRANSLATE_VALUES. |
| config/scripts/locale-prose-term-exemptions.mjs | File deleted; its key-specific exemption logic is superseded by the locale-wide canonical-rendering guard in locale-generic-ui-terms.mjs. No remaining imports found. |
| config/scripts/locale-ko-key-overrides.json | Single entry updated: SshTargetForm.137e88ce8d now uses 터미널 instead of the English word 'terminals' that was previously hard-coded into the Korean override. |
| config/scripts/locale-zh-value-overrides.mjs | Two fixes: Split Terminal Down/Right overrides updated to use 终端 instead of Latin Terminal; Local project… override updated to use 仓库 instead of English repo/repos. |
| config/scripts/verify-localization-catalog.test.mjs | Two new tests added for collectGenericTermRegressions: canonical translations not flagged as regressions, interpolation placeholder names not misidentified as English terms. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[repairTranslatedValue] --> B{shouldPreserveEnglishValue?}
B -- yes --> C[return enValue]
B -- no --> D{keyOverride or valueOverride?}
D -- yes --> E[use override text]
D -- no --> F[use localeValue]
E --> G[applyBrandMistranslationFixes]
F --> G
G --> H{for each brand in BRAND_MISTRANSLATIONS}
H --> I{enValue contains brand?}
I -- no --> H
I -- yes --> K{for each wrong form}
K --> L{isCanonicalGenericRendering?
brand, locale, wrong}
L -- yes NEW GUARD --> H
L -- no --> M[replaceMistranslatedForm]
M --> N{for each occurrence of wrong}
N --> O{overlapsCanonicalRendering?
brand, locale, value, at, end}
O -- yes NEW GUARD --> P[keep wrong form]
O -- no --> Q[replace with brand Latin]
P --> N
Q --> N
N -- done --> H
H -- done --> R[applyPhraseFixes]
R --> S[return repaired value]
Reviews (2): Last reviewed commit: "fix(localization): stop the repair polic..." | Re-trigger Greptile
…c terms to English Closes stablyai#12113. repair-locale-catalog rewrote ~2400 committed values back to English. Feeding every catalog value through repairTranslatedValue on main (11775 string values per locale): locale before after ko 279 88 ja 461 254 zh 1180 687 es 479 23 Root cause is not only NEVER_TRANSLATE_VALUES. Two lists share one defect: both treat ordinary UI words as brands. - shouldPreserveEnglishValue returns NEVER_TRANSLATE_VALUES.has(enValue), so a value that *is* the word ("Terminal") is pinned to English. 38 ko / 38 ja / 94 zh / 68 es. - BRAND_MISTRANSLATIONS lists each word's *correct translation* as a "wrong form" (ko Terminal: ['터미널']), so applyBrandMistranslationFixes reverts it anywhere inside a sentence. This is the dominant path: 189 of ko's 279. Both populations are the destructive class the issue reports. The 13 desirable English->localized repairs and the ~38 spacing/phrasing normalizations come from the override, phrase-fix and spacing stages, which are untouched here. Scope of the term list (config/scripts/locale-generic-ui-terms.mjs): a word qualifies as generic when it is an ordinary noun/verb whose standard rendering in ko/ja/zh/es is a translation, and whose product-name homonym is already pinned by ENGLISH_ONLY_KEY_PREFIXES. Removed from NEVER_TRANSLATE_VALUES (17): Agent/Agents/agent/agents, Commit/Commits/commit/commits, Repo/Repos/repo/repos, Terminal/Terminals/terminal/terminals, Continue. Continue.dev exists only at auto.lib.agent.catalog.9e2a9bb87b; the other four en.json occurrences are the English verb. Deliberately kept: Markdown/markdown (a format name; caused zero destructive rewrites), Token, idle, ui/ai/ci/ide, lint, MD, and every brand, path and code token. BRAND_MISTRANSLATIONS keeps its entries. Rather than hand-prune each array and duplicate the judgement, the revert now skips a form that is the term's canonical rendering. Genuine nonsense sharing the same English term still reverts: zh 回购 "repurchase agreement", ja/zh 端子 "electrical connector", es Comprometerse "to pledge", zh 降价 / ko 가격 인하 "price reduction". Also fixed, surfaced by the new gate: - 端子 was reverted inside zh 终端子进程 ("terminal sub-process"), cutting a valid word (main produced "Terminal子进程"). Reverts now skip matches overlapping a canonical form. - Seven override entries hard-coded English generic terms into translated sentences (zh Split Terminal Down/Right, the "Local project, Git repo…" value in all four locales, ko SshTargetForm.137e88ce8d). Each is set to the wording already committed in the catalog, so no catalog value moves. locale-prose-term-exemptions.mjs is deleted: it exempted exactly these terms for two macOS TCC keys, which the global rule now subsumes. verify-localization-catalog gains a gate for this drift, since key parity never saw it. It fails when the policy would replace a canonical rendering with its English term, which is zero across all four catalogs now and was 228+ per locale before. A full round-trip assertion is deliberately not added: the remaining 88/254/687/23 differences are the stale catalog that repair-locale-catalog exists to fix, and gating on those would force the catalog-wide rewrite this change avoids. No locale JSON is modified. Tests: locale-generic-ui-terms.test.mjs pins the contract and fails 5/8 on main. Nine assertions across the existing suites encoded the destructive behaviour ("keeps workflow terms in English"); they are updated with the reasoning, not silently flipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1149b8b to
8c51730
Compare
…c terms to English (#12192) Fixes #12113. shouldPreserveEnglishValue keyed on the English value, so any key whose source string equalled a NEVER_TRANSLATE_VALUES entry was forced back to English on every repair run — agent, commit, repo, terminal and Continue were all on that list. Measured on a clean checkout: ko 279, ja 461, zh 1180, es 479 values rewritten, the large majority destroying translator work. 17 generic terms move into locale-generic-ui-terms.mjs, and the brand revert now skips a term's canonical rendering, so genuinely nonsensical forms (zh 回购, ja/zh 端子, es Comprometerse) still fire while 터미널/커밋/エージェント survive. Brand, path, and code tokens are untouched — MD -> 医学博士 and HEAD -> CABEZA are why that list still earns its keep. No catalog values change; every file is under config/scripts/. Co-authored-by: AnddyAgudelo <44873492+AnddyAgudelo@users.noreply.github.com>
The override for SshTargetForm.137e88ce8d held a truncated relay-TTL sentence ending mid-clause at "최대:", which renders "Timeout after disconnect (seconds)" — the sibling key 55c56cf2c7 — not its own English source. #12192 only corrected the terminals token inside that wrong sentence. Point it at the value ko.json already ships, so a catalog repair cannot overwrite the correct string with the wrong one.
|
Merged into Closing this in favour of that. Thank you for the fix, and sorry it took as long as it did to get through. |
2 participants
Summary
Fixes #12113.
repair-locale-catalogrewrote translated values back to English on every run. Any value that translated a term listed inNEVER_TRANSLATE_VALUESwas reverted to the English term, so committed translator wording was temporary — and becauseverify:localization-catalogdidn't flag it and the app never runs the policy, the drift was invisible until someone regenerated the catalog.Measured on a clean checkout (11775 string values per locale): ko 279, ja 461, zh 1180, es 479 values rewritten. Classifying the ko cases showed the shape of the problem — 228 were localized term → English (translator work destroyed), 13 were English → localized (the policy correctly filling in a translation, desirable), and 38 were spacing/phrasing normalization. Only the first class is the defect, so the fix is scoped to it: the other two still happen.
Root cause
shouldPreserveEnglishValuetestedNEVER_TRANSLATE_VALUES.has(enValue)— keyed on the English value. Any key whose English string happened to equal a listed token forced the localized value back to English, with no way to distinguish a genuine brand from an ordinary word that every locale should translate.agent,terminal,commit,repo, andContinuewere all on that list.What changed
NEVER_TRANSLATE_VALUESinto a newconfig/scripts/locale-generic-ui-terms.mjs:Agent/Agents/agent/agents,Commit/Commits/commit/commits,Repo/Repos/repo/repos,Terminal/Terminals/terminal/terminals,Continue. Criterion: an ordinary noun or verb whose standard ko/ja/zh/es rendering is a translation, and whose product homonym is already pinned byENGLISH_ONLY_KEY_PREFIXES. ForContinuethe catalog was checked directly — the product sense exists only atauto.lib.agent.catalog.9e2a9bb87b; the other four occurrences are the English verb.Markdown/markdown(format name; it caused zero destructive rewrites — its only hit was a casing normalization),Token,idle,ui/ai/ci/ide,lint,MD, and every brand/path/code token.MD→ 医学博士 ("Doctor of Medicine") andHEAD→ CABEZA are why that list still earns its keep.BRAND_MISTRANSLATIONSentries were not hand-pruned. Instead the revert skips a form that is the term's canonical rendering, so the judgement lives in one place rather than two. The genuinely nonsensical forms provably still fire: zh 回购 (repurchase agreement), ja/zh 端子 (electrical connector), es Comprometerse, zh 降价 / ko 가격 인하 (price reduction).locale-prose-term-exemptions.mjsis removed — the new mechanism subsumes it, so it would have been dead code.verify-localization-catalognow fails when the policy would replace a canonical rendering with its English term: 0 across all four catalogs, from 228+ per locale before.Two further bugs the new gate surfaced
main, which producesTerminal子进程. Reverts now skip matches that overlap a canonical form.Split Terminal Down/Right, theLocal project, Git repo…value in all four locales, koSshTargetForm.137e88ce8d). Each is set to the wording already committed in the catalog, so no catalog value moves.What this does not do
No catalog values are changed. The defect is in the policy, and a catalog-wide regeneration would be unreviewable and would bury the actual fix. Every file in this PR is under
config/scripts/.A residual class remains that this does not gate: 88 ko / 254 ja / 687 zh / 23 es values that the policy would still rewrite. Those are the stale-catalog class
repair-locale-catalogexists to repair — gating on them would force exactly the catalog-wide rewrite described above. A separate residual of localized-term-to-English rows (5 ko / 6 ja / 39 zh) is unrelated: theme names (Blue,Zinc),reviewed-by,/path/to/destination, and the repo'sJira Issueconvention.Screenshots
No visual change — these are build-time localization scripts. No runtime UI is touched.
Testing
pnpm lint(all three localization gates green)pnpm typecheckpnpm testpnpm buildconfig/scripts/locale-generic-ui-terms.test.mjs(8 tests). Fail-before/pass-after verified by running it both ways: copied onto pristinemainit produces 5 failures, exit 1 — including 터미널 글꼴과 동일 →terminal 글꼴과 동일, the example from #11573. On this branch, 8 pass.Nine existing assertions encoded the destructive contract (keeping workflow terms in English, keeping
Terminalas a product surface term). They were found by temporarily converting the suite toexpect.soft— Vitest short-circuits each block at the first failure, so a naive run hid 13 further failing assertions. Each is updated with aWhy: #12113comment and its block renamed; the contract changed, and every genuine-brand assertion in those same blocks still passes untouched.npx vitest run config/scripts/→ 643 passed, exit 0.pnpm testexits 1 with 7 failures, all pre-existing and none reachable from anything this PR changes (nothing undersrc/imports a modified module — verified by grep). Running the six implicated files on pristinemainreproduces 5 of them; the other 2 aggregate large batches and fuzz ~800 near-cap payloads, take 32 s and 47 s under full-suite load, and pass in isolation — load-sensitive timeouts on this machine.AI Review Report
overlapsCanonicalRenderingcheck runs per revert candidate during catalog repair, an offline script. No runtime path.BRAND_MISTRANSLATIONSwas left intact and the nonsense forms were verified to still fire, rather than pruning the list by hand..github/CODEOWNERSroutesconfig/scripts/*locale*.mjsand*localization*.mjsfor focused review. Every term added or removed from a policy list is justified above, with the catalog evidence for the one ambiguous case (Continue).Security Audit
No runtime code, IPC, command execution, network calls, auth, secrets, or new dependencies. These scripts run offline against repo-local JSON. Path handling is unchanged. The only behavioural change is which string replacements a build-time policy performs. No follow-up needed.
Notes
ko SshTargetForm.137e88ce8d's override text is a different sentence from its English source — a mis-keyed override. Only the English-term injection it caused is fixed here; the mis-keying is left alone as a separate concern.