From 26ad36e95b88e416e7766fd365decd3efdbc0d52 Mon Sep 17 00:00:00 2001 From: MikeWang0316tw Date: Fri, 19 Jun 2026 13:39:12 +0800 Subject: [PATCH] feat(cli): show follow-up suggestion in input placeholder (#5145) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cli): show follow-up suggestion in input placeholder When enableFollowupSuggestions is true, display the generated follow-up suggestion as the input placeholder text (replacing the default "Type your message..."). Tab/Enter/Right arrow accepts the suggestion; typing dismisses it. Also change the default of enableFollowupSuggestions from false to true so the feature is on by default. Key changes: - AppContainer: dismissPromptSuggestion no longer clears promptSuggestion state, preserving it for placeholder restore after user types then deletes - InputPrompt: Tab/Enter/Right arrow/typing handlers check promptSuggestion prop as fallback when followup.state is not visible (e.g. after 300ms delay or user dismissed) - Composer: placeholder shows suggestion text when available - hasTabConsumer: include promptSuggestion to prevent Windows bare Tab from cycling approval mode * chore: update settings.schema.json (enableFollowupSuggestions default: false → true) * test(cli): add tests for promptSuggestion prop fallback paths (#5145) - Add unit tests for Tab/Right arrow/Enter accepting promptSuggestion when followup.state.suggestion is null (type-then-delete path). - Add unit test for hasTabConsumer reporting true immediately when promptSuggestion prop is set (no followup debounce needed). - Update stale comment on speculation abort useEffect in AppContainer. Co-authored-by: Qwen-Coder * fix(cli): address PR #5145 review feedback for promptSuggestion - Fix Enter key to fill buffer instead of submitting suggestion (matches Tab/Right-arrow behavior and Claude Code design) - Add suggestionDismissed state to hasTabConsumer for Windows Tab cycling - Fix suggestionDismissed to be set to true on user input (paste/typing) - Add speculation abort to dismissPromptSuggestion callback - Remove dead placeholder branch from Composer.tsx - Update tests to reflect Enter no longer auto-submits suggestion * fix(cli): address PR #5145 review from wenshao + telemetry gap wenshao's review (posted after the previous fixes) flagged two issues, both still valid against the current code; doudouOUC's telemetry gap is addressed too. - settings description: replace stale "Enter to accept and submit" with "Press Tab, Right Arrow, or Enter to accept into the input buffer" in both settingsSchema.ts and settings.schema.json (Enter now only fills the buffer, and the feature defaults to enabled). - hasTabConsumer / handler consistency: drop the redundant `suggestionDismissed` state and gate hasTabConsumer on `buffer.text.length === 0` — the exact condition the Tab/Right/Enter handlers already use. Fixes the type-then-delete desync where Windows bare Tab would both insert the suggestion and cycle approval mode (regression of #4171). - fallback telemetry: add a `fallbackText` option to the followup controller's accept() so the prop-fallback path (no live suggestion, e.g. within the show delay or after type-then-delete) routes through accept() and logs onOutcome instead of silently bypassing telemetry. Tab/Right/Enter handlers now call accept(method, { fallbackText }). - tests: add core-level coverage for accept() with/without fallbackText, and fix the InputPrompt "fallback" tests that advanced 700ms (which silently exercised the normal visible-suggestion path) to advance only 100ms so followup.state.suggestion truly stays null. Co-Authored-By: Claude Opus 4.8 * feat(cli): add accept_source telemetry + tests for promptSuggestion fallback Follow-up to wenshao's second review pass on #5145. - accept_source telemetry: fallback accepts report time_to_accept_ms: 0 (the suggestion was never shown via the timer), which is indistinguishable from an instant accept. Add an `accept_source: 'live' | 'fallback'` field to the followup controller's onOutcome and PromptSuggestionEvent so analytics can tell the two apart. The controller derives it from whether a live `currentState.suggestion` was present before applying `fallbackText`. - tests: assert accept_source on the fallback accept; add a test that a live suggestion takes priority over fallbackText (guards the `?? fallbackText` ordering); add an InputPrompt test pinning the new buffer.text.length === 0 gate — hasTabConsumer reports false when a promptSuggestion is set but the buffer is non-empty (the old Boolean(promptSuggestion) gate wrongly reported true). The empty-buffer → true direction stays covered by the existing test. Co-Authored-By: Claude Opus 4.8 * refactor(cli): address doudouOUC review on #5145 (dedupe, rename, telemetry) Three [Suggestion]-level items from the latest review pass. - Extract `availableSuggestion`: the compound condition `(followup.state.isVisible || promptSuggestion) && (followup.state.suggestion ?? promptSuggestion)` was copy-pasted across the Tab/Right/Enter accept guards, both typing-dismiss guards, and the placeholder prop. Collapse them into one derived value so the sites can't drift apart. Behavior is unchanged (the controller keeps `isVisible` and `suggestion` in lockstep). - Rename `dismissPromptSuggestion` -> `abortPromptSuggestion` across the UIState context, AppContainer, Composer, and the MainContent mock. The function only aborts in-flight generation/speculation and deliberately does NOT clear `promptSuggestion` (so the placeholder can restore it); the "dismiss" name implied the suggestion was gone. - Omit `time_to_first_keystroke_ms` for fallback accepts. With `accept_source: 'fallback'` the suggestion was never shown via the timer (shownAt stayed 0), so `prevShownAtRef` still holds a previous suggestion's timestamp and the delta would be meaningless. Co-Authored-By: Claude Opus 4.8 * feat(cli): actually enable followup suggestions by default PR #5145 changed the schema default to `true`, but `mergeSettings` never applies SETTINGS_SCHEMA defaults, so the runtime `=== true` gates left the feature off while the settings panel read it as on (verified by wenshao). - Flip both runtime gates to treat an unset value as enabled — only an explicit `false` opts out: `AppContainer.tsx` and the ACP `Session.ts` (`#maybeEmitFollowupSuggestion`). - Add a Session test for the unset/default-on path. - Fix the stale `UIStateContext` JSDoc left over from the dismiss→abort rename (it no longer clears state). - Docs: mark the feature on-by-default, correct Enter (fills the input, does not submit), ghost-text → placeholder text, and add a cost note that `fastModel` forks to a separate cache and can cost more than the default main-model + shared-cache path on long conversations. Co-Authored-By: Claude Opus 4.8 * fix(core): reject control chars and ANSI escapes in prompt suggestions The follow-up suggestion is influenceable through conversation history (tool/file/web output) and is rendered verbatim in the input placeholder now that enableFollowupSuggestions defaults to on. Raw control bytes (CR, ESC/CSI, C1) reached the terminal because getFilterReason only rejected newlines and asterisks. Reject them at the source so the displayed and inserted text always match. Co-Authored-By: Claude Opus 4.8 * fix(cli): clear promptSuggestion on submit and accept paths Addresses doudouOUC review on #5145. Since abortPromptSuggestion was changed to preserve `promptSuggestion` for type-then-delete restore, the submit and accept paths leaked stale suggestion text: - handleSubmitAndClear only called followup.dismiss(); after a synchronous command (/clear, /help) that never triggers AppContainer's streaming transition, the placeholder kept showing the old suggestion. - Tab/Right/Enter accept never cleared the prop, so clearing the buffer without submitting (Ctrl+U) made the accepted suggestion reappear as a ghost placeholder. Both now call onPromptSuggestionDismiss?.() after the followup action. Also reuse the availableSuggestion single-source-of-truth in hasTabConsumer instead of an inlined parallel expression, and add useFollowupSuggestions tests asserting the accept_source guard suppresses time_to_first_keystroke_ms on fallback accepts. Co-Authored-By: Claude Opus 4.8 * test(cli): assert promptSuggestion is cleared on accept and submit Regression coverage for the state-leak fixed in 04fcffd1c (doudouOUC Critical #1/#2, confirmed by wenshao's maintainer re-verification): Tab, Right-arrow and Enter accepts plus message submit must each call onPromptSuggestionDismiss, so the persisted promptSuggestion can't reappear as a ghost placeholder when the buffer is next cleared. Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Qwen-Coder Co-authored-by: Claude Opus 4.8 --- docs/users/configuration/settings.md | 2 +- docs/users/features/followup-suggestions.md | 16 +- .../acp-integration/session/Session.test.ts | 17 ++ .../src/acp-integration/session/Session.ts | 5 +- packages/cli/src/config/settingsSchema.ts | 4 +- packages/cli/src/ui/AppContainer.tsx | 31 ++- packages/cli/src/ui/components/Composer.tsx | 2 +- .../src/ui/components/InputPrompt.test.tsx | 261 +++++++++++++++++- .../cli/src/ui/components/InputPrompt.tsx | 81 ++++-- .../src/ui/components/MainContent.test.tsx | 2 +- .../cli/src/ui/contexts/UIStateContext.tsx | 8 +- .../ui/hooks/useFollowupSuggestions.test.tsx | 104 +++++++ .../src/ui/hooks/useFollowupSuggestions.tsx | 10 +- .../core/src/followup/followupState.test.ts | 85 ++++++ packages/core/src/followup/followupState.ts | 26 +- .../src/followup/suggestionGenerator.test.ts | 10 + .../core/src/followup/suggestionGenerator.ts | 9 + packages/core/src/telemetry/loggers.ts | 3 + packages/core/src/telemetry/types.ts | 3 + .../schemas/settings.schema.json | 4 +- 20 files changed, 617 insertions(+), 66 deletions(-) create mode 100644 packages/cli/src/ui/hooks/useFollowupSuggestions.test.tsx diff --git a/docs/users/configuration/settings.md b/docs/users/configuration/settings.md index 3696dce858..a81f95308b 100644 --- a/docs/users/configuration/settings.md +++ b/docs/users/configuration/settings.md @@ -117,7 +117,7 @@ Settings are organized into categories. Most settings should be placed within th | `ui.accessibility.enableLoadingPhrases` | boolean | Enable loading phrases (disable for accessibility). | `true` | | `ui.accessibility.screenReader` | boolean | Enables screen reader mode, which adjusts the TUI for better compatibility with screen readers. | `false` | | `ui.customWittyPhrases` | array of strings | A list of custom phrases to display during loading states. When provided, the CLI will cycle through these phrases instead of the default ones. | `[]` | -| `ui.enableFollowupSuggestions` | boolean | Enable [followup suggestions](../features/followup-suggestions) that predict what you want to type next after the model responds. Suggestions appear as ghost text and can be accepted with Tab, Enter, or Right Arrow. | `false` | +| `ui.enableFollowupSuggestions` | boolean | Enable [followup suggestions](../features/followup-suggestions) that predict what you want to type next after the model responds. Suggestions appear as placeholder text and are accepted with Tab, Enter, or Right Arrow (which fill the input — they do not auto-submit). On by default; set to `false` to opt out. | `true` | | `ui.enableCacheSharing` | boolean | Use cache-aware forked queries for suggestion generation. Reduces cost on providers that support prefix caching (experimental). | `true` | | `ui.enableSpeculation` | boolean | Speculatively execute accepted suggestions before submission. Results appear instantly when you accept (experimental). | `false` | | `experimental.emitToolUseSummaries` | boolean | Generate short LLM-based labels summarizing each tool-call batch. See [Tool-Use Summaries](../features/tool-use-summaries). Requires `fastModel` to be configured; silently skipped otherwise. Can be overridden per-session with `QWEN_CODE_EMIT_TOOL_USE_SUMMARIES=0` or `=1`. | `true` | diff --git a/docs/users/features/followup-suggestions.md b/docs/users/features/followup-suggestions.md index aaaf463b43..6393ee84ea 100644 --- a/docs/users/features/followup-suggestions.md +++ b/docs/users/features/followup-suggestions.md @@ -1,12 +1,12 @@ # Followup Suggestions -Qwen Code can predict what you want to type next and show it as ghost text in the input area. This feature uses an LLM call to analyze the conversation context and generate a natural next step suggestion. +Qwen Code can predict what you want to type next and show it as placeholder text in the input area. This feature uses an LLM call to analyze the conversation context and generate a natural next step suggestion. This feature works end-to-end in the CLI. In the WebUI, the hook and UI plumbing are available, but host applications must trigger suggestion generation and wire the followup state for suggestions to appear. ## How It Works -After Qwen Code finishes responding, a suggestion appears as dimmed text in the input area after a short delay (~300ms). For example, after fixing a bug, you might see: +After Qwen Code finishes responding, a suggestion appears as dimmed placeholder text in the input area after a short delay (~300ms). For example, after fixing a bug, you might see: ``` > run the tests @@ -19,10 +19,12 @@ The suggestion is generated by sending the conversation history to the model, wh | Key | Action | | ------------- | ------------------------------------------------ | | `Tab` | Accept the suggestion and fill it into the input | -| `Enter` | Accept the suggestion and submit it immediately | +| `Enter` | Accept the suggestion and fill it into the input | | `Right Arrow` | Accept the suggestion and fill it into the input | | Any typing | Dismiss the suggestion and type normally | +`Enter` fills the input rather than submitting, so accepting a suggested slash command (e.g. `/clear`) never auto-executes — you submit it yourself with a second `Enter`. + ## When Suggestions Appear Suggestions are generated when all of the following conditions are met: @@ -32,7 +34,7 @@ Suggestions are generated when all of the following conditions are met: - There are no errors in the most recent response - No confirmation dialogs are pending (e.g., shell confirmation, permissions) - The approval mode is not set to `plan` -- The feature is enabled in settings (disabled by default — set `ui.enableFollowupSuggestions` to `true` to turn it on) +- The feature is enabled (on by default — set `ui.enableFollowupSuggestions` to `false` to turn it off) Suggestions will not appear in non-interactive mode (e.g., headless/SDK mode). @@ -44,7 +46,7 @@ Suggestions are automatically dismissed when: ## Fast Model -By default, suggestions use the same model as your main conversation. For faster and cheaper suggestions, configure a dedicated fast model: +By default, suggestions use the same model as your main conversation. For lower-latency suggestions, configure a dedicated fast model: ### Via command @@ -64,6 +66,8 @@ Or use `/model --fast` (without a model name) to open a selection dialog. The fast model is used for prompt suggestions and speculative execution. When not configured, the main conversation model is used as fallback. +> **Cost note:** A fast model lowers latency, but it does not always lower cost. Suggestion generation reuses your conversation's prefix cache (via `ui.enableCacheSharing`, on by default) — but a prefix cache is per-model. Pointing `fastModel` at a different model forks to a separate cache, so the whole conversation history is re-billed as uncached input on the fast model. On long conversations, the default (main model + shared cache) can be **cheaper** than a fast model, since most of the history is billed at the discounted cached rate. Set `fastModel` when latency matters more than per-turn cost. + Thinking/reasoning mode is automatically disabled for all background tasks (suggestion generation and speculation), regardless of your main model's thinking configuration. This avoids wasting tokens on internal reasoning that isn't needed for these tasks. ## Configuration @@ -72,7 +76,7 @@ These settings can be configured in `settings.json`: | Setting | Type | Default | Description | | ------------------------------ | ------- | ------- | ------------------------------------------------------------------ | -| `ui.enableFollowupSuggestions` | boolean | `false` | Enable or disable followup suggestions | +| `ui.enableFollowupSuggestions` | boolean | `true` | Enable or disable followup suggestions | | `ui.enableCacheSharing` | boolean | `true` | Use cache-aware forked queries to reduce cost (experimental) | | `ui.enableSpeculation` | boolean | `false` | Speculatively execute suggestions before submission (experimental) | | `fastModel` | string | `""` | Model for prompt suggestions and speculative execution | diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 79c31dc92d..e31a813213 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -8256,6 +8256,23 @@ describe('Session', () => { ).toBeUndefined(); }); + it('emits when the setting is unset (on by default)', async () => { + // Regression for #5145 review: the schema default isn't applied by + // mergeSettings, so an unset value must be treated as enabled — only an + // explicit `false` opts out. + (mockSettings as unknown as { merged: { ui: unknown } }).merged.ui = {}; + generateMock.mockResolvedValue({ suggestion: 'Run the tests next?' }); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + + await vi.waitFor(() => { + expect(generateMock).toHaveBeenCalled(); + }); + }); + it('does not emit in PLAN approval mode', async () => { mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.PLAN); generateMock.mockResolvedValue({ suggestion: 'something' }); diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 40acf4b210..74d215f19b 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -1135,7 +1135,10 @@ export class Session implements SessionContext { */ #maybeEmitFollowupSuggestion(result: PromptResponse): void { if (result.stopReason !== 'end_turn') return; - if (this.settings.merged.ui?.enableFollowupSuggestions !== true) return; + // Enabled by default — only an explicit `false` opts out. The schema + // `default: true` isn't applied at runtime by `mergeSettings`, so an unset + // value must be treated as enabled here. + if (this.settings.merged.ui?.enableFollowupSuggestions === false) return; if (this.config.getApprovalMode() === ApprovalMode.PLAN) return; const chat = this.config.getGeminiClient()?.getChat(); diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index d33ac421d8..626d075af1 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -778,9 +778,9 @@ const SETTINGS_SCHEMA = { label: 'Enable Follow-up Suggestions', category: 'UI', requiresRestart: false, - default: false, + default: true, description: - 'Show context-aware follow-up suggestions after task completion. Press Tab or Right Arrow to accept, Enter to accept and submit.', + 'Show context-aware follow-up suggestions after task completion. Press Tab, Right Arrow, or Enter to accept into the input buffer.', showInDialog: true, }, enableCacheSharing: { diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 6abbe7f86e..4a84a1f177 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -1519,11 +1519,19 @@ export const AppContainer = (props: AppContainerProps) => { const speculationRef = useRef(IDLE_SPECULATION); const suggestionAbortRef = useRef(null); - // Dismiss callback — clears suggestion + aborts in-flight generation/speculation - const dismissPromptSuggestion = useCallback(() => { - setPromptSuggestion(null); + // Aborts in-flight suggestion generation/speculation only. It deliberately + // does NOT clear `promptSuggestion`, so the placeholder can restore the + // suggestion when the buffer becomes empty again (user types then deletes). + // Named "abort" (not "dismiss") precisely because the suggestion text + // survives — see #5145 review. + const abortPromptSuggestion = useCallback(() => { suggestionAbortRef.current?.abort(); suggestionAbortRef.current = null; + // Also abort the speculation so it doesn't continue running after abort. + if (speculationRef.current.status !== 'idle') { + abortSpeculation(speculationRef.current).catch(() => {}); + speculationRef.current = IDLE_SPECULATION; + } }, []); // Auto-accept indicator — disabled on agent tabs (agents handle their own) @@ -2144,9 +2152,11 @@ export const AppContainer = (props: AppContainerProps) => { geminiClient, ]); - // Generate prompt suggestions when streaming completes + // Generate prompt suggestions when streaming completes. Enabled by default: + // `mergeSettings` doesn't apply the schema `default: true`, so the runtime + // gate must treat an unset value as enabled. Only an explicit `false` opts out. const followupSuggestionsEnabled = - settings.merged.ui?.enableFollowupSuggestions === true; + settings.merged.ui?.enableFollowupSuggestions !== false; useEffect(() => { // Clear suggestion when feature is disabled at runtime @@ -2256,9 +2266,10 @@ export const AppContainer = (props: AppContainerProps) => { settingInputRequests, ]); - // Abort speculation when promptSuggestion is cleared (new turn, feature toggle, or - // user-initiated dismiss via typing/paste). InputPrompt calls onPromptSuggestionDismiss - // on user input, which clears promptSuggestion, triggering this effect to abort speculation. + // Abort speculation when promptSuggestion is cleared (new turn or feature toggle). + // promptSuggestion is only cleared when the model responds or the feature is disabled; + // user typing/paste no longer dismisses it — the AbortController in InputPrompt handles + // that path, so this effect only fires on state changes from non-user-input sources. useEffect(() => { if (!promptSuggestion && speculationRef.current.status !== 'idle') { abortSpeculation(speculationRef.current).catch(() => {}); @@ -3438,7 +3449,7 @@ export const AppContainer = (props: AppContainerProps) => { setSessionName, // Prompt suggestion promptSuggestion, - dismissPromptSuggestion, + abortPromptSuggestion, // Rewind selector isRewindSelectorOpen, rewindEscPending, @@ -3571,7 +3582,7 @@ export const AppContainer = (props: AppContainerProps) => { setSessionName, // Prompt suggestion promptSuggestion, - dismissPromptSuggestion, + abortPromptSuggestion, // Rewind selector isRewindSelectorOpen, rewindEscPending, diff --git a/packages/cli/src/ui/components/Composer.tsx b/packages/cli/src/ui/components/Composer.tsx index 1bd2a155e8..284ad688d2 100644 --- a/packages/cli/src/ui/components/Composer.tsx +++ b/packages/cli/src/ui/components/Composer.tsx @@ -155,7 +155,7 @@ export const Composer = () => { : ' ' + t('Type your message or @path/to/file') } promptSuggestion={uiState.promptSuggestion} - onPromptSuggestionDismiss={uiState.dismissPromptSuggestion} + onPromptSuggestionDismiss={uiState.abortPromptSuggestion} /> )} diff --git a/packages/cli/src/ui/components/InputPrompt.test.tsx b/packages/cli/src/ui/components/InputPrompt.test.tsx index 1530117b72..81b522d62d 100644 --- a/packages/cli/src/ui/components/InputPrompt.test.tsx +++ b/packages/cli/src/ui/components/InputPrompt.test.tsx @@ -384,7 +384,10 @@ describe('InputPrompt', () => { // generous buffer (renderWithProviders cold start can be 100-200ms). const SUGGESTION_VISIBLE_WAIT_MS = 700; - it('accepts and submits the prompt suggestion on Enter when the buffer is empty', async () => { + // Regression: Enter on suggestion should fill buffer, NOT submit — matches + // Tab/Right-arrow behavior and Claude Code's design. This prevents accidental + // execution of destructive slash commands (/clear, /quit). + it('fills buffer on Enter when suggestion is available (does not submit)', async () => { vi.useFakeTimers(); const { stdin, unmount } = renderWithProviders( , @@ -397,11 +400,9 @@ describe('InputPrompt', () => { }); await flush(); - expect(props.onSubmit).toHaveBeenCalledWith('commit this'); - // Enter path must NOT call buffer.insert — it passes text directly to - // handleSubmitAndClear. Calling insert would re-fill the buffer after - // it was already cleared (the microtask race bug). - expect(mockBuffer.insert).not.toHaveBeenCalled(); + // Enter on suggestion should fill buffer, NOT submit + expect(props.onSubmit).not.toHaveBeenCalled(); + expect(mockBuffer.insert).toHaveBeenCalledWith('commit this'); } finally { vi.useRealTimers(); unmount(); @@ -457,6 +458,210 @@ describe('InputPrompt', () => { unmount(); } }); + + // Regression for #5145: the `promptSuggestion` prop fallback path must work + // when `followup.state.suggestion` is null (e.g. after user typed and + // deleted — the followup controller was dismissed but the placeholder + // text is still available via the prop). + // + // These tests deliberately advance LESS than SUGGESTION_DELAY_MS (300ms), + // so the followup controller's show-timer never fires and + // `followup.state.suggestion` stays null — exercising the prop fallback + // branch rather than the normal visible-suggestion path. (Earlier versions + // waited 700ms here, which silently tested the normal flow instead.) + describe('promptSuggestion prop fallback (when followup.state.suggestion is null)', () => { + // Comfortably under the 300ms SUGGESTION_DELAY_MS so the controller's + // state.suggestion remains null. + const BEFORE_SUGGESTION_VISIBLE_MS = 100; + + it('accepts promptSuggestion via Tab when followup.state.suggestion is null', async () => { + vi.useFakeTimers(); + const { stdin, unmount } = renderWithProviders( + , + ); + try { + await advanceTimers(BEFORE_SUGGESTION_VISIBLE_MS); + + act(() => { + stdin.write('\t'); + }); + await flush(); + + // Tab should insert the suggestion text into the buffer + expect(mockBuffer.insert).toHaveBeenCalledWith('commit this'); + } finally { + vi.useRealTimers(); + unmount(); + } + }); + + it('accepts promptSuggestion via Right arrow when followup.state.suggestion is null', async () => { + vi.useFakeTimers(); + const { stdin, unmount } = renderWithProviders( + , + ); + try { + await advanceTimers(BEFORE_SUGGESTION_VISIBLE_MS); + + act(() => { + stdin.write('\x1b[C'); // right arrow + }); + await flush(); + + // Right arrow should insert the suggestion text into the buffer + expect(mockBuffer.insert).toHaveBeenCalledWith('commit this'); + } finally { + vi.useRealTimers(); + unmount(); + } + }); + + it('fills buffer on Enter (does not submit) when followup.state.suggestion is null', async () => { + vi.useFakeTimers(); + const { stdin, unmount } = renderWithProviders( + , + ); + try { + await advanceTimers(BEFORE_SUGGESTION_VISIBLE_MS); + + act(() => { + stdin.write('\r'); + }); + await flush(); + + // Enter on suggestion should fill buffer, NOT submit (matches Tab/Right-arrow behavior) + expect(props.onSubmit).not.toHaveBeenCalled(); + expect(mockBuffer.insert).toHaveBeenCalledWith('commit this'); + } finally { + vi.useRealTimers(); + unmount(); + } + }); + }); + + // Regression for #5145 (doudouOUC Critical #1/#2, confirmed by wenshao's + // re-verification): accepting or submitting must clear the persisted + // `promptSuggestion` via onPromptSuggestionDismiss. Otherwise the prop + // survives, and the next time the buffer empties `availableSuggestion` + // re-derives from it and the just-accepted/submitted suggestion reappears + // as a ghost placeholder. Typing (#1380) and paste (#665) already clear it; + // accept and submit must match. + describe('clears promptSuggestion on accept/submit (no ghost placeholder)', () => { + // Under the 300ms SUGGESTION_DELAY_MS so the accept goes through the + // promptSuggestion fallback path (followup.state.suggestion stays null). + const BEFORE_SUGGESTION_VISIBLE_MS = 100; + + it('calls onPromptSuggestionDismiss when Tab accepts the suggestion', async () => { + vi.useFakeTimers(); + const onPromptSuggestionDismiss = vi.fn(); + const { stdin, unmount } = renderWithProviders( + , + ); + try { + await advanceTimers(BEFORE_SUGGESTION_VISIBLE_MS); + + act(() => { + stdin.write('\t'); + }); + await flush(); + + expect(mockBuffer.insert).toHaveBeenCalledWith('commit this'); + expect(onPromptSuggestionDismiss).toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + unmount(); + } + }); + + it('calls onPromptSuggestionDismiss when Right arrow accepts the suggestion', async () => { + vi.useFakeTimers(); + const onPromptSuggestionDismiss = vi.fn(); + const { stdin, unmount } = renderWithProviders( + , + ); + try { + await advanceTimers(BEFORE_SUGGESTION_VISIBLE_MS); + + act(() => { + stdin.write('\x1b[C'); // right arrow + }); + await flush(); + + expect(mockBuffer.insert).toHaveBeenCalledWith('commit this'); + expect(onPromptSuggestionDismiss).toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + unmount(); + } + }); + + it('calls onPromptSuggestionDismiss when Enter accepts the suggestion', async () => { + vi.useFakeTimers(); + const onPromptSuggestionDismiss = vi.fn(); + const { stdin, unmount } = renderWithProviders( + , + ); + try { + await advanceTimers(BEFORE_SUGGESTION_VISIBLE_MS); + + act(() => { + stdin.write('\r'); + }); + await flush(); + + // Enter accepts into the buffer (does not submit) and clears the prop. + expect(props.onSubmit).not.toHaveBeenCalled(); + expect(mockBuffer.insert).toHaveBeenCalledWith('commit this'); + expect(onPromptSuggestionDismiss).toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + unmount(); + } + }); + + it('calls onPromptSuggestionDismiss when a typed message is submitted', async () => { + vi.useFakeTimers(); + const onPromptSuggestionDismiss = vi.fn(); + mockBuffer.text = 'ship it'; + mockBuffer.lines = ['ship it']; + mockBuffer.cursor = [0, 'ship it'.length]; + const { stdin, unmount } = renderWithProviders( + , + ); + try { + await advanceTimers(BEFORE_SUGGESTION_VISIBLE_MS); + + act(() => { + stdin.write('\r'); + }); + await flush(); + + // Submitting a non-empty buffer clears the stale suggestion too, so a + // synchronous slash command (/clear, /help) can't leave a ghost. + expect(props.onSubmit).toHaveBeenCalledWith('ship it'); + expect(onPromptSuggestionDismiss).toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + unmount(); + } + }); + }); }); // Regression for #4171: `onTabConsumerChange` (consumed by AppContainer @@ -483,6 +688,50 @@ describe('InputPrompt', () => { unmount(); }); + // Regression for #5145: `hasTabConsumer` must be true when ONLY + // `promptSuggestion` prop is set (followup.state.suggestion is null), + // so Windows Tab approval-mode cycling is blocked even before the + // followup controller's debounce fires. + it('reports true immediately when promptSuggestion prop is set (no followup debounce needed)', async () => { + const onTabConsumerChange = vi.fn(); + const { unmount } = renderWithProviders( + , + ); + // Must report true immediately — no need to wait for followup debounce + // because `hasTabConsumer` includes `Boolean(promptSuggestion)`. + expect(onTabConsumerChange).toHaveBeenCalledWith(true); + unmount(); + }); + + // Regression for #5145 (wenshao review): `hasTabConsumer` is now gated on + // `buffer.text.length === 0` instead of the old sticky `suggestionDismissed` + // flag, so it reacts to the *current* buffer. With a promptSuggestion present + // but a NON-empty buffer (the user is mid-typing), Tab must NOT be consumed — + // Windows approval-mode cycling stays enabled. The old `Boolean(promptSuggestion)` + // gate wrongly reported true here. The complementary empty-buffer → true + // direction (i.e. restored after deleting back to empty) is pinned by + // "reports true immediately when promptSuggestion prop is set" above. + it('reports false when a promptSuggestion is set but the buffer is non-empty', async () => { + mockBuffer.text = 'commit'; + const onTabConsumerChange = vi.fn(); + const { unmount } = renderWithProviders( + , + ); + await wait(); + + expect(onTabConsumerChange).toHaveBeenCalledWith(false); + expect(onTabConsumerChange).not.toHaveBeenCalledWith(true); + unmount(); + }); + it('reports true while mid-input ghost text offers an accept', async () => { mockCommandCompletion.midInputGhostText = { text: 'ile.txt', diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index 4fca54b1e3..a56a2856c9 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -395,8 +395,13 @@ export const InputPrompt: React.FC = ({ // entry rather than advancing from whatever index the user picked. resetHistoryNavRef.current(); - // Dismiss follow-up suggestion after submit + // Dismiss follow-up suggestion after submit. `followup.dismiss()` only + // resets the controller; `onPromptSuggestionDismiss` also clears the + // persisted `promptSuggestion` prop so the placeholder doesn't leak a + // stale suggestion after synchronous commands (e.g. /clear, /help) that + // never trigger the streaming-transition effect in AppContainer. followup.dismiss(); + onPromptSuggestionDismiss?.(); // Clear attachments after submit setAttachments([]); @@ -418,6 +423,7 @@ export const InputPrompt: React.FC = ({ config, pendingPastes, followup, + onPromptSuggestionDismiss, ], ); @@ -535,6 +541,16 @@ export const InputPrompt: React.FC = ({ setAgentTabBarFocused, ]); + // Single source of truth for "is there a suggestion the user can accept right + // now": the live followup suggestion if visible, otherwise the persisted + // `promptSuggestion` prop (type-then-delete / pre-show-delay). Tab/Right/Enter + // accept, the typing-dismiss guards, and the placeholder all derive from this + // so they can never drift apart. + const availableSuggestion: string | null = + followup.state.isVisible || promptSuggestion + ? (followup.state.suggestion ?? promptSuggestion ?? null) + : null; + const handleInput = useCallback( (key: Key): boolean => { // When the Arena tab bar or background pill has focus, block @@ -650,7 +666,7 @@ export const InputPrompt: React.FC = ({ if (key.paste) { // Dismiss follow-up suggestion when user starts typing/pasting - if (buffer.text.length === 0 && followup.state.isVisible) { + if (buffer.text.length === 0 && availableSuggestion) { followup.dismiss(); onPromptSuggestionDismiss?.(); } @@ -991,10 +1007,16 @@ export const InputPrompt: React.FC = ({ !showCompletionSuggestions && !reverseSearchActive && !commandSearchActive && - followup.state.isVisible && - followup.state.suggestion + availableSuggestion ) { - followup.accept('tab'); + // Use the normal accept path. When the followup controller has no live + // suggestion (e.g. after type-then-delete), `fallbackText` carries the + // still-available placeholder text so telemetry is logged either way. + followup.accept('tab', { fallbackText: promptSuggestion ?? undefined }); + // Clear the persisted `promptSuggestion` prop too, otherwise it survives + // the accept and reappears as a ghost placeholder once the buffer is + // cleared without submitting (e.g. Ctrl+U). + onPromptSuggestionDismiss?.(); return true; } @@ -1004,10 +1026,12 @@ export const InputPrompt: React.FC = ({ !key.ctrl && !key.meta && buffer.text.length === 0 && - followup.state.isVisible && - followup.state.suggestion + availableSuggestion ) { - followup.accept('right'); + followup.accept('right', { + fallbackText: promptSuggestion ?? undefined, + }); + onPromptSuggestionDismiss?.(); return true; } @@ -1255,19 +1279,16 @@ export const InputPrompt: React.FC = ({ } if (keyMatchers[Command.SUBMIT](key)) { - // Accept and submit prompt suggestion on Enter when input is truly empty - if ( - buffer.text.length === 0 && - followup.state.isVisible && - followup.state.suggestion - ) { - const text = followup.state.suggestion; - // Skip onAccept (buffer.insert) — we pass the text directly to - // handleSubmitAndClear which clears the buffer synchronously. - // Without skipOnAccept the microtask in accept() would re-insert - // the suggestion into the buffer after it was already cleared. - followup.accept('enter', { skipOnAccept: true }); - handleSubmitAndClear(text); + // When buffer is empty and a suggestion is available, Enter fills the + // buffer instead of submitting — matching Tab/Right-arrow behavior. + // This prevents accidental execution of destructive slash commands + // (/clear, /quit) and aligns with Claude Code's design: suggestion + // acceptance requires explicit Tab or arrow-key action. + if (buffer.text.length === 0 && availableSuggestion) { + followup.accept('enter', { + fallbackText: promptSuggestion ?? undefined, + }); + onPromptSuggestionDismiss?.(); return true; } if (buffer.text.trim()) { @@ -1360,7 +1381,7 @@ export const InputPrompt: React.FC = ({ // Dismiss follow-up suggestion only on printable character input if ( buffer.text.length === 0 && - followup.state.isVisible && + availableSuggestion && key.sequence && key.sequence.length === 1 && !key.ctrl && @@ -1442,7 +1463,9 @@ export const InputPrompt: React.FC = ({ enterBgDetailFromPanel, setBgSelectedIndex, followup, + availableSuggestion, onPromptSuggestionDismiss, + promptSuggestion, exportCompletion, isHistoryRestoredText, showCompletionSuggestions, @@ -1595,9 +1618,15 @@ export const InputPrompt: React.FC = ({ // consumed (ACCEPT_SUGGESTION_REVERSE_SEARCH). When they are active with no // matches, Tab is not consumed — so the bare `reverseSearchActive` / // `commandSearchActive` flags are intentionally NOT included here. + // Mirror exactly when the Tab/Right/Enter handlers actually consume the key: + // the buffer must be empty and a suggestion must be available (either from the + // followup controller or the `promptSuggestion` prop after type-then-delete). + // Tying this to `buffer.text.length === 0` — the same gate the handlers use — + // keeps Windows Tab approval-mode cycling correct: as soon as the user types, + // this drops to false; deleting back to empty restores it. const hasTabConsumer = shouldShowSuggestions || - (followup.state.isVisible && Boolean(followup.state.suggestion)) || + (buffer.text.length === 0 && Boolean(availableSuggestion)) || Boolean(completion.midInputGhostText?.acceptText); // Narrow signal — autocomplete dropdown only. Composer hides Footer / @@ -1706,11 +1735,7 @@ export const InputPrompt: React.FC = ({ onSubmit={handleSubmitAndClear} onKeypress={handleInput} showCursor={showCursor} - placeholder={ - followup.state.isVisible && followup.state.suggestion - ? followup.state.suggestion - : placeholder - } + placeholder={availableSuggestion ?? placeholder} prefix={prefixNode} prefixWidth={prefixWidth} borderColor={borderColor} diff --git a/packages/cli/src/ui/components/MainContent.test.tsx b/packages/cli/src/ui/components/MainContent.test.tsx index a11386c07a..a751bb8e38 100644 --- a/packages/cli/src/ui/components/MainContent.test.tsx +++ b/packages/cli/src/ui/components/MainContent.test.tsx @@ -225,7 +225,7 @@ const createUIState = (overrides: Partial = {}): UIState => sessionName: null, setSessionName: vi.fn(), promptSuggestion: null, - dismissPromptSuggestion: vi.fn(), + abortPromptSuggestion: vi.fn(), isRewindSelectorOpen: false, rewindEscPending: false, ...overrides, diff --git a/packages/cli/src/ui/contexts/UIStateContext.tsx b/packages/cli/src/ui/contexts/UIStateContext.tsx index dc3ce8af8e..8b70a897b3 100644 --- a/packages/cli/src/ui/contexts/UIStateContext.tsx +++ b/packages/cli/src/ui/contexts/UIStateContext.tsx @@ -186,8 +186,12 @@ export interface UIState { setSessionName: (name: string | null) => void; // Prompt suggestion promptSuggestion: string | null; - /** Dismiss prompt suggestion (clears state, aborts speculation) */ - dismissPromptSuggestion: () => void; + /** + * Abort in-flight suggestion generation/speculation; intentionally preserves + * `promptSuggestion` so the placeholder can restore it when the buffer is + * emptied again. + */ + abortPromptSuggestion: () => void; // Rewind selector isRewindSelectorOpen: boolean; rewindEscPending: boolean; diff --git a/packages/cli/src/ui/hooks/useFollowupSuggestions.test.tsx b/packages/cli/src/ui/hooks/useFollowupSuggestions.test.tsx new file mode 100644 index 0000000000..3ca712777e --- /dev/null +++ b/packages/cli/src/ui/hooks/useFollowupSuggestions.test.tsx @@ -0,0 +1,104 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { act, renderHook } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Config, PromptSuggestionEvent } from '@qwen-code/qwen-code-core'; + +const { mockLogPromptSuggestion } = vi.hoisted(() => ({ + mockLogPromptSuggestion: vi.fn(), +})); + +vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + logPromptSuggestion: mockLogPromptSuggestion, + }; +}); + +import { useFollowupSuggestionsCLI } from './useFollowupSuggestions.js'; + +// Mirror of SUGGESTION_DELAY_MS in core's followupState controller. +const SUGGESTION_DELAY_MS = 300; +const config = {} as Config; + +function acceptedEvent(): PromptSuggestionEvent | undefined { + const call = mockLogPromptSuggestion.mock.calls.find( + ([, event]) => (event as PromptSuggestionEvent).outcome === 'accepted', + ); + return call?.[1] as PromptSuggestionEvent | undefined; +} + +describe('useFollowupSuggestionsCLI telemetry', () => { + beforeEach(() => { + vi.useFakeTimers(); + mockLogPromptSuggestion.mockReset(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('omits time_to_first_keystroke_ms on fallback accepts even with stale shown/keystroke refs', () => { + const { result } = renderHook(() => + useFollowupSuggestionsCLI({ enabled: true, config }), + ); + + // Show a live suggestion so prevShownAtRef + firstKeystrokeAtRef get set + // from this turn — exactly the stale state that would corrupt a later + // fallback accept if the accept_source guard were missing. + act(() => { + result.current.setSuggestion('run the tests'); + vi.advanceTimersByTime(SUGGESTION_DELAY_MS); + }); + expect(result.current.state.isVisible).toBe(true); + + act(() => { + vi.advanceTimersByTime(50); + result.current.recordKeystroke(); + }); + + // Drop the live suggestion and accept via fallback in the same tick, before + // the ref-reset effect runs. accept_source is 'fallback' (no live + // suggestion), so the keystroke delta must be suppressed. + act(() => { + result.current.setSuggestion(null); + result.current.accept('tab', { fallbackText: 'run the tests' }); + }); + + const event = acceptedEvent(); + expect(event).toBeDefined(); + expect(event?.accept_source).toBe('fallback'); + expect(event?.time_to_first_keystroke_ms).toBeUndefined(); + }); + + it('includes time_to_first_keystroke_ms on live accepts', () => { + const { result } = renderHook(() => + useFollowupSuggestionsCLI({ enabled: true, config }), + ); + + act(() => { + result.current.setSuggestion('run the tests'); + vi.advanceTimersByTime(SUGGESTION_DELAY_MS); + }); + + act(() => { + vi.advanceTimersByTime(50); + result.current.recordKeystroke(); + }); + + act(() => { + result.current.accept('tab'); + }); + + const event = acceptedEvent(); + expect(event).toBeDefined(); + expect(event?.accept_source).toBe('live'); + expect(event?.time_to_first_keystroke_ms).toBe(50); + }); +}); diff --git a/packages/cli/src/ui/hooks/useFollowupSuggestions.tsx b/packages/cli/src/ui/hooks/useFollowupSuggestions.tsx index 4ff8e4d686..ab504c177d 100644 --- a/packages/cli/src/ui/hooks/useFollowupSuggestions.tsx +++ b/packages/cli/src/ui/hooks/useFollowupSuggestions.tsx @@ -45,7 +45,7 @@ export interface UseFollowupSuggestionsReturn { /** Accept the current suggestion */ accept: ( method?: 'tab' | 'enter' | 'right', - options?: { skipOnAccept?: boolean }, + options?: { skipOnAccept?: boolean; fallbackText?: string }, ) => void; /** Dismiss the current suggestion */ dismiss: () => void; @@ -104,6 +104,7 @@ export function useFollowupSuggestionsCLI( (params: { outcome: 'accepted' | 'ignored'; accept_method?: 'tab' | 'enter' | 'right'; + accept_source?: 'live' | 'fallback'; time_ms: number; suggestion_length: number; }) => { @@ -114,10 +115,15 @@ export function useFollowupSuggestionsCLI( new PromptSuggestionEvent({ outcome: params.outcome, accept_method: params.accept_method, + accept_source: params.accept_source, ...(params.outcome === 'accepted' ? { time_to_accept_ms: params.time_ms } : { time_to_ignore_ms: params.time_ms }), - ...(firstKeystrokeAtRef.current > 0 && + // Skip for fallback accepts: the suggestion was never shown via the + // timer (shownAt stayed 0), so `prevShownAtRef` still holds a previous + // suggestion's timestamp and the delta would be meaningless. + ...(params.accept_source !== 'fallback' && + firstKeystrokeAtRef.current > 0 && prevShownAtRef.current > 0 && { time_to_first_keystroke_ms: firstKeystrokeAtRef.current - prevShownAtRef.current, diff --git a/packages/core/src/followup/followupState.test.ts b/packages/core/src/followup/followupState.test.ts index 331a28db5e..f0f787498b 100644 --- a/packages/core/src/followup/followupState.test.ts +++ b/packages/core/src/followup/followupState.test.ts @@ -190,6 +190,91 @@ describe('createFollowupController', () => { ctrl.cleanup(); }); + it('accept with fallbackText logs telemetry and inserts when there is no live suggestion', async () => { + const onStateChange = vi.fn(); + const onOutcome = vi.fn(); + const onAccept = vi.fn(); + const ctrl = createFollowupController({ + onStateChange, + onOutcome, + getOnAccept: () => onAccept, + }); + + // No setSuggestion + advance, so currentState.suggestion stays null — + // mirrors the InputPrompt type-then-delete / pre-delay fallback where the + // placeholder text only lives in the `promptSuggestion` prop. + ctrl.accept('right', { fallbackText: 'commit this' }); + + expect(onOutcome).toHaveBeenCalledTimes(1); + expect(onOutcome).toHaveBeenCalledWith( + expect.objectContaining({ + outcome: 'accepted', + accept_method: 'right', + accept_source: 'fallback', + suggestion_length: 11, + }), + ); + + // onAccept still fires via microtask with the fallback text + await Promise.resolve(); + expect(onAccept).toHaveBeenCalledTimes(1); + expect(onAccept).toHaveBeenCalledWith('commit this'); + + ctrl.cleanup(); + }); + + it('accept prefers the live suggestion over fallbackText and reports source "live"', async () => { + const onStateChange = vi.fn(); + const onOutcome = vi.fn(); + const onAccept = vi.fn(); + const ctrl = createFollowupController({ + onStateChange, + onOutcome, + getOnAccept: () => onAccept, + }); + + ctrl.setSuggestion('live suggestion'); + vi.advanceTimersByTime(300); + + // A live suggestion is present; fallbackText must be ignored. Guards the + // `currentState.suggestion ?? options.fallbackText` ordering — a flip would + // silently corrupt the accepted text and telemetry length. + ctrl.accept('tab', { fallbackText: 'fallback text' }); + + expect(onOutcome).toHaveBeenCalledWith( + expect.objectContaining({ + outcome: 'accepted', + accept_source: 'live', + suggestion_length: 'live suggestion'.length, + }), + ); + + await Promise.resolve(); + expect(onAccept).toHaveBeenCalledTimes(1); + expect(onAccept).toHaveBeenCalledWith('live suggestion'); + + ctrl.cleanup(); + }); + + it('accept without a live suggestion or fallbackText is a no-op', async () => { + const onStateChange = vi.fn(); + const onOutcome = vi.fn(); + const onAccept = vi.fn(); + const ctrl = createFollowupController({ + onStateChange, + onOutcome, + getOnAccept: () => onAccept, + }); + + ctrl.accept('tab'); + + await Promise.resolve(); + expect(onOutcome).not.toHaveBeenCalled(); + expect(onAccept).not.toHaveBeenCalled(); + + ctrl.cleanup(); + }); + it('onOutcome fires with ignored on dismiss', () => { const onStateChange = vi.fn(); const onOutcome = vi.fn(); diff --git a/packages/core/src/followup/followupState.ts b/packages/core/src/followup/followupState.ts index 60e8f717a0..e62c5e0313 100644 --- a/packages/core/src/followup/followupState.ts +++ b/packages/core/src/followup/followupState.ts @@ -59,6 +59,13 @@ export interface FollowupControllerOptions { onOutcome?: (params: { outcome: 'accepted' | 'ignored'; accept_method?: 'tab' | 'enter' | 'right'; + /** + * Whether the accepted text came from the live controller suggestion or + * from `fallbackText` (no live suggestion — e.g. type-then-delete or + * within the show delay). Lets analytics distinguish the two, since a + * fallback accept reports `time_ms: 0` (it was never shown via the timer). + */ + accept_source?: 'live' | 'fallback'; time_ms: number; suggestion_length: number; }) => void; @@ -71,10 +78,17 @@ export interface FollowupControllerOptions { export interface FollowupControllerActions { /** Set suggestion text (with delayed show). Null clears immediately. */ setSuggestion: (text: string | null) => void; - /** Accept the current suggestion and invoke onAccept callback */ + /** + * Accept the current suggestion and invoke onAccept callback. + * + * When the controller has no live suggestion (e.g. still within the show + * delay, or dismissed after type-then-delete while the placeholder text is + * still available), pass `fallbackText` so the accept — including telemetry — + * is logged instead of silently bypassing this path. + */ accept: ( method?: 'tab' | 'enter' | 'right', - options?: { skipOnAccept?: boolean }, + options?: { skipOnAccept?: boolean; fallbackText?: string }, ) => void; /** Dismiss/clear suggestion */ dismiss: () => void; @@ -140,7 +154,7 @@ export function createFollowupController( const accept = ( method?: 'tab' | 'enter' | 'right', - options?: { skipOnAccept?: boolean }, + options?: { skipOnAccept?: boolean; fallbackText?: string }, ): void => { if (accepting) { return; @@ -153,7 +167,10 @@ export function createFollowupController( accepting = true; - const text = currentState.suggestion; + const accept_source: 'live' | 'fallback' = currentState.suggestion + ? 'live' + : 'fallback'; + const text = currentState.suggestion ?? options?.fallbackText ?? null; const { shownAt } = currentState; if (!text) { accepting = false; @@ -164,6 +181,7 @@ export function createFollowupController( onOutcome?.({ outcome: 'accepted', accept_method: method, + accept_source, time_ms: shownAt > 0 ? Date.now() - shownAt : 0, suggestion_length: text.length, }); diff --git a/packages/core/src/followup/suggestionGenerator.test.ts b/packages/core/src/followup/suggestionGenerator.test.ts index 15abf5081a..adac3a4f02 100644 --- a/packages/core/src/followup/suggestionGenerator.test.ts +++ b/packages/core/src/followup/suggestionGenerator.test.ts @@ -39,6 +39,7 @@ vi.mock('../telemetry/uiTelemetry.js', async (importOriginal) => { import { generatePromptSuggestion, + getFilterReason, shouldFilterSuggestion, } from './suggestionGenerator.js'; @@ -178,6 +179,15 @@ describe('shouldFilterSuggestion', () => { expect(shouldFilterSuggestion('line1\nline2')).toBe(true); }); + it('filters control characters and ANSI escapes', () => { + expect(shouldFilterSuggestion('run\rtests')).toBe(true); // carriage return + expect(shouldFilterSuggestion('run\x1b[31mtests')).toBe(true); // ESC/CSI + expect(shouldFilterSuggestion('run\ttests')).toBe(true); // tab (C0) + expect(shouldFilterSuggestion('run\x7ftests')).toBe(true); // DEL + expect(shouldFilterSuggestion('run\x9btests')).toBe(true); // C1 CSI + expect(getFilterReason('run\x1b[31mtests')).toBe('control_chars'); + }); + it('filters evaluative language', () => { expect(shouldFilterSuggestion('looks good to me')).toBe(true); expect(shouldFilterSuggestion('thanks for the help')).toBe(true); diff --git a/packages/core/src/followup/suggestionGenerator.ts b/packages/core/src/followup/suggestionGenerator.ts index e84fa2c97e..5f79cfe488 100644 --- a/packages/core/src/followup/suggestionGenerator.ts +++ b/packages/core/src/followup/suggestionGenerator.ts @@ -274,6 +274,15 @@ export function getFilterReason(suggestion: string): string | null { const lower = suggestion.toLowerCase(); const wordCount = suggestion.trim().split(/\s+/).length; + // Reject C0/C1 control bytes and ANSI escapes first. The suggestion is + // influenceable through conversation history (tool/file/web output) and is + // rendered verbatim in the input placeholder, so raw control chars (CR, + // ESC/CSI, etc.) could be injected into the terminal. Rejecting here keeps the + // displayed and inserted text consistent, since the accept path strips them on + // buffer.insert. + // eslint-disable-next-line no-control-regex + if (/[\u0000-\u001f\u007f-\u009f]/.test(suggestion)) return 'control_chars'; + if (lower === 'done') return 'done'; if ( diff --git a/packages/core/src/telemetry/loggers.ts b/packages/core/src/telemetry/loggers.ts index 6084d3f06b..143d2ce688 100644 --- a/packages/core/src/telemetry/loggers.ts +++ b/packages/core/src/telemetry/loggers.ts @@ -1144,6 +1144,9 @@ export function logPromptSuggestion( if (event.accept_method) { attributes['accept_method'] = event.accept_method; } + if (event.accept_source) { + attributes['accept_source'] = event.accept_source; + } if (event.time_to_accept_ms !== undefined) { attributes['time_to_accept_ms'] = event.time_to_accept_ms; } diff --git a/packages/core/src/telemetry/types.ts b/packages/core/src/telemetry/types.ts index 64ae817eb8..8f5c9f56ed 100644 --- a/packages/core/src/telemetry/types.ts +++ b/packages/core/src/telemetry/types.ts @@ -1211,6 +1211,7 @@ export class PromptSuggestionEvent implements BaseTelemetryEvent { outcome: 'accepted' | 'ignored' | 'suppressed'; prompt_id?: string; accept_method?: 'tab' | 'enter' | 'right'; + accept_source?: 'live' | 'fallback'; time_to_accept_ms?: number; time_to_ignore_ms?: number; time_to_first_keystroke_ms?: number; @@ -1223,6 +1224,7 @@ export class PromptSuggestionEvent implements BaseTelemetryEvent { outcome: 'accepted' | 'ignored' | 'suppressed'; prompt_id?: string; accept_method?: 'tab' | 'enter' | 'right'; + accept_source?: 'live' | 'fallback'; time_to_accept_ms?: number; time_to_ignore_ms?: number; time_to_first_keystroke_ms?: number; @@ -1236,6 +1238,7 @@ export class PromptSuggestionEvent implements BaseTelemetryEvent { this.outcome = params.outcome; this.prompt_id = params.prompt_id ?? 'user_intent'; this.accept_method = params.accept_method; + this.accept_source = params.accept_source; this.time_to_accept_ms = params.time_to_accept_ms; this.time_to_ignore_ms = params.time_to_ignore_ms; this.time_to_first_keystroke_ms = params.time_to_first_keystroke_ms; diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index 5671d67cd0..a54a13bd92 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -255,9 +255,9 @@ "default": true }, "enableFollowupSuggestions": { - "description": "Show context-aware follow-up suggestions after task completion. Press Tab or Right Arrow to accept, Enter to accept and submit.", + "description": "Show context-aware follow-up suggestions after task completion. Press Tab, Right Arrow, or Enter to accept into the input buffer.", "type": "boolean", - "default": false + "default": true }, "enableCacheSharing": { "description": "Use cache-aware forked queries for suggestion generation. Reduces cost on providers that support prefix caching (experimental).",