Repository navigation
refactor(i18n): move Settings-only strings into src/lib/i18n/settings.*.ts - #1673
Merged
Merged
Conversation
….*.ts
The 281 keys that only the Settings dialog's files use move, byte for byte, from
src/lib/i18n.{ko,ja,zh}.ts into src/lib/i18n/settings.{ko,ja,zh}.ts. The base
dictionaries keep every other key and compose KO, JA and ZH from their parts
(KO_PARTS and the like), so KO, JA, ZH, t() and every call site are unchanged.
i18n.test.ts skips the part files when it scans for t() calls and fails when a
key sits in two parts (a spread keeps only the last copy), when the three
languages keep a key in different parts, or when a part passes 400 lines.
CONTRIBUTING.md says where a feature's new strings go.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description check |  |
The description clearly explains the change, resulting behavior, implementation details, validation results, and lack of a changelog entry. It uses equivalent headings instead of the template headings… |
| Title check | |
The title clearly and concisely identifies the main change: moving Settings-only strings into language-specific i18n files. |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 7 files. (1 skipped: 1 … |
| 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. |
Full details: Description check
-
Explanation
The description clearly explains the change, resulting behavior, implementation details, validation results, and lack of a changelog entry. It uses equivalent headings instead of the template headings, but all required information is present.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 7 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- 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.
Owner
Author
|
Merge-pass review: the dictionaries are unchanged, and the new tests fail when a split goes wrong. Re-run on the PR head f33cb84 in a separate checkout:
The first Integration and browser run failed in scripts/omo-question-wheel-regression.ts (waitForResponse 10 s timeout, twice), the same failure #1663 hit; this PR does not touch that path, and the rerun passed. A fix for that script is in progress. |
devswha
added a commit
that referenced
this pull request
Oct 22, 2026
….*.ts (#1674) Second PR of the i18n split (#2 in the split plan, after the Settings pilot #1673). The strings that only the terminal's files use move out of the three base dictionaries into `src/lib/i18n/terminal.{ko,ja,zh}.ts`. `t()`, the English keys, every translation, placeholders, the English fallback and all call sites stay the same. ## What changes - **Moved:** 113 keys in each language, byte for byte, in the order each language's file had them. A key moves only when every file that uses it is a terminal file: `components/PaneTerminal.tsx`, `components/PaneCanvas.tsx`, `components/KeyBar.tsx` (with its `ARROWS` and `KEY_SPOKEN_NAMES` label maps), `components/FindBar.tsx`, `components/TerminalViewportTools.tsx`, `components/SplitDropPreview.tsx`, `components/TabStrip.tsx`, `lib/terminalLabel.ts` and `lib/tabName.ts`. Keys these files share with another feature stay in the base files. - **Composition:** each base file gains one import and one `KO_PARTS` / `JA_PARTS` / `ZH_PARTS` entry (`terminal: TERMINAL_KO`, …). Nothing else in the base files changes. - No test, docs or CONTRIBUTING change: the pilot's tests already cover every part, and CONTRIBUTING already says where a feature's strings go. ## Key list I regenerated the list with the split plan's ownership script on this branch's base, `a717621f`. That script collects keys the same way as `i18n.test.ts`: `t()`/`tt()` literals plus the 13 label maps, recorded per file. It finds 113 terminal keys, identical to the plan's list from `8b5aaba2`, with no key added or removed. Since then main gained only #1672 (no i18n change) and the pilot #1673 (which moved only Settings-only keys). ## Checks - **Same dictionaries:** the composed `KO`, `JA` and `ZH` (every key and value) dumped as sorted JSON on `a717621f` and on this branch are byte-identical (`EQUIVALENT`). - **`--color-moved` summary:** `git diff --color-moved=plain origin/main...HEAD` over the base and part files shows all 339 removed entry lines (113 × 3) as moved. The only non-moved lines are 33 added lines, 11 per language: - the import; - the parts entry; - the part file's 7-line header, its `export const` line and its closing `};`. No line is removed other than moved entries. - **Same keys moved:** the keys of `TERMINAL_KO`, `TERMINAL_JA` and `TERMINAL_ZH` each equal the 113-key list (`SAME-KEYS` for ko, ja and zh). The scan still finds all 1,389 keys, the same count as main. - **Part sizes:** each part file is 122 lines, under the 400-line cap. `i18n.ko.ts` goes from 1,180 to 1,069 lines, `i18n.ja.ts` from 1,182 to 1,071 and `i18n.zh.ts` from 1,184 to 1,073. - `HERDR_TEST_MODE=unit bun test src/lib/i18n.test.ts`: 21 pass, 0 fail. This covers missing, unused, placeholder and untranslated entries, a key in two parts, and different placement across languages. - `bun run typecheck`: passes. - `bun run check fast`: passes (unit 6127 pass, 0 fail, 12 skip; workflow syntax, generated types/themes, typecheck and build ok). This PR contains only moved lines, one import and one parts entry per language, and the part file header. It should qualify for the move-only merge path if the parts entry and header are counted as part of the move. ## Note After this move the `// ---- key bar ----` section comment in each base file has no entries left: its keys went to Settings (#1673) and here. The comment is left in place to keep this diff move-only; the last split PR can drop empty section comments. No changelog fragment: users see no change.
devswha
added a commit
that referenced
this pull request
Oct 22, 2026
…n the omo question wheel script (#1675) ## Change `scripts/omo-question-wheel-regression.ts` failed twice in a row (first run and the lane's retry) on #1663 and #1673, with `waitForResponse: Timeout 10000ms exceeded`. Neither PR touched the omo question UI. **Cause.** The script holds the initial metrics response in its route (line 102) until after terminal startup and the wheel gestures. It started that response's 10-second deadline (line 105) before navigating, though. Under CPU contention the deadline ran out before the script itself released the request: an instrumented reproduction timed out at 10.12 s and released the request at 10.78 s. **Fix.** The script now subscribes to the held response just before releasing the request (lines 135-139). The 10-second bound and every wheel, clamp, touch, input and settlement assertion stay. Test-only: 7 insertions, 2 deletions. No sleeps, raised timeouts, new mocks or production changes. ## Validation Six browser scripts ran at once with 24 CPU workers and no retries: - **Before the fix:** 36/36 passed under CPU load. With CDP renderer throttling added, 0/18 passed: the premature deadline reproduced. - **After the fix:** 36/36 consecutive passes under CPU load with 8-CPU affinity: ``` CHECK_WAIT_SECONDS=600 bun run check run --build taskset -c 0-7 bun <load harness> 36 final ``` Under artificial 6x renderer throttling, the patched script gets past the formerly failing step. It then hits two other deadlines, which are harness timing bounds, not product bugs, and were left unchanged: - the clamp test's deadline, while dispatching 126 synthetic wheel events; - the `.conn-live` readiness bound. `bun run check fast` passes.
devswha
added a commit
that referenced
this pull request
Oct 22, 2026
The last PR of the i18n split (split plan, section 4 and the deferred items of section 2.4). The split PRs (#1673, #1674, #1676, #1677, #1679, #1680, #1681, #1683) moved every feature-only key into `src/lib/i18n/<feature>.{ko,ja,zh}.ts` and were move-only. This PR carries the non-move changes the split left for the end. It is **not move-only**: it changes a test, a test message and CONTRIBUTING.md, so it takes the normal review path. ## Changes - **Placement check (`src/lib/i18n.test.ts`).** A new test in "dictionary parts" fails when: - a part keeps a key that a file outside the part also uses, reported as `Cancel (in palette, belongs in base)`; - the base keeps a key that only one part's files use, reported as `Enable notifications (in base, belongs in palette)`; - `KO_PARTS` has a part that `PART_FILES` does not list. `PART_FILES` lists each part's files, the same lists as the part headers. A key's uses are the files that call `t()`/`tt()` with it plus, for the 13 label maps, the map's own file. To support this, `labelMapKeys()` becomes `labelMapUses()` and returns `[key, file]` pairs; the existing missing/unused test takes just the keys, so it checks the same set as before. File paths are normalized to `/` so the check also holds on Windows. Only `KO_PARTS` is checked, because the existing "same part in every language" test already ties JA and ZH to it. - **Emptied section comments.** `// ---- key bar ----`, `// ---- markdown ----`, `// ---- install help ----` and `// ---- sign-in ----` no longer have any entries in the base files (ko, ja, zh) and are removed. - **`src/conventions.test.ts`.** The "New workspace" failure message now points to `src/lib/i18n.*.ts or src/lib/i18n/*.ts`. - **`CONTRIBUTING.md`.** The UI text rule now says `bun run test:unit` also fails on an entry kept in the wrong file. ## Checks - `HERDR_TEST_MODE=unit bun test src/lib/i18n.test.ts src/conventions.test.ts`: 28 pass, 0 fail. - The placement test was checked by breaking it on purpose, in all three languages, then restoring the files: - moving the shared key "Cancel" from the base into the palette part fails with `Cancel (in palette, belongs in base)`; - moving a palette-only key into the base fails with `Enable notifications (in base, belongs in palette)`. - The composed `KO`, `JA` and `ZH` are identical to main's (sorted-JSON dump diff empty, `EQUIVALENT`): only comment lines leave the dictionaries. - `bun run typecheck`: passes. - `bun run check fast`: passes (unit 6128 pass, 0 fail, 12 skip in 434 files, 8 at a time, 58.2s). No changelog fragment: users see no change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First PR of the i18n split: the strings that only the Settings dialog's files use move out of the three big dictionaries into
src/lib/i18n/settings.{ko,ja,zh}.ts.t(), the English keys, every translation, placeholders, the English fallback and all call sites stay the same.What changes
src/lib/i18n.{ko,ja,zh}.tsand keep the order they had in each language's file. A key moves only when every file that uses it is a Settings file:components/SettingsDialog.tsx(withTHEME_NAMES),components/KeyBarSettings.tsx,components/SettingsControls.tsx,components/TelemetryControls.tsxandlib/paletteSettings.ts. A key another feature also uses stays in the base file, for example "Settings" (also used in CommandPalette and MachineSidebar) and "Pin/Unpin agent" (AgentSidebar). The// ---- <area> ----sections are not used to decide ownership: the 281 keys come from 12 different sections.BASEand exportsKO_PARTS/JA_PARTS/ZH_PARTS({ base, settings }).KO/JA/ZHare built from those parts withObject.assignand keep their names and paths, sosrc/lib/i18n.tsand the three scripts that import them by path are untouched. The first sentence of each base header now says what the base file holds.src/lib/i18n.test.ts:t()calls now skipslib/i18n/. The old file-name regex does not match the part files, and a dictionary header containst("…").CONTRIBUTING.md: the UI text rule now says where a feature's new strings go. A string only one feature's files use goes in that feature'ssrc/lib/i18n/files when it has them; any other string, or one a second feature starts using, goes in the base files.This is not a move-only PR: alongside the moved lines it adds the composition, the tests and the docs rule. The following split PRs (terminal, composer, chat, …; one at a time) will contain only moved lines, one import and one parts entry per feature, and the part file header.
Key list
I regenerated the list on this branch's base,
dc70d60a, with the ownership script from the split plan. That script collects keys the same way asi18n.test.ts:t()/tt()literals plus the 13 label maps, recorded per file. The list is identical to the plan's list from8b5aaba2, with no difference. Main moved only by #1672, which touches no i18n file.Checks
KO,JAandZH(every key and value) dumped as sorted JSON ondc70d60aand on this branch are byte-identical (EQUIVALENT).git diff --color-moved=plain origin/main...HEADover the six dictionary files shows every entry line as moved. The 72 non-moved+/-lines, 24 per language, are:export const KO … = {→const BASE … = {;export constline and its closing};.SETTINGS_KO,SETTINGS_JAandSETTINGS_ZHeach equal the 281-key list (SAME-KEYSfor ko, ja and zh). The scan still finds all 1,389 keys, the same count as main.i18n.ko.tsgoes from 1,449 to 1,180 lines,i18n.ja.tsfrom 1,451 to 1,182 andi18n.zh.tsfrom 1,453 to 1,184.HERDR_TEST_MODE=unit bun test src/lib/i18n.test.ts: 21 pass, 0 fail. The missing, unused, placeholder and untranslated tests still run on the composed dictionaries.SETTINGS_KOfails withSettings (base, settings). One key moved back to the ja base fails the placement test.bun run typecheck: passes. Both tsconfigs compile, including the three browser scripts that importKO/JA/ZH.bun run check fast: passes.Nothing drops out of a check's scope: no test file moves,
scripts/ci-tests.tsfinds tests with a recursive glob, andsrc/conventions.test.tsscanssrc/**/*.{ts,tsx}, which includessrc/lib/i18n/.Overlap with open PRs
#1639, #1598 and #1427 also edit
src/lib/i18n.{ko,ja,zh}.ts. None of them changes any of the moved lines; all three only add keys. If one of them merges first, this branch is rebased, the key list is regenerated, and any new Settings-only key moves into the part. If this PR merges first, a new Settings-only key goes insettings.*.tsunder the new CONTRIBUTING rule. #1639 adds strings toSettingsDialog.tsx, so its keys are the likely case.No changelog fragment: users see no change.