Repository navigation
fix(i18n): localize titles, bodies, and action labels for every contextual tour step - #12713
Conversation
|
Note Reviews pausedIt 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
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title uses conventional-commit form and names the main change: localizing titles, bodies, and action labels for every contextual tour step. It matches the changeset, which adds IDs to all steps, m… Description check The description follows the repository template. It includes ELI5, What Changed, Why, Linked Issue, Visual Proof, Testing, AI Disclosure, Review, the agent skill boundary, Notes, and Checklist. It exp… Full details: Title check
-
Explanation
The title uses conventional-commit form and names the main change: localizing titles, bodies, and action labels for every contextual tour step. It matches the changeset, which adds IDs to all steps, moves copy lookup into a new module, and adds localized strings across seven locales. It is specific and concise, so it passes.
Full details: Description check
-
Explanation
The description follows the repository template. It includes ELI5, What Changed, Why, Linked Issue, Visual Proof, Testing, AI Disclosure, Review, the agent skill boundary, Notes, and Checklist. It explains the before and after behavior and the mechanism, and it gives the reasoning for the approach. Visual proof is present for the UI change. Gaps: the Linked Issue section says 'None' and cites an upstream PR (
#12713) instead of a linked issue, which the template says outside contributors must provide. The AI Disclosure section is empty. The final lint/typecheck/test/build checklist item is unchecked. The Testing section also says the author ran the tests, but the PR objectives note that some suites failed on the Windows host, which the description does not mention.Resolution
Mostly complete. Consider these fixes before merge: (1) Add a linked issue with 'Fixes #<number>', or explain why none exists, since the template says outside contributors must link an actual issue. (2) Fill in the AI Disclosure section, or leave it empty only if the author is a StablyAI team member. (3) Check the final checklist item only after pnpm lint, typecheck, test, and build pass, or note which ones CI will cover. (4) Note the known failing tests on the Windows host, and state that they also fail on clean main.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 4 files. (7 skipped: 7 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- 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.
Comment @coderabbitai help to get the list of available commands.
Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
![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
🧹 Nitpick comments (1)
src/shared/contextual-tours.ts (1)
32-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider making
idrequired now that every step has one.All steps in
CONTEXTUAL_TOURS, including the ones not touched by this PR, now setid. Keepid?: stringoptional, and a future step added without anidwill compile without error. That step then has no stable anchor for localized copy, and it can silently fall back to the wrong copy or to English only.Change the field to
id: string(non-optional) so the type checker catches a missingidon any new step at the source, instead of relying on the copy-lookup layer or tests to catch it downstream.♻️ Proposed change
export type ContextualTourStep = { // Stable anchor for localized copy — position-keyed translations shift onto the wrong step when one is inserted. - id?: string + id: string title: string
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9a408867-6c00-4ebe-b603-4b680b5d7b57
📒 Files selected for processing (7)
config/scripts/locale-ko-key-overrides.jsonsrc/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.tssrc/renderer/src/components/contextual-tours/contextual-tour-step-copy.test.tssrc/renderer/src/components/contextual-tours/contextual-tour-step-copy.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/shared/contextual-tours.ts
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 53185d10-ad30-4499-85ee-7b70064aa1a5
📒 Files selected for processing (3)
src/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/zh.json
Only the automations and client-hosted-browser steps had an id-keyed entry in the localized step copy map, so every other contextual tour (workspace board, agent sessions, browser, tasks, floating workspace, workspace creation) rendered English titles, bodies, and action labels next to localized Back/Next/Done controls. Give every tour step a required stable id and move the copy lookup into contextual-tour-step-copy.ts. The map `satisfies` a record over all step ids, so a new step without localized copy is a type error. Primary and secondary action labels (Split terminal, Got it, Browser settings) are localized through the same entry. Existing automations and client-hosted-browser keys are unchanged so shipped translations keep resolving. Add the new strings to en and translate them for ko, es, ja, zh, fr, and pt. Co-authored-by: Neil <neil@stably.ai>
3f406b5 to
a72ae15
Compare
nwparker
changed the title
ELI5
When Orca's language is set to something other than English, the little "tour" pop-ups that explain features were still mostly in English, even though their Back / Next / Done buttons were translated. Now every tour pop-up — title, explanation, and buttons — shows up in the chosen language.
What Changed
Before: Only the Automations and client-hosted-browser tour steps had localized copy. Every other contextual tour (workspace board, agent sessions, browser, tasks, floating workspace, workspace creation) rendered English titles and bodies, and action labels like "Split terminal", "Got it" and "Browser settings" were always English.
After: Every tour step's title, body, and primary/secondary action labels are localized in en, ko, es, ja, zh, fr and pt.
Mechanism:
src/shared/contextual-tours.tsnow has a required, stableid, and aContextualTourStepIdtype is derived fromCONTEXTUAL_TOURS.contextual-tour-overlay-measurement.tsintocontextual-tour-step-copy.ts. The map usessatisfies Record<ContextualTourStepId, ...>, so adding a step without localized copy (or leaving a stray key) is a type error.en.jsonand translated for ko, es, ja, zh, fr and pt. Shortcut markers such as{terminal.splitRight}stay verbatim, so the platform-correct shortcut (⌘ on macOS, Ctrl on Linux/Windows) is still substituted.Why
The overlay only translated steps that had both an
idand an entry in a whitelist map. Most steps had noid, so they fell back to the raw English. Keying every step by id and making coverage a compile-time check fixes all of the tours at once and stops the next new step from shipping untranslated. A runtime test that requires a translation for every step in every locale was considered and dropped: under the catalog policy, feature PRs own the English declarations and missing target translations fall back through i18next, so that test would break future feature PRs.Linked Issue
None. Original fix by @5Hyeons in #12713.
Visual Proof
Contextual tour overlay surface, UI language Korean (component fixture screenshots, dark theme).
Testing
Reproduced on
mainby walkingCONTEXTUAL_TOURSinko, mounting each target and callingmeasureContextualTourOverlayRenderState: 32 untranslated titles, bodies or action labels. All of them are localized on this branch.Added a regression test in
contextual-tour-overlay-measurement.test.tsthat covers the title, the shortcut-substituted body, and the action label of a non-automation step inko. It fails onmainand passes here.pnpm test src/renderer/src/components/contextual-tours/ src/shared/contextual-tours.test.ts: 11 files, 71 tests pass.verify:localization-catalog,-catalogs,-runtime-catalog,-extractionand-coverageall pass.oxlint,oxfmt --check, the anti-slop and code-quality oxlint configs,ast-grepand the changed-code quality gate are clean on the touched files.Tested on macOS. The change only adds renderer copy and an id lookup, with no platform, path or IPC code. The shortcut chips are still substituted per platform.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Review
Only the Korean is native-reviewed (by @5Hyeons). The es/ja/zh strings are the contributor's, and fr/pt were added during landing. All of them follow the existing catalog conventions and the
repairTranslatedValuepolicy, but would still benefit from native review.Agent skill upstream boundary
Notes
translate()is only called with constant keys.Original fix by @5Hyeons in #12713. Authorship is preserved on the commit.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)