feat(web-shell): improve slash command discovery (taller menu, group counts, fuzzy search) - #6267
Conversation
The slash-command menu capped its visible height at exactly four rows, so with 40+ merged commands users had to scroll a thin list to find anything, and the built-in custom/skill/system grouping was only a faint 1px divider with no label. Raise the cap to min(12 rows, 40vh) and render the category name as a visible header at each group boundary (custom / skill / system), keeping the divider between groups. Sub-command menus are ungrouped and unchanged.
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual, before/after screenshots included. On direction: this solves a real UX problem — 60+ commands crammed into a 4-row menu with no labels and substring-only matching. Fuzzy search also brings the web shell to parity with the TUI, which already uses On approach: the four changes (taller menu, category headers, per-group counts, fuzzy search) are tightly scoped to the stated goal. Each change is independently justified and the diff doesn't carry anything extraneous. The Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有必要章节齐全,双语,含前后对比截图。 方向:解决了真实的 UX 问题——60+ 命令挤在 4 行菜单里,没有分类标题,只有子串匹配。模糊搜索也让 Web Shell 与 TUI 对齐(TUI 已经对同一批命令使用 方案:四项改动(加高菜单、分类标题、分组计数、模糊搜索)严格围绕目标,没有夹带无关变更。 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewClean implementation. The No correctness bugs, security issues, or AGENTS.md violations found. The Test ResultsUnit tests (worktree, PR branch): The full web-shell suite has 6 pre-existing failures unrelated to this PR (worktree Real-Scenario Testing (tmux)PR code (
|
|
This is a well-scoped, well-implemented PR that solves a genuine UX problem. Before reading the diff, my independent proposal would have been: increase the menu height via CSS, add section labels with counts in the render loop, and integrate Every change in the diff is needed for the stated goal — no drive-by refactors, no scope creep. The code is straightforward and I'd be happy to maintain it in six months. Tests are thorough (22 slash completion + 6 section plan), the server starts cleanly, and the bundle correctly includes the new code. The only thing I can't verify in this environment is the actual browser rendering, but the PR author provided Playwright screenshots and the CSS/JS evidence checks out. Approving. ✅ 中文说明这是一个范围合理、实现良好的 PR,解决了真实的 UX 问题。我在看 diff 之前的独立方案是:通过 CSS 加高菜单、在渲染循环中加分类标题和计数、集成 diff 中每项改动都是目标所需的——没有顺手重构,没有范围蔓延。代码清晰,六个月后维护也不会头疼。测试充分(22 个斜杠补全 + 6 个 section plan),服务器启动正常,包正确包含新代码。唯一无法在此环境验证的是实际浏览器渲染,但 PR 作者提供了 Playwright 截图且 CSS/JS 证据已核实。 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
Typing in the slash menu now fuzzy-ranks commands with the same fzf engine the TUI uses, so abbreviated input like "mdl" finds "model" and "arf" finds "agent-reproduce-feature" — substring matching alone could not. An empty query still browses the category-ordered list; a non-empty query switches to a flat relevance-ranked list (headers are dropped since results interleave categories). Each category header also shows how many commands the group holds (e.g. "Skill commands 28"), so the volume hidden below the fold is visible at a glance. The fzf index is built once per command set (keyed on the array identity) and falls back to substring filtering if construction fails.
wenshao
changed the title
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No high-confidence issues found. Downgraded from Approve to Comment: CI still running.
Minor items worth human review (terminal only, not blocking):
fuzzyRankCommandscatch block silently falls back to substring filtering with no logging — consider addingconsole.warn- Fallback path returns unsorted results, inconsistent with pre-PR sorting behavior
- Missing test for null result when fuzzy query matches nothing
All 739 web-shell tests pass. tsc and eslint clean.
— qwen3.7-max via Qwen Code /review
- Extract the section header/divider boundary logic into a pure `planSlashSectionRows` helper and unit-test it (headers at group boundaries, first row header without a divider, no repeated headers for adjacent duplicate sections, per-group counts). This also moves the section-count computation past the `!anchorRect` early return so it no longer runs on first render. - Simplify `--slash-panel-max-height` to a round `min(460px, 45vh)` instead of a `12 * rowHeight` formula that ignored header/divider overhead and so showed only ~9-10 rows; the panel now shows ~12-13 rows. - Log a warning when fzf fuzzy search throws before falling back to substring matching, so a silent failure is diagnosable. - Add a completion test for the zero-match case returning null.
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
Incremental review passed for new commit 5dc3a86. Changes address all review feedback:
- Added console.warn in catch block for better error visibility
- Added test for fuzzy search returning null when no matches
- Refactored section counting logic into pure utility function (slashSectionPlan.ts) with comprehensive tests
- Fixed early return ordering (section counting now happens before anchorRect check)
- Adjusted CSS max-height to 45vh for better viewport sizing
All tests pass (28/28). LGTM ✅
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
|
|
||
| it('honors panel category order and filtered commands', () => { | ||
| const commands: CommandInfo[] = [ | ||
| { |
There was a problem hiding this comment.
[Suggestion] getCommandFzf silently deduplicates commands by name (if (byName.has(command.name)) continue), keeping only the first occurrence — but no test exercises this path. A test with duplicate command names would lock in the dedup contract and catch regressions if upstream mergeCommands behavior ever changes.
it('deduplicates commands with the same name for fuzzy search', () => {
const commands: CommandInfo[] = [
{ name: 'model', description: 'First', source: 'builtin-command' },
{ name: 'model', description: 'Duplicate', source: 'skill-dir-command' },
{ name: 'memory', description: 'Manage memory', source: 'builtin-command' },
];
const result = getSlashCommandCompletionResult(
'/model', 6, commands, [], 'en', getTranslator('en'),
);
expect(result?.items).toHaveLength(1);
expect(result?.items[0]?.detail).toBe('First');
});— qwen3.7-max via Qwen Code /review
| import { describe, expect, it } from 'vitest'; | ||
| import { planSlashSectionRows } from './slashSectionPlan'; | ||
|
|
||
| describe('planSlashSectionRows', () => { |
There was a problem hiding this comment.
[Suggestion] All existing tests use either all-section-defined or all-section-undefined items. A mixed test — e.g. [{section: 'A'}, {section: undefined}, {section: 'B'}] — would pin the lastSection = section ?? lastSection behavior (undefined items don't break group boundaries) and catch regressions if the ?? is ever changed to a plain =.
it('handles mixed section and undefined items correctly', () => {
const plans = planSlashSectionRows(
[
{ section: 'Custom commands' },
{ section: undefined },
{ section: 'System commands' },
],
'command',
);
expect(plans[0]).toMatchObject({ showHeader: true, showDivider: false, count: 1 });
expect(plans[1]).toMatchObject({ showHeader: false, showDivider: false, count: 0 });
expect(plans[2]).toMatchObject({ showHeader: true, showDivider: true, count: 1 });
});— qwen3.7-max via Qwen Code /review
3 participants
What this PR does
Makes the web-shell slash-command menu easy to search and browse when there are many commands. Four changes:
Skill commands 28), so the volume hidden below the fold is visible at a glance.fzfengine the TUI already uses, so abbreviated input likemdlfinds/modelandarffinds/agent-reproduce-feature. An empty query keeps the grouped browse list; a non-empty query switches to a flat relevance-ranked list.Why it's needed
With built-in commands, project commands, and skills merged together the menu holds 40+ entries (60 in this repo). Only four were ever visible, the grouping was invisible, and matching was plain substring — so
mdlmatched nothing and users had to know a command's exact spelling or scroll a thin list. Fuzzy ranking plus a taller, labeled, counted menu makes commands far easier to find. Fuzzy search also brings the web shell to parity with the TUI, which already usesfzffor the same commands.Reviewer Test Plan
How to verify
npm run build --workspace=@qwen-code/qwen-code-core, then from the repo root:node packages/cli/dist/index.js serve --web --port 18811(cwd determines which project/skill commands load)././mdl,/arf,/thm./model,/agent-reproduce-feature,/themerespectively surface — plain substring matching returns nothing for these.Verified locally against a real
qwen serve --webbinary with Playwright: browse shows 60 commands grouped asSkill commands 28/System commands 32;/mdl→/model,/thm→/theme,/arfincludes/agent-reproduce-feature— all fuzzy-only matches with no console errors. Unit tests (slashCompletion.test.ts+2 fuzzy cases, 21 total; full web-shell suite 739 passed), prettier, eslint, andtsc -p tsconfig.lib.json(0 errors) all pass. The fzf index is built once per command set and falls back to substring filtering on error.Evidence (Before & After)
Before — capped at 4 rows, no labels, substring-only:
After — browse — up to 12 rows, category headers with counts:
After — fuzzy
/mdl— abbreviation (not a substring) finds/model:After — fuzzy
/arf— finds/agent-reproduce-feature:Tested on
Environment (optional)
macOS,
qwen serve --webwith an isolated HOME; Playwright (headless Chromium) for screenshots and DOM assertions.Risk & Scope
fzf(^0.5.2, already apackages/clidependency, pure-JS, browser-safe) topackages/web-shell. Fuzzy ranking changes result ordering for non-empty queries from category-first to relevance-first — intentional and matching the TUI.slashCompletionSourceCodeMirror source (not wired into the live editor) still does substring filtering; only the livegetSlashCommandCompletionResultpath gained fuzzy — noted in a code comment.Linked Issues
None.
中文说明
这个 PR 做了什么
让 web-shell 的斜杠命令菜单在命令很多时更好搜索和浏览。四项改动:
Skill commands 28),一眼看出折叠线下藏了多少。fzf引擎做模糊排序,所以缩写mdl能找到/model、arf能找到/agent-reproduce-feature。空查询保持分组浏览列表,非空查询切换为按相关度排序的扁平列表。为什么需要
内置命令、项目命令、skill 合并后菜单有 40+ 条(本仓库 60 条)。此前只露 4 条、分组不可见、匹配是纯子串——
mdl匹配不到任何东西,用户得知道命令确切拼写或滚动细列表。模糊排序 + 更高/带标题/带计数的菜单让命令好找得多。模糊搜索也让 Web Shell 与 TUI 对齐(TUI 对同一批命令早已用fzf)。复现与验证
npm run build --workspace=@qwen-code/qwen-code-core,然后仓库根目录:node packages/cli/dist/index.js serve --web --port 18811(cwd 决定加载哪些项目命令/skill)。/:预期随窗口高度显示约 9–12 行(原 4 行),带 自定义 / Skill / 系统 标题,每个标题显示命令数。/mdl、/arf、/thm:预期分别浮现/model、/agent-reproduce-feature、/theme——纯子串匹配对这些返回空。已用真实
qwen serve --web二进制 + Playwright 本地验证:浏览显示 60 条命令,分组为Skill commands 28/System commands 32;/mdl→/model、/thm→/theme、/arf含/agent-reproduce-feature——全是纯模糊命中且无 console 报错。单测(slashCompletion.test.ts新增 2 个模糊用例、共 21;完整 web-shell 套件 739 通过)、prettier、eslint、tsc -p tsconfig.lib.json(0 错误)全过。fzf 索引按命令集构建一次,失败时回退子串过滤。风险与范围
packages/web-shell增加fzf(^0.5.2,已是packages/cli依赖,纯 JS、浏览器安全)。模糊排序把非空查询的结果顺序从"分类优先"改为"相关度优先"——有意为之,且与 TUI 一致。slashCompletionSource(CodeMirror 源,未接入实际编辑器)仍用子串;只有活路径getSlashCommandCompletionResult加了模糊——已在代码注释中说明。