Repository navigation
Conversation
Greptile SummaryThis PR routes the Automation contextual-tour copy and the overlay's default Next/Done labels through the renderer localization catalog, then adds Korean translations for both. The previous code rendered tour titles and bodies directly from the shared constants (bypassing
Confidence Score: 5/5The change is additive and well-scoped: all touched code paths have existing tests plus new focused regression coverage, and the localization tooling passes confirm catalog consistency. The localization lookup is now keyed by a stable step ID rather than a positional index, the thunk pattern correctly defers translate() to render time, the fallback chain to English copy is intact for non-localized steps, and the new tests cover both the happy path and the step-insertion regression. No incorrect data, broken contracts, or behavioral regressions were found in the changed code. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| src/shared/contextual-tours.ts | Adds optional id field to ContextualTourStep and stamps both automation steps with stable IDs; also corrects the results step body copy. |
| src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.ts | Introduces a LOCALIZED_STEP_COPY map keyed by stable step ID using thunks so translate() resolves at render time rather than module load. |
| src/renderer/src/components/contextual-tours/ContextualTourOverlaySurface.tsx | Routes default "Next" and "Done" labels through translate() so the surface action buttons are fully localized. |
| src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.test.ts | Adds three new Korean-locale tests including a step-insertion stability regression. |
| src/renderer/src/components/contextual-tours/ContextualTourOverlaySurface.localization.test.tsx | New SSR-based surface localization test asserting Korean labels appear and English strings do not. |
| src/renderer/src/i18n/locales/en.json | Adds the complete key and four automation tour copy keys to the English catalog. |
| src/renderer/src/i18n/locales/ko.json | Mirrors English catalog additions with Korean translations. |
| config/scripts/locale-ko-key-overrides.json | Adds five new Korean override keys in sorted order. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[measureContextualTourOverlayRenderState] --> B{activeStep has id?}
B -- yes --> C{id in LOCALIZED_STEP_COPY?}
B -- no --> F[getContextualTourStepCopy]
C -- yes --> D[localizedCopy thunk calls translate at render time]
C -- no --> F
D --> G[formatContextualTourStepCopy]
F --> G
G --> H[renderState.title / .body]
I[ContextualTourOverlaySurface] --> J{isLastStep?}
J -- yes --> K[translate complete key - Done]
J -- no --> L[translate 38b3155418 key - Next]
K --> M[defaultPrimaryAction label]
L --> M
Reviews (3): Last reviewed commit: "fix(i18n): key tour copy off the step id..." | Re-trigger Greptile
|
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. 🎉 ℹ️ 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 d075781e6e11ce02b4f7681e4d35a9343ff6cf4a and 3af175c. 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change adds stable identifiers to contextual tour steps and assigns identifiers to automation introduction and results steps. The automation overlay resolves localized titles and body text by step ID, with fallback to existing metadata. The overlay localizes Next and Done action labels. English and Korean translations include automation guidance and completion text. Tests cover Korean automation content, action labels, locale cleanup, and stable localization after inserting a preceding step. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 32b26b94-7553-4f44-9c66-118ac0c5592c
📒 Files selected for processing (7)
config/scripts/locale-ko-key-overrides.jsonsrc/renderer/src/components/contextual-tours/ContextualTourOverlaySurface.localization.test.tsxsrc/renderer/src/components/contextual-tours/ContextualTourOverlaySurface.tsxsrc/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.test.tssrc/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/ko.json
Localized automation tour copy was selected by matching tour id plus the raw activeStepIndex. Inserting a step ahead of those two, or adding a third, silently shifts the mapping: the intro copy lands on the wrong step and the new one falls through to untranslated English, with no type or test error to catch it. Give ContextualTourStep an optional stable `id` and look the copy up by that instead. Thunks keep translate() out of module scope so the lookup still resolves in the language active at render time, and the four keys stay statically referenced for the extraction gate. Addresses the review comment on stablyai#11694. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/shared/contextual-tours.ts (1)
33-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce the duplicated multi-line rationale comments.
Keep one concise comment for the non-obvious stable-ID requirement. Remove the test comment or reduce it to one line.
src/shared/contextual-tours.ts#L33-L35: Replace the three-line comment with one concise stable-ID rationale.src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.test.ts#L69-L71: Remove the duplicate rationale or replace it with one concise test-specific comment.As per coding guidelines, comments must be concise, limited to non-obvious information, and preferably one line.
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 496cf61d-7063-4432-a35a-c673c6226ce7
📒 Files selected for processing (3)
src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.test.tssrc/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.tssrc/shared/contextual-tours.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.ts
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dc1b2a71-1089-4afd-92f2-89103a49a084
📥 Commits
Reviewing files that changed from the base of the PR and between 09003f6 and d075781e6e11ce02b4f7681e4d35a9343ff6cf4a.
⛔ Files ignored due to path filters (2)
.omo/evidence/automation-tour-ko/automation-onboarding-ko.pngis excluded by!**/*.png.omo/evidence/automation-tour-ko/automation-results-ko.pngis excluded by!**/*.png
📒 Files selected for processing (18)
.omc/project-memory.json.omc/sessions/148ff172-83c2-48d9-b272-f3cc154946a2.json.omc/state/agent-replay-2ace6150-331a-4c4c-b0ca-54062860fd57.jsonl.omc/state/hud-stdin-cache.json.omc/state/idle-notif-cooldown.json.omc/state/session-end-jobs/148ff172-83c2-48d9-b272-f3cc154946a2.json.omc/state/session-end-jobs/discovery.json.omc/state/sessions/148ff172-83c2-48d9-b272-f3cc154946a2/hud-state.json.omc/state/sessions/148ff172-83c2-48d9-b272-f3cc154946a2/pre-tool-advisory-throttle.json.omc/state/sessions/148ff172-83c2-48d9-b272-f3cc154946a2/subagent-tracking-state.json.omc/state/sessions/2ace6150-331a-4c4c-b0ca-54062860fd57/hud-state.json.omc/state/sessions/2ace6150-331a-4c4c-b0ca-54062860fd57/pre-tool-advisory-throttle.json.omc/state/sessions/2ace6150-331a-4c4c-b0ca-54062860fd57/session-started.json.omc/state/sessions/2ace6150-331a-4c4c-b0ca-54062860fd57/subagent-tracking-state.json.omo/evidence/automation-tour-ko/automation-tour-ko-manual-qa.md.omo/evidence/automation-tour-ko/review-ledger.mdsrc/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.test.tssrc/shared/contextual-tours.ts
💤 Files with no reviewable changes (1)
- src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/shared/contextual-tours.ts
| { | ||
| "version": "1.0.0", | ||
| "lastScanned": 1785738167322, | ||
| "projectRoot": "/Users/hans/codes/orca", |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Remove machine-specific absolute paths from committed artifacts.
These records persist /Users/hans/codes/orca. This exposes a developer-specific path and makes the records non-portable.
.omc/project-memory.json#L4: store a repository-relative root or omitprojectRoot..omo/evidence/automation-tour-ko/automation-tour-ko-manual-qa.md#L27-L30: use repository-relative artifact paths or stable CI artifact IDs..omc/state/sessions/2ace6150-331a-4c4c-b0ca-54062860fd57/session-started.json#L4: remove the absolutecwdor keep this runtime file outside version control.
📍 Affects 3 files
.omc/project-memory.json#L4-L4(this comment).omo/evidence/automation-tour-ko/automation-tour-ko-manual-qa.md#L27-L30.omc/state/sessions/2ace6150-331a-4c4c-b0ca-54062860fd57/session-started.json#L4-L4
| "session_id": "2ace6150-331a-4c4c-b0ca-54062860fd57", | ||
| "transcript_path": "/Users/hans/.claude/projects/-Users-hans-codes-orca/2ace6150-331a-4c4c-b0ca-54062860fd57.jsonl", | ||
| "cwd": "/Users/hans/codes/orca", | ||
| "prompt_id": "7c25d3d8-23f2-48ad-b2d3-869bcd6864ca", | ||
| "effort": { "level": "high" }, | ||
| "session_name": "메인으로 체크아웃하기", | ||
| "model": { "id": "claude-sonnet-5", "display_name": "Sonnet 5" }, | ||
| "workspace": { | ||
| "current_dir": "/Users/hans/codes/orca", | ||
| "project_dir": "/Users/hans/codes/orca", | ||
| "added_dirs": [], | ||
| "repo": { "host": "github.com", "owner": "stablyai", "name": "orca" } | ||
| }, | ||
| "version": "2.1.220", |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove generated .omc/state files from this PR.
These files are local runtime output. They disclose session activity. They also expose the local username and absolute /Users/hans/... paths. Remove them from version control and add an ignore rule for generated .omc/state/ content.
.omc/state/hud-stdin-cache.json#L2-L15: Remove cached local session metadata and paths..omc/state/agent-replay-2ace6150-331a-4c4c-b0ca-54062860fd57.jsonl#L1-L1: Remove generated agent replay state..omc/state/idle-notif-cooldown.json#L1-L3: Remove generated notification state..omc/state/session-end-jobs/148ff172-83c2-48d9-b272-f3cc154946a2.json#L32-L59: Remove generated job data and local paths..omc/state/session-end-jobs/discovery.json#L1-L12: Remove generated discovery state..omc/state/sessions/148ff172-83c2-48d9-b272-f3cc154946a2/hud-state.json#L1-L6: Remove generated HUD state..omc/state/sessions/148ff172-83c2-48d9-b272-f3cc154946a2/pre-tool-advisory-throttle.json#L1-L14: Remove generated throttle state..omc/state/sessions/148ff172-83c2-48d9-b272-f3cc154946a2/subagent-tracking-state.json#L1-L41: Remove generated subagent telemetry.
📍 Affects 8 files
.omc/state/hud-stdin-cache.json#L2-L15(this comment).omc/state/agent-replay-2ace6150-331a-4c4c-b0ca-54062860fd57.jsonl#L1-L1.omc/state/idle-notif-cooldown.json#L1-L3.omc/state/session-end-jobs/148ff172-83c2-48d9-b272-f3cc154946a2.json#L32-L59.omc/state/session-end-jobs/discovery.json#L1-L12.omc/state/sessions/148ff172-83c2-48d9-b272-f3cc154946a2/hud-state.json#L1-L6.omc/state/sessions/148ff172-83c2-48d9-b272-f3cc154946a2/pre-tool-advisory-throttle.json#L1-L14.omc/state/sessions/148ff172-83c2-48d9-b272-f3cc154946a2/subagent-tracking-state.json#L1-L41
| | scenario id | criterion reference | surface | exact invocation | verdict | artifactRefs | | ||
| |---|---|---|---|---|---| | ||
| | S1 | English runtime/catalog copy is grammatical and consistent | Shared contextual-tour data, renderer overlay measurement fallback, and `en.json` catalog | `pnpm exec vitest run --config config/vitest.config.ts src/shared/contextual-tours.test.ts`; then `pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.test.ts src/renderer/src/components/contextual-tours/ContextualTourOverlaySurface.localization.test.tsx` | PASS (15 tests total) | A1, A2 | | ||
| | S2 | Localization catalog remains valid | Localization catalog, extraction, and coverage verification scripts | `pnpm run verify:localization-catalog && pnpm run verify:localization-extraction && pnpm run verify:localization-coverage` | PASS | A3 | | ||
| | S3 | Exact locale values preserve English correction and Korean copy | Parsed `en.json`/`ko.json` runtime catalogs | `node - <<'NODE' ... JSON.parse(en.json/ko.json) ... NODE` | PASS; English exact target and Korean exact existing translation observed | A4 | | ||
|
|
||
| ### adversarialCases | ||
|
|
||
| | scenario id | criterion reference | adversarial class | expected behavior | verdict | artifactRefs | | ||
| |---|---|---|---|---|---| | ||
| | A1-adv | English copy | stale-string regression | No runtime/catalog source retains “automations executed”; all automation-results paths use “automations ran”. | PASS; `rg` found only the corrected string in all three runtime/catalog sources. | A2 | | ||
| | A2-adv | Korean unaffected | locale regression | Korean results body remains its existing Korean translation while English changes. | PASS; parsed catalog assertion returned `koreanPreserved: true`. | A4 | | ||
| | A3-adv | Runtime fallback | fallback/source mismatch | Overlay fallback and shared tour data agree with the English catalog text, and targeted UI/localization tests pass. | PASS; 2 files / 6 tests passed plus shared-tour assertions. | A1 | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Add Korean Electron evidence before marking validation complete.
The recorded scenarios cover catalog text, fallback text, and Vitest assertions. They do not show either Korean tour step rendered in Electron or assert the 다음 and 완료 actions. A1–A4 point to this Markdown record instead of inspectable runtime captures. Add one Electron scenario and artifact for each step before keeping PASS. No blockers.
As per the PR objectives, validation must include real Electron captures for both Korean tour steps.
Also applies to: 27-30, 32-45
| |---|---|---|---|---|---| | ||
| | S1 | English runtime/catalog copy is grammatical and consistent | Shared contextual-tour data, renderer overlay measurement fallback, and `en.json` catalog | `pnpm exec vitest run --config config/vitest.config.ts src/shared/contextual-tours.test.ts`; then `pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.test.ts src/renderer/src/components/contextual-tours/ContextualTourOverlaySurface.localization.test.tsx` | PASS (15 tests total) | A1, A2 | | ||
| | S2 | Localization catalog remains valid | Localization catalog, extraction, and coverage verification scripts | `pnpm run verify:localization-catalog && pnpm run verify:localization-extraction && pnpm run verify:localization-coverage` | PASS | A3 | | ||
| | S3 | Exact locale values preserve English correction and Korean copy | Parsed `en.json`/`ko.json` runtime catalogs | `node - <<'NODE' ... JSON.parse(en.json/ko.json) ... NODE` | PASS; English exact target and Korean exact existing translation observed | A4 | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the S3 invocation reproducible.
The exact invocation contains ... and the placeholder en.json/ko.json. Include the complete command or a checked-in verification script with repository-relative paths.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Orca <help@stably.ai>
d075781 to
3af175c
Compare
|
Superseded by #12270, which carries the identical four commits from a clean branch. Closing this one because an earlier push accidentally included local agent runtime state ( |
|
@stablyai maintainers — apologies for the noise here. I accidentally pushed local agent runtime state to this PR, and while I've since removed it, the orphaned commit Whenever you have a moment, would you be able to file a GitHub Support request to delete this PR along with The change itself continues in #12270, so there's nothing to review here. Thanks! |
Summary
Root cause
The shared Automation tour copy was rendered directly without passing through
translate(), while the overlay surface hardcoded its defaultNextandDonelabels.Screenshots
Step 1: Automation introduction
Step 2: Results
Validation
pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/contextual-tours/contextual-tour-overlay-measurement.test.ts src/renderer/src/components/contextual-tours/ContextualTourOverlay.test.tsx src/renderer/src/components/contextual-tours/ContextualTourOverlaySurface.localization.test.tsxpnpm run typecheck:webpnpm run verify:localization-catalogpnpm run verify:localization-extractionpnpm run verify:localization-coveragepnpm run check:max-lines-ratchetFixes #11693