Skip to content

fix: nest settings translation keys for language packs - #28251

Merged
nwparker merged 2 commits into
mainfrom
nwparker/scan-catalog-shape
Oct 14, 2026
Merged

nwparker merged 2 commits into
mainfrom
nwparker/scan-catalog-shape

Conversation

@nwparker

@nwparker nwparker commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​80 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​80
Prod 7 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​107 $\color{#cf222e}{\Huge{\mathbf{−}}}$​78 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​29

ELI5

A language pack could be rejected because some settings labels were stored under one long name instead of inside their folders. Those labels now use the same nested structure as the rest of the translation file. The words on screen stay the same.

What Changed

The eleven agent settings labels move into the existing settings.agents object in English, Spanish, French, Japanese, Korean, and Chinese. English's duplicate chat title is removed; its identical nested entry already exists. The existing catalog check now reports keys containing dots in their names, including nested mistakes, before an English --fix can rewrite the file.

Why

The app's translation library accepted both shapes, hiding the mismatch with the language-pack parser. Nesting the entries preserves every translation lookup and keeps the parser's existing safety rules intact. The verifier reports ambiguous entries for review rather than automatically choosing which value to keep.

Linked Issue

Fixes #28017

Visual Proof

N/A. No layout, text, or interaction changes. Every translation path and value is identical across all six locales.

Parser proof using the actual English settings translations:

Check Before After
Language-pack parser Rejected: catalog key settings.appearance.chat.title is not safe Accepted: 142 entries
Translation paths and values in all six locales Original values Zero changes
Dotted English/target catalog regression tests Original verifier incorrectly returns success Verifier rejects both shapes and leaves files unchanged

The parser proof uses the settings translations, including the formerly misplaced labels. Whole bundled catalogs also contain text restricted by other plugin safety limits; those limits remain enforced.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Reproduced rejection before editing, then verified all six settings subtrees with the actual parser and their eleven labels with flat-key fallback disabled. Compared every flattened translation path/value against the original commit: zero differences. The new verifier tests failed against the original implementation and pass after the fix.

pnpm test config/scripts/verify-localization-catalog.test.mjs config/scripts/verify-localization-catalogs.test.mjs src/renderer/src/i18n/catalog-nested-settings.test.ts src/shared/plugins/plugin-language-pack-artifact.test.ts --maxWorkers=2: 54 tests in four suites pass with ORCA_BACKGROUND_LAUNCH=1 and --maxWorkers=2. Main and renderer typechecks, changed-code quality, catalog/runtime verification under Node and Bun, localization extraction, and localization coverage pass. No app windows were launched. Validation ran on macOS; the data and verification script are platform independent.

Final platform verification: tested together with the other reviewed fixes in 59 selected suites. macOS: 748 passed. Linux: 748 passed. Windows: 725 passed, 13 platform skips (including the Bash-only suite). Core typecheck, changed-code quality (zero new findings), and the production Electron bundle build passed. UI screenshots are hidden macOS renderers; fixture and live-host limits are described above.

All CI checks passed on the final head 951a33a43bdb22427f8657ed67e67dd4d55d833d before merge.

Review

Independent review found no remaining issues and confirmed identical translation paths and values across all six locales. Cross-referenced community PR #28018 after reproducing the bug and designing the fix. Evgenii (@smwbev) receives co-author credit. This change preserves all Chinese values as well; the community PR's additional wording changes are outside this structural repair.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Low merge risk; no known negative user-facing tradeoffs. No runtime parser, permission, wire, workspace, path, or shortcut changes. Nested language-pack keys are already supported by older hosts.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Move misplaced catalog entries into their existing nested settings paths without changing translation values. Extend catalog verification to reject dotted keys and cover the real bundled settings labels with the language-pack parser and strict nested lookups.

Co-authored-by: Evgenii <kumiro@me.com>
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9294ba20-0d66-490e-aff4-7f00e71b1440
📥 Commits

Reviewing files that changed from the base of the PR and between f4da672 and 951a33a.

📒 Files selected for processing: 9
  • config/scripts/verify-localization-catalog.mjs
  • config/scripts/verify-localization-catalog.test.mjs
  • src/renderer/src/i18n/catalog-nested-settings.test.ts
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/zh.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

📝 Walkthrough

Locale catalogs now store agent settings translations as nested objects. Tests check nested translation parsing and lookup across six locales. The catalog verifier detects dotted key names and stops validation for affected English and target locale catalogs.

Estimated code review effort

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 951a3

This change moves agent settings translations into the nested structure that the language-pack parser accepts, and it adds a check to stop dotted keys from coming back. No merge-blocking risk is evident.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (6 skipped: 6… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: nesting settings translation keys for language packs.
Description check Passed The description follows the required template, explains the change and rationale, links issue #28017, documents testing and platform coverage, and marks visual proof as not applicable with a reason. I…
Linked Issues check Passed Issue #28017 requires nested storage for the twelve dotted catalog keys and a verifier check. The PR moves the eleven agent keys into settings.agents in all six listed locales, removes the duplicate…
Out of Scope Changes check Passed The changes stay within issue #28017. The locale edits implement the required catalog shape, the verifier change prevents recurrence, and the added tests verify both behaviors. No app call-site or unr…
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (6 skipped: 6 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@smwbev

smwbev commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

@nwparker Thanks for picking this up. I compared this branch (951a33a43b) with #28018 leaf by leaf: in all six catalogs the same keys are removed and the same nested paths are added with identical values, and the dotted-key check is the same. The only difference is the five Chinese wording edits, which you left out on purpose — fine by me, they were not part of the structural fix. #28018 has nothing this PR lacks, so close it whenever this lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Twelve catalog keys are stored with dots in their names, a shape language packs cannot use

2 participants