diff --git a/packages/types/src/message.ts b/packages/types/src/message.ts index 77c055c6e1..fa55cf44be 100644 --- a/packages/types/src/message.ts +++ b/packages/types/src/message.ts @@ -225,6 +225,8 @@ export const clineMessageSchema = z.object({ reasoning_summary: z.string().optional(), }) .optional(), + condenseId: z.string().optional(), + condenseParent: z.string().optional(), }) .optional(), }) diff --git a/src/core/condense/__tests__/condense.spec.ts b/src/core/condense/__tests__/condense.spec.ts index 5eb97b3e8a..586ba30048 100644 --- a/src/core/condense/__tests__/condense.spec.ts +++ b/src/core/condense/__tests__/condense.spec.ts @@ -84,8 +84,9 @@ describe("Condense", () => { 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 condense, we keep ALL messages plus the summary + // [first message, middle messages (tagged), summary, last N messages] + expect(result.messages.length).toBe(messages.length + 1) // All original messages + 1 summary // Verify the last N messages are preserved const lastMessages = result.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 6a03298aa6..7a61a4ab3d 100644 --- a/src/core/condense/__tests__/index.spec.ts +++ b/src/core/condense/__tests__/index.spec.ts @@ -188,21 +188,33 @@ describe("summarizeConversation", () => { 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 condense, all original messages are preserved plus the summary + 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 should be inserted after the first message and before tail) + 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) + expect(summaryMessage?.condenseId).toBeDefined() + + // Check that middle messages are tagged with condenseParent + const middleMessages = result.messages.slice(1, messages.length - N_MESSAGES_TO_KEEP + 1) + middleMessages.forEach((msg) => { + if (!msg.isSummary) { + expect(msg.condenseParent).toBe(summaryMessage?.condenseId) + } + }) - // 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) + // Check that the last N_MESSAGES_TO_KEEP messages are preserved without condenseParent + const tailMessages = result.messages.slice(-N_MESSAGES_TO_KEEP) + tailMessages.forEach((msg) => { + expect(msg.condenseParent).toBeUndefined() + }) // Check the cost and token counts expect(result.cost).toBe(0.05) @@ -424,8 +436,8 @@ describe("summarizeConversation", () => { ) // 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 condense, all original messages are preserved plus the summary + expect(result.messages.length).toBe(messages.length + 1) // All original messages + summary expect(result.cost).toBe(0.03) expect(result.summary).toBe("Concise summary") expect(result.error).toBeUndefined() diff --git a/src/core/condense/__tests__/non-destructive-condense.spec.ts b/src/core/condense/__tests__/non-destructive-condense.spec.ts new file mode 100644 index 0000000000..df4ec2721f --- /dev/null +++ b/src/core/condense/__tests__/non-destructive-condense.spec.ts @@ -0,0 +1,281 @@ +// npx vitest src/core/condense/__tests__/non-destructive-condense.spec.ts + +import { describe, it, expect, beforeEach, vi } from "vitest" +import { summarizeConversation } from "../index" +import { ApiMessage } from "../../task-persistence/apiMessages" +import { ApiHandler } from "../../../api" + +// Mock the translation function +vi.mock("../../../i18n", () => ({ + t: (key: string) => key, +})) + +// Mock TelemetryService +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { + captureContextCondensed: vi.fn(), + }, + }, +})) + +describe("Non-destructive condense", () => { + let mockApiHandler: ApiHandler + let messages: ApiMessage[] + + beforeEach(() => { + // Create a mock API handler + mockApiHandler = { + createMessage: vi.fn().mockImplementation(() => { + // Return an async generator that yields a summary + return (async function* () { + yield { type: "text", text: "This is a summary of the conversation" } + yield { type: "usage", totalCost: 0.01, outputTokens: 50 } + })() + }), + countTokens: vi.fn().mockResolvedValue(100), + getModel: vi.fn().mockReturnValue({ info: {} }), + } as any + + // Create test messages + messages = [ + { role: "user", content: "First message", ts: 1000 }, + { role: "assistant", content: "Response 1", ts: 2000 }, + { role: "user", content: "Second message", ts: 3000 }, + { role: "assistant", content: "Response 2", ts: 4000 }, + { role: "user", content: "Third message", ts: 5000 }, + { role: "assistant", content: "Response 3", ts: 6000 }, + { role: "user", content: "Fourth message", ts: 7000 }, + { role: "assistant", content: "Response 4", ts: 8000 }, + ] + }) + + describe("summarizeConversation", () => { + it("should preserve all messages with condenseParent tags", async () => { + const result = await summarizeConversation( + messages, + mockApiHandler, + "system prompt", + "task-123", + 1000, // prevContextTokens + false, + ) + + // Should not have an error + expect(result.error).toBeUndefined() + + // Should have more messages than before (all original + summary) + expect(result.messages.length).toBeGreaterThan(messages.length) + + // First message should be preserved + expect(result.messages[0]).toEqual(messages[0]) + + // Should have a summary message with condenseId + const summaryMessage = result.messages.find((m) => m.isSummary) + expect(summaryMessage).toBeDefined() + expect(summaryMessage?.condenseId).toBeDefined() + expect(summaryMessage?.condenseId).toMatch(/^condense-\d+-[a-z0-9]+$/) + + // Middle messages should have condenseParent + const middleMessages = result.messages.filter((m) => m.condenseParent) + expect(middleMessages.length).toBeGreaterThan(0) + expect(middleMessages.every((m) => m.condenseParent === summaryMessage?.condenseId)).toBe(true) + + // Last N messages should not have condenseParent + const tailMessages = result.messages.slice(-3) // N_MESSAGES_TO_KEEP = 3 + expect(tailMessages.every((m) => !m.condenseParent)).toBe(true) + }) + + it("should generate unique condenseId for each condensation", async () => { + const result1 = await summarizeConversation( + messages, + mockApiHandler, + "system prompt", + "task-123", + 1000, + false, + ) + + const result2 = await summarizeConversation( + messages, + mockApiHandler, + "system prompt", + "task-123", + 1000, + false, + ) + + const summaryMessage1 = result1.messages.find((m) => m.isSummary) + const summaryMessage2 = result2.messages.find((m) => m.isSummary) + + expect(summaryMessage1?.condenseId).toBeDefined() + expect(summaryMessage2?.condenseId).toBeDefined() + expect(summaryMessage1?.condenseId).not.toEqual(summaryMessage2?.condenseId) + }) + + it("should not condense if not enough messages", async () => { + const shortMessages = messages.slice(0, 3) + const result = await summarizeConversation( + shortMessages, + mockApiHandler, + "system prompt", + "task-123", + 1000, + false, + ) + + expect(result.error).toBe("common:errors.condense_not_enough_messages") + expect(result.messages).toEqual(shortMessages) + }) + + it("should not condense if recent summary exists", async () => { + const messagesWithSummary: ApiMessage[] = [ + ...messages.slice(0, -2), + { role: "assistant" as const, content: "Previous summary", ts: 6500, isSummary: true }, + ...messages.slice(-2), + ] + + const result = await summarizeConversation( + messagesWithSummary, + mockApiHandler, + "system prompt", + "task-123", + 1000, + false, + ) + + expect(result.error).toBe("common:errors.condensed_recently") + expect(result.messages).toEqual(messagesWithSummary) + }) + }) + + describe("Message filtering with active summaries", () => { + it("should filter out messages with condenseParent matching active summary", () => { + const messagesWithCondense: ApiMessage[] = [ + { role: "user", content: "First", ts: 1000 }, + { role: "user", content: "Second", ts: 2000, condenseParent: "condense-123-abc" }, + { role: "assistant", content: "Response", ts: 3000, condenseParent: "condense-123-abc" }, + { + role: "assistant", + content: "Summary", + ts: 4000, + isSummary: true, + condenseId: "condense-123-abc", + }, + { role: "user", content: "Latest", ts: 5000 }, + ] + + // Simulate filtering logic from Task.attemptApiRequest + const activeCondenseIds = new Set( + messagesWithCondense.filter((m) => m.isSummary && m.condenseId).map((m) => m.condenseId!), + ) + + const effectiveHistory = messagesWithCondense.filter( + (m) => !m.condenseParent || !activeCondenseIds.has(m.condenseParent), + ) + + // Should filter out the middle messages with condenseParent + expect(effectiveHistory.length).toBe(3) + expect(effectiveHistory[0].content).toBe("First") + expect(effectiveHistory[1].content).toBe("Summary") + expect(effectiveHistory[2].content).toBe("Latest") + }) + + it("should include messages with orphaned condenseParent", () => { + const messagesWithOrphan: ApiMessage[] = [ + { role: "user", content: "First", ts: 1000 }, + { role: "user", content: "Second", ts: 2000, condenseParent: "condense-old-xyz" }, // Orphaned + { role: "assistant", content: "Response", ts: 3000 }, + ] + + // No active summaries + const activeCondenseIds = new Set( + messagesWithOrphan.filter((m) => m.isSummary && m.condenseId).map((m) => m.condenseId!), + ) + + const effectiveHistory = messagesWithOrphan.filter( + (m) => !m.condenseParent || !activeCondenseIds.has(m.condenseParent), + ) + + // Should include the orphaned message since its condenseParent doesn't match any active summary + expect(effectiveHistory.length).toBe(3) + }) + }) + + describe("Nested condense support", () => { + it("should handle multiple condensations with different condenseIds", async () => { + // First condensation + const result1 = await summarizeConversation( + messages, + mockApiHandler, + "system prompt", + "task-123", + 1000, + false, + ) + + // Add more messages + const extendedMessages: ApiMessage[] = [ + ...result1.messages, + { role: "user" as const, content: "Fifth message", ts: 9000 }, + { role: "assistant" as const, content: "Response 5", ts: 10000 }, + { role: "user" as const, content: "Sixth message", ts: 11000 }, + { role: "assistant" as const, content: "Response 6", ts: 12000 }, + ] + + // Second condensation + const result2 = await summarizeConversation( + extendedMessages, + mockApiHandler, + "system prompt", + "task-123", + 1000, + false, + ) + + // Should have two different summaries with different condenseIds + const summaries = result2.messages.filter((m) => m.isSummary) + expect(summaries.length).toBeGreaterThanOrEqual(1) + + // Messages should have different condenseParent values + const condenseParents = new Set( + result2.messages.filter((m) => m.condenseParent).map((m) => m.condenseParent), + ) + expect(condenseParents.size).toBeGreaterThanOrEqual(1) + }) + }) + + describe("Rollback behavior", () => { + it("should support rollback by removing summary and cleaning condenseParent", () => { + const messagesWithCondense: ApiMessage[] = [ + { role: "user", content: "First", ts: 1000 }, + { role: "user", content: "Second", ts: 2000, condenseParent: "condense-123-abc" }, + { role: "assistant", content: "Response", ts: 3000, condenseParent: "condense-123-abc" }, + { + role: "assistant", + content: "Summary", + ts: 4000, + isSummary: true, + condenseId: "condense-123-abc", + }, + { role: "user", content: "Latest", ts: 5000 }, + ] + + // Simulate rollback: remove summary + const afterRollback = messagesWithCondense.filter((m) => !m.isSummary) + + // Simulate hygiene: clean orphaned condenseParent + const activeCondenseIds = new Set(afterRollback.filter((m) => m.condenseId).map((m) => m.condenseId!)) + + afterRollback.forEach((m) => { + if (m.condenseParent && !activeCondenseIds.has(m.condenseParent)) { + delete m.condenseParent + } + }) + + // All messages should have condenseParent removed + expect(afterRollback.every((m) => !m.condenseParent)).toBe(true) + expect(afterRollback.length).toBe(4) + }) + }) +}) diff --git a/src/core/condense/index.ts b/src/core/condense/index.ts index 86cfa7ab1e..47389b60aa 100644 --- a/src/core/condense/index.ts +++ b/src/core/condense/index.ts @@ -181,15 +181,35 @@ export async function summarizeConversation( return { ...response, cost, error } } + // Generate a unique condenseId for this condensation + const condenseId = `condense-${Date.now()}-${Math.random().toString(36).slice(2, 11)}` + + // Choose a unique timestamp that sorts just before the first kept tail message + const summaryTs = Math.min((keepMessages[0]?.ts ?? Date.now()) - 1, Date.now()) + + // Create the summary message with condenseId const summaryMessage: ApiMessage = { role: "assistant", content: summary, - ts: keepMessages[0].ts, + ts: summaryTs, isSummary: true, + condenseId: condenseId, } - // Reconstruct messages: [first message, summary, last N messages] - const newMessages = [firstMessage, summaryMessage, ...keepMessages] + // Tag middle messages from the full middle span, including any previous summaries + // that were part of this summarization window. Preserve existing condenseId/isSummary. + const windowTs = new Set(messagesToSummarize.map((m) => m.ts).filter((ts): ts is number => typeof ts === "number")) + const middleMessages = messages.slice(1, -N_MESSAGES_TO_KEEP).map((msg) => { + if (typeof msg.ts === "number" && windowTs.has(msg.ts)) { + // Do not alter isSummary or condenseId on prior summaries; only add condenseParent if missing + return { ...msg, condenseParent: msg.condenseParent ?? condenseId } + } + return msg + }) + + // Reconstruct messages: [first message, tagged middle messages, summary, last N messages] + // This preserves ALL messages, with middle ones tagged for filtering + const newMessages = [firstMessage, ...middleMessages, summaryMessage, ...keepMessages] // 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 diff --git a/src/core/task-persistence/apiMessages.ts b/src/core/task-persistence/apiMessages.ts index f846aaf13f..b28ff4289d 100644 --- a/src/core/task-persistence/apiMessages.ts +++ b/src/core/task-persistence/apiMessages.ts @@ -9,7 +9,12 @@ import { fileExistsAtPath } from "../../utils/fs" import { GlobalFileNames } from "../../shared/globalFileNames" import { getTaskDirectoryPath } from "../../utils/storage" -export type ApiMessage = Anthropic.MessageParam & { ts?: number; isSummary?: boolean } +export type ApiMessage = Anthropic.MessageParam & { + ts?: number + isSummary?: boolean + condenseId?: string + condenseParent?: string +} export async function readApiMessages({ taskId, diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 851df91e6c..0aa0474211 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1040,6 +1040,29 @@ export class Task extends EventEmitter implements TaskLike { // Set flag to skip previous_response_id on the next API call after manual condense this.skipPrevResponseIdOnce = true + // If there was a previous condense_context in the UI, tag it with condenseParent = newCondenseId + try { + const newCondenseId = messages.find((m) => m.isSummary && m.condenseId)?.condenseId + if (newCondenseId) { + const lastCondenseIdx = findLastIndex( + this.clineMessages, + (m) => m.type === "say" && m.say === "condense_context", + ) + if (lastCondenseIdx !== -1) { + const lastCondenseMsg = this.clineMessages[lastCondenseIdx] as ClineMessage & + ClineMessageWithMetadata + lastCondenseMsg.metadata = lastCondenseMsg.metadata ?? {} + if (!lastCondenseMsg.metadata.condenseParent) { + lastCondenseMsg.metadata.condenseParent = newCondenseId + await this.saveClineMessages() + await this.updateClineMessage(lastCondenseMsg) + } + } + } + } catch { + // non-fatal; UI metadata tagging is best-effort + } + const contextCondense: ContextCondense = { summary, cost, newContextTokens, prevContextTokens } await this.say( "condense_context", @@ -1048,7 +1071,12 @@ export class Task extends EventEmitter implements TaskLike { false /* partial */, undefined /* checkpoint */, undefined /* progressStatus */, - { isNonInteractive: true } /* options */, + { + isNonInteractive: true, + metadata: { + condenseId: messages.find((m) => m.condenseId)?.condenseId, + }, + } /* options */, contextCondense, ) } @@ -2622,6 +2650,39 @@ export class Task extends EventEmitter implements TaskLike { // send previous_response_id so the request reflects the fresh condensed context. this.skipPrevResponseIdOnce = true + // Determine the condenseId of the newly created summary in API history + let newCondenseId: string | undefined + try { + const lastSummary = [...this.apiConversationHistory] + .reverse() + .find((m) => m.isSummary && m.condenseId) + newCondenseId = lastSummary?.condenseId + } catch { + // non-fatal + } + + // Tag the previous UI condense_context (if any) with condenseParent to maintain lineage + try { + if (newCondenseId) { + const lastCondenseIdx = findLastIndex( + this.clineMessages, + (m) => m.type === "say" && m.say === "condense_context", + ) + if (lastCondenseIdx !== -1) { + const lastCondenseMsg = this.clineMessages[lastCondenseIdx] as ClineMessage & + ClineMessageWithMetadata + lastCondenseMsg.metadata = lastCondenseMsg.metadata ?? {} + if (!lastCondenseMsg.metadata.condenseParent) { + lastCondenseMsg.metadata.condenseParent = newCondenseId + await this.saveClineMessages() + await this.updateClineMessage(lastCondenseMsg) + } + } + } + } catch { + // best-effort + } + const { summary, cost, prevContextTokens, newContextTokens = 0 } = truncateResult const contextCondense: ContextCondense = { summary, cost, newContextTokens, prevContextTokens } await this.say( @@ -2631,13 +2692,26 @@ export class Task extends EventEmitter implements TaskLike { false /* partial */, undefined /* checkpoint */, undefined /* progressStatus */, - { isNonInteractive: true } /* options */, + { + isNonInteractive: true, + metadata: newCondenseId ? { condenseId: newCondenseId } : undefined, + } /* options */, contextCondense, ) } } - const messagesSinceLastSummary = getMessagesSinceLastSummary(this.apiConversationHistory) + // Filter messages based on active summaries (non-destructive condense) + const activeCondenseIds = new Set( + this.apiConversationHistory.filter((m) => m.isSummary && m.condenseId).map((m) => m.condenseId!), + ) + + // Filter out messages that have been condensed (have condenseParent that matches an active summary) + const effectiveHistory = this.apiConversationHistory.filter( + (m) => !m.condenseParent || !activeCondenseIds.has(m.condenseParent), + ) + + const messagesSinceLastSummary = getMessagesSinceLastSummary(effectiveHistory) let cleanConversationHistory = maybeRemoveImageBlocks(messagesSinceLastSummary, this.api).map( ({ role, content }) => ({ role, content }), ) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index af5f9925c3..03ec8a7b35 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -16,7 +16,7 @@ import { import { CloudService } from "@roo-code/cloud" import { TelemetryService } from "@roo-code/telemetry" -import { type ApiMessage } from "../task-persistence/apiMessages" +import { type ApiMessage, saveApiMessages } from "../task-persistence/apiMessages" import { saveTaskMessages } from "../task-persistence" import { ClineProvider } from "./ClineProvider" @@ -112,6 +112,83 @@ export const webviewMessageHandler = async ( currentCline.apiConversationHistory.slice(0, apiConversationHistoryIndex), ) } + + // Perform hygiene: UI is source-of-truth during rewinds (purge API summaries whose UI counterparts were removed) + await performCondenseHygiene(currentCline, { uiOnly: true }) + } + + /** + * Clean up orphaned condenseParent references after truncation + */ + const performCondenseHygiene = async (currentCline: any, opts?: { uiOnly?: boolean }) => { + // Build active condenseIds. If uiOnly, treat UI as source-of-truth (used during rewind/delete). + // Otherwise, include API summaries too (general hygiene). + const activeCondenseIds = new Set() + + // Always include UI-derived condenseIds + currentCline.clineMessages.forEach((msg: any) => { + if (msg.say === "condense_context" && msg.metadata?.condenseId) { + activeCondenseIds.add(msg.metadata.condenseId) + } + }) + + // Optionally include API summaries that remain in history (non-delete hygiene) + if (!opts?.uiOnly) { + currentCline.apiConversationHistory.forEach((msg: ApiMessage) => { + if (msg.isSummary && msg.condenseId) { + activeCondenseIds.add(msg.condenseId) + } + }) + } + + let apiHistoryModified = false + let uiMessagesModified = false + + // Purge API summaries that are not represented by UI when uiOnly=true (rewind to pre-condense state). + // In general hygiene (uiOnly=false), we only remove summaries with condenseIds not present anywhere. + const beforeLen = currentCline.apiConversationHistory.length + currentCline.apiConversationHistory = currentCline.apiConversationHistory.filter((msg: ApiMessage) => { + if (msg.isSummary && msg.condenseId && !activeCondenseIds.has(msg.condenseId)) { + return false + } + return true + }) + if (currentCline.apiConversationHistory.length !== beforeLen) { + apiHistoryModified = true + } + + // Clean up orphaned condenseParent references in API history + currentCline.apiConversationHistory.forEach((msg: ApiMessage) => { + if (msg.condenseParent && !activeCondenseIds.has(msg.condenseParent)) { + delete msg.condenseParent + apiHistoryModified = true + } + }) + + // Clean up orphaned condenseParent references in UI messages + currentCline.clineMessages.forEach((msg: any) => { + if (msg.metadata?.condenseParent && !activeCondenseIds.has(msg.metadata.condenseParent)) { + delete msg.metadata.condenseParent + uiMessagesModified = true + } + }) + + // Save if any modifications were made + if (apiHistoryModified) { + await saveApiMessages({ + messages: currentCline.apiConversationHistory, + taskId: currentCline.taskId, + globalStoragePath: provider.contextProxy.globalStorageUri.fsPath, + }) + } + + if (uiMessagesModified) { + await saveTaskMessages({ + messages: currentCline.clineMessages, + taskId: currentCline.taskId, + globalStoragePath: provider.contextProxy.globalStorageUri.fsPath, + }) + } } /**