-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix: Preserve first message during conversation condensing #7910
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
Conversation
When users invoke slash commands, the command content is appended to the first message. This fix ensures the first message is always preserved during condensing operations, preventing loss of vital context like slash command instructions. - Always preserve the first message intact during summarization - Only summarize messages from index 1 onwards - Maintains command context and initial user request throughout the conversation
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.
Thank you for this fix! The implementation correctly addresses the issue of preserving the first message during conversation condensing, which is crucial for maintaining slash command context. The approach elegantly mirrors the existing pattern in truncateConversation for consistency. I've left a few suggestions for consideration below.
| const messagesToSummarize = getMessagesSinceLastSummary(messages.slice(0, -N_MESSAGES_TO_KEEP)) | ||
|
|
||
| // Always preserve the first message (which may contain slash command content) | ||
| const firstMessage = messages[0] |
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.
Could we add a safety check here to handle the edge case where the messages array might be empty? While unlikely in practice, it would make the code more robust. Consider checking if messages.length === 0 before accessing messages[0].
| const response: SummarizeResponse = { messages, cost: 0, summary: "" } | ||
| const messagesToSummarize = getMessagesSinceLastSummary(messages.slice(0, -N_MESSAGES_TO_KEEP)) | ||
|
|
||
| // Always preserve the first message (which may contain slash command content) |
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.
Consider enhancing this comment to explicitly mention why we preserve the first message: 'Always preserve the first message (which may contain slash command content, initial user request, and important context)'. This would help future maintainers understand the critical importance of this preservation.
| // Always preserve the first message (which may contain slash command content) | ||
| const firstMessage = messages[0] | ||
| // Get messages to summarize, excluding the first message and last N messages | ||
| const messagesToSummarize = getMessagesSinceLastSummary(messages.slice(1, -N_MESSAGES_TO_KEEP)) |
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.
The logic here creates an implicit minimum of 4 messages (first + 3 to keep). Would it be clearer to extract this as a constant like MIN_MESSAGES_FOR_CONDENSING = N_MESSAGES_TO_KEEP + 1? Then the condition could be more explicit about the requirement.
|
|
||
| const newMessages = [...messages.slice(0, -N_MESSAGES_TO_KEEP), summaryMessage, ...keepMessages] | ||
| // Reconstruct messages: [first message, summary, last N messages] | ||
| const newMessages = [firstMessage, summaryMessage, ...keepMessages] |
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.
Nice work maintaining consistency with the structure. The reconstruction clearly shows the preserved first message, followed by the summary, and then the recent messages.
* Follow symlinks in rooignore checks (RooCodeInc#7405) * Sonic -> Grok Code Fast (RooCodeInc#7426) * chore: add changeset for v3.26.0 (RooCodeInc#7428) * Changeset version bump (RooCodeInc#7429) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Matt Rubens <[email protected]> * feat: Add Vercel AI Gateway provider integration (RooCodeInc#7396) Co-authored-by: daniel-lxs <[email protected]> Co-authored-by: cte <[email protected]> * feat: Enable on-disk storage for Qdrant vectors and HNSW index (RooCodeInc#7182) * fix: use anthropic protocol for token counting when using anthropic models via Vercel AI Gateway (RooCodeInc#7433) - Added condition in getApiProtocol to return 'anthropic' for vercel-ai-gateway when modelId starts with 'anthropic/' - Added tests for Vercel AI Gateway provider protocol detection This ensures proper token counting for Anthropic models accessed through Vercel AI Gateway, as Anthropic and OpenAI count tokens differently (Anthropic excludes cache tokens from input count, OpenAI includes them). * fix: remove duplicate cache display in task header (RooCodeInc#7443) * Random chat text area cleanup (RooCodeInc#7436) * Update @roo-code/cloud to enable roomote control for cloud agents (RooCodeInc#7446) Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com> * Always set remoteControlEnabled to true for cloud agents (RooCodeInc#7448) * chore: add changeset for v3.26.1 (RooCodeInc#7459) * feat: show model ID in API configuration dropdown (RooCodeInc#7423) * feat: update tooltip component to match native VSCode tooltip shadow styling (RooCodeInc#7457) Co-authored-by: Roo Code <[email protected]> Co-authored-by: cte <[email protected]> * Add support for Vercel embeddings (RooCodeInc#7445) Co-authored-by: daniel-lxs <[email protected]> * Remove dot before model display (RooCodeInc#7461) * Update contributors list (RooCodeInc#7109) Co-authored-by: mrubens <[email protected]> * Update 3.26.1 changeset (RooCodeInc#7463) * Changeset version bump (RooCodeInc#7460) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Matt Rubens <[email protected]> * Add type for RooCodeEventName.TaskSpawned (RooCodeInc#7465) * fix: hide .rooignore'd files from environment details by default (RooCodeInc#7369) * fix: change default showRooIgnoredFiles to false to hide ignored files - Changed default value from true to false across all files - Updated tests to reflect the new default behavior - This prevents ignored files from appearing in environment details Fixes RooCodeInc#7368 * fix: update tests to match new showRooIgnoredFiles default * fix: update test expectation to match new showRooIgnoredFiles default value The PR changed the default value of showRooIgnoredFiles from true to false, so the test needs to expect false instead of true when calling formatFilesList. --------- Co-authored-by: Roo Code <[email protected]> Co-authored-by: daniel-lxs <[email protected]> * fix: exclude browser scroll actions from repetition detection (RooCodeInc#7471) - Modified ToolRepetitionDetector to skip repetition detection for browser_action scroll_down and scroll_up actions - Added isBrowserScrollAction() helper method to identify scroll actions - Added comprehensive tests for the new behavior - Fixes issue where multiple scroll actions were incorrectly flagged as being stuck in a loop Resolves: RooCodeInc#7470 Co-authored-by: Roo Code <[email protected]> * Fix GPT-5 Responses API issues with condensing and image support (RooCodeInc#7067) Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com> Co-authored-by: Roo Code <[email protected]> Co-authored-by: Hannes Rudolph <[email protected]> * Bump cloud to 0.25.0 (RooCodeInc#7475) * feat: add image generation tool with OpenRouter integration (RooCodeInc#7474) Co-authored-by: Matt Rubens <[email protected]> Co-authored-by: cte <[email protected]> * Make the default image filename more generic (RooCodeInc#7479) * Release v3.26.2 (RooCodeInc#7490) * Support free imagegen (RooCodeInc#7493) * feat: update OpenRouter API to support input/output modalities and filter image generation models (RooCodeInc#7492) * Add padding to image model picker (
When users invoke slash commands, the command content is appended to the first message. This fix ensures the first message is always preserved during condensing operations, preventing loss of vital context like slash command instructions.
Changes
Testing
src/core/condense/__tests__/condense.spec.tsProblem
When a user types a slash command (e.g.,
/prr), the command content is appended to the first message viaparseMentions. However, when the conversation is condensed, this first message was replaced with a summary, losing the vital command content that was appended.Solution
Modified the
summarizeConversationfunction insrc/core/condense/index.tsto always preserve the first message intact, similar to howtruncateConversationinsrc/core/sliding-window/index.tsdoes.Important
Preserve the first message during conversation summarization to retain slash command content.
summarizeConversationinindex.tsnow preserves the first message during summarization.condense.spec.tsto verify first message preservation, handling of slash commands, and complex content.summarizeConversationto preserve the first message, similar totruncateConversation.This description was created by
for 3f504e7. You can customize this summary. It will automatically update as commits are pushed.