Repository navigation
Conversation
The copy is built with a template literal (it interpolates the platform
shortcut label), so the localization codemod never wrapped it in
translate() and it rendered hard-coded English in every locale. Wrap it
with {{value0}} interpolation and add catalog entries for all locales.
📝 WalkthroughWalkthroughUpdated browser link routing descriptions to use i18n translation with the computed shortcut label passed as 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6bb0e0e0-7a11-49fd-8b58-2aa1beb6892c
📒 Files selected for processing (6)
src/renderer/src/components/settings/browser-search.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.json
👮 Files not reviewed due to content moderation or server errors (1)
- src/renderer/src/i18n/locales/ja.json
…cut hint
The shortcut label read as the sentence's actor and a hard-coded particle
could mismatch other labels; switch to the {{value0}}을(를) 누르면 form,
matching the action-oriented style of the existing Korean catalog.
Existing browser-search coverage only asserted the English copy, which was byte-identical before and after the fix. Add a locale-switching test that fails on main for every non-English catalog and pins the shortcut label per platform. Original patch by @m-a-king. Co-authored-by: Orca <help@stably.ai>
The existing assertion (no leaked {{...}}) cannot fail on main: the
hardcoded English literal has no placeholder either. Switch to ko and
assert the rendered copy matches the catalog entry.
Co-authored-by: Orca <help@stably.ai>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/components/settings/browser-search.test.ts (1)
11-19: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReset the global i18n state around every test.
The first suite assumes the shared
i18nsingleton is already English, while the localization suite changes it to Korean and only restores English if all assertions pass. A failed assertion can leak locale state into later tests. Add file-level setup/cleanup, or usetry/finallyaround the locale switch.Suggested test isolation
-import { beforeEach, describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +beforeEach(async () => { + await i18n.changeLanguage('en') +}) + +afterEach(async () => { + await i18n.changeLanguage('en') +}) + describe('browser settings search copy', () => { ... describe('Link Routing description localization', () => { - beforeEach(async () => { - await i18n.changeLanguage('en') - })Also applies to: 34-40, 57-59, 68-83
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a80e3600-9d01-4a09-a006-bd43de10da05
📒 Files selected for processing (1)
src/renderer/src/components/settings/browser-search.test.ts
…tion stablyai#10991 rebuilt getBrowserLinkRoutingDescription with a modifierInverts branch as hardcoded English, which conflicted with the translate() wrap. The resolution keeps both intents: the invert-off branch reuses the existing 904ce58440 catalog entry untouched, and the invert-on base sentence gets a hand-named key — its codemod hash collides with 904ce58440, whose source text was this same sentence as the template head — plus ko/ja/zh/es entries. The localization tests now cover the invert-on variant as well.
Greptile SummaryThis PR fixes a localization gap where the Link Routing description in Settings → Workflow → Browser always rendered in English regardless of the UI language, because the string was a bare template literal that the localization codemod couldn't classify as a translatable candidate. The fix wraps the copy in
Confidence Score: 5/5Safe to merge — pure renderer string change with no runtime behavior differences for English users and a targeted, well-tested fix for non-English locales. The change is narrow: one translate() call replaces one template literal, five catalog entries are added, and the TypeScript build is clean. All ten locale x platform combinations are covered by new tests confirmed to fail on main and pass with the fix. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| src/renderer/src/components/settings/browser-link-routing-copy.ts | Renamed from browser-link-routing-modifier-copy.ts; adds getBrowserLinkRoutingShortcutLabel and the localized getBrowserLinkRoutingDescription with correct translate() calls and {{shortcut}} interpolation. |
| src/renderer/src/components/settings/browser-search.ts | Removes the old unlocalized getBrowserLinkRoutingDescription and imports it from browser-link-routing-copy; call site passes platform explicitly so no default-parameter regression. |
| src/renderer/src/components/settings/BrowserPane.tsx | Import updated from browser-search to browser-link-routing-copy; TypeScript build is clean. |
| src/renderer/src/components/settings/BrowserLinkRoutingModifierSetting.tsx | One-line import path update reflecting the file rename; no functional change. |
| src/renderer/src/components/settings/browser-link-routing-localization.test.ts | New regression guard covering all 5 locales x 2 platforms; asserts translation differs from English, shortcut label is preserved, and no raw {{...}} placeholder leaks. |
| src/renderer/src/components/settings/browser-search.test.ts | Adds a Korean round-trip test and a catalog-key-existence check; both fail on main and pass with the fix. |
| src/renderer/src/i18n/locales/en.json | Adds BrowserLinkRoutingSetting.description (with {{shortcut}}) and descriptionBase; English fallback text is byte-identical to the old literal. |
| src/renderer/src/i18n/locales/ko.json | Korean entry correctly handles grammatical particle ambiguity for both platform shortcut labels. |
| src/renderer/src/i18n/locales/es.json | Spanish translation added for both description and descriptionBase. |
| src/renderer/src/i18n/locales/ja.json | Japanese translation added for both description and descriptionBase. |
| src/renderer/src/i18n/locales/zh.json | Simplified Chinese translation added for both variants with no English fallbacks. |
Reviews (2): Last reviewed commit: "refactor(i18n): move Link Routing copy i..." | Re-trigger Greptile
…odule pattern The interim state mixed a codemod hash key with a hand-named key inside browser-search.ts — forced by a hash collision: the invert-on base sentence is byte-identical to the template head that produced the full-copy key's hash. Align with the copy-module pattern stablyai#10991 introduced instead: rename browser-link-routing-modifier-copy.ts to browser-link-routing-copy.ts (it now carries all Link Routing copy), re-key both description variants as hand-named entries (BrowserLinkRoutingSetting.description / .descriptionBase), and use a named {{shortcut}} placeholder to match the module's convention. Catalog values move verbatim across all five locales, so the reviewed translations are unchanged. Platform detection stays in browser-search.ts — the copy module keeps its resolved-arguments-only contract, and the description's unused default-platform argument is dropped.
…9444) The description was assembled from a bare template literal, so it stayed English under every language pack. It now lives in the catalog as two entries — description with a {{shortcut}} placeholder and descriptionBase for the invert-on variant — following the copy-module pattern from #10991. Co-authored-by: 조재중 <126754298+m-a-king@users.noreply.github.com>
|
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. |
5 participants
Summary
With a non-English UI language, the Link Routing description in Settings → Workflow → Browser rendered hard-coded English while every sibling string on the pane is localized.
getBrowserLinkRoutingDescription()returned a bare template literal — because it interpolates the platform-specific shortcut label, the localization codemod never classified it as a candidate, so it never entered the catalog.This PR wraps the copy in
translate()with a placeholder for the shortcut label and adds catalog entries for all five locales (en,es,ja,ko,zh), following each catalog's existing vocabulary. The same description feeds the pane's search entry, so settings search now matches the localized copy too. The entries originally shipped under the codemod's hash key with{{value0}}; after #10991 landed they were re-keyed to the hand-named copy-module convention — see Update (2026-07-31).Three PRs independently fixed this bug; this one was kept, and #9681 (@Chang-Jin-Lee) and #9654 (@seandoesdev) were closed in its favour. From #9654 I ported the regression guard asserting the shortcut is actually interpolated — no raw
{{...}}leaks into the rendered copy — generalized tonot.toMatch(/\{\{.+?\}\}/)so it catches any placeholder name rather than one specific name. #9681 independently confirmed the same root cause and the same{{value0}}convention, which raised confidence in this approach.Update (2026-07-31)
Two things happened after the sections below were written:
feat(browser): let Shift invert link routing instead of always forcing the system browser #10991 landed on
mainand rewrotegetBrowserLinkRoutingDescription(). The description now has two variants — with the Shift-invert option on, the "always uses your system browser" sentence is dropped — and both variants were rebuilt as hardcoded English literals, which is exactly the regression class this PR's tests guard against.origin/mainwas merged into the branch (5e1c45f) and the localization re-applied onto the two-variant shape: the invert-off variant reuses the reviewed catalog values untouched, and the invert-on base sentence gets its own complete entry per locale (no suffix-stitching — sentence joining differs across locales, e.g.ja/zhjoin sentences without a space).The keys moved to the copy-module convention feat(browser): let Shift invert link routing instead of always forcing the system browser #10991 itself introduced (73760e7). The invert-on variant's text is byte-identical to the whitespace-compacted template head that produced
904ce58440(per the derivation under Notes), so the codemod hash formula collides — it cannot key both variants. Rather than mixing one hash key with one hand-named key, both entries now follow the hand-named pattern of the feat(browser): let Shift invert link routing instead of always forcing the system browser #10991 copy module (titleOrca,descriptionOrca,{{chord}}):browser-link-routing-modifier-copy.ts→ renamedbrowser-link-routing-copy.ts, now carrying all Link Routing copy (description variants, shortcut label, modifier rows);auto.components.settings.BrowserLinkRoutingSetting.description/.descriptionBase, placeholder{{value0}}→{{shortcut}};browser-search.ts— the copy module keeps its resolved-arguments-only contract.Invert-on variant rendered output (the invert-off output is unchanged from the block under Proof of fix):
Full gates at
73760e78a:pnpm lint✓ ·pnpm typecheck✓ ·pnpm build✓ ·pnpm test41,470 passed / 4 failed — all four unrelated to this diff: two insrc/relay/agent-exec-handler.test.ts, which assert the exact spawned-process environment and fail identically on a pristineorigin/maincheckout under the same dev shell (it injects extra variables — e.g.GIT_CONFIG_COUNTdoubles under nesting); and two insrc/main/native-chat/transcript-watch-liveness.test.ts, which pass 7/7 in isolation and fail only under full-suite parallel load.verify-localization-catalog.mjs✓, and the Link Routing localization tests pass 15/15.ELI5
One line of text in the settings screen was written directly into the code instead of being put in the app's dictionary of translated phrases. Every other line on that screen was in the dictionary, so when you switched Orca to Korean, Japanese, Spanish, or Chinese, that one sentence stubbornly stayed in English. It got skipped because it has a keyboard shortcut glued into the middle of it, and the tool that collects text for translation didn't know how to handle that. Now the sentence lives in the dictionary too, with a slot where the shortcut goes — so it translates, and the shortcut still shows the right keys for your operating system.
Proof of fix
The pre-existing
browser-search.test.tscould not catch this: English output is byte-identical before and after, so it passed onmaineither way. Addedbrowser-link-routing-localization.test.ts, which switches the catalog through every non-English locale and asserts the copy actually changes and the platform label survives.Failing on
origin/main(source + locales reverted to main, new test kept):A second guard was added in
browser-search.test.tsitself, so the file that previously could not fail onmainnow does. It switches tokoand asserts the rendered description equals the catalog entry with the shortcut interpolated:With the fix restored, all six pass:
Passing with the fix:
The catalog key originally used here (
904ce58440) is the one the codemod itself would generate — see Notes for the derivation, which is the load-bearing check on a PR like this. It has since been re-keyed to the hand-named convention (see Update (2026-07-31)). Both catalog gates are green:Rendered output, all five locales × both platform branches:
Proof of no regressions
No user-visible behavior changes beyond the fix. Under English the rendered string is byte-identical to before — the fallback preserves the original wording verbatim, so
translate()returns the same characters. The only change any user sees is that the four non-English locales now render translated copy where they previously rendered English. The same function feeds the settings-search entry for this row, so search now matches on the localized copy too, which is the intended behavior for a localized description.Compatibility
getBrowserLinkRoutingShortcutLabel()platform branch (⇧⌘-clickon macOS,Shift+Ctrl+clickelsewhere) and is passed as an interpolation value, so it stays literal and is never translated. Per AGENTS.md the modifier is not hardcoded; the platform check is untouched. Verified by asserting both platform branches across all five locales, and that neither platform's label leaks into the other's copy.translate()call replaces one template literal, on a settings pane rendered on demand. The catalog entry adds ~140 bytes per locale, and the non-English catalogs are already lazy-loaded on language switch.Screenshots
Before (Korean UI — the Link Routing description stays English):
https://github.com/user-attachments/assets/79ebc828-8046-4f1a-b7cf-607f669ae77b
After (same pane — description localized, platform shortcut label preserved):
The rendered-output block under Proof of fix covers the remaining four locales and both platform branches, which is not capturable in a single screenshot.
Testing
pnpm lint— the constituent gates covering this change all pass:verify:localization-catalog,verify:localization-coverage, andoxlinton the three changed files. The stale-baselint:switch-exhaustivenesscaveat below is resolved — fullpnpm lintis clean at 73760e7 (see Update (2026-07-31)).pnpm typecheck— all three projects clean at 73760e7 (see Update (2026-07-31)).pnpm test— scoped runs described below, plus the full suite at 73760e7 (see Update (2026-07-31)).pnpm build— passes at 73760e7 (see Update (2026-07-31)).browser-link-routing-localization.test.tsfails onmainand passes here;browser-search.test.tsgained a placeholder-leak guard ported from fix(i18n): localize browser link-routing description #9654.Commands actually run, in full:
pnpm run lint:switch-exhaustivenessreported one error insrc/renderer/src/components/skills/skill-freshness-group.tsx, which this PR does not touch. It was an artifact of this branch's base being 499 commits behindmain:mainfixed that file in e133d93. Resolved —origin/mainwas merged into the branch on 2026-07-30, and the command is clean at 73760e7.AI Review Report
Reviewed as maintainer with an AI coding agent, adversarially, against the three competing PRs for this bug.
⇧⌘-click) vs non-Mac (Shift+Ctrl+click) ingetBrowserLinkRoutingShortcutLabel(), and that branch is untouched by this PR. The label is passed as an interpolation value, so it is never sent through translation and cannot be corrupted by a translator. I verified interpolation on both platform branches in every locale — all ten combinations are in the rendered-output block under Proof of fix — and the test additionally asserts neither platform's label appears in the other's copy. Per AGENTS.md nometaKey/modifier is hardcoded. No paths, shell behavior, keyboard handling, or Electron platform APIs touched.browser.searchentries for house style and vocabulary. Korean particle handling was flagged by CodeRabbit and fixed in 1093c31 ({{value0}}은 항상…→{{value0}}을(를) 누르면 항상…), which reads correctly regardless of the trailing consonant of the interpolated label.zhincluded — no English fallbacks left in non-English catalogs, which for this specific bug would have left the reported symptom visible to the user.Security Audit
Thin surface, and honestly so: this is a string-catalog change plus one
translate()call. No input handling, no command execution, no path handling, no authentication, no secrets, no dependency changes, no IPC. The interpolated value is a hard-coded platform label rather than user input, soescapeValue: falseinterpolation adds no injection surface. Nothing to follow up.Notes
Key-hash derivation — worth recording, because nothing in CI enforces it.
localize-renderer-strings.mjsderives auto keys assha1("<relative path>:<candidate text>").slice(0, 10). The subtlety is which text. For a template literal,stringParts()emits the head plus every span, butuniqueCandidates()dedupes bystart:end:kind— all parts share the same node range, so only the head survives — andcompactText()(/\s+/g → ' ', then.trim()) runs before hashing.Two consequences:
{{value0}}→{{shortcut}}does not change the key.verify-localization-catalog.mjsnever recomputes hashes — it checks key existence, placeholder consistency, and locale parity only. A wrong hash therefore passes review and CI, then orphans the entry (silently dropping every translation) the next time someone re-runs the codemod.Re-deriving over the
{{value0}}keys already in this catalog confirms the form: restricting to keys whose head ends in whitespace (the only case where trimmed and untrimmed differ), the trimmed head reproduced 29/29 resolvable samples and the untrimmed head 0/55. For this string that yields904ce58440, the key originally used here.{{value0}}is likewise the convention rather than taste — the codemod names interpolations positionally (`value${index}`), and across ~10.9kauto.*keys the placeholders arevalue0(769),value1(141),value2(30) and so on, with named placeholders only in the 218 hand-authored keys.Superseded 2026-07-31: this derivation is precisely why the hash scheme could not survive #10991 — the invert-on variant's text is the compacted template head, so its hash collides with
904ce58440. Both entries now use hand-named keys in the #10991 copy-module convention, where named placeholders are the norm; see Update (2026-07-31). The derivation above stays as the record of that constraint.Other notes:
{{value0}}convention.config/scripts/locale-ko-key-overrides.jsonwas deliberately not touched. fix(i18n): localize browser link-routing description #9654 added an override entry for its key; that mechanism pins curated Korean against the machine-translation pass, and it is not needed here — only ~27% of the keys in this namespace carry one, and per the derivation above the hash does not vary with the placeholder name, so no regeneration path would clobber this value.origin/mainwas merged into the branch on 2026-07-30 (including the feat(browser): let Shift invert link routing instead of always forcing the system browser #10991 conflict — see Update (2026-07-31)), so the unrelatedskill-freshness-group.tsxlint error no longer applies andgit merge-treeremains conflict-free.Closes #9442
Closes #9652
Original patch by @m-a-king.