From 7de790fbf236193191a33482793ee3ec5943d62d Mon Sep 17 00:00:00 2001 From: Marat Boshernitsan Date: Tue, 3 Jun 2025 23:01:26 -0700 Subject: [PATCH] Fix several bugs in prompt history (#734) Co-authored-by: Marat Boshernitsan --- packages/cli/src/ui/App.tsx | 43 ++++++++----- .../cli/src/ui/components/InputPrompt.tsx | 36 +++++++++-- .../src/ui/hooks/useHistoryManager.test.ts | 61 +++++++++++++++++++ .../cli/src/ui/hooks/useHistoryManager.ts | 18 +++++- 4 files changed, 136 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/ui/App.tsx b/packages/cli/src/ui/App.tsx index 9ebefaa9..ddaf2f02 100644 --- a/packages/cli/src/ui/App.tsx +++ b/packages/cli/src/ui/App.tsx @@ -227,21 +227,36 @@ export const App = ({ useEffect(() => { const fetchUserMessages = async () => { - const pastMessages = (await logger?.getPreviousUserMessages()) || []; - if (pastMessages.length > 0) { - setUserMessages(pastMessages.reverse()); - } else { - setUserMessages( - history - .filter( - (item): item is HistoryItem & { type: 'user'; text: string } => - item.type === 'user' && - typeof item.text === 'string' && - item.text.trim() !== '', - ) - .map((item) => item.text), - ); + const pastMessagesRaw = (await logger?.getPreviousUserMessages()) || []; // Newest first + + const currentSessionUserMessages = history + .filter( + (item): item is HistoryItem & { type: 'user'; text: string } => + item.type === 'user' && + typeof item.text === 'string' && + item.text.trim() !== '', + ) + .map((item) => item.text) + .reverse(); // Newest first, to match pastMessagesRaw sorting + + // Combine, with current session messages being more recent + const combinedMessages = [ + ...currentSessionUserMessages, + ...pastMessagesRaw, + ]; + + // Deduplicate consecutive identical messages from the combined list (still newest first) + const deduplicatedMessages: string[] = []; + if (combinedMessages.length > 0) { + deduplicatedMessages.push(combinedMessages[0]); // Add the newest one unconditionally + for (let i = 1; i < combinedMessages.length; i++) { + if (combinedMessages[i] !== combinedMessages[i - 1]) { + deduplicatedMessages.push(combinedMessages[i]); + } + } } + // Reverse to oldest first for useInputHistory + setUserMessages(deduplicatedMessages.reverse()); }; fetchUserMessages(); }, [history, logger]); diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index c131f5e0..7e4c8c8b 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useCallback } from 'react'; +import React, { useCallback, useEffect, useState } from 'react'; import { Text, Box, useInput, useStdin } from 'ink'; import { Colors } from '../colors.js'; import { SuggestionsDisplay } from './SuggestionsDisplay.js'; @@ -19,7 +19,7 @@ import { isAtCommand, isSlashCommand } from '../utils/commandUtils.js'; import { SlashCommand } from '../hooks/slashCommandProcessor.js'; import { Config } from '@gemini-code/core'; -interface InputPromptProps { +export interface InputPromptProps { onSubmit: (value: string) => void; userMessages: readonly string[]; onClearScreen: () => void; @@ -54,6 +54,8 @@ export const InputPrompt: React.FC = ({ ); const suggestionsWidth = Math.max(60, Math.floor(terminalSize.columns * 0.8)); + const [justNavigatedHistory, setJustNavigatedHistory] = useState(false); + const { stdin, setRawMode } = useStdin(); const buffer = useTextBuffer({ @@ -84,14 +86,35 @@ export const InputPrompt: React.FC = ({ [onSubmit, buffer, resetCompletionState], ); + const customSetTextAndResetCompletionSignal = useCallback( + (newText: string) => { + buffer.setText(newText); + setJustNavigatedHistory(true); + }, + [buffer, setJustNavigatedHistory], + ); + const inputHistory = useInputHistory({ userMessages, onSubmit: handleSubmitAndClear, isActive: !completion.showSuggestions, currentQuery: buffer.text, - onChange: buffer.setText, + onChange: customSetTextAndResetCompletionSignal, }); + // Effect to reset completion if history navigation just occurred and set the text + useEffect(() => { + if (justNavigatedHistory) { + resetCompletionState(); + setJustNavigatedHistory(false); + } + }, [ + justNavigatedHistory, + buffer.text, + resetCompletionState, + setJustNavigatedHistory, + ]); + const completionSuggestions = completion.suggestions; const handleAutocomplete = useCallback( (indexToUse: number) => { @@ -276,8 +299,8 @@ export const InputPrompt: React.FC = ({ // Standard arrow navigation within the buffer if (key.upArrow && !completion.showSuggestions) { if ( - buffer.visualCursor[0] === 0 && - buffer.visualScrollRow === 0 && + (buffer.allVisualLines.length === 1 || // Always navigate for single line + (buffer.visualCursor[0] === 0 && buffer.visualScrollRow === 0)) && inputHistory.navigateUp ) { inputHistory.navigateUp(); @@ -288,7 +311,8 @@ export const InputPrompt: React.FC = ({ } if (key.downArrow && !completion.showSuggestions) { if ( - buffer.visualCursor[0] === buffer.allVisualLines.length - 1 && + (buffer.allVisualLines.length === 1 || // Always navigate for single line + buffer.visualCursor[0] === buffer.allVisualLines.length - 1) && inputHistory.navigateDown ) { inputHistory.navigateDown(); diff --git a/packages/cli/src/ui/hooks/useHistoryManager.test.ts b/packages/cli/src/ui/hooks/useHistoryManager.test.ts index 35964612..c7f925e2 100644 --- a/packages/cli/src/ui/hooks/useHistoryManager.test.ts +++ b/packages/cli/src/ui/hooks/useHistoryManager.test.ts @@ -138,4 +138,65 @@ describe('useHistoryManager', () => { expect(result.current.history).toEqual([]); }); + + it('should not add consecutive duplicate user messages', () => { + const { result } = renderHook(() => useHistory()); + const timestamp = Date.now(); + const itemData1: Omit = { + type: 'user', // Replaced HistoryItemType.User + text: 'Duplicate message', + }; + const itemData2: Omit = { + type: 'user', // Replaced HistoryItemType.User + text: 'Duplicate message', + }; + const itemData3: Omit = { + type: 'gemini', // Replaced HistoryItemType.Gemini + text: 'Gemini response', + }; + const itemData4: Omit = { + type: 'user', // Replaced HistoryItemType.User + text: 'Another user message', + }; + + act(() => { + result.current.addItem(itemData1, timestamp); + result.current.addItem(itemData2, timestamp + 1); // Same text, different timestamp + result.current.addItem(itemData3, timestamp + 2); + result.current.addItem(itemData4, timestamp + 3); + }); + + expect(result.current.history).toHaveLength(3); + expect(result.current.history[0].text).toBe('Duplicate message'); + expect(result.current.history[1].text).toBe('Gemini response'); + expect(result.current.history[2].text).toBe('Another user message'); + }); + + it('should add duplicate user messages if they are not consecutive', () => { + const { result } = renderHook(() => useHistory()); + const timestamp = Date.now(); + const itemData1: Omit = { + type: 'user', // Replaced HistoryItemType.User + text: 'Message 1', + }; + const itemData2: Omit = { + type: 'gemini', // Replaced HistoryItemType.Gemini + text: 'Gemini response', + }; + const itemData3: Omit = { + type: 'user', // Replaced HistoryItemType.User + text: 'Message 1', // Duplicate text, but not consecutive + }; + + act(() => { + result.current.addItem(itemData1, timestamp); + result.current.addItem(itemData2, timestamp + 1); + result.current.addItem(itemData3, timestamp + 2); + }); + + expect(result.current.history).toHaveLength(3); + expect(result.current.history[0].text).toBe('Message 1'); + expect(result.current.history[1].text).toBe('Gemini response'); + expect(result.current.history[2].text).toBe('Message 1'); + }); }); diff --git a/packages/cli/src/ui/hooks/useHistoryManager.ts b/packages/cli/src/ui/hooks/useHistoryManager.ts index 424f1fb6..f82707ef 100644 --- a/packages/cli/src/ui/hooks/useHistoryManager.ts +++ b/packages/cli/src/ui/hooks/useHistoryManager.ts @@ -43,8 +43,22 @@ export function useHistory(): UseHistoryManagerReturn { (itemData: Omit, baseTimestamp: number): number => { const id = getNextMessageId(baseTimestamp); const newItem: HistoryItem = { ...itemData, id } as HistoryItem; - setHistory((prevHistory) => [...prevHistory, newItem]); - return id; // Return the generated ID + + setHistory((prevHistory) => { + if (prevHistory.length > 0) { + const lastItem = prevHistory[prevHistory.length - 1]; + // Prevent adding duplicate consecutive user messages + if ( + lastItem.type === 'user' && + newItem.type === 'user' && + lastItem.text === newItem.text + ) { + return prevHistory; // Don't add the duplicate + } + } + return [...prevHistory, newItem]; + }); + return id; // Return the generated ID (even if not added, to keep signature) }, [getNextMessageId], );