-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(cli): improve slash command history feedback #8365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
90a67a7
e7183bb
facb38f
296961e
d7a14bf
ce6fbb9
4b2b65f
8d19d46
81618a6
15227e1
d1140e6
90f0145
c1dfe04
b44d4d3
deed974
24b12bf
c2ab3fc
cf3cc8c
64fc673
751deb0
a3df7ee
0502eed
cd49a42
92d1dc3
b6b0cc9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| # Slash command history feedback | ||
|
|
||
| ## Problem | ||
|
|
||
| Interactive slash commands are added to the TUI history before their action is | ||
| known. Commands that only open a dialog can therefore leave a bare invocation | ||
| behind after the dialog closes. The model picker has the same problem when it | ||
| is dismissed without a selection. | ||
|
|
||
| ## Design | ||
|
|
||
| - Do not add the built-in `/auth`, `/settings`, `/status`, `/help`, `/theme`, | ||
| `/editor`, `/diff`, or `/stats` invocations to visible TUI history. Bare | ||
| `/effort`, `/model`, and `/statusline` pickers are hidden too. Their existing | ||
| UI remains unchanged, as do chat recording and slash-command telemetry. User | ||
| and project commands that override those names keep their invocation | ||
| history. | ||
| - Root matches apply to the bare command only; subcommands keep their | ||
| invocation because they perform work (for example `/status paths` prints | ||
| session paths). | ||
| - Resolve the command before adding its invocation so aliases use the canonical | ||
| command name for this decision. | ||
| - Preserve invocations for commands that directly perform work, change session | ||
| state, write data, or enter a management/security workflow. Argument-sensitive | ||
| commands only hide their bare picker form; for example, `/effort` is hidden | ||
| while `/effort high` remains visible, and `/model` is hidden while | ||
| `/model <id>` remains visible. | ||
| - Commands that fail before opening their dialog keep the invocation paired | ||
| with the failure message: `/theme` under `NO_COLOR` is not hidden because it | ||
| prints feedback instead of opening the picker, and a hidden picker-shaped | ||
| `/model` invocation is revealed when its arguments are rejected. | ||
| - Record the hiding decision in the chat record (`hiddenInvocation`) so | ||
| `/resume`, `/branch`, and session previews reconstruct the same history the | ||
| live session displayed instead of bringing the bare invocation row back. | ||
| - When the primary model picker is dismissed without a selection, add an info | ||
| message identifying the unchanged model. Successful selections keep their | ||
| existing feedback. The other pickers leave no trace when dismissed; the | ||
| model picker states the outcome explicitly because the active model is | ||
| session-critical and otherwise invisible in history, so a silent close would | ||
| leave it ambiguous whether the model changed. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,24 @@ const COMPACTION_MODEL_CONFIGURATION_HINT = | |
| const IMAGE_MODEL_CONFIGURATION_HINT = | ||
| 'Configure a model with imageOnly: true, baseUrl, and envKey in settings.modelProviders. Run /model --image <model-id> to select it.'; | ||
|
|
||
| const MODEL_PICKER_FLAGS = [ | ||
| 'fast', | ||
| 'voice', | ||
| 'vision', | ||
| 'compaction', | ||
| 'image', | ||
| 'project', | ||
| 'global', | ||
| ] as const; | ||
| const MODEL_PICKER_FLAG_PATTERN = `(?:${MODEL_PICKER_FLAGS.join('|')})`; | ||
| const MODEL_PICKER_ONLY_PATTERN = new RegExp( | ||
| `^(?:\\s*--${MODEL_PICKER_FLAG_PATTERN})*\\s*$`, | ||
| ); | ||
|
|
||
| export function isPickerOnlyModelInvocation(args: string): boolean { | ||
|
DragonnZhang marked this conversation as resolved.
|
||
| return MODEL_PICKER_ONLY_PATTERN.test(args); | ||
| } | ||
|
Comment on lines
+64
to
+66
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. **[Suggestion]**R5-2: The picker-only classification is never tested for the flag-plus-model-id shape ( // add a keep-case to the direct-action it.each, e.g.
['/model --fast qwen3-max', 'model'],
// or unit-test:
// isPickerOnlyModelInvocation('--fast qwen3-max') === false中文说明picker-only 分类从未测试过 flag+模型 id 形态( — qwen3.8-max via Qwen Code /review (v0.21.7) |
||
|
|
||
| /** | ||
| * Parse --project / --global scope flags from the argument string. | ||
| * Returns the resolved scope override and the remaining args with flags stripped. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] The new
configwiring from AppContainer intouseEditorSettings(and identically intouseThemeCommandat ~1426-1431) has no test:AppContainer.test.tsxmocks both hooks wholesale (vi.mock('./hooks/useThemeCommand.js'),vi.mock('./hooks/useEditorSettings.js')), and the parameter is optional (config?: Config), so nothing type-checks the wiring either. — Failure scenario: a future change that drops theconfigargument from either call site compiles and leaves every test green (the hook-level tests inject their ownconfig), while/editorselection feedback and/theme-under-NO_COLOR feedback silently stop callingrecordSlashCommand— resumed/branched sessions and desktop reconstruction then lose those outcome lines with no error anywhere.中文说明
[Suggestion] AppContainer 向
useEditorSettings传入config的新接线(以及 ~1426-1431 处useThemeCommand的相同接线)没有任何测试:AppContainer.test.tsx对这两个 hook 做了整体 mock(vi.mock('./hooks/useThemeCommand.js')、vi.mock('./hooks/useEditorSettings.js')),且该参数是可选的(config?: Config),因此类型检查也无法兜住这个接线。— 失败场景:未来某个改动把任一调用点的config参数删掉后,编译通过、所有测试仍为绿色(hook 层测试自带config注入),而/editor选择反馈与 NO_COLOR 下的/theme反馈会静默停止调用recordSlashCommand——resume/branch 会话与桌面端重建将丢失这些结果行,且任何地方都不会报错。修复见英文部分代码块:在
AppContainer.test.tsx中断言两个 hook mock 的末尾参数是 config 实例。— qwen3.8-max via Qwen Code /review (v0.21.10)