Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/cli/src/config/settingsSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ const SETTINGS_SCHEMA = {
label: 'Enable Follow-up Suggestions',
category: 'UI',
requiresRestart: false,
default: false,
default: true,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] The description on line 783 still says "Enter to accept and submit", but this PR changed Enter to only fill the buffer (matching Tab/Right Arrow). Since enableFollowupSuggestions now defaults to true, every user will see this misleading description in settings.

Update the description in both settingsSchema.ts and settings.schema.json:

Suggested change
default: true,
default: true,
description:
'Show context-aware follow-up suggestions after task completion. Press Tab, Right Arrow, or Enter to accept into the input buffer.',

— qwen3.7-max via Qwen Code /review

description:
'Show context-aware follow-up suggestions after task completion. Press Tab or Right Arrow to accept, Enter to accept and submit.',
showInDialog: true,
Expand Down
12 changes: 7 additions & 5 deletions packages/cli/src/ui/AppContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1470,9 +1470,10 @@ export const AppContainer = (props: AppContainerProps) => {
const speculationRef = useRef<SpeculationState>(IDLE_SPECULATION);
const suggestionAbortRef = useRef<AbortController | null>(null);

// Dismiss callback — clears suggestion + aborts in-flight generation/speculation
// Dismiss callback — aborts in-flight generation/speculation. Does NOT
// clear `promptSuggestion` so the placeholder can restore it when the
// buffer becomes empty again (user types then deletes).
const dismissPromptSuggestion = useCallback(() => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] dismissPromptSuggestion no longer clears promptSuggestion state despite the "dismiss" name. The comment acknowledges this and suggests renaming but defers it. Future maintainers adding call sites will assume the suggestion disappears from the UI.

The rename is a one-line change — do it now before the misleading name ships to main. abortPromptSuggestion matches what the function actually does (abort generation/speculation only).

— qwen3.7-max via Qwen Code /review

setPromptSuggestion(null);
suggestionAbortRef.current?.abort();
suggestionAbortRef.current = null;
}, []);
Expand Down Expand Up @@ -2207,9 +2208,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(() => {});
Expand Down
4 changes: 3 additions & 1 deletion packages/cli/src/ui/components/Composer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,9 @@ export const Composer = () => {
placeholder={
vimEnabled
? ' ' + t("Press 'i' for INSERT mode and 'Esc' for NORMAL mode.")
: ' ' + t('Type your message or @path/to/file')
: uiState.promptSuggestion
? ' ' + uiState.promptSuggestion
: ' ' + t('Type your message or @path/to/file')
}
promptSuggestion={uiState.promptSuggestion}
onPromptSuggestionDismiss={uiState.dismissPromptSuggestion}
Expand Down
90 changes: 90 additions & 0 deletions packages/cli/src/ui/components/InputPrompt.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,77 @@ 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).
describe('promptSuggestion prop fallback (when followup.state.suggestion is null)', () => {
it('accepts promptSuggestion via Tab when followup.state.suggestion is null', async () => {
vi.useFakeTimers();
const { stdin, unmount } = renderWithProviders(
<InputPrompt {...props} promptSuggestion="commit this" />,
);
try {
await advanceTimers(SUGGESTION_VISIBLE_WAIT_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(
<InputPrompt {...props} promptSuggestion="commit this" />,
);
try {
await advanceTimers(SUGGESTION_VISIBLE_WAIT_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('submits promptSuggestion via Enter when followup.state.suggestion is null', async () => {
vi.useFakeTimers();
const { stdin, unmount } = renderWithProviders(
<InputPrompt {...props} promptSuggestion="commit this" />,
);
try {
await advanceTimers(SUGGESTION_VISIBLE_WAIT_MS);

act(() => {
stdin.write('\r');
});
await flush();

expect(props.onSubmit).toHaveBeenCalledWith('commit this');
// Enter path must NOT call buffer.insert — it passes text directly to
// handleSubmitAndClear.
expect(mockBuffer.insert).not.toHaveBeenCalled();
} finally {
vi.useRealTimers();
unmount();
}
});
});
});

// Regression for #4171: `onTabConsumerChange` (consumed by AppContainer
Expand All @@ -478,6 +549,25 @@ 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(
<InputPrompt
{...props}
promptSuggestion="commit this"
onTabConsumerChange={onTabConsumerChange}
/>,
);
// Must report true immediately — no need to wait for followup debounce
// because `hasTabConsumer` includes `Boolean(promptSuggestion)`.
expect(onTabConsumerChange).toHaveBeenCalledWith(true);
unmount();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The core behavioral change of this PR — replacing suggestionDismissed (sticky until new suggestion) with buffer.text.length === 0 (reactive to current buffer) — has two untested transitions:

  1. Type-then-delete round trip: after typing then deleting back to empty, hasTabConsumer should restore to true. The old suggestionDismissed would have kept it false. No test covers this divergence.

  2. Type-to-non-empty: hasTabConsumer should drop to false when the user types while promptSuggestion is set. This is the exact transition the old setSuggestionDismissed(true) handled.

Consider adding a test that renders with promptSuggestion, types a character (asserts onTabConsumerChange(false)), backspaces to empty (asserts onTabConsumerChange(true) again). This pins the behavioral contract of the new buffer.text.length === 0 gate.

— qwen3.7-max via Qwen Code /review

});

it('reports true while mid-input ghost text offers an accept', async () => {
mockCommandCompletion.midInputGhostText = {
text: 'ile.txt',
Expand Down
44 changes: 31 additions & 13 deletions packages/cli/src/ui/components/InputPrompt.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,10 @@ export const InputPrompt: React.FC<InputPromptProps> = ({

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 &&
(followup.state.isVisible || promptSuggestion)
) {
followup.dismiss();
onPromptSuggestionDismiss?.();
}
Expand Down Expand Up @@ -987,10 +990,18 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
!showCompletionSuggestions &&
!reverseSearchActive &&
!commandSearchActive &&
followup.state.isVisible &&
followup.state.suggestion
(followup.state.isVisible || promptSuggestion) &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The expression (followup.state.isVisible || promptSuggestion) && (followup.state.suggestion ?? promptSuggestion) is copy-pasted across 5 handler sites (Tab, Right-arrow, Enter, printable-char dismiss) plus the placeholder prop. Each site must stay structurally identical — a partial edit to one silently creates a handler inconsistency.

Consider extracting a single derived value near the top of handleInput:

const availableSuggestion =
  (followup.state.isVisible || promptSuggestion)
    ? (followup.state.suggestion ?? promptSuggestion ?? null)
    : null;

Then each handler checks buffer.text.length === 0 && availableSuggestion and the placeholder becomes availableSuggestion ?? placeholder. One site to change, five sites protected.

— qwen3.7-max via Qwen Code /review

(followup.state.suggestion ?? promptSuggestion)
) {
followup.accept('tab');
// If followup state has a suggestion, use the normal accept path.
// Otherwise use `promptSuggestion` directly (e.g. after user typed
// and deleted — the followup controller was dismissed but the
// placeholder text is still available).
if (followup.state.suggestion) {
followup.accept('tab');
} else if (promptSuggestion) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] When accepting a suggestion via the promptSuggestion fallback path, buffer.insert(promptSuggestion) bypasses followup.accept() entirely. The same applies to Right Arrow (line 1019) and Enter (line 1263, where followup.accept() early-returns because currentState.suggestion is null). No onOutcome telemetry event is logged for these fallback accepts — they appear as "ignored" in analytics, understating the feature's acceptance rate.

Consider logging telemetry explicitly in each fallback branch.

— qwen3.7-max via Qwen Code /review

buffer.insert(promptSuggestion);
}
return true;
}

Expand All @@ -1000,10 +1011,14 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
!key.ctrl &&
!key.meta &&
buffer.text.length === 0 &&
followup.state.isVisible &&
followup.state.suggestion
(followup.state.isVisible || promptSuggestion) &&
(followup.state.suggestion ?? promptSuggestion)
) {
followup.accept('right');
if (followup.state.suggestion) {
followup.accept('right');
} else if (promptSuggestion) {
buffer.insert(promptSuggestion);
}
return true;
}

Expand Down Expand Up @@ -1242,10 +1257,10 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
// Accept and submit prompt suggestion on Enter when input is truly empty
if (
buffer.text.length === 0 &&
followup.state.isVisible &&
followup.state.suggestion
(followup.state.isVisible || promptSuggestion) &&
(followup.state.suggestion ?? promptSuggestion)
) {
const text = followup.state.suggestion;
const text = followup.state.suggestion ?? promptSuggestion!;
// 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
Expand Down Expand Up @@ -1344,7 +1359,7 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
// Dismiss follow-up suggestion only on printable character input
if (
buffer.text.length === 0 &&
followup.state.isVisible &&
(followup.state.isVisible || promptSuggestion) &&
key.sequence &&
key.sequence.length === 1 &&
!key.ctrl &&
Expand Down Expand Up @@ -1426,6 +1441,7 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
setBgSelectedIndex,
followup,
onPromptSuggestionDismiss,
promptSuggestion,
exportCompletion,
isHistoryRestoredText,
showCompletionSuggestions,
Expand Down Expand Up @@ -1581,6 +1597,7 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
const hasTabConsumer =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] hasTabConsumer manually inlines the equivalent of availableSuggestion instead of reusing it. The comment above says "Mirror exactly when the Tab/Right/Enter handlers actually consume the key" yet uses a different expression. If availableSuggestion semantics change in the future, this parallel expression must be updated independently.

Suggested change
const hasTabConsumer =
const hasTabConsumer =
shouldShowSuggestions ||
(buffer.text.length === 0 && Boolean(availableSuggestion)) ||
Boolean(completion.midInputGhostText?.acceptText);

— qwen3.7-max via Qwen Code /review

shouldShowSuggestions ||
(followup.state.isVisible && Boolean(followup.state.suggestion)) ||
Boolean(promptSuggestion) ||

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] Boolean(promptSuggestion) stays true for the entire idle period after a suggestion is generated — it is only cleared on new turn or feature toggle, not on user input. This means hasTabConsumer reports true even when buffer.text.length > 0 and Tab wouldn't actually be consumed.

On Windows, shouldBlockTab returns true permanently, blocking Tab-cycles-approval-mode — a regression of the fix from issue #4171.

Suggested change
Boolean(promptSuggestion) ||
Boolean(promptSuggestion && buffer.text.length === 0) ||

— qwen3.7-max via Qwen Code /review

Boolean(completion.midInputGhostText?.acceptText);

// Narrow signal — autocomplete dropdown only. Composer hides Footer /
Expand Down Expand Up @@ -1679,8 +1696,9 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
onKeypress={handleInput}
showCursor={showCursor}
placeholder={
followup.state.isVisible && followup.state.suggestion
? followup.state.suggestion
(followup.state.isVisible || promptSuggestion) &&
(followup.state.suggestion ?? promptSuggestion)
? (followup.state.suggestion ?? promptSuggestion ?? undefined)
: placeholder
}
prefix={prefixNode}
Expand Down
2 changes: 1 addition & 1 deletion packages/vscode-ide-companion/schemas/settings.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@
"enableFollowupSuggestions": {
"description": "Show context-aware follow-up suggestions after task completion. Press Tab or Right Arrow to accept, Enter to accept and submit.",
"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).",
Expand Down
Loading