Repository navigation
feat(cli): show follow-up suggestion in input placeholder #5145
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 7 commits
9e2549c
0a3adce
7404c71
876d49a
06b62cb
e4d893f
e40f5a4
45aa1c3
fbd34b8
1796a14
bb6eb37
04fcffd
c791b20
7eebe85
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 |
|---|---|---|
|
|
@@ -379,7 +379,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( | ||
| <InputPrompt {...props} promptSuggestion="commit this" />, | ||
|
|
@@ -392,11 +395,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(); | ||
|
|
@@ -452,6 +453,86 @@ 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( | ||
| <InputPrompt {...props} promptSuggestion="commit this" />, | ||
| ); | ||
| 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( | ||
| <InputPrompt {...props} promptSuggestion="commit this" />, | ||
| ); | ||
| 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( | ||
| <InputPrompt {...props} promptSuggestion="commit this" />, | ||
| ); | ||
| 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 #4171: `onTabConsumerChange` (consumed by AppContainer | ||
|
|
@@ -478,6 +559,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( | ||
| <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(); | ||
|
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] The core behavioral change of this PR — replacing
Consider adding a test that renders with — qwen3.7-max via Qwen Code /review |
||
| }); | ||
|
|
||
| // 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( | ||
| <InputPrompt | ||
| {...props} | ||
| promptSuggestion="commit this" | ||
| onTabConsumerChange={onTabConsumerChange} | ||
| />, | ||
| ); | ||
| 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', | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -531,6 +531,16 @@ export const InputPrompt: React.FC<InputPromptProps> = ({ | |||||||||||||||||||||
| 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 | ||||||||||||||||||||||
|
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. [Critical] After running a synchronous slash command (e.g.,
Suggested change
Suggested fix: Add followup.dismiss();
onPromptSuggestionDismiss?.(); // abort generation + speculation + clear state— qwen3.7-max via Qwen Code /review |
||||||||||||||||||||||
| // 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 | ||||||||||||||||||||||
|
|
@@ -646,7 +656,7 @@ 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 && availableSuggestion) { | ||||||||||||||||||||||
| followup.dismiss(); | ||||||||||||||||||||||
| onPromptSuggestionDismiss?.(); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
@@ -987,10 +997,12 @@ export const InputPrompt: React.FC<InputPromptProps> = ({ | |||||||||||||||||||||
| !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 }); | ||||||||||||||||||||||
|
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. [Critical] After accepting a suggestion via Tab, Right Arrow, or Enter, Repro: Model suggests "/clear" → Tab to accept (buffer now shows "/clear") → Ctrl+U to clear buffer → "/clear" reappears as placeholder. This applies to all three accept handlers (Tab at line 1005, Right-arrow at line 1017, Enter at line 1261). Suggested fix: Add followup.accept('tab', { fallbackText: promptSuggestion ?? undefined });
onPromptSuggestionDismiss?.();
return true;— qwen3.7-max via Qwen Code /review |
||||||||||||||||||||||
| return true; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -1000,10 +1012,11 @@ export const InputPrompt: React.FC<InputPromptProps> = ({ | |||||||||||||||||||||
| !key.ctrl && | ||||||||||||||||||||||
| !key.meta && | ||||||||||||||||||||||
| buffer.text.length === 0 && | ||||||||||||||||||||||
| followup.state.isVisible && | ||||||||||||||||||||||
| followup.state.suggestion | ||||||||||||||||||||||
| availableSuggestion | ||||||||||||||||||||||
| ) { | ||||||||||||||||||||||
| followup.accept('right'); | ||||||||||||||||||||||
| followup.accept('right', { | ||||||||||||||||||||||
| fallbackText: promptSuggestion ?? undefined, | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
| return true; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -1239,19 +1252,15 @@ export const InputPrompt: React.FC<InputPromptProps> = ({ | |||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 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, | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
| return true; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| if (buffer.text.trim()) { | ||||||||||||||||||||||
|
|
@@ -1344,7 +1353,7 @@ export const InputPrompt: React.FC<InputPromptProps> = ({ | |||||||||||||||||||||
| // 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 && | ||||||||||||||||||||||
|
|
@@ -1425,7 +1434,9 @@ export const InputPrompt: React.FC<InputPromptProps> = ({ | |||||||||||||||||||||
| enterBgDetailFromPanel, | ||||||||||||||||||||||
| setBgSelectedIndex, | ||||||||||||||||||||||
| followup, | ||||||||||||||||||||||
| availableSuggestion, | ||||||||||||||||||||||
| onPromptSuggestionDismiss, | ||||||||||||||||||||||
| promptSuggestion, | ||||||||||||||||||||||
| exportCompletion, | ||||||||||||||||||||||
| isHistoryRestoredText, | ||||||||||||||||||||||
| showCompletionSuggestions, | ||||||||||||||||||||||
|
|
@@ -1578,9 +1589,17 @@ export const InputPrompt: React.FC<InputPromptProps> = ({ | |||||||||||||||||||||
| // 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 = | ||||||||||||||||||||||
|
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]
Suggested change
— qwen3.7-max via Qwen Code /review |
||||||||||||||||||||||
| shouldShowSuggestions || | ||||||||||||||||||||||
| (followup.state.isVisible && Boolean(followup.state.suggestion)) || | ||||||||||||||||||||||
| (buffer.text.length === 0 && | ||||||||||||||||||||||
| ((followup.state.isVisible && Boolean(followup.state.suggestion)) || | ||||||||||||||||||||||
| Boolean(promptSuggestion))) || | ||||||||||||||||||||||
| Boolean(completion.midInputGhostText?.acceptText); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Narrow signal — autocomplete dropdown only. Composer hides Footer / | ||||||||||||||||||||||
|
|
@@ -1678,11 +1697,7 @@ export const InputPrompt: React.FC<InputPromptProps> = ({ | |||||||||||||||||||||
| onSubmit={handleSubmitAndClear} | ||||||||||||||||||||||
| onKeypress={handleInput} | ||||||||||||||||||||||
| showCursor={showCursor} | ||||||||||||||||||||||
| placeholder={ | ||||||||||||||||||||||
| followup.state.isVisible && followup.state.suggestion | ||||||||||||||||||||||
| ? followup.state.suggestion | ||||||||||||||||||||||
| : placeholder | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| placeholder={availableSuggestion ?? placeholder} | ||||||||||||||||||||||
|
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] Now that The render sink is pre-existing (main already showed — claude-opus-4-8 via Claude Code /qreview
Contributor
Author
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. Addressed in Strip
Reject (chosen)
On balance reject wins here: the only thing strip buys us is salvaging a suggestion that contains control bytes — which in a 2-12 word hint is near-certainly noise/injection, so the lost-recall downside is negligible, while reject is simpler, matches the function's existing contract, and resolves the consistency issue more directly. Happy to switch to strip if you'd prefer to keep those suggestions. |
||||||||||||||||||||||
| prefix={prefixNode} | ||||||||||||||||||||||
| borderColor={borderColor} | ||||||||||||||||||||||
| topRightLabel={uiState.sessionName || undefined} | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
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.
[Critical] The
descriptionon line 783 still says "Enter to accept and submit", but this PR changed Enter to only fill the buffer (matching Tab/Right Arrow). SinceenableFollowupSuggestionsnow defaults totrue, every user will see this misleading description in settings.Update the description in both
settingsSchema.tsandsettings.schema.json:— qwen3.7-max via Qwen Code /review