Fix several bugs in prompt history (#734)
Co-authored-by: Marat Boshernitsan <maratb@google.com>
This commit is contained in:
parent
c313762ba0
commit
7de790fbf2
|
@ -227,21 +227,36 @@ export const App = ({
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const fetchUserMessages = async () => {
|
const fetchUserMessages = async () => {
|
||||||
const pastMessages = (await logger?.getPreviousUserMessages()) || [];
|
const pastMessagesRaw = (await logger?.getPreviousUserMessages()) || []; // Newest first
|
||||||
if (pastMessages.length > 0) {
|
|
||||||
setUserMessages(pastMessages.reverse());
|
const currentSessionUserMessages = history
|
||||||
} else {
|
|
||||||
setUserMessages(
|
|
||||||
history
|
|
||||||
.filter(
|
.filter(
|
||||||
(item): item is HistoryItem & { type: 'user'; text: string } =>
|
(item): item is HistoryItem & { type: 'user'; text: string } =>
|
||||||
item.type === 'user' &&
|
item.type === 'user' &&
|
||||||
typeof item.text === 'string' &&
|
typeof item.text === 'string' &&
|
||||||
item.text.trim() !== '',
|
item.text.trim() !== '',
|
||||||
)
|
)
|
||||||
.map((item) => item.text),
|
.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();
|
fetchUserMessages();
|
||||||
}, [history, logger]);
|
}, [history, logger]);
|
||||||
|
|
|
@ -4,7 +4,7 @@
|
||||||
* SPDX-License-Identifier: Apache-2.0
|
* 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 { Text, Box, useInput, useStdin } from 'ink';
|
||||||
import { Colors } from '../colors.js';
|
import { Colors } from '../colors.js';
|
||||||
import { SuggestionsDisplay } from './SuggestionsDisplay.js';
|
import { SuggestionsDisplay } from './SuggestionsDisplay.js';
|
||||||
|
@ -19,7 +19,7 @@ import { isAtCommand, isSlashCommand } from '../utils/commandUtils.js';
|
||||||
import { SlashCommand } from '../hooks/slashCommandProcessor.js';
|
import { SlashCommand } from '../hooks/slashCommandProcessor.js';
|
||||||
import { Config } from '@gemini-code/core';
|
import { Config } from '@gemini-code/core';
|
||||||
|
|
||||||
interface InputPromptProps {
|
export interface InputPromptProps {
|
||||||
onSubmit: (value: string) => void;
|
onSubmit: (value: string) => void;
|
||||||
userMessages: readonly string[];
|
userMessages: readonly string[];
|
||||||
onClearScreen: () => void;
|
onClearScreen: () => void;
|
||||||
|
@ -54,6 +54,8 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
|
||||||
);
|
);
|
||||||
const suggestionsWidth = Math.max(60, Math.floor(terminalSize.columns * 0.8));
|
const suggestionsWidth = Math.max(60, Math.floor(terminalSize.columns * 0.8));
|
||||||
|
|
||||||
|
const [justNavigatedHistory, setJustNavigatedHistory] = useState(false);
|
||||||
|
|
||||||
const { stdin, setRawMode } = useStdin();
|
const { stdin, setRawMode } = useStdin();
|
||||||
|
|
||||||
const buffer = useTextBuffer({
|
const buffer = useTextBuffer({
|
||||||
|
@ -84,14 +86,35 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
|
||||||
[onSubmit, buffer, resetCompletionState],
|
[onSubmit, buffer, resetCompletionState],
|
||||||
);
|
);
|
||||||
|
|
||||||
|
const customSetTextAndResetCompletionSignal = useCallback(
|
||||||
|
(newText: string) => {
|
||||||
|
buffer.setText(newText);
|
||||||
|
setJustNavigatedHistory(true);
|
||||||
|
},
|
||||||
|
[buffer, setJustNavigatedHistory],
|
||||||
|
);
|
||||||
|
|
||||||
const inputHistory = useInputHistory({
|
const inputHistory = useInputHistory({
|
||||||
userMessages,
|
userMessages,
|
||||||
onSubmit: handleSubmitAndClear,
|
onSubmit: handleSubmitAndClear,
|
||||||
isActive: !completion.showSuggestions,
|
isActive: !completion.showSuggestions,
|
||||||
currentQuery: buffer.text,
|
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 completionSuggestions = completion.suggestions;
|
||||||
const handleAutocomplete = useCallback(
|
const handleAutocomplete = useCallback(
|
||||||
(indexToUse: number) => {
|
(indexToUse: number) => {
|
||||||
|
@ -276,8 +299,8 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
|
||||||
// Standard arrow navigation within the buffer
|
// Standard arrow navigation within the buffer
|
||||||
if (key.upArrow && !completion.showSuggestions) {
|
if (key.upArrow && !completion.showSuggestions) {
|
||||||
if (
|
if (
|
||||||
buffer.visualCursor[0] === 0 &&
|
(buffer.allVisualLines.length === 1 || // Always navigate for single line
|
||||||
buffer.visualScrollRow === 0 &&
|
(buffer.visualCursor[0] === 0 && buffer.visualScrollRow === 0)) &&
|
||||||
inputHistory.navigateUp
|
inputHistory.navigateUp
|
||||||
) {
|
) {
|
||||||
inputHistory.navigateUp();
|
inputHistory.navigateUp();
|
||||||
|
@ -288,7 +311,8 @@ export const InputPrompt: React.FC<InputPromptProps> = ({
|
||||||
}
|
}
|
||||||
if (key.downArrow && !completion.showSuggestions) {
|
if (key.downArrow && !completion.showSuggestions) {
|
||||||
if (
|
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
|
||||||
) {
|
) {
|
||||||
inputHistory.navigateDown();
|
inputHistory.navigateDown();
|
||||||
|
|
|
@ -138,4 +138,65 @@ describe('useHistoryManager', () => {
|
||||||
|
|
||||||
expect(result.current.history).toEqual([]);
|
expect(result.current.history).toEqual([]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should not add consecutive duplicate user messages', () => {
|
||||||
|
const { result } = renderHook(() => useHistory());
|
||||||
|
const timestamp = Date.now();
|
||||||
|
const itemData1: Omit<HistoryItem, 'id'> = {
|
||||||
|
type: 'user', // Replaced HistoryItemType.User
|
||||||
|
text: 'Duplicate message',
|
||||||
|
};
|
||||||
|
const itemData2: Omit<HistoryItem, 'id'> = {
|
||||||
|
type: 'user', // Replaced HistoryItemType.User
|
||||||
|
text: 'Duplicate message',
|
||||||
|
};
|
||||||
|
const itemData3: Omit<HistoryItem, 'id'> = {
|
||||||
|
type: 'gemini', // Replaced HistoryItemType.Gemini
|
||||||
|
text: 'Gemini response',
|
||||||
|
};
|
||||||
|
const itemData4: Omit<HistoryItem, 'id'> = {
|
||||||
|
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<HistoryItem, 'id'> = {
|
||||||
|
type: 'user', // Replaced HistoryItemType.User
|
||||||
|
text: 'Message 1',
|
||||||
|
};
|
||||||
|
const itemData2: Omit<HistoryItem, 'id'> = {
|
||||||
|
type: 'gemini', // Replaced HistoryItemType.Gemini
|
||||||
|
text: 'Gemini response',
|
||||||
|
};
|
||||||
|
const itemData3: Omit<HistoryItem, 'id'> = {
|
||||||
|
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');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
@ -43,8 +43,22 @@ export function useHistory(): UseHistoryManagerReturn {
|
||||||
(itemData: Omit<HistoryItem, 'id'>, baseTimestamp: number): number => {
|
(itemData: Omit<HistoryItem, 'id'>, baseTimestamp: number): number => {
|
||||||
const id = getNextMessageId(baseTimestamp);
|
const id = getNextMessageId(baseTimestamp);
|
||||||
const newItem: HistoryItem = { ...itemData, id } as HistoryItem;
|
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],
|
[getNextMessageId],
|
||||||
);
|
);
|
||||||
|
|
Loading…
Reference in New Issue