fix(web-shell): stabilize slash command i18n in split-view panes - #6546
Merged
Conversation
Split-view panes showed English descriptions for slash commands while the main view showed Chinese. Two root causes: 1. ChatPane never merged getLocalCommands(t) into the command list, so ~33 built-in commands (help, model, clear, etc.) lacked i18n descriptions. 2. localizeBuiltinDescriptions required source === 'builtin-command', but the daemon omits _meta.source in some SSE event paths (available_commands_update), causing built-in commands to skip translation unpredictably across sessions. 3. Skill localization depended on connection.skills, which can be empty when SSE events deliver commands without availableSkills. Fix: make the entire localization pipeline name-based and session-independent — merge local commands, relax the source guard to also translate when source is missing, and use skillDescriptionKey directly instead of connection.skills for skill tagging. Also adds missing autofix skill translation (EN + ZH).
Collaborator
|
Thanks for the PR, @wenshao! Template: heading names differ from the PR template ("Summary" vs "What this PR does", etc.), and "Risk & Scope", "Linked Issues", and 中文说明 sections are missing. The body itself is thorough though — clear root cause analysis, test plan, and file-by-file breakdown. Not blocking on this, but worth aligning with the template for consistency. Problem: the three bugs are real and the root causes are correctly identified. Direction: aligned. CHANGELOG shows active work in this area — #6326 added slash menu localization, #6400 added split-view, and several follow-ups (#6418, #6447, #6454) fixed adjacent split-view issues. This PR closes a gap from the original localization work. Size: not applicable — all changes in Approach: scope feels right. Four source files + two test files, each change directly addresses one of the three root causes. No drive-by refactors or scope creep. The relaxation of Moving on to code review. 🔍 中文说明感谢 PR！ 模板：标题名称与 PR 模板不同（"Summary" vs "What this PR does" 等），缺少 "Risk & Scope"、"Linked Issues" 和中文说明部分。但内容本身很充分——清晰的根因分析、测试计划和逐文件说明。不以此阻止，但建议与模板保持一致。 问题：三个 bug 是真实的，根因定位正确。 方向：对齐。CHANGELOG 显示该领域活跃——#6326 添加了斜杠菜单本地化，#6400 添加了分屏视图，后续 #6418、#6447、#6454 修复了相关分屏问题。本 PR 弥补了原始本地化工作的缺口。 规模：不适用——所有变更在 方案：范围合理。4 个源文件 + 2 个测试文件，每个变更直接针对三个根因之一。没有附带重构或范围蔓延。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewIndependent proposal before reading diff: I would have (1) merged The PR's approach matches this exactly. Clean and minimal. Specific findings:
No correctness bugs, security issues, or convention violations found. TestingUnit tests (39 tests, 2 files): all pass ✅ Typecheck: no errors in PR-changed files. Pre-existing errors in Dev server build (vite with PR applied): Compiles cleanly, no warnings or errors. Note: the actual i18n behavior in split-view panes requires browser testing with a running daemon session and cannot be exercised from CLI/tmux. The unit tests cover the core logic (command merging, source guard relaxation, skill key lookup). 中文说明代码审查独立方案：(1) 在 ChatPane 中合并 具体发现：所有文件变更正确无误。 测试单元测试（39 个测试，2 个文件）：全部通过 ✅ 类型检查：PR 变更文件无错误。 开发服务器构建（vite + PR 代码）：编译成功，无警告或错误。 注意：分屏视图中的实际 i18n 行为需要浏览器 + 运行中的 daemon 会话测试，无法从 CLI/tmux 验证。单元测试覆盖了核心逻辑。 — Qwen Code · qwen3.7-max |
Collaborator
|
This is a well-scoped fix for a real i18n gap in split-view panes. The three root causes are correctly identified and each is addressed with a minimal, targeted change. The approach matches what I would have done independently — merge local commands in ChatPane, relax the source guard, use static skill lookup instead of The code is clean, the tests cover the key scenarios (source missing, custom source shadowing, local command merge count), and the build compiles without issues. The PR is small (48 additions, 17 deletions across 6 files) and stays focused on the stated goal. The only gap is the lack of a visual before/after showing the split-view i18n behavior, which would require a browser screenshot. But the unit tests validate the underlying logic, and the CHANGELOG trail (#6326 → #6400 → this fix) makes the motivation clear. Approving. ✅ 中文说明这是一个范围合理的修复，解决了分屏视图中真实的 i18n 缺口。三个根因被正确识别，每个都用最小、针对性的变更解决。方案与我的独立方案一致——在 ChatPane 中合并本地命令、放宽 source 检查、使用静态 skill 查找替代 代码干净，测试覆盖关键场景（source 缺失、自定义 source 覆盖、本地命令合并计数），构建编译无问题。PR 很小（48 行增加，17 行删除，跨 6 个文件），专注于声明的目标。 唯一的缺口是缺少分屏 i18n 行为的视觉 before/after（需要浏览器截图）。但单元测试验证了底层逻辑，CHANGELOG 轨迹（#6326 → #6400 → 本修复）使动机清晰。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jul 8, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
ytahdn
approved these changes
Jul 8, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. 功能层面审查无缺陷：
localizeBuiltinDescriptionssource 放宽逻辑正确：source 缺失时按名字翻译，明确非 builtin 才跳过- ChatPane 补上
mergeCommands+getLocalCommands(t)+ skill 标签，与 App.tsx 对齐 - 移除
connection.skills依赖，不再受 SSE 事件顺序影响，更稳健 - 测试覆盖到位：source 缺失翻译、custom command 回归
一个小观察：skill 标签从 connection.skills 集合成员判断改为 SKILL_DESCRIPTION_KEYS 名字映射判断，当前安全（skill 名与内置命令名不重叠），但未来添加同名命令时需注意误标风险。
建议（非阻塞）：ChatPane 和 App.tsx 的 merge → localize → skill-tag 逻辑完全重复，可考虑提取为共享 hook 减少后续维护不一致。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Collaborator
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
localCommands.test.ts:118-140 |
Test pipeline() helper still uses old skillNames-set gating removed from App.tsx; validates stale behavior |
Rewrite pipeline() to use skillDescriptionKey() as the gate, matching new App.tsx |
ChatPane.test.tsx:428 |
Assertion weakened to toBeGreaterThan(30) — won't catch merge/dedup regressions |
Use exact count: expect(count).toBe(getLocalCommands(t).length + 1) |
App.tsx:4127-4134 + ChatPane.tsx:162-169 |
Skill-tagging .map() duplicated byte-for-byte between two files; this duplication caused the original bug |
Extract shared tagSkills(commands, t) helper in localCommands.ts |
localCommands.test.ts:85-92 |
Newly added autofix skill entry has no explicit test assertion |
Add expect(skillDescriptionKey('autofix')).toBe('skilldesc.autofix') |
— qwen3.7-max via Qwen Code /review
wenshao
commented
Jul 8, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Collaborator
Author
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
localCommands.ts:224-229 |
Relaxed source guard: a custom command named export (or any builtin name) arriving without source will have its description silently overwritten. The PR's own test had to add source: 'custom' to prevent this. |
Consider only translating when source is explicitly 'builtin-command' or when the description matches a known daemon-default pattern. At minimum, log when a non-empty description is replaced so collisions are debuggable. |
App.tsx:4125 / ChatPane.tsx:159 |
User-defined skills not in SKILL_DESCRIPTION_KEYS lose displayCategory: 'skill' on the welcome screen (pre-session, where source is unreliable). The old connection.skills check and t('skills.run') fallback are removed. |
Add displayCategory: 'skill' for commands where source is 'skill' or 'bundled-skill' in the .map() step as a safety net, or restore connection.skills as a secondary gate. |
localCommands.test.ts:122-145 |
The pipeline helper mirrors the old App.tsx logic (uses skillNames set, ` |
|
ChatPane.test.tsx:422-433 |
ChatPane's skill-tagging .map() is untested — the test only asserts count > 30 via a mocked testid. No test verifies skills get displayCategory: 'skill'. |
Extend the ChatEditor mock to expose command details, then add a test seeding a known skill and asserting displayCategory: 'skill'. |
ChatPane.test.tsx:427-431 |
Assertion toBeGreaterThan(30) is too loose — a mergeCommands dedup regression (e.g., clear appearing twice) would still pass. |
Use toBe(getLocalCommands(t).length + 1) to verify exact dedup behavior. |
— glm-5.2 via Qwen Code /review
This was referenced Aug 13, 2026
Closed
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
3 participants
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.
Summary
Split-view panes showed English descriptions for slash commands (e.g.
/help,/autofix) while the main view correctly showed Chinese. The behavior was unstable — sometimes Chinese, sometimes English, varying across panes and sessions.Root Causes
Three issues compounded:
ChatPane never merged
getLocalCommands(t)— the split-view commands memo only calledlocalizeBuiltinDescriptions()on raw daemon commands. ~33 built-in commands (/help,/model,/clear, etc.) had no i18n descriptions in split-view.localizeBuiltinDescriptionsrequiredsource === 'builtin-command'— the daemon omits_meta.sourcein some SSEavailable_commands_updateevent paths. Commands arriving withoutsourceskipped translation entirely. Different sessions received events at different times, causing the unstable behavior.Skill localization depended on
connection.skills— this value can be empty when SSE events deliver commands withoutavailableSkills. Skills like/autofixfell back to the daemon's English description.Fix
Make the entire localization pipeline name-based and session-independent:
mergeCommands(daemon, getLocalCommands(t))— local winssource === 'builtin-command'requiredsourceis explicitly non-builtin (e.g.skill,custom)connection.skillsmembership checkskillDescriptionKey(name)static lookupAlso adds missing
autofixskill translation (EN + ZH).Files Changed
ChatPane.tsx— full command localization pipeline (was missing merge + skill tagging)App.tsx— same pipeline simplification (removeconnection.skillsdependency)localCommands.ts— relaxsourceguard inlocalizeBuiltinDescriptions; addautofixtoSKILL_DESCRIPTION_KEYSi18n.tsx— addskilldesc.autofixEN + ZH translationsReviewer Test Plan
?lang=zh-CN)/in each pane's composer — all slash command descriptions should be in Chinese/autofix— skill description should be in Chinese