Skip to content

fix: preserve user messages at choice prompts in history (fixes #7316) #7317

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix: preserve user messages at choice prompts in history (fixes #7316) #7317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 14 additions & 7 deletions webview-ui/src/components/chat/ChatTextArea.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,13 +215,14 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
const [isFocused, setIsFocused] = useState(false)

// Use custom hook for prompt history navigation
const { handleHistoryNavigation, resetHistoryNavigation, resetOnInputChange } = usePromptHistory({
clineMessages,
taskHistory,
cwd,
inputValue,
setInputValue,
})
const { handleHistoryNavigation, resetHistoryNavigation, resetOnInputChange, addToLocalHistory } =
usePromptHistory({
clineMessages,
taskHistory,
cwd,
inputValue,
setInputValue,
})

// Fetch git commits when Git is selected or when typing a hash.
useEffect(() => {
Expand Down Expand Up @@ -465,6 +466,11 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
if (event.key === "Enter" && !event.shiftKey && !isComposing) {
event.preventDefault()

// Add to local history before sending
if (inputValue.trim()) {
addToLocalHistory(inputValue.trim())
Copy link
Contributor Author

@roomote roomote bot Aug 22, 2025

Choose a reason for hiding this comment

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

Is there a potential race condition between adding to local history and the actual send? If onSend() fails or is delayed, the message is already in local history. Is this intentional to preserve the message even on send failure, or should we only add after confirming the send was initiated?

}

// Always call onSend - let ChatView handle queueing when disabled
resetHistoryNavigation()
onSend()
Expand Down Expand Up @@ -531,6 +537,7 @@ const ChatTextArea = forwardRef<HTMLTextAreaElement, ChatTextAreaProps>(
handleHistoryNavigation,
resetHistoryNavigation,
commands,
addToLocalHistory,
],
)

Expand Down
46 changes: 43 additions & 3 deletions webview-ui/src/components/chat/hooks/usePromptHistory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface UsePromptHistoryReturn {
) => boolean
resetHistoryNavigation: () => void
resetOnInputChange: () => void
addToLocalHistory: (message: string) => void
Copy link
Contributor Author

@roomote roomote bot Aug 22, 2025

Choose a reason for hiding this comment

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

Consider adding JSDoc comments for the new addToLocalHistory function and localSentMessages state to explain their purpose and when they're used. This would help future maintainers understand why we need this local buffer for fixing the UX issue.

}

export const usePromptHistory = ({
Expand All @@ -38,6 +39,8 @@ export const usePromptHistory = ({
const [historyIndex, setHistoryIndex] = useState(-1)
const [tempInput, setTempInput] = useState("")
const [promptHistory, setPromptHistory] = useState<string[]>([])
// Local sent messages that haven't been confirmed by backend yet
const [localSentMessages, setLocalSentMessages] = useState<string[]>([])
Copy link
Contributor Author

@roomote roomote bot Aug 22, 2025

Choose a reason for hiding this comment

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

Potential memory concern here. The localSentMessages array grows up to MAX_PROMPT_HISTORY_SIZE but is only cleared when clineMessages becomes empty. In a long-running session, could this accumulate duplicates? Consider adding deduplication when adding to localSentMessages or clearing confirmed messages from the local buffer.


// Initialize prompt history with hybrid approach: conversation messages if in task, otherwise task history
const filteredPromptHistory = useMemo(() => {
Expand All @@ -46,9 +49,30 @@ export const usePromptHistory = ({
?.filter((message) => message.type === "say" && message.say === "user_feedback" && message.text?.trim())
.map((message) => message.text!)

// If we have conversation messages, use those (newest first when navigating up)
// Combine conversation prompts with local sent messages (deduplicated)
Copy link
Contributor Author

@roomote roomote bot Aug 22, 2025

Choose a reason for hiding this comment

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

This deduplication logic (lines 52-71) could be extracted into a utility function for better readability and potential reuse. Something like deduplicateAndMergePrompts(conversationPrompts, localSentMessages) would make the intent clearer.

const allPrompts: string[] = []
const seen = new Set<string>()

// Add conversation prompts first
if (conversationPrompts?.length) {
return conversationPrompts.slice(-MAX_PROMPT_HISTORY_SIZE).reverse()
conversationPrompts.forEach((prompt) => {
if (!seen.has(prompt)) {
seen.add(prompt)
allPrompts.push(prompt)
}
})
}

// Add local sent messages that aren't in conversation yet
localSentMessages.forEach((msg) => {
if (!seen.has(msg)) {
allPrompts.push(msg)
}
})

// If we have any prompts, use those (newest first when navigating up)
if (allPrompts.length) {
return allPrompts.slice(-MAX_PROMPT_HISTORY_SIZE).reverse()
}

// If we have clineMessages array (meaning we're in an active task), don't fall back to task history
Expand All @@ -67,7 +91,7 @@ export const usePromptHistory = ({
.filter((item) => item.task?.trim() && (!item.workspace || item.workspace === cwd))
.map((item) => item.task)
.slice(0, MAX_PROMPT_HISTORY_SIZE)
}, [clineMessages, taskHistory, cwd])
}, [clineMessages, taskHistory, cwd, localSentMessages])

// Update prompt history when filtered history changes and reset navigation
useEffect(() => {
Expand Down Expand Up @@ -175,6 +199,21 @@ export const usePromptHistory = ({
setTempInput("")
}, [])

// Add a message to local sent history
const addToLocalHistory = useCallback((message: string) => {
Copy link
Contributor Author

@roomote roomote bot Aug 22, 2025

Choose a reason for hiding this comment

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

Missing test coverage for this new functionality. Could we add tests to verify that messages are properly saved to local history when Enter is pressed at choice prompts? The existing tests don't cover the addToLocalHistory function or the local message preservation logic.

if (message?.trim()) {
setLocalSentMessages((prev) => [...prev, message].slice(-MAX_PROMPT_HISTORY_SIZE))
}
}, [])

// Clear local sent messages when task changes
useEffect(() => {
// When clineMessages changes significantly (new task), clear local messages
if (!clineMessages?.length) {
setLocalSentMessages([])
}
}, [clineMessages?.length])

return {
historyIndex,
setHistoryIndex,
Expand All @@ -184,5 +223,6 @@ export const usePromptHistory = ({
handleHistoryNavigation,
resetHistoryNavigation,
resetOnInputChange,
addToLocalHistory,
}
}
Loading