feat(desktop): localize built-in personality labels in the settings picker - #65406
hansai-art wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused picker change. The current-main premise is valid: apps/desktop/src/app/settings/config-settings.tsx:134 falls back to prettyName, and current main does not supply personality option labels at :472. The PR’s optionLabels wiring at apps/desktop/src/app/settings/config-settings.tsx:472-477 correctly preserves custom-personality fallback behavior.
Problems
- Japanese remains incomplete.
apps/desktop/src/i18n/catalog.ts:7-12registersja, whileapps/desktop/src/i18n/define-locale.ts:39-40merges missing keys from English. This PR addspersonalityLabelsonly toen.ts,zh.ts, andzh-hant.ts;ja.tshas no override. Japanese users would therefore still see English labels such asPirate. The existing test only checks truthiness, so the English fallback passes it. This conflicts with the Desktop checklist inapps/desktop/AGENTS.md:197-198to update all locales.
Suggested changes
- Add the 14 Japanese labels in
apps/desktop/src/i18n/ja.tsand assert at least one representative Japanese label differs from English.
Automated hermes-sweeper review.
| invalidJson: 'Invalid config JSON', | ||
| // Display names for the built-in personalities (config IDs stay English). | ||
| // Custom personalities fall back to prettyName(id) in the picker. | ||
| personalityLabels: { |
There was a problem hiding this comment.
Please add a matching Japanese override in ja.ts. ja is registered in the catalog and defineLocale() fills absent keys from this English map, so without an override the Japanese picker still displays English labels; the current truthiness test will not catch that fallback.
|
Addressed in b14a613 - added the 14 ja.ts labels and a representative-difference assertion so the fallback can't satisfy the test. Thanks for the review! |
… catch-up The Personality picker rendered raw English ids (`pirate`, `uwu`, `noir`) in every locale (NousResearch#64132). The ids are already stable i18n keys, so no backend change is needed: - `i18n/types.ts` + `en.ts`: new `settings.config.personalityLabels` map covering all 14 built-ins. - `zh.ts` / `ja.ts` / `zh-hant.ts`: localized display names. Locales that do not override fall back to English via `defineLocale()`. - `config-settings.tsx`: pass the map as `optionLabels` for `display.personality`, mirroring the `tts.elevenlabs.voice_id` pattern. - Config ids, `/personality <name>` args and custom personalities are untouched; customs still render via `prettyName`. Also brings the zh-Hant catalog back to parity with `en.ts` (313 keys: keybinds panel, desktop plugins, Hermes Cloud gateway, MCP catalog, model defaults, skills hub, maintenance/ops, starmap sharing, tab and composer strings). This supersedes NousResearch#65405 and folds in its 7 unique keys. All keys `main` gained after these branches were cut (`settings.toolsets.terminalBackend.*`, `settings.config.builtinOnly`, `settings.nav.billing`, `settings.toolsets.needsSetup` / `needsSignIn`, the fieldLabels/fieldDescriptions additions) are preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AG6LyYMvHC2o6HbVUozmVR
b14a613 to
bfe909a
Compare
|
Thanks for flagging #71231 rather than just opening it quietly, and for the offer to close it. I'd rather you keep it open. Two things make yours the better review path, independent of who wrote what:
So: I am not rebasing this one to compete with #71231. If a maintainer wants the personality fix, yours is the one to take. One note on the catalog half, in case it is useful to whoever picks this up: it should not be carried by a wholesale rewrite of No concern at all about the overlap. Narrowing the surface is the right call. |
SummaryThree PRs are in this complex: #65406 and #71231 both address #64132 by supplying localized display labels through the existing Related pull requests
Duplicates#65406 and #71231 duplicate the core personality-picker fix for #64132; #65406 additionally subsumes the separate catalog scope represented by closed #65405. Suggested consolidationKeep #71231 open with a salvage path as the focused vehicle: its visible diff retains the effective picker fix and addresses both objections from the contributor's keep_open review. Close #65406 as a duplicate of #71231 for the #64132 scope despite its recorded best-fix and keep_open verdicts, because its author explicitly declined to rebase it, endorsed #71231, and its diff bundles the independent zh-Hant catalog work; retain that catalog work only as a separate focused follow-up, while #65405 remains closed as superseded reference material. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I64132(["issue #64132 (open)"])
subgraph Dup65406 ["PRs duplicating each other"]
P65406["PR #65406 (open)"]
P71231["PR #71231 (open)"]
end
P65406 -->|best fix| I64132
class I64132 open
class P65406 open
class P71231 open
class P65406 best
class P71231 best
class P65406 target
click I64132 "https://github.com/NousResearch/hermes-agent/issues/64132"
click P65406 "https://github.com/NousResearch/hermes-agent/pull/65406"
click P71231 "https://github.com/NousResearch/hermes-agent/pull/71231"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 3 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 49 kB of PR diffs, 15 kB of issue/PR text, 11 kB of discussion (16 comments), 4 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
What does this PR do?
Fixes the Personality picker in Desktop Settings showing raw English ids (
pirate,uwu,noir) in every locale, and brings the Traditional Chinese catalog back to parity withen.ts.The personality ids are already stable i18n keys, so no backend change is needed: a new
settings.config.personalityLabelsmap is added to the catalog and passed to the existingoptionLabelsprop, exactly mirroring thetts.elevenlabs.voice_idpattern already inconfig-settings.tsx. Config ids,/personality <name>args, and custom personalities are untouched; customs still render viaprettyName.This PR supersedes #65405 (
feat/i18n-zh-hant-desktop). Both branches rewroteapps/desktop/src/i18n/zh-hant.tsfrom the same base, so they could never both merge. This one is the strict superset (catalog catch-up plus the personality feature), and it now folds in the 7 keys that were unique to #65405:keybinds.search,settings.nav.keybinds,settings.providers.localEndpoint.title,settings.providers.localEndpoint.description,onboarding.fireworksPitch,shell.statusbar.modelPinned,sidebar.row.ownedByProfile. #65405 can be closed in favour of this one.Rebuilt on current
main, additively. The branch is now a single commit on top ofmain, andzh-hant.tswas edited additively starting frommain's version rather than replaced wholesale, so the keysmaingained after these branches were cut are preserved: thesettings.toolsets.terminalBackend.*block (11 keys),settings.config.builtinOnly,settings.nav.billing,settings.toolsets.needsSetup,settings.toolsets.needsSignIn, and the 5fieldLabels.tts.xai.*additions. Verified mechanically — see "How to Test".Related Issue
Fixes #64132
Type of Change
Changes Made
apps/desktop/src/i18n/types.ts— addsettings.config.personalityLabels: Record<string, string>.apps/desktop/src/i18n/en.ts— English labels for the 14 built-in personalities (source of truth for fallbacks).apps/desktop/src/i18n/zh.ts,apps/desktop/src/i18n/ja.ts— localized personality labels.apps/desktop/src/i18n/zh-hant.ts— localized personality labels plus 313 catalog keys that zh-Hant was missing relative toen.ts: the keybinds panel, desktop plugins page, Hermes Cloud gateway flow, MCP catalog tab, model defaults/fallbacks, skills hub, maintenance/ops panel, starmap sharing, tab-bar and composer strings.apps/desktop/src/app/settings/config-settings.tsx— passt.settings.config.personalityLabelsasoptionLabelswhenkey === 'display.personality'.apps/desktop/src/app/settings/helpers.test.ts— two new tests (see below).How to Test
cd apps/desktop && npm run typecheck— passes (tsc -p . --noEmitandtsc -p tsconfig.electron.json --noEmit, 0 errors).cd apps/desktop && npx vitest run --project ui src/app/settings/helpers.test.ts— 32/32 pass, including the two newpersonalityLabelscases.cd apps/desktop && npx vitest run— 2043 passed / 2 failed / 2 skipped. The 2 failures are insrc/app/settings/billing/index.test.tsxand reproduce identically on unmodifiedmain(pre-existing, unrelated to this PR).fieldLabels/fieldDescriptionssubtree is excluded from the set math becauseen.tsuses the importedFIELD_LABELS/FIELD_DESCRIPTIONSconstants whilezh-hant.tsusesdefineFieldCopy({...})— a structural asymmetry, not a gap):en.ts: 2330 leaf keys (excluding that subtree).zh-hant.tson this branch: 2330 leaf keys (excluding that subtree) + 116 in the subtree.en.tsminuszh-hant.ts= 0 missing.zh-hant.tsminusen.ts= 0 extra (no junk keys).main'szh-hant.tsminus this branch's = 0 (nothing regressed), including the subtree.zh,zh-hant,ja; English elsewhere; custom personality ids still showprettyName(id).Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — N/A, this change is JS/TS only; ran the desktop vitest suite instead (see "How to Test")Documentation & Housekeeping
docs/, docstrings) — N/Acli-config.yaml.exampleif I added/changed config keys — N/A (no new config keys; only UI display names for existing ids)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/ANotes for reviewers
helpers.test.ts: one asserts everyBUILTIN_PERSONALITIESid has a label in every locale (guards the list and the map drifting apart); the other assertszh/zh-hant/jaactually differ from English rather than silently falling throughdefineLocale()./personalitylist via apersonality.namesmap inlocales/*.yaml.apps/desktop/src/i18n/zh-hant.ts(theallowSessionwording). It touches a different region of the file, but whichever lands second will need a trivial rebase.