diff --git a/packages/types/src/message.ts b/packages/types/src/message.ts index 548da850b62..9186da62491 100644 --- a/packages/types/src/message.ts +++ b/packages/types/src/message.ts @@ -175,6 +175,7 @@ export const clineSays = [ "diff_error", "condense_context", "condense_context_error", + "sliding_window_truncation", "codebase_search_result", "user_edit_todos", ] as const @@ -203,10 +204,25 @@ export const contextCondenseSchema = z.object({ prevContextTokens: z.number(), newContextTokens: z.number(), summary: z.string(), + condenseId: z.string().optional(), }) export type ContextCondense = z.infer +/** + * ContextTruncation + * + * Used to track sliding window truncation events for the UI. + */ + +export const contextTruncationSchema = z.object({ + truncationId: z.string(), + messagesRemoved: z.number(), + prevContextTokens: z.number(), +}) + +export type ContextTruncation = z.infer + /** * ClineMessage */ @@ -224,6 +240,7 @@ export const clineMessageSchema = z.object({ checkpoint: z.record(z.string(), z.unknown()).optional(), progressStatus: toolProgressStatusSchema.optional(), contextCondense: contextCondenseSchema.optional(), + contextTruncation: contextTruncationSchema.optional(), isProtected: z.boolean().optional(), apiProtocol: z.union([z.literal("openai"), z.literal("anthropic")]).optional(), isAnswered: z.boolean().optional(), diff --git a/src/core/condense/__tests__/condense.spec.ts b/src/core/condense/__tests__/condense.spec.ts index 5eb97b3e8a3..2558ee5b33a 100644 --- a/src/core/condense/__tests__/condense.spec.ts +++ b/src/core/condense/__tests__/condense.spec.ts @@ -6,7 +6,12 @@ import { TelemetryService } from "@roo-code/telemetry" import { BaseProvider } from "../../../api/providers/base-provider" import { ApiMessage } from "../../task-persistence/apiMessages" -import { summarizeConversation, getMessagesSinceLastSummary, N_MESSAGES_TO_KEEP } from "../index" +import { + summarizeConversation, + getMessagesSinceLastSummary, + getEffectiveApiHistory, + N_MESSAGES_TO_KEEP, +} from "../index" // Create a mock ApiHandler for testing class MockApiHandler extends BaseProvider { @@ -83,11 +88,13 @@ describe("Condense", () => { expect(summaryMessage).toBeTruthy() expect(summaryMessage?.content).toBe("Mock summary of the conversation") - // Verify we have the expected number of messages - // [first message, summary, last N messages] - expect(result.messages.length).toBe(1 + 1 + N_MESSAGES_TO_KEEP) + // With non-destructive condensing, all messages are retained (tagged but not deleted) + // Use getEffectiveApiHistory to verify the effective view matches the old behavior + expect(result.messages.length).toBe(messages.length + 1) // All original messages + summary + const effectiveHistory = getEffectiveApiHistory(result.messages) + expect(effectiveHistory.length).toBe(1 + 1 + N_MESSAGES_TO_KEEP) // first + summary + last N - // Verify the last N messages are preserved + // Verify the last N messages are preserved (same messages by reference) const lastMessages = result.messages.slice(-N_MESSAGES_TO_KEEP) expect(lastMessages).toEqual(messages.slice(-N_MESSAGES_TO_KEEP)) }) diff --git a/src/core/condense/__tests__/index.spec.ts b/src/core/condense/__tests__/index.spec.ts index 4e20a972950..fe17b09ee37 100644 --- a/src/core/condense/__tests__/index.spec.ts +++ b/src/core/condense/__tests__/index.spec.ts @@ -2,6 +2,7 @@ import type { Mock } from "vitest" +import { Anthropic } from "@anthropic-ai/sdk" import { TelemetryService } from "@roo-code/telemetry" import { ApiHandler } from "../../../api" @@ -11,6 +12,8 @@ import { summarizeConversation, getMessagesSinceLastSummary, getKeepMessagesWithToolBlocks, + getEffectiveApiHistory, + cleanupAfterTruncation, N_MESSAGES_TO_KEEP, } from "../index" @@ -407,22 +410,28 @@ describe("summarizeConversation", () => { expect(mockApiHandler.createMessage).toHaveBeenCalled() expect(maybeRemoveImageBlocks).toHaveBeenCalled() - // Verify the structure of the result - // The result should be: first message + summary + last N messages - expect(result.messages.length).toBe(1 + 1 + N_MESSAGES_TO_KEEP) // First + summary + last N + // With non-destructive condensing, the result contains ALL original messages + // plus the summary message. Condensed messages are tagged but not deleted. + // Use getEffectiveApiHistory to verify the effective API view matches the old behavior. + expect(result.messages.length).toBe(messages.length + 1) // All original messages + summary // Check that the first message is preserved expect(result.messages[0]).toEqual(messages[0]) - // Check that the summary message was inserted correctly - const summaryMessage = result.messages[1] - expect(summaryMessage.role).toBe("assistant") - expect(summaryMessage.content).toBe("This is a summary") - expect(summaryMessage.isSummary).toBe(true) + // Find the summary message (it has isSummary: true) + const summaryMessage = result.messages.find((m) => m.isSummary) + expect(summaryMessage).toBeDefined() + expect(summaryMessage!.role).toBe("assistant") + expect(summaryMessage!.content).toBe("This is a summary") + expect(summaryMessage!.isSummary).toBe(true) - // Check that the last N_MESSAGES_TO_KEEP messages are preserved - const lastMessages = messages.slice(-N_MESSAGES_TO_KEEP) - expect(result.messages.slice(-N_MESSAGES_TO_KEEP)).toEqual(lastMessages) + // Verify that the effective API history matches expected: first + summary + last N messages + const effectiveHistory = getEffectiveApiHistory(result.messages) + expect(effectiveHistory.length).toBe(1 + 1 + N_MESSAGES_TO_KEEP) // First + summary + last N + + // Check that condensed messages are properly tagged + const condensedMessages = result.messages.filter((m) => m.condenseParent !== undefined) + expect(condensedMessages.length).toBeGreaterThan(0) // Check the cost and token counts expect(result.cost).toBe(0.05) @@ -643,9 +652,11 @@ describe("summarizeConversation", () => { prevContextTokens, ) - // Should successfully summarize - // Result should be: first message + summary + last N messages - expect(result.messages.length).toBe(1 + 1 + N_MESSAGES_TO_KEEP) // First + summary + last N + // With non-destructive condensing, result contains all messages plus summary + // Use getEffectiveApiHistory to verify the effective API view + expect(result.messages.length).toBe(messages.length + 1) // All messages + summary + const effectiveHistory = getEffectiveApiHistory(result.messages) + expect(effectiveHistory.length).toBe(1 + 1 + N_MESSAGES_TO_KEEP) // First + summary + last N expect(result.cost).toBe(0.03) expect(result.summary).toBe("Concise summary") expect(result.error).toBeUndefined() @@ -809,23 +820,27 @@ describe("summarizeConversation", () => { true, // useNativeTools - required for tool_use block preservation ) - // Verify the summary message has content array with text and tool_use blocks - const summaryMessage = result.messages[1] - expect(summaryMessage.role).toBe("assistant") - expect(summaryMessage.isSummary).toBe(true) - expect(Array.isArray(summaryMessage.content)).toBe(true) + // Find the summary message + const summaryMessage = result.messages.find((m) => m.isSummary) + expect(summaryMessage).toBeDefined() + expect(summaryMessage!.role).toBe("assistant") + expect(summaryMessage!.isSummary).toBe(true) + expect(Array.isArray(summaryMessage!.content)).toBe(true) // Content should be [text block, tool_use block] - const content = summaryMessage.content as any[] + const content = summaryMessage!.content as Anthropic.Messages.ContentBlockParam[] expect(content).toHaveLength(2) expect(content[0].type).toBe("text") - expect(content[0].text).toBe("Summary of conversation") + expect((content[0] as Anthropic.Messages.TextBlockParam).text).toBe("Summary of conversation") expect(content[1].type).toBe("tool_use") - expect(content[1].id).toBe("toolu_123") - expect(content[1].name).toBe("read_file") - - // Verify the keepMessages are preserved correctly - expect(result.messages).toHaveLength(1 + 1 + N_MESSAGES_TO_KEEP) // first + summary + last 3 + expect((content[1] as Anthropic.Messages.ToolUseBlockParam).id).toBe("toolu_123") + expect((content[1] as Anthropic.Messages.ToolUseBlockParam).name).toBe("read_file") + + // With non-destructive condensing, all messages are retained plus the summary + expect(result.messages.length).toBe(messages.length + 1) // all original + summary + // Verify effective history matches expected + const effectiveHistory = getEffectiveApiHistory(result.messages) + expect(effectiveHistory.length).toBe(1 + 1 + N_MESSAGES_TO_KEEP) // first + summary + last 3 expect(result.error).toBeUndefined() }) @@ -961,12 +976,16 @@ describe("summarizeConversation", () => { true, ) - const summaryMessage = result.messages[1] - expect(Array.isArray(summaryMessage.content)).toBe(true) - const summaryContent = summaryMessage.content as any[] + // Find the summary message (it has isSummary: true) + const summaryMessage = result.messages.find((m) => m.isSummary) + expect(summaryMessage).toBeDefined() + expect(Array.isArray(summaryMessage!.content)).toBe(true) + const summaryContent = summaryMessage!.content as Anthropic.Messages.ContentBlockParam[] expect(summaryContent[0]).toEqual({ type: "text", text: "This is a summary" }) - const preservedToolUses = summaryContent.filter((block) => block.type === "tool_use") + const preservedToolUses = summaryContent.filter( + (block): block is Anthropic.Messages.ToolUseBlockParam => block.type === "tool_use", + ) expect(preservedToolUses).toHaveLength(2) expect(preservedToolUses.map((block) => block.id)).toEqual(["toolu_parallel_1", "toolu_parallel_2"]) }) diff --git a/src/core/condense/__tests__/rewind-after-condense.spec.ts b/src/core/condense/__tests__/rewind-after-condense.spec.ts new file mode 100644 index 00000000000..f5f1a09380c --- /dev/null +++ b/src/core/condense/__tests__/rewind-after-condense.spec.ts @@ -0,0 +1,542 @@ +// npx vitest src/core/condense/__tests__/rewind-after-condense.spec.ts + +/** + * Regression tests for the issue: "Rewind after Condense is broken" + * https://github.com/RooCodeInc/Roo-Code/issues/8295 + * + * These tests verify that when a user rewinds (deletes/truncates) their conversation + * after a condense operation, the orphaned condensed messages are properly reactivated + * so they can be sent to the API again. + */ + +import { TelemetryService } from "@roo-code/telemetry" + +import { getEffectiveApiHistory, cleanupAfterTruncation } from "../index" +import { ApiMessage } from "../../task-persistence/apiMessages" + +describe("Rewind After Condense - Issue #8295", () => { + beforeEach(() => { + if (!TelemetryService.hasInstance()) { + TelemetryService.createInstance([]) + } + }) + + describe("getEffectiveApiHistory", () => { + it("should filter out messages tagged with condenseParent", () => { + const condenseId = "summary-123" + const messages: ApiMessage[] = [ + { role: "user", content: "First message", ts: 1 }, + { role: "assistant", content: "First response", ts: 2, condenseParent: condenseId }, + { role: "user", content: "Second message", ts: 3, condenseParent: condenseId }, + { role: "assistant", content: "Summary", ts: 4, isSummary: true, condenseId }, + { role: "user", content: "Third message", ts: 5 }, + { role: "assistant", content: "Third response", ts: 6 }, + ] + + const effective = getEffectiveApiHistory(messages) + + // Effective history should be: first message, summary, third message, third response + expect(effective.length).toBe(4) + expect(effective[0].content).toBe("First message") + expect(effective[1].isSummary).toBe(true) + expect(effective[2].content).toBe("Third message") + expect(effective[3].content).toBe("Third response") + }) + + it("should include messages without condenseParent", () => { + const messages: ApiMessage[] = [ + { role: "user", content: "Hello", ts: 1 }, + { role: "assistant", content: "Hi", ts: 2 }, + ] + + const effective = getEffectiveApiHistory(messages) + + expect(effective.length).toBe(2) + expect(effective).toEqual(messages) + }) + + it("should handle empty messages array", () => { + const effective = getEffectiveApiHistory([]) + expect(effective).toEqual([]) + }) + }) + + describe("cleanupAfterTruncation", () => { + it("should clear condenseParent when summary message is deleted", () => { + const condenseId = "summary-123" + const messages: ApiMessage[] = [ + { role: "user", content: "First message", ts: 1 }, + { role: "assistant", content: "First response", ts: 2, condenseParent: condenseId }, + { role: "user", content: "Second message", ts: 3, condenseParent: condenseId }, + // Summary is NOT in the array (was truncated/deleted) + ] + + const cleaned = cleanupAfterTruncation(messages) + + // All condenseParent tags should be cleared since summary is gone + expect(cleaned[1].condenseParent).toBeUndefined() + expect(cleaned[2].condenseParent).toBeUndefined() + }) + + it("should preserve condenseParent when summary message still exists", () => { + const condenseId = "summary-123" + const messages: ApiMessage[] = [ + { role: "user", content: "First message", ts: 1 }, + { role: "assistant", content: "First response", ts: 2, condenseParent: condenseId }, + { role: "assistant", content: "Summary", ts: 3, isSummary: true, condenseId }, + ] + + const cleaned = cleanupAfterTruncation(messages) + + // condenseParent should remain since summary exists + expect(cleaned[1].condenseParent).toBe(condenseId) + }) + + it("should handle multiple condense operations with different IDs", () => { + const condenseId1 = "summary-1" + const condenseId2 = "summary-2" + const messages: ApiMessage[] = [ + { role: "user", content: "Message 1", ts: 1, condenseParent: condenseId1 }, + { role: "assistant", content: "Summary 1", ts: 2, isSummary: true, condenseId: condenseId1 }, + { role: "user", content: "Message 2", ts: 3, condenseParent: condenseId2 }, + // Summary 2 is NOT present (was truncated) + ] + + const cleaned = cleanupAfterTruncation(messages) + + // condenseId1 should remain (summary exists) + expect(cleaned[0].condenseParent).toBe(condenseId1) + // condenseId2 should be cleared (summary deleted) + expect(cleaned[2].condenseParent).toBeUndefined() + }) + + it("should not modify messages without condenseParent", () => { + const messages: ApiMessage[] = [ + { role: "user", content: "Hello", ts: 1 }, + { role: "assistant", content: "Hi", ts: 2 }, + ] + + const cleaned = cleanupAfterTruncation(messages) + + expect(cleaned).toEqual(messages) + }) + + it("should handle empty messages array", () => { + const cleaned = cleanupAfterTruncation([]) + expect(cleaned).toEqual([]) + }) + }) + + describe("Rewind scenario: truncate after condense", () => { + it("should reactivate condensed messages when their summary is deleted via truncation", () => { + const condenseId = "summary-abc" + + // Simulate a conversation after condensing + const fullHistory: ApiMessage[] = [ + { role: "user", content: "Initial task", ts: 1 }, + { role: "assistant", content: "Working on it", ts: 2, condenseParent: condenseId }, + { role: "user", content: "Continue", ts: 3, condenseParent: condenseId }, + { role: "assistant", content: "Summary of work so far", ts: 4, isSummary: true, condenseId }, + { role: "user", content: "Now do this", ts: 5 }, + { role: "assistant", content: "Done", ts: 6 }, + { role: "user", content: "And this", ts: 7 }, + { role: "assistant", content: "Also done", ts: 8 }, + ] + + // Verify effective history before truncation + const effectiveBefore = getEffectiveApiHistory(fullHistory) + // Should be: first message, summary, last 4 messages + expect(effectiveBefore.length).toBe(6) + + // Simulate rewind: user truncates back to message ts=4 (keeping 0-3) + const truncatedHistory = fullHistory.slice(0, 4) // Keep first, condensed1, condensed2, summary + + // After truncation, the summary is still there, so condensed messages remain condensed + const cleanedAfterKeepingSummary = cleanupAfterTruncation(truncatedHistory) + expect(cleanedAfterKeepingSummary[1].condenseParent).toBe(condenseId) + expect(cleanedAfterKeepingSummary[2].condenseParent).toBe(condenseId) + + // Now simulate a more aggressive rewind: delete back to message ts=2 + const aggressiveTruncate = fullHistory.slice(0, 2) // Keep only first message and first response + + // The condensed messages should now be reactivated since summary is gone + const cleanedAfterDeletingSummary = cleanupAfterTruncation(aggressiveTruncate) + expect(cleanedAfterDeletingSummary[1].condenseParent).toBeUndefined() + + // Verify effective history after cleanup + const effectiveAfterCleanup = getEffectiveApiHistory(cleanedAfterDeletingSummary) + // Now both messages should be active (no condensed filtering) + expect(effectiveAfterCleanup.length).toBe(2) + expect(effectiveAfterCleanup[0].content).toBe("Initial task") + expect(effectiveAfterCleanup[1].content).toBe("Working on it") + }) + + it("should properly restore context after rewind when summary was deleted", () => { + const condenseId = "summary-xyz" + + // Scenario: Most of the conversation was condensed, but the summary was deleted. + // getEffectiveApiHistory already correctly handles orphaned messages (includes them + // when their summary doesn't exist). cleanupAfterTruncation cleans up the tags. + const messages: ApiMessage[] = [ + { role: "user", content: "Start", ts: 1 }, + { role: "assistant", content: "Response 1", ts: 2, condenseParent: condenseId }, + { role: "user", content: "More", ts: 3, condenseParent: condenseId }, + { role: "assistant", content: "Response 2", ts: 4, condenseParent: condenseId }, + { role: "user", content: "Even more", ts: 5, condenseParent: condenseId }, + // Summary was deleted (not present), so these are "orphaned" messages + ] + + // getEffectiveApiHistory already includes orphaned messages (summary doesn't exist) + const effectiveBefore = getEffectiveApiHistory(messages) + expect(effectiveBefore.length).toBe(5) // All messages visible since summary was deleted + expect(effectiveBefore[0].content).toBe("Start") + expect(effectiveBefore[1].content).toBe("Response 1") + + // cleanupAfterTruncation clears the orphaned condenseParent tags for data hygiene + const cleaned = cleanupAfterTruncation(messages) + + // Verify condenseParent was cleared for all orphaned messages + expect(cleaned[1].condenseParent).toBeUndefined() + expect(cleaned[2].condenseParent).toBeUndefined() + expect(cleaned[3].condenseParent).toBeUndefined() + expect(cleaned[4].condenseParent).toBeUndefined() + + // After cleanup, effective history is the same (all visible) + const effectiveAfter = getEffectiveApiHistory(cleaned) + expect(effectiveAfter.length).toBe(5) // All messages visible + }) + + it("should hide condensed messages when their summary still exists", () => { + const condenseId = "summary-exists" + + // Scenario: Messages were condensed and summary exists - condensed messages should be hidden + const messages: ApiMessage[] = [ + { role: "user", content: "Start", ts: 1 }, + { role: "assistant", content: "Response 1", ts: 2, condenseParent: condenseId }, + { role: "user", content: "More", ts: 3, condenseParent: condenseId }, + { role: "assistant", content: "Summary", ts: 4, isSummary: true, condenseId }, + { role: "user", content: "After summary", ts: 5 }, + ] + + // Effective history should hide condensed messages since summary exists + const effective = getEffectiveApiHistory(messages) + expect(effective.length).toBe(3) // Start, Summary, After summary + expect(effective[0].content).toBe("Start") + expect(effective[1].content).toBe("Summary") + expect(effective[2].content).toBe("After summary") + + // cleanupAfterTruncation should NOT clear condenseParent since summary exists + const cleaned = cleanupAfterTruncation(messages) + expect(cleaned[1].condenseParent).toBe(condenseId) + expect(cleaned[2].condenseParent).toBe(condenseId) + }) + + describe("Summary timestamp collision prevention", () => { + it("should give summary a unique timestamp that does not collide with first kept message", () => { + // Scenario: After condense, the summary message should have a unique timestamp + // (ts - 1 from the first kept message) to prevent lookup collisions. + // + // This test verifies the expected data structure after condense: + // - Summary should have ts = firstKeptMessage.ts - 1 + // - Summary and first kept message should NOT share the same timestamp + // + // With real millisecond timestamps, the summary timestamp (firstKeptTs - 1) will + // never collide with an existing message since timestamps are always increasing. + + const condenseId = "summary-unique-ts" + // Use timestamps that represent a realistic scenario where firstKeptTs - 1 + // does not collide with any existing message timestamp + const firstKeptTs = 1000 + + // Simulate post-condense state where summary has unique timestamp (firstKeptTs - 1) + // In real usage, condensed messages have timestamps like 100, 200, 300... + // and firstKeptTs is much larger, so firstKeptTs - 1 = 999 is unique + const messagesAfterCondense: ApiMessage[] = [ + { role: "user", content: "Initial task", ts: 100 }, + { role: "assistant", content: "Response 1", ts: 200, condenseParent: condenseId }, + { role: "user", content: "Continue", ts: 300, condenseParent: condenseId }, + { role: "assistant", content: "Response 2", ts: 400, condenseParent: condenseId }, + { role: "user", content: "More work", ts: 500, condenseParent: condenseId }, + { role: "assistant", content: "Response 3", ts: 600, condenseParent: condenseId }, + { role: "user", content: "Even more", ts: 700, condenseParent: condenseId }, + // Summary gets ts = firstKeptTs - 1 = 999, which is unique + { role: "assistant", content: "Summary", ts: firstKeptTs - 1, isSummary: true, condenseId }, + // First kept message + { role: "user", content: "First kept message", ts: firstKeptTs }, + { role: "assistant", content: "Response to first kept", ts: 1100 }, + { role: "user", content: "Last message", ts: 1200 }, + ] + + // Find all messages with the same timestamp as the summary + const summaryTs = firstKeptTs - 1 + const messagesWithSummaryTs = messagesAfterCondense.filter((msg) => msg.ts === summaryTs) + + // There should be exactly one message with the summary timestamp + expect(messagesWithSummaryTs.length).toBe(1) + expect(messagesWithSummaryTs[0].isSummary).toBe(true) + + // The first kept message should have a different timestamp + const firstKeptMessage = messagesAfterCondense.find( + (msg) => msg.ts === firstKeptTs && !msg.isSummary && !msg.condenseParent, + ) + expect(firstKeptMessage).toBeDefined() + expect(firstKeptMessage?.content).toBe("First kept message") + expect(firstKeptMessage?.ts).not.toBe(summaryTs) + }) + + it("should not target summary message when looking up first kept message by timestamp", () => { + // This test verifies that when timestamps are unique (as they should be after the fix), + // looking up by the first kept message's timestamp returns that message, not the summary. + + const condenseId = "summary-lookup-test" + const firstKeptTs = 8 + + const messages: ApiMessage[] = [ + { role: "user", content: "Initial", ts: 1 }, + { role: "assistant", content: "Summary", ts: firstKeptTs - 1, isSummary: true, condenseId }, + { role: "user", content: "First kept message", ts: firstKeptTs }, + { role: "assistant", content: "Response", ts: 9 }, + ] + + // Look up by first kept message's timestamp + const foundMessage = messages.find((msg) => msg.ts === firstKeptTs) + expect(foundMessage).toBeDefined() + expect(foundMessage?.content).toBe("First kept message") + expect(foundMessage?.isSummary).toBeUndefined() + + // Looking up by summary's timestamp should find the summary + const foundSummary = messages.find((msg) => msg.ts === firstKeptTs - 1) + expect(foundSummary).toBeDefined() + expect(foundSummary?.isSummary).toBe(true) + }) + }) + + describe("Message preservation after condense operations", () => { + /** + * These tests verify that the correct user and assistant messages are preserved + * and sent to the LLM after condense operations. With N_MESSAGES_TO_KEEP = 3, + * condense should always preserve: + * - The first message (never condensed) + * - The active summary + * - The last 3 kept messages + */ + + it("should preserve correct messages after single condense (10 messages)", () => { + const condenseId = "summary-single" + + // Simulate storage state after condensing 10 messages with N_MESSAGES_TO_KEEP = 3 + // Original: [msg1, msg2, msg3, msg4, msg5, msg6, msg7, msg8, msg9, msg10] + // After condense: + // - msg1 preserved (first message) + // - msg2-msg7 tagged with condenseParent + // - summary inserted with ts = msg8.ts - 1 + // - msg8, msg9, msg10 kept + const storageAfterCondense: ApiMessage[] = [ + { role: "user", content: "Task: Build a feature", ts: 100 }, + { role: "assistant", content: "I'll help with that", ts: 200, condenseParent: condenseId }, + { role: "user", content: "Start with the API", ts: 300, condenseParent: condenseId }, + { role: "assistant", content: "Creating API endpoints", ts: 400, condenseParent: condenseId }, + { role: "user", content: "Add validation", ts: 500, condenseParent: condenseId }, + { role: "assistant", content: "Added validation logic", ts: 600, condenseParent: condenseId }, + { role: "user", content: "Now the tests", ts: 700, condenseParent: condenseId }, + // Summary inserted before first kept message + { + role: "assistant", + content: "Summary: Built API with validation, working on tests", + ts: 799, // msg8.ts - 1 + isSummary: true, + condenseId, + }, + // Last 3 kept messages (N_MESSAGES_TO_KEEP = 3) + { role: "assistant", content: "Writing unit tests now", ts: 800 }, + { role: "user", content: "Include edge cases", ts: 900 }, + { role: "assistant", content: "Added edge case tests", ts: 1000 }, + ] + + const effective = getEffectiveApiHistory(storageAfterCondense) + + // Should send exactly 5 messages to LLM: + // 1. First message (user) - preserved + // 2. Summary (assistant) + // 3-5. Last 3 kept messages + expect(effective.length).toBe(5) + + // Verify exact order and content + expect(effective[0].role).toBe("user") + expect(effective[0].content).toBe("Task: Build a feature") + + expect(effective[1].role).toBe("assistant") + expect(effective[1].isSummary).toBe(true) + expect(effective[1].content).toBe("Summary: Built API with validation, working on tests") + + expect(effective[2].role).toBe("assistant") + expect(effective[2].content).toBe("Writing unit tests now") + + expect(effective[3].role).toBe("user") + expect(effective[3].content).toBe("Include edge cases") + + expect(effective[4].role).toBe("assistant") + expect(effective[4].content).toBe("Added edge case tests") + + // Verify condensed messages are NOT in effective history + const condensedContents = ["I'll help with that", "Start with the API", "Creating API endpoints"] + for (const content of condensedContents) { + expect(effective.find((m) => m.content === content)).toBeUndefined() + } + }) + + it("should preserve correct messages after double condense (10 msgs → condense → 10 more msgs → condense)", () => { + const condenseId1 = "summary-first" + const condenseId2 = "summary-second" + + // Simulate storage state after TWO condense operations + // First condense: 10 messages condensed, summary1 created + // Then: 10 more messages added (making effective history have 15 messages) + // Second condense: summary1 + msg8-msg17 condensed, summary2 created + // + // Storage after double condense: + const storageAfterDoubleCondense: ApiMessage[] = [ + // First message - never condensed + { role: "user", content: "Initial task: Build a full app", ts: 100 }, + + // Messages from first condense (tagged with condenseId1) + { role: "assistant", content: "Starting the project", ts: 200, condenseParent: condenseId1 }, + { role: "user", content: "Add authentication", ts: 300, condenseParent: condenseId1 }, + { role: "assistant", content: "Added auth module", ts: 400, condenseParent: condenseId1 }, + { role: "user", content: "Add database", ts: 500, condenseParent: condenseId1 }, + { role: "assistant", content: "Set up database", ts: 600, condenseParent: condenseId1 }, + { role: "user", content: "Connect them", ts: 700, condenseParent: condenseId1 }, + + // First summary - now ALSO tagged with condenseId2 (from second condense) + { + role: "assistant", + content: "Summary1: Built auth and database", + ts: 799, + isSummary: true, + condenseId: condenseId1, + condenseParent: condenseId2, // Tagged during second condense! + }, + + // Messages after first condense but before second (tagged with condenseId2) + { role: "assistant", content: "Continuing development", ts: 800, condenseParent: condenseId2 }, + { role: "user", content: "Add API routes", ts: 900, condenseParent: condenseId2 }, + { role: "assistant", content: "Created REST endpoints", ts: 1000, condenseParent: condenseId2 }, + { role: "user", content: "Add validation", ts: 1100, condenseParent: condenseId2 }, + { role: "assistant", content: "Added input validation", ts: 1200, condenseParent: condenseId2 }, + { role: "user", content: "Add error handling", ts: 1300, condenseParent: condenseId2 }, + { role: "assistant", content: "Implemented error handlers", ts: 1400, condenseParent: condenseId2 }, + { role: "user", content: "Add logging", ts: 1500, condenseParent: condenseId2 }, + { role: "assistant", content: "Set up logging system", ts: 1600, condenseParent: condenseId2 }, + { role: "user", content: "Now write tests", ts: 1700, condenseParent: condenseId2 }, + + // Second summary - inserted before the last 3 kept messages + { + role: "assistant", + content: "Summary2: App complete with auth, DB, API, validation, errors, logging. Now testing.", + ts: 1799, // msg18.ts - 1 + isSummary: true, + condenseId: condenseId2, + }, + + // Last 3 kept messages (N_MESSAGES_TO_KEEP = 3) + { role: "assistant", content: "Writing integration tests", ts: 1800 }, + { role: "user", content: "Test the auth flow", ts: 1900 }, + { role: "assistant", content: "Auth tests passing", ts: 2000 }, + ] + + const effective = getEffectiveApiHistory(storageAfterDoubleCondense) + + // Should send exactly 5 messages to LLM: + // 1. First message (user) - preserved + // 2. Summary2 (assistant) - the ACTIVE summary + // 3-5. Last 3 kept messages + expect(effective.length).toBe(5) + + // Verify exact order and content + expect(effective[0].role).toBe("user") + expect(effective[0].content).toBe("Initial task: Build a full app") + + expect(effective[1].role).toBe("assistant") + expect(effective[1].isSummary).toBe(true) + expect(effective[1].condenseId).toBe(condenseId2) // Must be the SECOND summary + expect(effective[1].content).toContain("Summary2") + + expect(effective[2].role).toBe("assistant") + expect(effective[2].content).toBe("Writing integration tests") + + expect(effective[3].role).toBe("user") + expect(effective[3].content).toBe("Test the auth flow") + + expect(effective[4].role).toBe("assistant") + expect(effective[4].content).toBe("Auth tests passing") + + // Verify Summary1 is NOT in effective history (it's tagged with condenseParent) + const summary1 = effective.find((m) => m.content?.toString().includes("Summary1")) + expect(summary1).toBeUndefined() + + // Verify all condensed messages are NOT in effective history + const condensedContents = [ + "Starting the project", + "Added auth module", + "Continuing development", + "Created REST endpoints", + "Implemented error handlers", + ] + for (const content of condensedContents) { + expect(effective.find((m) => m.content === content)).toBeUndefined() + } + }) + + it("should maintain proper user/assistant alternation in effective history", () => { + const condenseId = "summary-alternation" + + // Verify that after condense, the effective history maintains proper + // user/assistant message alternation (important for API compatibility) + const storage: ApiMessage[] = [ + { role: "user", content: "Start task", ts: 100 }, + { role: "assistant", content: "Response 1", ts: 200, condenseParent: condenseId }, + { role: "user", content: "Continue", ts: 300, condenseParent: condenseId }, + { role: "assistant", content: "Summary text", ts: 399, isSummary: true, condenseId }, + // Kept messages - should alternate properly + { role: "assistant", content: "Response after summary", ts: 400 }, + { role: "user", content: "User message", ts: 500 }, + { role: "assistant", content: "Final response", ts: 600 }, + ] + + const effective = getEffectiveApiHistory(storage) + + // Verify the sequence: user, assistant(summary), assistant, user, assistant + // Note: Having two assistant messages in a row (summary + next response) is valid + // because the summary replaces what would have been multiple messages + expect(effective[0].role).toBe("user") + expect(effective[1].role).toBe("assistant") + expect(effective[1].isSummary).toBe(true) + expect(effective[2].role).toBe("assistant") + expect(effective[3].role).toBe("user") + expect(effective[4].role).toBe("assistant") + }) + + it("should preserve timestamps in chronological order in effective history", () => { + const condenseId = "summary-timestamps" + + const storage: ApiMessage[] = [ + { role: "user", content: "First", ts: 100 }, + { role: "assistant", content: "Condensed", ts: 200, condenseParent: condenseId }, + { role: "assistant", content: "Summary", ts: 299, isSummary: true, condenseId }, + { role: "user", content: "Kept 1", ts: 300 }, + { role: "assistant", content: "Kept 2", ts: 400 }, + { role: "user", content: "Kept 3", ts: 500 }, + ] + + const effective = getEffectiveApiHistory(storage) + + // Verify timestamps are in ascending order + for (let i = 1; i < effective.length; i++) { + const prevTs = effective[i - 1].ts ?? 0 + const currTs = effective[i].ts ?? 0 + expect(currTs).toBeGreaterThan(prevTs) + } + }) + }) + }) +}) diff --git a/src/core/condense/index.ts b/src/core/condense/index.ts index d330716df10..b8af4d1de24 100644 --- a/src/core/condense/index.ts +++ b/src/core/condense/index.ts @@ -1,4 +1,5 @@ import Anthropic from "@anthropic-ai/sdk" +import crypto from "crypto" import { TelemetryService } from "@roo-code/telemetry" @@ -117,6 +118,7 @@ export type SummarizeResponse = { cost: number // The cost of the summarization operation newContextTokens?: number // The number of tokens in the context for the next API request error?: string // Populated iff the operation fails: error message shown to the user on failure (see Task.ts) + condenseId?: string // The unique ID of the created Summary message, for linking to condense_context clineMessage } /** @@ -266,15 +268,51 @@ export async function summarizeConversation( summaryContent = summary } + // Generate a unique condenseId for this summary + const condenseId = crypto.randomUUID() + + // Use first kept message's timestamp minus 1 to ensure unique timestamp for summary. + // Fallback to Date.now() if keepMessages is empty (shouldn't happen due to earlier checks). + const firstKeptTs = keepMessages[0]?.ts ?? Date.now() + const summaryMessage: ApiMessage = { role: "assistant", content: summaryContent, - ts: keepMessages[0].ts, + ts: firstKeptTs - 1, // Unique timestamp before first kept message to avoid collision isSummary: true, + condenseId, // Unique ID for this summary, used to track which messages it replaces } - // Reconstruct messages: [first message, summary, last N messages] - const newMessages = [firstMessage, summaryMessage, ...keepMessages] + // NON-DESTRUCTIVE CONDENSE: + // Instead of deleting middle messages, tag them with condenseParent so they can be + // restored if the user rewinds to a point before the summary. + // + // Storage structure after condense: + // [firstMessage, msg2(parent=X), ..., msg8(parent=X), summary(id=X), msg9, msg10, msg11] + // + // Effective for API (filtered by getEffectiveApiHistory): + // [firstMessage, summary, msg9, msg10, msg11] + + // Tag middle messages with condenseParent (skip first message, skip last N messages) + const newMessages = messages.map((msg, index) => { + // First message stays as-is + if (index === 0) { + return msg + } + // Messages in the "keep" range stay as-is + if (index >= keepStartIndex) { + return msg + } + // Middle messages get tagged with condenseParent (unless they already have one from a previous condense) + // If they already have a condenseParent, we leave it - nested condense is handled by filtering + if (!msg.condenseParent) { + return { ...msg, condenseParent: condenseId } + } + return msg + }) + + // Insert the summary message right before the keep messages + newMessages.splice(keepStartIndex, 0, summaryMessage) // Count the tokens in the context for the next API request // We only estimate the tokens in summaryMesage if outputTokens is 0, otherwise we use outputTokens @@ -293,7 +331,7 @@ export async function summarizeConversation( const error = t("common:errors.condense_context_grew") return { ...response, cost, error } } - return { messages: newMessages, summary, cost, newContextTokens } + return { messages: newMessages, summary, cost, newContextTokens, condenseId } } /* Returns the list of all messages since the last summary message, including the summary. Returns all messages if there is no summary. */ @@ -329,3 +367,108 @@ export function getMessagesSinceLastSummary(messages: ApiMessage[]): ApiMessage[ return messagesSinceSummary } + +/** + * Filters the API conversation history to get the "effective" messages to send to the API. + * Messages with a condenseParent that points to an existing summary are filtered out, + * as they have been replaced by that summary. + * Messages with a truncationParent that points to an existing truncation marker are also filtered out, + * as they have been hidden by sliding window truncation. + * + * This allows non-destructive condensing and truncation where messages are tagged but not deleted, + * enabling accurate rewind operations while still sending condensed/truncated history to the API. + * + * @param messages - The full API conversation history including tagged messages + * @returns The filtered history that should be sent to the API + */ +export function getEffectiveApiHistory(messages: ApiMessage[]): ApiMessage[] { + // Collect all condenseIds of summaries that exist in the current history + const existingSummaryIds = new Set() + // Collect all truncationIds of truncation markers that exist in the current history + const existingTruncationIds = new Set() + + for (const msg of messages) { + if (msg.isSummary && msg.condenseId) { + existingSummaryIds.add(msg.condenseId) + } + if (msg.isTruncationMarker && msg.truncationId) { + existingTruncationIds.add(msg.truncationId) + } + } + + // Filter out messages whose condenseParent points to an existing summary + // or whose truncationParent points to an existing truncation marker. + // Messages with orphaned parents (summary/marker was deleted) are included + return messages.filter((msg) => { + // Filter out condensed messages if their summary exists + if (msg.condenseParent && existingSummaryIds.has(msg.condenseParent)) { + return false + } + // Filter out truncated messages if their truncation marker exists + if (msg.truncationParent && existingTruncationIds.has(msg.truncationParent)) { + return false + } + return true + }) +} + +/** + * Cleans up orphaned condenseParent and truncationParent references after a truncation operation (rewind/delete). + * When a summary message or truncation marker is deleted, messages that were tagged with its ID + * should have their parent reference cleared so they become active again. + * + * This function should be called after any operation that truncates the API history + * to ensure messages are properly restored when their summary or truncation marker is deleted. + * + * @param messages - The API conversation history after truncation + * @returns The cleaned history with orphaned condenseParent and truncationParent fields cleared + */ +export function cleanupAfterTruncation(messages: ApiMessage[]): ApiMessage[] { + // Collect all condenseIds of summaries that still exist + const existingSummaryIds = new Set() + // Collect all truncationIds of truncation markers that still exist + const existingTruncationIds = new Set() + + for (const msg of messages) { + if (msg.isSummary && msg.condenseId) { + existingSummaryIds.add(msg.condenseId) + } + if (msg.isTruncationMarker && msg.truncationId) { + existingTruncationIds.add(msg.truncationId) + } + } + + // Clear orphaned parent references for messages whose summary or truncation marker was deleted + return messages.map((msg) => { + let needsUpdate = false + + // Check for orphaned condenseParent + if (msg.condenseParent && !existingSummaryIds.has(msg.condenseParent)) { + needsUpdate = true + } + + // Check for orphaned truncationParent + if (msg.truncationParent && !existingTruncationIds.has(msg.truncationParent)) { + needsUpdate = true + } + + if (needsUpdate) { + // Create a new object without orphaned parent references + const { condenseParent, truncationParent, ...rest } = msg + const result: ApiMessage = rest as ApiMessage + + // Keep condenseParent if its summary still exists + if (condenseParent && existingSummaryIds.has(condenseParent)) { + result.condenseParent = condenseParent + } + + // Keep truncationParent if its truncation marker still exists + if (truncationParent && existingTruncationIds.has(truncationParent)) { + result.truncationParent = truncationParent + } + + return result + } + return msg + }) +} diff --git a/src/core/context-management/__tests__/context-management.spec.ts b/src/core/context-management/__tests__/context-management.spec.ts index 712785b7cd8..308d0e2e965 100644 --- a/src/core/context-management/__tests__/context-management.spec.ts +++ b/src/core/context-management/__tests__/context-management.spec.ts @@ -65,10 +65,14 @@ describe("Context Management", () => { // With 2 messages after the first, 0.5 fraction means remove 1 message // But 1 is odd, so it rounds down to 0 (to make it even) - expect(result.length).toBe(3) // First message + 2 remaining messages - expect(result[0]).toEqual(messages[0]) - expect(result[1]).toEqual(messages[1]) - expect(result[2]).toEqual(messages[2]) + // Result should have messages + truncation marker + expect(result.messages.length).toBe(4) // First message + truncation marker + 2 remaining messages + expect(result.messages[0]).toEqual(messages[0]) + // messages[1] is the truncation marker + expect(result.messages[1].isTruncationMarker).toBe(true) + // Original messages[1] and messages[2] are at indices 2 and 3 now + expect(result.messages[2].content).toEqual(messages[1].content) + expect(result.messages[3].content).toEqual(messages[2].content) }) it("should remove the specified fraction of messages (rounded to even number)", () => { @@ -84,10 +88,17 @@ describe("Context Management", () => { // 2 is already even, so no rounding needed const result = truncateConversation(messages, 0.5, taskId) - expect(result.length).toBe(3) - expect(result[0]).toEqual(messages[0]) - expect(result[1]).toEqual(messages[3]) - expect(result[2]).toEqual(messages[4]) + // Should have all original messages + truncation marker + expect(result.messages.length).toBe(6) // 5 original + 1 marker + expect(result.messagesRemoved).toBe(2) + expect(result.messages[0]).toEqual(messages[0]) + expect(result.messages[1].isTruncationMarker).toBe(true) + // Messages 2 and 3 (indices 1 and 2 from original) should be tagged + expect(result.messages[2].truncationParent).toBe(result.truncationId) + expect(result.messages[3].truncationParent).toBe(result.truncationId) + // Messages 4 and 5 (indices 3 and 4 from original) should NOT be tagged + expect(result.messages[4].truncationParent).toBeUndefined() + expect(result.messages[5].truncationParent).toBeUndefined() }) it("should round to an even number of messages to remove", () => { @@ -105,8 +116,10 @@ describe("Context Management", () => { // 1.8 rounds down to 1, then to 0 to make it even const result = truncateConversation(messages, 0.3, taskId) - expect(result.length).toBe(7) // No messages removed - expect(result).toEqual(messages) + expect(result.messagesRemoved).toBe(0) // No messages removed + // Should still have truncation marker inserted + expect(result.messages.length).toBe(8) // 7 original + 1 marker + expect(result.messages[1].isTruncationMarker).toBe(true) }) it("should handle edge case with fracToRemove = 0", () => { @@ -118,7 +131,10 @@ describe("Context Management", () => { const result = truncateConversation(messages, 0, taskId) - expect(result).toEqual(messages) + expect(result.messagesRemoved).toBe(0) + // Should have original messages + truncation marker + expect(result.messages.length).toBe(4) + expect(result.messages[1].isTruncationMarker).toBe(true) }) it("should handle edge case with fracToRemove = 1", () => { @@ -133,9 +149,16 @@ describe("Context Management", () => { // But 3 is odd, so it rounds down to 2 to make it even const result = truncateConversation(messages, 1, taskId) - expect(result.length).toBe(2) - expect(result[0]).toEqual(messages[0]) - expect(result[1]).toEqual(messages[3]) + expect(result.messagesRemoved).toBe(2) + // Should have all original messages + truncation marker + expect(result.messages.length).toBe(5) // 4 original + 1 marker + expect(result.messages[0]).toEqual(messages[0]) + expect(result.messages[1].isTruncationMarker).toBe(true) + // Messages at indices 2 and 3 should be tagged (original indices 1 and 2) + expect(result.messages[2].truncationParent).toBe(result.truncationId) + expect(result.messages[3].truncationParent).toBe(result.truncationId) + // Last message should NOT be tagged + expect(result.messages[4].truncationParent).toBeUndefined() }) }) @@ -289,14 +312,6 @@ describe("Context Management", () => { { ...messages[messages.length - 1], content: "" }, ] - // When truncating, always uses 0.5 fraction - // With 4 messages after the first, 0.5 fraction means remove 2 messages - const expectedMessages = [ - messagesWithSmallContent[0], - messagesWithSmallContent[3], - messagesWithSmallContent[4], - ] - const result = await manageContext({ messages: messagesWithSmallContent, totalTokens, @@ -311,12 +326,14 @@ describe("Context Management", () => { currentProfileId: "default", }) - expect(result).toEqual({ - messages: expectedMessages, - summary: "", - cost: 0, - prevContextTokens: totalTokens, - }) + // Should have truncation + expect(result.truncationId).toBeDefined() + expect(result.messagesRemoved).toBe(2) // With 4 messages after first, 0.5 fraction = 2 to remove + expect(result.summary).toBe("") + expect(result.cost).toBe(0) + expect(result.prevContextTokens).toBe(totalTokens) + // Should have all original messages + truncation marker (non-destructive) + expect(result.messages.length).toBe(6) // 5 original + 1 marker }) it("should work with non-prompt caching models the same as prompt caching models", async () => { @@ -360,10 +377,14 @@ describe("Context Management", () => { currentProfileId: "default", }) - expect(result1.messages).toEqual(result2.messages) + // For truncation results, we can't compare messages directly because + // truncationId is randomly generated. Compare structure instead. + expect(result1.messages.length).toEqual(result2.messages.length) expect(result1.summary).toEqual(result2.summary) expect(result1.cost).toEqual(result2.cost) expect(result1.prevContextTokens).toEqual(result2.prevContextTokens) + expect(result1.truncationId).toBeDefined() + expect(result2.truncationId).toBeDefined() // Test above threshold const aboveThreshold = 70001 @@ -395,10 +416,14 @@ describe("Context Management", () => { currentProfileId: "default", }) - expect(result3.messages).toEqual(result4.messages) + // For truncation results, we can't compare messages directly because + // truncationId is randomly generated. Compare structure instead. + expect(result3.messages.length).toEqual(result4.messages.length) expect(result3.summary).toEqual(result4.summary) expect(result3.cost).toEqual(result4.cost) expect(result3.prevContextTokens).toEqual(result4.prevContextTokens) + expect(result3.truncationId).toBeDefined() + expect(result4.truncationId).toBeDefined() }) it("should consider incoming content when deciding to truncate", async () => { @@ -510,14 +535,6 @@ describe("Context Management", () => { { ...messages[messages.length - 1], content: "" }, ] - // When truncating, always uses 0.5 fraction - // With 4 messages after the first, 0.5 fraction means remove 2 messages - const expectedResult = [ - messagesWithSmallContent[0], - messagesWithSmallContent[3], - messagesWithSmallContent[4], - ] - const result = await manageContext({ messages: messagesWithSmallContent, totalTokens, @@ -531,12 +548,15 @@ describe("Context Management", () => { profileThresholds: {}, currentProfileId: "default", }) - expect(result).toEqual({ - messages: expectedResult, - summary: "", - cost: 0, - prevContextTokens: totalTokens, - }) + + // Should have truncation + expect(result.truncationId).toBeDefined() + expect(result.messagesRemoved).toBe(2) // With 4 messages after first, 0.5 fraction = 2 to remove + expect(result.summary).toBe("") + expect(result.cost).toBe(0) + expect(result.prevContextTokens).toBe(totalTokens) + // Should have all original messages + truncation marker (non-destructive) + expect(result.messages.length).toBe(6) // 5 original + 1 marker }) it("should use summarizeConversation when autoCondenseContext is true and tokens exceed threshold", async () => { @@ -650,10 +670,13 @@ describe("Context Management", () => { // Verify summarizeConversation was called expect(summarizeSpy).toHaveBeenCalled() - // Verify it fell back to truncation - expect(result.messages).toEqual(expectedMessages) + // Verify it fell back to truncation (non-destructive) + expect(result.truncationId).toBeDefined() + expect(result.messagesRemoved).toBe(2) expect(result.summary).toBe("") expect(result.prevContextTokens).toBe(totalTokens) + // Should have all original messages + truncation marker + expect(result.messages.length).toBe(6) // 5 original + 1 marker // The cost might be different than expected, so we don't check it // Clean up @@ -697,13 +720,14 @@ describe("Context Management", () => { // Verify summarizeConversation was not called expect(summarizeSpy).not.toHaveBeenCalled() - // Verify it used truncation - expect(result).toEqual({ - messages: expectedMessages, - summary: "", - cost: 0, - prevContextTokens: totalTokens, - }) + // Verify it used truncation (non-destructive) + expect(result.truncationId).toBeDefined() + expect(result.messagesRemoved).toBe(2) + expect(result.summary).toBe("") + expect(result.cost).toBe(0) + expect(result.prevContextTokens).toBe(totalTokens) + // Should have all original messages + truncation marker + expect(result.messages.length).toBe(6) // 5 original + 1 marker // Clean up summarizeSpy.mockRestore() @@ -1093,7 +1117,10 @@ describe("Context Management", () => { currentProfileId: "default", }) expect(result2.messages).not.toEqual(messagesWithSmallContent) - expect(result2.messages.length).toBe(3) // Truncated with 0.5 fraction + // Should have all original messages + truncation marker (non-destructive) + expect(result2.messages.length).toBe(6) // 5 original + 1 marker + expect(result2.truncationId).toBeDefined() + expect(result2.messagesRemoved).toBe(2) expect(result2.summary).toBe("") expect(result2.cost).toBe(0) expect(result2.prevContextTokens).toBe(50001) @@ -1146,7 +1173,9 @@ describe("Context Management", () => { currentProfileId: "default", }) expect(result2.messages).not.toEqual(messagesWithSmallContent) - expect(result2.messages.length).toBe(3) // Truncated with 0.5 fraction + // Should have all original messages + truncation marker (non-destructive) + expect(result2.messages.length).toBe(6) // 5 original + 1 marker + expect(result2.truncationId).toBeDefined() expect(result2.summary).toBe("") expect(result2.cost).toBe(0) expect(result2.prevContextTokens).toBe(81809) @@ -1192,8 +1221,10 @@ describe("Context Management", () => { profileThresholds: {}, currentProfileId: "default", }) - expect(result2).not.toEqual(messagesWithSmallContent) - expect(result2.messages.length).toBe(3) // Truncated with 0.5 fraction + expect(result2.messages).not.toEqual(messagesWithSmallContent) + // Should have all original messages + truncation marker (non-destructive) + expect(result2.messages.length).toBe(6) // 5 original + 1 marker + expect(result2.truncationId).toBeDefined() }) it("should handle large context windows appropriately", async () => { @@ -1237,8 +1268,10 @@ describe("Context Management", () => { profileThresholds: {}, currentProfileId: "default", }) - expect(result2).not.toEqual(messagesWithSmallContent) - expect(result2.messages.length).toBe(3) // Truncated with 0.5 fraction + expect(result2.messages).not.toEqual(messagesWithSmallContent) + // Should have all original messages + truncation marker (non-destructive) + expect(result2.messages.length).toBe(6) // 5 original + 1 marker + expect(result2.truncationId).toBeDefined() }) }) }) diff --git a/src/core/context-management/__tests__/truncation.spec.ts b/src/core/context-management/__tests__/truncation.spec.ts new file mode 100644 index 00000000000..185a69ca7a0 --- /dev/null +++ b/src/core/context-management/__tests__/truncation.spec.ts @@ -0,0 +1,402 @@ +import { describe, it, expect, beforeEach } from "vitest" +import { TelemetryService } from "@roo-code/telemetry" +import { truncateConversation } from "../index" +import { getEffectiveApiHistory, cleanupAfterTruncation } from "../../condense" +import { ApiMessage } from "../../task-persistence/apiMessages" + +describe("Non-Destructive Sliding Window Truncation", () => { + let messages: ApiMessage[] + + beforeEach(() => { + // Initialize TelemetryService for tests + if (!TelemetryService.hasInstance()) { + TelemetryService.createInstance([]) + } + + // Create a sample conversation with 11 messages (1 initial + 10 conversation messages) + messages = [ + { role: "user", content: "Initial task", ts: 1000 }, + { role: "assistant", content: "Response 1", ts: 1100 }, + { role: "user", content: "Message 2", ts: 1200 }, + { role: "assistant", content: "Response 2", ts: 1300 }, + { role: "user", content: "Message 3", ts: 1400 }, + { role: "assistant", content: "Response 3", ts: 1500 }, + { role: "user", content: "Message 4", ts: 1600 }, + { role: "assistant", content: "Response 4", ts: 1700 }, + { role: "user", content: "Message 5", ts: 1800 }, + { role: "assistant", content: "Response 5", ts: 1900 }, + { role: "user", content: "Message 6", ts: 2000 }, + ] + }) + + describe("truncateConversation()", () => { + it("should tag messages with truncationParent instead of deleting", () => { + const result = truncateConversation(messages, 0.5, "test-task-id") + + // All messages should still be present + expect(result.messages.length).toBe(messages.length + 1) // +1 for truncation marker + + // Calculate expected messages to remove: floor((11-1) * 0.5) = 5, rounded to even = 4 + const expectedMessagesToRemove = 4 + + // Messages 1-4 should be tagged with truncationParent + for (let i = 1; i <= expectedMessagesToRemove; i++) { + // Account for truncation marker inserted at position 1 + const msgIndex = i < 1 ? i : i + 1 + expect(result.messages[msgIndex].truncationParent).toBeDefined() + expect(result.messages[msgIndex].truncationParent).toBe(result.truncationId) + } + + // First message should not be tagged + expect(result.messages[0].truncationParent).toBeUndefined() + + // Remaining messages should not be tagged + for (let i = expectedMessagesToRemove + 2; i < result.messages.length; i++) { + expect(result.messages[i].truncationParent).toBeUndefined() + } + }) + + it("should insert truncation marker with truncationId", () => { + const result = truncateConversation(messages, 0.5, "test-task-id") + + // Truncation marker should be at index 1 (after first message) + const marker = result.messages[1] + expect(marker.isTruncationMarker).toBe(true) + expect(marker.truncationId).toBeDefined() + expect(marker.truncationId).toBe(result.truncationId) + expect(marker.role).toBe("assistant") + expect(marker.content).toContain("Sliding window truncation") + }) + + it("should return truncationId and messagesRemoved", () => { + const result = truncateConversation(messages, 0.5, "test-task-id") + + expect(result.truncationId).toBeDefined() + expect(typeof result.truncationId).toBe("string") + expect(result.messagesRemoved).toBe(4) // floor((11-1) * 0.5) rounded to even + }) + + it("should round messagesToRemove to an even number", () => { + // Test with 12 messages (1 initial + 11 conversation) + const manyMessages: ApiMessage[] = [ + { role: "user", content: "Initial", ts: 1000 }, + ...Array.from({ length: 11 }, (_, i) => ({ + role: (i % 2 === 0 ? "assistant" : "user") as "assistant" | "user", + content: `Message ${i + 1}`, + ts: 1100 + i * 100, + })), + ] + + // fracToRemove=0.5 -> rawMessagesToRemove = floor(11 * 0.5) = 5 + // messagesToRemove = 5 - (5 % 2) = 4 (rounded down to even) + const result = truncateConversation(manyMessages, 0.5, "test-task-id") + expect(result.messagesRemoved).toBe(4) + }) + }) + + describe("getEffectiveApiHistory()", () => { + it("should filter out truncated messages when truncation marker exists", () => { + const truncationResult = truncateConversation(messages, 0.5, "test-task-id") + const effective = getEffectiveApiHistory(truncationResult.messages) + + // Should exclude 4 truncated messages but keep the first message and truncation marker + // Original: 11 messages + // After truncation: 11 + 1 marker = 12 + // Effective: 11 - 4 (hidden) + 1 (marker) = 8 + expect(effective.length).toBe(8) + + // First message should be present + expect(effective[0].content).toBe("Initial task") + + // Truncation marker should be present + expect(effective[1].isTruncationMarker).toBe(true) + + // Messages with truncationParent should be filtered out + for (const msg of effective) { + if (msg.truncationParent) { + throw new Error("Message with truncationParent should be filtered out") + } + } + }) + + it("should include truncated messages when truncation marker is removed", () => { + const truncationResult = truncateConversation(messages, 0.5, "test-task-id") + + // Remove the truncation marker (simulate rewind past truncation) + const messagesWithoutMarker = truncationResult.messages.filter((msg) => !msg.isTruncationMarker) + + const effective = getEffectiveApiHistory(messagesWithoutMarker) + + // All messages should be visible now + expect(effective.length).toBe(messages.length) + + // Verify first and last messages are present + expect(effective[0].content).toBe("Initial task") + expect(effective[effective.length - 1].content).toBe("Message 6") + }) + + it("should handle both condenseParent and truncationParent filtering", () => { + // Create a scenario with both condensing and truncation + const messagesWithCondense: ApiMessage[] = [ + { role: "user", content: "Initial", ts: 1000 }, + { role: "assistant", content: "Msg 1", ts: 1100, condenseParent: "condense-1" }, + { role: "user", content: "Msg 2", ts: 1200, condenseParent: "condense-1" }, + { + role: "assistant", + content: "Summary 1", + ts: 1250, + isSummary: true, + condenseId: "condense-1", + }, + { role: "user", content: "Msg 3", ts: 1300 }, + { role: "assistant", content: "Msg 4", ts: 1400 }, + ] + + const truncationResult = truncateConversation(messagesWithCondense, 0.5, "test-task-id") + const effective = getEffectiveApiHistory(truncationResult.messages) + + // Should filter both condensed messages and truncated messages + // Messages with condenseParent="condense-1" should be filtered (summary exists) + // Messages with truncationParent should be filtered (marker exists) + const hasCondensedMessage = effective.some((msg) => msg.condenseParent === "condense-1") + const hasTruncatedMessage = effective.some((msg) => msg.truncationParent) + + expect(hasCondensedMessage).toBe(false) + expect(hasTruncatedMessage).toBe(false) + }) + }) + + describe("cleanupAfterTruncation()", () => { + it("should clear orphaned truncationParent tags when marker is deleted", () => { + const truncationResult = truncateConversation(messages, 0.5, "test-task-id") + + // Remove the truncation marker (simulate rewind) + const messagesWithoutMarker = truncationResult.messages.filter((msg) => !msg.isTruncationMarker) + + const cleaned = cleanupAfterTruncation(messagesWithoutMarker) + + // All truncationParent tags should be cleared + for (const msg of cleaned) { + expect(msg.truncationParent).toBeUndefined() + } + }) + + it("should preserve truncationParent tags when marker still exists", () => { + const truncationResult = truncateConversation(messages, 0.5, "test-task-id") + + const cleaned = cleanupAfterTruncation(truncationResult.messages) + + // truncationParent tags should be preserved (marker still exists) + const taggedMessages = cleaned.filter((msg) => msg.truncationParent) + expect(taggedMessages.length).toBeGreaterThan(0) + + // All tagged messages should point to the existing marker + for (const msg of taggedMessages) { + expect(msg.truncationParent).toBe(truncationResult.truncationId) + } + }) + + it("should handle both condenseParent and truncationParent cleanup", () => { + const messagesWithBoth: ApiMessage[] = [ + { role: "user", content: "Initial", ts: 1000 }, + { role: "assistant", content: "Msg 1", ts: 1100, condenseParent: "orphan-condense" }, + { role: "user", content: "Msg 2", ts: 1200, truncationParent: "orphan-truncation" }, + { role: "assistant", content: "Msg 3", ts: 1300 }, + ] + + const cleaned = cleanupAfterTruncation(messagesWithBoth) + + // Both orphaned parent references should be cleared + expect(cleaned[1].condenseParent).toBeUndefined() + expect(cleaned[2].truncationParent).toBeUndefined() + }) + + it("should preserve valid parent references", () => { + const messagesWithValidParents: ApiMessage[] = [ + { role: "user", content: "Initial", ts: 1000 }, + { role: "assistant", content: "Msg 1", ts: 1100, condenseParent: "valid-condense" }, + { + role: "assistant", + content: "Summary", + ts: 1150, + isSummary: true, + condenseId: "valid-condense", + }, + { role: "user", content: "Msg 2", ts: 1200, truncationParent: "valid-truncation" }, + { + role: "assistant", + content: "Truncation marker", + ts: 1250, + isTruncationMarker: true, + truncationId: "valid-truncation", + }, + ] + + const cleaned = cleanupAfterTruncation(messagesWithValidParents) + + // Valid parent references should be preserved + expect(cleaned[1].condenseParent).toBe("valid-condense") + expect(cleaned[3].truncationParent).toBe("valid-truncation") + }) + }) + + describe("Rewind past truncation integration", () => { + it("should restore hidden messages when rewinding past truncation point", () => { + // Step 1: Perform truncation + const truncationResult = truncateConversation(messages, 0.5, "test-task-id") + + // Step 2: Verify messages are hidden initially + const effectiveBeforeRewind = getEffectiveApiHistory(truncationResult.messages) + expect(effectiveBeforeRewind.length).toBeLessThan(messages.length) + + // Step 3: Simulate rewind by removing truncation marker and subsequent messages + // In practice this would be done via removeMessagesThisAndSubsequent + const markerIndex = truncationResult.messages.findIndex((msg) => msg.isTruncationMarker) + const messagesAfterRewind = truncationResult.messages.slice(0, markerIndex) + + // Step 4: Clean up orphaned parent references + const cleanedAfterRewind = cleanupAfterTruncation(messagesAfterRewind) + + // Step 5: Get effective history after cleanup + const effectiveAfterRewind = getEffectiveApiHistory(cleanedAfterRewind) + + // All original messages before the marker should be restored + expect(effectiveAfterRewind.length).toBe(markerIndex) + + // No messages should have truncationParent + for (const msg of effectiveAfterRewind) { + expect(msg.truncationParent).toBeUndefined() + } + }) + + it("should handle multiple truncations correctly", () => { + // Step 1: First truncation + const firstTruncation = truncateConversation(messages, 0.5, "task-1") + + // Step 2: Get effective history and simulate more messages being added + const effectiveAfterFirst = getEffectiveApiHistory(firstTruncation.messages) + const moreMessages: ApiMessage[] = [ + ...firstTruncation.messages, + { role: "user", content: "New message 1", ts: 3000 }, + { role: "assistant", content: "New response 1", ts: 3100 }, + { role: "user", content: "New message 2", ts: 3200 }, + { role: "assistant", content: "New response 2", ts: 3300 }, + ] + + // Step 3: Second truncation + const secondTruncation = truncateConversation(moreMessages, 0.5, "task-1") + + // Step 4: Get effective history after second truncation + const effectiveAfterSecond = getEffectiveApiHistory(secondTruncation.messages) + + // Should have messages hidden by both truncations filtered out + const firstMarker = secondTruncation.messages.find( + (msg) => msg.isTruncationMarker && msg.truncationId === firstTruncation.truncationId, + ) + const secondMarker = secondTruncation.messages.find( + (msg) => msg.isTruncationMarker && msg.truncationId === secondTruncation.truncationId, + ) + + expect(firstMarker).toBeDefined() + expect(secondMarker).toBeDefined() + + // Messages tagged with either truncationId should be filtered + for (const msg of effectiveAfterSecond) { + if (msg.truncationParent === firstTruncation.truncationId) { + throw new Error("First truncation messages should be filtered") + } + if (msg.truncationParent === secondTruncation.truncationId) { + throw new Error("Second truncation messages should be filtered") + } + } + }) + + it("should handle rewinding when second truncation affects first truncation marker", () => { + // Step 1: First truncation + const firstTruncation = truncateConversation(messages, 0.5, "task-1") + + // Step 2: Add more messages AFTER getting effective history + // This simulates real usage where we only send effective messages to API + const effectiveAfterFirst = getEffectiveApiHistory(firstTruncation.messages) + const moreMessages: ApiMessage[] = [ + ...firstTruncation.messages, // Keep full history with tagged messages + { role: "user", content: "New message 1", ts: 3000 }, + { role: "assistant", content: "New response 1", ts: 3100 }, + { role: "user", content: "New message 2", ts: 3200 }, + { role: "assistant", content: "New response 2", ts: 3300 }, + ] + + // Step 3: Second truncation - this will tag some messages including possibly the first marker + const secondTruncation = truncateConversation(moreMessages, 0.5, "task-1") + + // Step 4: Simulate rewind past second truncation marker + const secondMarkerIndex = secondTruncation.messages.findIndex( + (msg) => msg.isTruncationMarker && msg.truncationId === secondTruncation.truncationId, + ) + const afterSecondRewind = secondTruncation.messages.slice(0, secondMarkerIndex) + + // Step 5: Clean up orphaned references + const cleaned = cleanupAfterTruncation(afterSecondRewind) + + // Step 6: Get effective history + const effective = getEffectiveApiHistory(cleaned) + + // The second truncation marker should be removed + const hasSecondTruncationMarker = effective.some( + (msg) => msg.isTruncationMarker && msg.truncationId === secondTruncation.truncationId, + ) + expect(hasSecondTruncationMarker).toBe(false) + + // Messages that were tagged by the second truncation should have those tags cleared + const hasSecondTruncationParent = cleaned.some( + (msg) => msg.truncationParent === secondTruncation.truncationId, + ) + expect(hasSecondTruncationParent).toBe(false) + + // First truncation marker and its tagged messages may or may not be present + // depending on whether the second truncation affected them + // The important thing is that cleanup works correctly + expect(cleaned.length).toBeGreaterThan(0) + }) + }) + + describe("Edge cases", () => { + it("should handle truncateConversation with fracToRemove=0", () => { + const result = truncateConversation(messages, 0, "test-task-id") + + // No messages should be tagged (messagesToRemove = 0) + const taggedMessages = result.messages.filter((msg) => msg.truncationParent) + expect(taggedMessages.length).toBe(0) + + // Should still have truncation marker + const marker = result.messages.find((msg) => msg.isTruncationMarker) + expect(marker).toBeDefined() + }) + + it("should handle truncateConversation with very few messages", () => { + const fewMessages: ApiMessage[] = [ + { role: "user", content: "Initial", ts: 1000 }, + { role: "assistant", content: "Response", ts: 1100 }, + ] + + const result = truncateConversation(fewMessages, 0.5, "test-task-id") + + // Should not crash and should still create marker + expect(result.messages.length).toBeGreaterThan(0) + const marker = result.messages.find((msg) => msg.isTruncationMarker) + expect(marker).toBeDefined() + }) + + it("should handle empty condenseParent and truncationParent gracefully", () => { + const messagesWithoutTags: ApiMessage[] = [ + { role: "user", content: "Message 1", ts: 1000 }, + { role: "assistant", content: "Response 1", ts: 1100 }, + ] + + const cleaned = cleanupAfterTruncation(messagesWithoutTags) + + // Should return same messages unchanged + expect(cleaned).toEqual(messagesWithoutTags) + }) + }) +}) diff --git a/src/core/context-management/index.ts b/src/core/context-management/index.ts index 40ad688ae9a..54d2ff09c93 100644 --- a/src/core/context-management/index.ts +++ b/src/core/context-management/index.ts @@ -1,4 +1,5 @@ import { Anthropic } from "@anthropic-ai/sdk" +import crypto from "crypto" import { TelemetryService } from "@roo-code/telemetry" @@ -39,27 +40,62 @@ export async function estimateTokenCount( } /** - * Truncates a conversation by removing a fraction of the messages. + * Result of truncation operation, includes the truncation ID for UI events. + */ +export type TruncationResult = { + messages: ApiMessage[] + truncationId: string + messagesRemoved: number +} + +/** + * Truncates a conversation by tagging messages as hidden instead of removing them. * * The first message is always retained, and a specified fraction (rounded to an even number) - * of messages from the beginning (excluding the first) is removed. + * of messages from the beginning (excluding the first) is tagged with truncationParent. + * A truncation marker is inserted to track where truncation occurred. * - * This implements the sliding window truncation behavior. + * This implements non-destructive sliding window truncation, allowing messages to be + * restored if the user rewinds past the truncation point. * * @param {ApiMessage[]} messages - The conversation messages. - * @param {number} fracToRemove - The fraction (between 0 and 1) of messages (excluding the first) to remove. + * @param {number} fracToRemove - The fraction (between 0 and 1) of messages (excluding the first) to hide. * @param {string} taskId - The task ID for the conversation, used for telemetry - * @returns {ApiMessage[]} The truncated conversation messages. + * @returns {TruncationResult} Object containing the tagged messages, truncation ID, and count of messages removed. */ -export function truncateConversation(messages: ApiMessage[], fracToRemove: number, taskId: string): ApiMessage[] { +export function truncateConversation(messages: ApiMessage[], fracToRemove: number, taskId: string): TruncationResult { TelemetryService.instance.captureSlidingWindowTruncation(taskId) - const truncatedMessages = [messages[0]] + + const truncationId = crypto.randomUUID() const rawMessagesToRemove = Math.floor((messages.length - 1) * fracToRemove) const messagesToRemove = rawMessagesToRemove - (rawMessagesToRemove % 2) - const remainingMessages = messages.slice(messagesToRemove + 1) - truncatedMessages.push(...remainingMessages) - return truncatedMessages + // Tag messages that are being "truncated" (hidden from API calls) + const taggedMessages = messages.map((msg, index) => { + if (index > 0 && index <= messagesToRemove) { + return { ...msg, truncationParent: truncationId } + } + return msg + }) + + // Insert truncation marker after first message (so we know a truncation happened) + const firstKeptTs = messages[messagesToRemove + 1]?.ts ?? Date.now() + const truncationMarker: ApiMessage = { + role: "assistant", + content: `[Sliding window truncation: ${messagesToRemove} messages hidden to reduce context]`, + ts: firstKeptTs - 1, + isTruncationMarker: true, + truncationId, + } + + // Insert marker after first message + const result = [taggedMessages[0], truncationMarker, ...taggedMessages.slice(1)] + + return { + messages: result, + truncationId, + messagesRemoved: messagesToRemove, + } } /** @@ -89,7 +125,11 @@ export type ContextManagementOptions = { useNativeTools?: boolean } -export type ContextManagementResult = SummarizeResponse & { prevContextTokens: number } +export type ContextManagementResult = SummarizeResponse & { + prevContextTokens: number + truncationId?: string + messagesRemoved?: number +} /** * Conditionally manages conversation context (condense and fallback truncation). @@ -178,8 +218,16 @@ export async function manageContext({ // Fall back to sliding window truncation if needed if (prevContextTokens > allowedTokens) { - const truncatedMessages = truncateConversation(messages, 0.5, taskId) - return { messages: truncatedMessages, prevContextTokens, summary: "", cost, error } + const truncationResult = truncateConversation(messages, 0.5, taskId) + return { + messages: truncationResult.messages, + prevContextTokens, + summary: "", + cost, + error, + truncationId: truncationResult.truncationId, + messagesRemoved: truncationResult.messagesRemoved, + } } // No truncation or condensation needed return { messages, summary: "", cost, prevContextTokens, error } diff --git a/src/core/task-persistence/apiMessages.ts b/src/core/task-persistence/apiMessages.ts index 26285681353..9263115c60e 100644 --- a/src/core/task-persistence/apiMessages.ts +++ b/src/core/task-persistence/apiMessages.ts @@ -20,6 +20,18 @@ export type ApiMessage = Anthropic.MessageParam & { text?: string // For OpenRouter reasoning_details array format (used by Gemini 3, etc.) reasoning_details?: any[] + // For non-destructive condense: unique identifier for summary messages + condenseId?: string + // For non-destructive condense: points to the condenseId of the summary that replaces this message + // Messages with condenseParent are filtered out when sending to API if the summary exists + condenseParent?: string + // For non-destructive truncation: unique identifier for truncation marker messages + truncationId?: string + // For non-destructive truncation: points to the truncationId of the marker that hides this message + // Messages with truncationParent are filtered out when sending to API if the marker exists + truncationParent?: string + // Identifies a message as a truncation boundary marker + isTruncationMarker?: boolean } export async function readApiMessages({ diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index e0abd536553..31d73e88e06 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -21,6 +21,7 @@ import { type ToolUsage, type ToolName, type ContextCondense, + type ContextTruncation, type ClineMessage, type ClineSay, type ClineAsk, @@ -121,7 +122,7 @@ import { checkpointDiff, } from "../checkpoints" import { processUserContentMentions } from "../mentions/processUserContentMentions" -import { getMessagesSinceLastSummary, summarizeConversation } from "../condense" +import { getMessagesSinceLastSummary, summarizeConversation, getEffectiveApiHistory } from "../condense" import { MessageQueueService } from "../message-queue/MessageQueueService" import { AutoApprovalHandler, checkAutoApproval } from "../auto-approval" @@ -1327,6 +1328,7 @@ export class Task extends EventEmitter implements TaskLike { cost, newContextTokens = 0, error, + condenseId, } = await summarizeConversation( this.apiConversationHistory, this.api, // Main API handler (fallback) @@ -1352,7 +1354,13 @@ export class Task extends EventEmitter implements TaskLike { } await this.overwriteApiConversationHistory(messages) - const contextCondense: ContextCondense = { summary, cost, newContextTokens, prevContextTokens } + const contextCondense: ContextCondense = { + summary, + cost, + newContextTokens, + prevContextTokens, + condenseId: condenseId!, + } await this.say( "condense_context", undefined /* text */, @@ -1379,6 +1387,7 @@ export class Task extends EventEmitter implements TaskLike { isNonInteractive?: boolean } = {}, contextCondense?: ContextCondense, + contextTruncation?: ContextTruncation, ): Promise { if (this.abort) { throw new Error(`[RooCode#say] task ${this.taskId}.${this.instanceId} aborted`) @@ -1414,6 +1423,7 @@ export class Task extends EventEmitter implements TaskLike { images, partial, contextCondense, + contextTruncation, }) } } else { @@ -1451,6 +1461,7 @@ export class Task extends EventEmitter implements TaskLike { text, images, contextCondense, + contextTruncation, }) } } @@ -1474,6 +1485,7 @@ export class Task extends EventEmitter implements TaskLike { images, checkpoint, contextCondense, + contextTruncation, }) } @@ -3339,6 +3351,24 @@ export class Task extends EventEmitter implements TaskLike { { isNonInteractive: true } /* options */, contextCondense, ) + } else if (truncateResult.truncationId) { + // Sliding window truncation occurred (fallback when condensing fails or is disabled) + const contextTruncation: ContextTruncation = { + truncationId: truncateResult.truncationId, + messagesRemoved: truncateResult.messagesRemoved ?? 0, + prevContextTokens: truncateResult.prevContextTokens, + } + await this.say( + "sliding_window_truncation", + undefined /* text */, + undefined /* images */, + false /* partial */, + undefined /* checkpoint */, + undefined /* progressStatus */, + { isNonInteractive: true } /* options */, + undefined /* contextCondense */, + contextTruncation, + ) } } @@ -3449,8 +3479,14 @@ export class Task extends EventEmitter implements TaskLike { if (truncateResult.error) { await this.say("condense_context_error", truncateResult.error) } else if (truncateResult.summary) { - const { summary, cost, prevContextTokens, newContextTokens = 0 } = truncateResult - const contextCondense: ContextCondense = { summary, cost, newContextTokens, prevContextTokens } + const { summary, cost, prevContextTokens, newContextTokens = 0, condenseId } = truncateResult + const contextCondense: ContextCondense = { + summary, + cost, + newContextTokens, + prevContextTokens, + condenseId, + } await this.say( "condense_context", undefined /* text */, @@ -3461,10 +3497,32 @@ export class Task extends EventEmitter implements TaskLike { { isNonInteractive: true } /* options */, contextCondense, ) + } else if (truncateResult.truncationId) { + // Sliding window truncation occurred (fallback when condensing fails or is disabled) + const contextTruncation: ContextTruncation = { + truncationId: truncateResult.truncationId, + messagesRemoved: truncateResult.messagesRemoved ?? 0, + prevContextTokens: truncateResult.prevContextTokens, + } + await this.say( + "sliding_window_truncation", + undefined /* text */, + undefined /* images */, + false /* partial */, + undefined /* checkpoint */, + undefined /* progressStatus */, + { isNonInteractive: true } /* options */, + undefined /* contextCondense */, + contextTruncation, + ) } } - const messagesSinceLastSummary = getMessagesSinceLastSummary(this.apiConversationHistory) + // Get the effective API history by filtering out condensed messages + // This allows non-destructive condensing where messages are tagged but not deleted, + // enabling accurate rewind operations while still sending condensed history to the API. + const effectiveHistory = getEffectiveApiHistory(this.apiConversationHistory) + const messagesSinceLastSummary = getMessagesSinceLastSummary(effectiveHistory) const messagesWithoutImages = maybeRemoveImageBlocks(messagesSinceLastSummary, this.api) const cleanConversationHistory = this.buildCleanConversationHistory(messagesWithoutImages as ApiMessage[]) diff --git a/src/core/webview/__tests__/webviewMessageHandler.delete.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.delete.spec.ts index 28f6ba9cf88..8450ab5d7ab 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.delete.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.delete.spec.ts @@ -241,5 +241,322 @@ describe("webviewMessageHandler delete functionality", () => { { ts: 1500, role: "assistant", content: { type: "text", text: "First response" } }, ]) }) + + describe("condense preservation behavior", () => { + it("should preserve summary and condensed messages when deleting after the summary", async () => { + // Design: Rewind/delete preserves summaries that were created BEFORE the rewind point. + // Only summaries removed by truncation have their associated condenseParent tags cleared. + const condenseId = "summary-abc" + + getCurrentTaskMock.clineMessages = [ + { ts: 100, say: "user", text: "First message" }, + { ts: 200, say: "assistant", text: "Response 1" }, + { ts: 300, say: "user", text: "Second message" }, + { ts: 799, say: "assistant", text: "Summary" }, + { ts: 800, say: "assistant", text: "Kept message 1" }, + { ts: 900, say: "user", text: "Kept message 2" }, + { ts: 1000, say: "assistant", text: "Kept message 3" }, + ] + + // API history after condense: msg1, msg2(tagged), msg3(tagged), summary, kept1, kept2, kept3 + getCurrentTaskMock.apiConversationHistory = [ + { ts: 100, role: "user", content: "First message" }, + { ts: 200, role: "assistant", content: "Response 1", condenseParent: condenseId }, + { ts: 300, role: "user", content: "Second message", condenseParent: condenseId }, + { ts: 799, role: "assistant", content: "Summary", isSummary: true, condenseId }, + { ts: 800, role: "assistant", content: "Kept message 1" }, + { ts: 900, role: "user", content: "Kept message 2" }, + { ts: 1000, role: "assistant", content: "Kept message 3" }, + ] + + // Delete kept message 2 (ts=900) - summary is BEFORE truncation point so should be preserved + await webviewMessageHandler(provider, { + type: "deleteMessageConfirm", + messageTs: 900, + }) + + expect(getCurrentTaskMock.overwriteApiConversationHistory).toHaveBeenCalled() + const result = getCurrentTaskMock.overwriteApiConversationHistory.mock.calls[0][0] + + // Summary should be PRESERVED, condensed messages should KEEP their tags + // Expected: [msg1, msg2(tagged), msg3(tagged), summary, kept1] + expect(result.length).toBe(5) + expect(result[0].content).toBe("First message") + expect(result[1].content).toBe("Response 1") + expect(result[1].condenseParent).toBe(condenseId) // Tag preserved + expect(result[2].content).toBe("Second message") + expect(result[2].condenseParent).toBe(condenseId) // Tag preserved + expect(result[3].content).toBe("Summary") + expect(result[3].isSummary).toBe(true) // Summary preserved + expect(result[4].content).toBe("Kept message 1") + }) + + it("should restore condensed messages when summary is removed by truncation", async () => { + // Scenario: Condensed messages exist, user deletes in a way that removes the summary + // The orphaned condenseParent tags should be cleared + const condenseId = "summary-xyz" + + getCurrentTaskMock.clineMessages = [ + { ts: 100, say: "user", text: "Task start" }, + { ts: 200, say: "assistant", text: "Response 1" }, + { ts: 300, say: "user", text: "Message 2" }, + { ts: 999, say: "assistant", text: "Summary displayed" }, + { ts: 1000, say: "user", text: "First kept" }, + ] + + // API history with condensed messages and summary + getCurrentTaskMock.apiConversationHistory = [ + { ts: 100, role: "user", content: "Task start" }, + { ts: 200, role: "assistant", content: "Response 1", condenseParent: condenseId }, + { ts: 300, role: "user", content: "Message 2", condenseParent: condenseId }, + { ts: 999, role: "assistant", content: "Summary", isSummary: true, condenseId }, + { ts: 1000, role: "user", content: "First kept" }, + ] + + // Delete "Message 2" (ts=300) - this removes summary too, so orphaned tags should be cleared + await webviewMessageHandler(provider, { + type: "deleteMessageConfirm", + messageTs: 300, + }) + + expect(getCurrentTaskMock.overwriteApiConversationHistory).toHaveBeenCalled() + const result = getCurrentTaskMock.overwriteApiConversationHistory.mock.calls[0][0] + + // Summary was removed, so orphaned tags should be cleared + expect(result.length).toBe(2) + expect(result[0].content).toBe("Task start") + expect(result[1].content).toBe("Response 1") + expect(result[1].condenseParent).toBeUndefined() // Tag cleared since summary is gone + }) + + it("should preserve first condense but undo second when rewinding past second condense only", async () => { + // Scenario: Double condense, user deletes a message that removes the second summary + // but keeps the first summary. First condense should remain intact. + const condenseId1 = "summary-first" + const condenseId2 = "summary-second" + + getCurrentTaskMock.clineMessages = [ + { ts: 100, say: "user", text: "First message" }, + { ts: 799, say: "assistant", text: "Summary1" }, + { ts: 1799, say: "assistant", text: "Summary2" }, + { ts: 1800, say: "user", text: "Kept1" }, + { ts: 1900, say: "assistant", text: "Kept2" }, + { ts: 2000, say: "user", text: "To delete" }, + ] + + getCurrentTaskMock.apiConversationHistory = [ + { ts: 100, role: "user", content: "First message" }, + // Messages from first condense (tagged with condenseId1) + { ts: 200, role: "assistant", content: "Msg2", condenseParent: condenseId1 }, + { ts: 300, role: "user", content: "Msg3", condenseParent: condenseId1 }, + // First summary - ALSO tagged with condenseId2 from second condense + { + ts: 799, + role: "assistant", + content: "Summary1", + isSummary: true, + condenseId: condenseId1, + condenseParent: condenseId2, + }, + // Messages from second condense (tagged with condenseId2) + { ts: 1000, role: "assistant", content: "Msg after summary1", condenseParent: condenseId2 }, + { ts: 1100, role: "user", content: "More msgs", condenseParent: condenseId2 }, + // Second summary + { ts: 1799, role: "assistant", content: "Summary2", isSummary: true, condenseId: condenseId2 }, + // Kept messages + { ts: 1800, role: "user", content: "Kept1" }, + { ts: 1900, role: "assistant", content: "Kept2" }, + { ts: 2000, role: "user", content: "To delete" }, + ] + + // Delete "Kept2" (ts=1900) - summary2 is BEFORE truncation, so it's preserved + await webviewMessageHandler(provider, { + type: "deleteMessageConfirm", + messageTs: 1900, + }) + + expect(getCurrentTaskMock.overwriteApiConversationHistory).toHaveBeenCalled() + const result = getCurrentTaskMock.overwriteApiConversationHistory.mock.calls[0][0] + + // Both summaries should be preserved since they're before the truncation point + const summaries = result.filter((msg: any) => msg.isSummary) + expect(summaries.length).toBe(2) + + // Verify tags are preserved + const summary1 = result.find((msg: any) => msg.content === "Summary1") + expect(summary1.condenseParent).toBe(condenseId2) // Still tagged + }) + + it("should prefer non-summary message when timestamps collide for deletion target", async () => { + // When multiple messages share the same timestamp, prefer non-summary for targeting + const sharedTs = 1000 + + getCurrentTaskMock.clineMessages = [ + { ts: 900, say: "user", text: "Previous message" }, + { ts: sharedTs, say: "user", text: "First kept message" }, + { ts: 1100, say: "assistant", text: "Response" }, + ] + + // Summary and regular message share timestamp (edge case) + getCurrentTaskMock.apiConversationHistory = [ + { ts: 900, role: "user", content: "Previous message" }, + { ts: sharedTs, role: "assistant", content: "Summary", isSummary: true, condenseId: "abc" }, + { ts: sharedTs, role: "user", content: "First kept message" }, + { ts: 1100, role: "assistant", content: "Response" }, + ] + + // Delete at shared timestamp - should target non-summary message (index 2) + await webviewMessageHandler(provider, { + type: "deleteMessageConfirm", + messageTs: sharedTs, + }) + + expect(getCurrentTaskMock.overwriteApiConversationHistory).toHaveBeenCalled() + const result = getCurrentTaskMock.overwriteApiConversationHistory.mock.calls[0][0] + + // Truncation at index 2 means we keep indices 0-1: previous message and summary + expect(result.length).toBe(2) + expect(result[0].content).toBe("Previous message") + // The summary is kept since it's before truncation point + expect(result[1].content).toBe("Summary") + expect(result[1].isSummary).toBe(true) + }) + + it("should remove Summary when its condense_context clineMessage is deleted", async () => { + // Scenario: Summary has timestamp BEFORE the deletion point (so it survives truncation), + // BUT the condense_context UI message has timestamp AFTER the deletion point (so it gets removed). + // The fix links them via condenseId so the Summary is explicitly removed. + const condenseId = "summary-sync-test" + + getCurrentTaskMock.clineMessages = [ + { ts: 100, say: "user", text: "Task start" }, + { ts: 200, say: "assistant", text: "Response 1" }, + { ts: 300, say: "user", text: "Message to delete this and after" }, + { ts: 400, say: "assistant", text: "Response 2" }, + // condense_context is created AFTER the condense operation + { ts: 500, say: "condense_context", contextCondense: { condenseId, summary: "Summary text" } }, + { ts: 600, say: "user", text: "Post-condense message" }, + ] + + // Summary has ts=299 (before first kept message), so it would survive basic truncation + // But since condense_context (ts=500) is being removed, Summary should be removed too + getCurrentTaskMock.apiConversationHistory = [ + { ts: 100, role: "user", content: "Task start" }, + { ts: 200, role: "assistant", content: "Response 1", condenseParent: condenseId }, + // Summary timestamp is BEFORE the kept messages (this is the bug scenario) + { ts: 299, role: "assistant", content: "Summary text", isSummary: true, condenseId }, + { ts: 300, role: "user", content: "Message to delete this and after" }, + { ts: 400, role: "assistant", content: "Response 2" }, + { ts: 600, role: "user", content: "Post-condense message" }, + ] + + // Delete at ts=300 - this removes condense_context (ts=500), so Summary should be removed too + await webviewMessageHandler(provider, { + type: "deleteMessageConfirm", + messageTs: 300, + }) + + expect(getCurrentTaskMock.overwriteApiConversationHistory).toHaveBeenCalled() + const result = getCurrentTaskMock.overwriteApiConversationHistory.mock.calls[0][0] + + // Summary should be REMOVED even though its timestamp (299) is before truncation point (300) + // because its corresponding condense_context message is being removed + expect(result.length).toBe(2) + expect(result[0].content).toBe("Task start") + expect(result[1].content).toBe("Response 1") + // condenseParent should be cleared since the Summary is gone + expect(result[1].condenseParent).toBeUndefined() + }) + + it("should preserve first Summary when only second condense_context is deleted (nested condense)", async () => { + // Scenario: Two condense operations occurred. User deletes a message that removes + // the second condense_context but keeps the first. First summary should stay intact. + const condenseId1 = "summary-first" + const condenseId2 = "summary-second" + + getCurrentTaskMock.clineMessages = [ + { ts: 100, say: "user", text: "First message" }, + { ts: 200, say: "assistant", text: "Response 1" }, + // First condense_context created after first condense + { + ts: 800, + say: "condense_context", + contextCondense: { condenseId: condenseId1, summary: "First summary" }, + }, + { ts: 900, say: "user", text: "After first condense" }, + { ts: 1000, say: "assistant", text: "Response after 1st condense" }, + // Delete target - deleting this will remove the second condense_context below + { ts: 1100, say: "user", text: "Message to delete this and after" }, + // Second condense_context created after second condense (AFTER delete target) + { + ts: 1800, + say: "condense_context", + contextCondense: { condenseId: condenseId2, summary: "Second summary" }, + }, + { ts: 1900, say: "user", text: "Post second condense" }, + { ts: 2000, say: "assistant", text: "Final response" }, + ] + + getCurrentTaskMock.apiConversationHistory = [ + { ts: 100, role: "user", content: "First message" }, + // Messages from first condense (tagged with condenseId1) + { ts: 200, role: "assistant", content: "Response 1", condenseParent: condenseId1 }, + // First summary (also tagged with condenseId2 from second condense) + { + ts: 799, + role: "assistant", + content: "First summary", + isSummary: true, + condenseId: condenseId1, + condenseParent: condenseId2, + }, + { ts: 900, role: "user", content: "After first condense", condenseParent: condenseId2 }, + { + ts: 1000, + role: "assistant", + content: "Response after 1st condense", + condenseParent: condenseId2, + }, + { ts: 1100, role: "user", content: "Message to delete this and after" }, + // Second summary (timestamp is BEFORE the messages it summarized for sort purposes) + { + ts: 1799, + role: "assistant", + content: "Second summary", + isSummary: true, + condenseId: condenseId2, + }, + { ts: 1900, role: "user", content: "Post second condense" }, + { ts: 2000, role: "assistant", content: "Final response" }, + ] + + // Delete at ts=1100 - this removes second condense_context (ts=1800) but keeps first (ts=800) + await webviewMessageHandler(provider, { + type: "deleteMessageConfirm", + messageTs: 1100, + }) + + expect(getCurrentTaskMock.overwriteApiConversationHistory).toHaveBeenCalled() + const result = getCurrentTaskMock.overwriteApiConversationHistory.mock.calls[0][0] + + // First summary should be PRESERVED (its condense_context is not being removed) + const firstSummary = result.find((msg: any) => msg.condenseId === condenseId1) + expect(firstSummary).toBeDefined() + expect(firstSummary.content).toBe("First summary") + expect(firstSummary.isSummary).toBe(true) + + // Second summary should be REMOVED (its condense_context is being removed) + const secondSummary = result.find((msg: any) => msg.condenseId === condenseId2) + expect(secondSummary).toBeUndefined() + + // Messages that were tagged with condenseId2 should have their tags cleared + const afterFirstCondense = result.find((msg: any) => msg.content === "After first condense") + expect(afterFirstCondense?.condenseParent).toBeUndefined() // Tag cleared + + // Messages tagged with condenseId1 should KEEP their tags + const response1 = result.find((msg: any) => msg.content === "Response 1") + expect(response1?.condenseParent).toBe(condenseId1) // Tag preserved + }) + }) }) }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 21a515b6107..0201c1da2a7 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -21,6 +21,7 @@ import { TelemetryService } from "@roo-code/telemetry" import { type ApiMessage } from "../task-persistence/apiMessages" import { saveTaskMessages } from "../task-persistence" +import { cleanupAfterTruncation } from "../condense" import { ClineProvider } from "./ClineProvider" import { BrowserSessionPanelManager } from "./BrowserSessionPanelManager" @@ -78,14 +79,24 @@ export const webviewMessageHandler = async ( return provider.getCurrentTask()?.cwd || provider.cwd } /** - * Shared utility to find message indices based on timestamp + * Shared utility to find message indices based on timestamp. + * When multiple messages share the same timestamp (e.g., after condense), + * this function prefers non-summary messages to ensure user operations + * target the intended message rather than the summary. */ const findMessageIndices = (messageTs: number, currentCline: any) => { // Find the exact message by timestamp, not the first one after a cutoff const messageIndex = currentCline.clineMessages.findIndex((msg: ClineMessage) => msg.ts === messageTs) - const apiConversationHistoryIndex = currentCline.apiConversationHistory.findIndex( - (msg: ApiMessage) => msg.ts === messageTs, - ) + + // Find all matching API messages by timestamp + const allApiMatches = currentCline.apiConversationHistory + .map((msg: ApiMessage, idx: number) => ({ msg, idx })) + .filter(({ msg }: { msg: ApiMessage }) => msg.ts === messageTs) + + // Prefer non-summary message if multiple matches exist (handles timestamp collision after condense) + const preferred = allApiMatches.find(({ msg }: { msg: ApiMessage }) => !msg.isSummary) || allApiMatches[0] + const apiConversationHistoryIndex = preferred?.idx ?? -1 + return { messageIndex, apiConversationHistoryIndex } } @@ -101,20 +112,81 @@ export const webviewMessageHandler = async ( } /** - * Removes the target message and all subsequent messages + * Removes the target message and all subsequent messages. + * After truncation, cleans up orphaned condenseParent and truncationParent references for any + * summaries or truncation markers that were removed by the truncation. + * + * Design: Rewind/delete operations preserve earlier condense and truncation states. + * Only summaries and truncation markers that are removed by the truncation (i.e., were created + * after the rewind point) have their associated tags cleared. + * This allows nested condensing and multiple truncations to work correctly - rewinding past the + * second condense restores visibility of messages condensed by it, while keeping the first condense intact. + * Same applies to truncation markers. */ const removeMessagesThisAndSubsequent = async ( currentCline: any, messageIndex: number, apiConversationHistoryIndex: number, ) => { - // Delete this message and all that follow + // Step 1: Collect condenseIds from condense_context messages being removed. + // These IDs link clineMessages to their corresponding Summaries in apiConversationHistory. + const removedCondenseIds = new Set() + // Step 1b: Collect truncationIds from sliding_window_truncation messages being removed. + // These IDs link clineMessages to their corresponding truncation markers in apiConversationHistory. + const removedTruncationIds = new Set() + + for (let i = messageIndex; i < currentCline.clineMessages.length; i++) { + const msg = currentCline.clineMessages[i] + if (msg.say === "condense_context" && msg.contextCondense?.condenseId) { + removedCondenseIds.add(msg.contextCondense.condenseId) + } + if (msg.say === "sliding_window_truncation" && msg.contextTruncation?.truncationId) { + removedTruncationIds.add(msg.contextTruncation.truncationId) + } + } + + // Step 2: Delete this message and all that follow await currentCline.overwriteClineMessages(currentCline.clineMessages.slice(0, messageIndex)) if (apiConversationHistoryIndex !== -1) { - await currentCline.overwriteApiConversationHistory( - currentCline.apiConversationHistory.slice(0, apiConversationHistoryIndex), - ) + // Step 3: Truncate API history by timestamp/index + let truncatedApiHistory = currentCline.apiConversationHistory.slice(0, apiConversationHistoryIndex) + + // Step 4: Remove Summaries whose condenseId was in a removed condense_context message. + // This handles the case where Summary.ts < truncation point but condense_context.ts > truncation point. + // Without this, the Summary would survive truncation but its corresponding UI event would be gone. + if (removedCondenseIds.size > 0) { + truncatedApiHistory = truncatedApiHistory.filter((msg: ApiMessage) => { + if (msg.isSummary && msg.condenseId && removedCondenseIds.has(msg.condenseId)) { + console.log( + `[removeMessagesThisAndSubsequent] Removing orphaned Summary with condenseId=${msg.condenseId}`, + ) + return false + } + return true + }) + } + + // Step 4b: Remove truncation markers whose truncationId was in a removed sliding_window_truncation message. + // Same logic as condense - without this, the marker would survive but its UI event would be gone. + if (removedTruncationIds.size > 0) { + truncatedApiHistory = truncatedApiHistory.filter((msg: ApiMessage) => { + if (msg.isTruncationMarker && msg.truncationId && removedTruncationIds.has(msg.truncationId)) { + console.log( + `[removeMessagesThisAndSubsequent] Removing orphaned truncation marker with truncationId=${msg.truncationId}`, + ) + return false + } + return true + }) + } + + // Step 5: Clean up orphaned condenseParent and truncationParent references for messages whose + // summary or truncation marker was removed by the truncation. Summaries, truncation markers, and messages + // from earlier condense/truncation operations are preserved. + const cleanedApiHistory = cleanupAfterTruncation(truncatedApiHistory) + + await currentCline.overwriteApiConversationHistory(cleanedApiHistory) } }