Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions .tmp/review/Roo-Code
Submodule Roo-Code added at 8dbd8c
44 changes: 43 additions & 1 deletion src/core/condense/__tests__/condense.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { summarizeConversation, getMessagesSinceLastSummary, N_MESSAGES_TO_KEEP

// Create a mock ApiHandler for testing
class MockApiHandler extends BaseProvider {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

P2: This test override uses Anthropic.Messages.MessageParam[], coupling the test to a specific provider SDK. Prefer a provider-agnostic message type to reduce brittleness across providers.

createMessage(): any {
createMessage(systemPrompt: string, messages: Anthropic.Messages.MessageParam[], metadata?: any): any {
// Mock implementation for testing - returns an async iterable stream
const mockStream = {
async *[Symbol.asyncIterator]() {
Expand Down Expand Up @@ -204,6 +204,48 @@ describe("Condense", () => {
expect(result.messages).toEqual(messages)
expect(result.cost).toBeGreaterThan(0)
})

it("should include the first message in the summarization content", async () => {
// Create a mock handler that captures what messages are sent for summarization
let capturedMessages: any[] = []
class CapturingMockApiHandler extends MockApiHandler {
override createMessage(
systemPrompt: string,
messages: Anthropic.Messages.MessageParam[],
metadata?: any,
): any {
// Capture the messages sent for summarization (excluding the final request message)
capturedMessages = messages.slice(0, -1)
return super.createMessage(systemPrompt, messages, metadata)
}
}

const capturingHandler = new CapturingMockApiHandler()
const messages: ApiMessage[] = [
{ role: "user", content: "Initial task: Create a TODO app" },
{ role: "assistant", content: "I'll help you create a TODO app" },
{ role: "user", content: "Add user authentication" },
{ role: "assistant", content: "Adding authentication" },
{ role: "user", content: "Add database support" },
{ role: "assistant", content: "Setting up database" },
{ role: "user", content: "Add API endpoints" },
{ role: "assistant", content: "Creating API endpoints" },
{ role: "user", content: "Deploy it" },
]

await summarizeConversation(messages, capturingHandler, "System prompt", taskId, 5000, false)

// Verify that the first message was included in what gets summarized
expect(capturedMessages.length).toBeGreaterThan(0)
expect(capturedMessages[0]).toEqual({
role: "user",
content: "Initial task: Create a TODO app",
})

// Verify all messages except the last N_MESSAGES_TO_KEEP were included
const expectedMessagesToSummarize = messages.slice(0, -N_MESSAGES_TO_KEEP)
expect(capturedMessages).toEqual(expectedMessagesToSummarize)
})
})

describe("getMessagesSinceLastSummary", () => {
Expand Down
4 changes: 2 additions & 2 deletions src/core/condense/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ export async function summarizeConversation(

// 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))
// Get messages to summarize, including the first message but excluding last N messages
Copy link
Contributor Author

Choose a reason for hiding this comment

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

P2: If N_MESSAGES_TO_KEEP === 0, slice(0, -0) yields an empty array, causing no messages to be summarized. Consider normalizing the end index to undefined when N_MESSAGES_TO_KEEP is 0 (e.g., const end = N_MESSAGES_TO_KEEP ? -N_MESSAGES_TO_KEEP : undefined; const messagesToSummarize = getMessagesSinceLastSummary(messages.slice(0, end));).

const messagesToSummarize = getMessagesSinceLastSummary(messages.slice(0, -N_MESSAGES_TO_KEEP))

if (messagesToSummarize.length <= 1) {
const error =
Expand Down
Loading