diff --git a/packages/types/src/experiment.ts b/packages/types/src/experiment.ts index 10384db8ed..b455a170b7 100644 --- a/packages/types/src/experiment.ts +++ b/packages/types/src/experiment.ts @@ -6,7 +6,7 @@ import type { Keys, Equals, AssertEqual } from "./type-fu.js" * ExperimentId */ -export const experimentIds = ["powerSteering", "multiFileApplyDiff"] as const +export const experimentIds = ["powerSteering", "multiFileApplyDiff", "readFileDeduplication"] as const export const experimentIdsSchema = z.enum(experimentIds) @@ -19,6 +19,7 @@ export type ExperimentId = z.infer export const experimentsSchema = z.object({ powerSteering: z.boolean().optional(), multiFileApplyDiff: z.boolean().optional(), + readFileDeduplication: z.boolean().optional(), }) export type Experiments = z.infer diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index fe8fd0f68f..99bf717286 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -329,6 +329,110 @@ export class Task extends EventEmitter { return readApiMessages({ taskId: this.taskId, globalStoragePath: this.globalStoragePath }) } + public async deduplicateReadFileHistory(): Promise { + // Check if the experimental feature is enabled + const state = await this.providerRef.deref()?.getState() + if (!state?.experiments || !experiments.isEnabled(state.experiments, EXPERIMENT_IDS.READ_FILE_DEDUPLICATION)) { + return + } + + const cacheWindowMs = 5 * 60 * 1000 // 5 minutes + const now = Date.now() + const seenFiles = new Map() + const blocksToRemove = new Map>() // messageIndex -> Set of blockIndexes to remove + + // Process messages in reverse order (newest first) to keep the most recent reads + for (let i = this.apiConversationHistory.length - 1; i >= 0; i--) { + const message = this.apiConversationHistory[i] + + // Only process user messages + if (message.role !== "user") { + continue + } + + // Skip messages within the cache window + if (message.ts && now - message.ts < cacheWindowMs) { + continue + } + + // Process content blocks + if (Array.isArray(message.content)) { + for (let j = 0; j < message.content.length; j++) { + const block = message.content[j] + if (block.type === "text" && typeof block.text === "string") { + // Check for read_file results in text blocks + const readFileMatch = block.text.match(/\[read_file(?:\s+for\s+'([^']+)')?.*?\]\s*Result:/i) + + if (readFileMatch) { + // Extract file paths from the result content + const resultContent = block.text.substring(block.text.indexOf("Result:") + 7).trim() + + // Handle new XML format + const xmlFileMatches = resultContent.matchAll(/\s*([^<]+)<\/path>/g) + const xmlFilePaths: string[] = [] + for (const match of xmlFileMatches) { + xmlFilePaths.push(match[1].trim()) + } + + // Handle legacy format (single file) + let filePaths: string[] = xmlFilePaths + if (xmlFilePaths.length === 0 && readFileMatch[1]) { + filePaths = [readFileMatch[1]] + } + + if (filePaths.length > 0) { + // For multi-file reads, only mark as duplicate if ALL files have been seen + const allFilesSeen = filePaths.every((path) => seenFiles.has(path)) + + if (allFilesSeen) { + // This is a duplicate - mark this block for removal + if (!blocksToRemove.has(i)) { + blocksToRemove.set(i, new Set()) + } + blocksToRemove.get(i)!.add(j) + } else { + // This is not a duplicate - update seen files + filePaths.forEach((path) => { + seenFiles.set(path, { messageIndex: i, blockIndex: j }) + }) + } + } + } + } + } + } + } + + // Build the updated history, removing marked blocks + const updatedHistory: ApiMessage[] = [] + for (let i = 0; i < this.apiConversationHistory.length; i++) { + const message = this.apiConversationHistory[i] + const blocksToRemoveForMessage = blocksToRemove.get(i) + + if (blocksToRemoveForMessage && blocksToRemoveForMessage.size > 0 && Array.isArray(message.content)) { + // Filter out marked blocks + const filteredContent: Anthropic.Messages.ContentBlockParam[] = [] + + for (let j = 0; j < message.content.length; j++) { + if (!blocksToRemoveForMessage.has(j)) { + filteredContent.push(message.content[j]) + } + } + + // Only add the message if it has content after filtering + if (filteredContent.length > 0) { + updatedHistory.push({ ...message, content: filteredContent }) + } + } else { + // Keep the message as-is + updatedHistory.push(message) + } + } + + // Update the conversation history + await this.overwriteApiConversationHistory(updatedHistory) + } + private async addToApiConversationHistory(message: Anthropic.MessageParam) { const messageWithTs = { ...message, ts: Date.now() } this.apiConversationHistory.push(messageWithTs) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 9aa5a8d7a8..549f4e05a1 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -17,6 +17,7 @@ import { processUserContentMentions } from "../../mentions/processUserContentMen import { MultiSearchReplaceDiffStrategy } from "../../diff/strategies/multi-search-replace" import { MultiFileSearchReplaceDiffStrategy } from "../../diff/strategies/multi-file-search-replace" import { EXPERIMENT_IDS } from "../../../shared/experiments" +import { ApiMessage } from "../../task-persistence/apiMessages" // Mock delay before any imports that might use it vi.mock("delay", () => ({ @@ -1493,5 +1494,456 @@ describe("Cline", () => { expect(noModelTask.apiConfiguration.apiProvider).toBe("openai") }) }) + + describe("deduplicateReadFileHistory", () => { + let mockProvider: any + let mockApiConfig: any + let cline: Task + + beforeEach(() => { + vi.clearAllMocks() + + mockApiConfig = { + apiProvider: "anthropic", + apiKey: "test-key", + } + + mockProvider = { + context: { + globalStorageUri: { fsPath: "/test/storage" }, + }, + getState: vi.fn().mockResolvedValue({ + experiments: { + [EXPERIMENT_IDS.READ_FILE_DEDUPLICATION]: true, + }, + }), + postStateToWebview: vi.fn().mockResolvedValue(undefined), + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + updateTaskHistory: vi.fn().mockResolvedValue(undefined), + } + + cline = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + }) + + it("should not deduplicate when feature is disabled", async () => { + mockProvider.getState.mockResolvedValue({ + experiments: { + [EXPERIMENT_IDS.READ_FILE_DEDUPLICATION]: false, + }, + }) + + const originalHistory: ApiMessage[] = [ + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'test.ts'] Result:\ntest.tstest content", + }, + ], + ts: Date.now() - 10 * 60 * 1000, // 10 minutes ago + }, + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'test.ts'] Result:\ntest.tstest content", + }, + ], + ts: Date.now() - 8 * 60 * 1000, // 8 minutes ago + }, + ] + + cline.apiConversationHistory = [...originalHistory] + await cline.deduplicateReadFileHistory() + + // Should not change when disabled + expect(cline.apiConversationHistory).toEqual(originalHistory) + }) + + it("should deduplicate duplicate file reads", async () => { + const now = Date.now() + cline.apiConversationHistory = [ + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'test.ts'] Result:\ntest.tsold content", + }, + ], + ts: now - 10 * 60 * 1000, // 10 minutes ago + }, + { + role: "assistant", + content: [{ type: "text" as const, text: "I read the file" }], + ts: now - 9 * 60 * 1000, + }, + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'test.ts'] Result:\ntest.tsnew content", + }, + ], + ts: now - 8 * 60 * 1000, // 8 minutes ago + }, + ] + + await cline.deduplicateReadFileHistory() + + // Should keep only the most recent read of test.ts + expect(cline.apiConversationHistory).toHaveLength(2) + const content0 = cline.apiConversationHistory[0].content + const content1 = cline.apiConversationHistory[1].content + if (Array.isArray(content0) && content0[0]?.type === "text") { + expect(content0[0].text).not.toContain("old content") + } + if (Array.isArray(content1) && content1[0]?.type === "text") { + expect(content1[0].text).toContain("new content") + } + }) + + it("should preserve messages within cache window", async () => { + const now = Date.now() + cline.apiConversationHistory = [ + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'test.ts'] Result:\ntest.tsold content", + }, + ], + ts: now - 10 * 60 * 1000, // 10 minutes ago + }, + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'test.ts'] Result:\ntest.tsrecent content", + }, + ], + ts: now - 2 * 60 * 1000, // 2 minutes ago (within 5 minute cache window) + }, + ] + + await cline.deduplicateReadFileHistory() + + // Should keep both messages (recent one is within cache window) + expect(cline.apiConversationHistory).toHaveLength(2) + }) + + it("should handle multi-file reads", async () => { + const now = Date.now() + cline.apiConversationHistory = [ + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'file1.ts', 'file2.ts'] Result:\nfile1.tscontent1file2.tscontent2", + }, + ], + ts: now - 10 * 60 * 1000, + }, + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'file1.ts', 'file2.ts'] Result:\nfile1.tsnew content1file2.tsnew content2", + }, + ], + ts: now - 8 * 60 * 1000, + }, + ] + + await cline.deduplicateReadFileHistory() + + // Should keep only the most recent multi-file read + expect(cline.apiConversationHistory).toHaveLength(1) + const content = cline.apiConversationHistory[0].content + if (Array.isArray(content) && content[0]?.type === "text") { + expect(content[0].text).toContain("new content1") + expect(content[0].text).toContain("new content2") + } + }) + + it("should preserve non-read_file content blocks", async () => { + const now = Date.now() + cline.apiConversationHistory = [ + { + role: "user", + content: [ + { type: "text" as const, text: "Please read the file" }, + { + type: "text" as const, + text: "[read_file for 'test.ts'] Result:\ntest.tscontent", + }, + { type: "text" as const, text: "And then do something with it" }, + ], + ts: now - 10 * 60 * 1000, + }, + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'test.ts'] Result:\ntest.tsnew content", + }, + ], + ts: now - 8 * 60 * 1000, + }, + ] + + await cline.deduplicateReadFileHistory() + + // Should preserve non-read_file blocks in the first message + expect(cline.apiConversationHistory).toHaveLength(2) + const firstContent = cline.apiConversationHistory[0].content + if (Array.isArray(firstContent)) { + expect(firstContent).toHaveLength(2) // Two non-read_file blocks + if (firstContent[0]?.type === "text") { + expect(firstContent[0].text).toBe("Please read the file") + } + if (firstContent[1]?.type === "text") { + expect(firstContent[1].text).toBe("And then do something with it") + } + } + }) + + it("should handle legacy read_file format", async () => { + const now = Date.now() + cline.apiConversationHistory = [ + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'legacy.ts'] Result:\nFile content without XML wrapper", + }, + ], + ts: now - 10 * 60 * 1000, + }, + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'legacy.ts'] Result:\nNew file content without XML wrapper", + }, + ], + ts: now - 8 * 60 * 1000, + }, + ] + + await cline.deduplicateReadFileHistory() + + // Should deduplicate legacy format + expect(cline.apiConversationHistory).toHaveLength(1) + const legacyContent = cline.apiConversationHistory[0].content + if (Array.isArray(legacyContent) && legacyContent[0]?.type === "text") { + expect(legacyContent[0].text).toContain("New file content") + } + }) + + it("should handle messages without timestamps", async () => { + cline.apiConversationHistory = [ + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'test.ts'] Result:\ntest.tscontent", + }, + ], + // No ts property + }, + { + role: "assistant", + content: [{ type: "text" as const, text: "Processing..." }], + }, + ] + + await cline.deduplicateReadFileHistory() + + // Should handle gracefully + expect(cline.apiConversationHistory).toHaveLength(2) + }) + + it("should only process user messages", async () => { + const now = Date.now() + cline.apiConversationHistory = [ + { + role: "assistant", + content: [ + { + type: "text" as const, + text: "[read_file for 'test.ts'] Result:\ntest.tscontent", + }, + ], + ts: now - 10 * 60 * 1000, + }, + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'test.ts'] Result:\ntest.tscontent", + }, + ], + ts: now - 8 * 60 * 1000, + }, + ] + + await cline.deduplicateReadFileHistory() + + // Should keep both (assistant messages are not processed) + expect(cline.apiConversationHistory).toHaveLength(2) + }) + + it("should handle empty conversation history", async () => { + cline.apiConversationHistory = [] + await cline.deduplicateReadFileHistory() + expect(cline.apiConversationHistory).toEqual([]) + }) + + it("should handle malformed content", async () => { + const now = Date.now() + cline.apiConversationHistory = [ + { + role: "user", + content: "string content instead of array", // Invalid format + ts: now - 10 * 60 * 1000, + }, + { + role: "user", + content: [ + { + type: "image" as const, + source: { type: "base64" as const, media_type: "image/png", data: "..." }, + }, + ], // Non-text block + ts: now - 8 * 60 * 1000, + }, + ] + + await cline.deduplicateReadFileHistory() + + // Should handle gracefully + expect(cline.apiConversationHistory).toHaveLength(2) + }) + + it("should not deduplicate multi-file reads that include new files", async () => { + const now = Date.now() + // Scenario: file1.ts and file3.ts read separately, then file1.ts + file2.ts together + cline.apiConversationHistory = [ + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'file1.ts'] Result:\nfile1.tscontent1", + }, + ], + ts: now - 10 * 60 * 1000, + }, + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'file3.ts'] Result:\nfile3.tscontent3", + }, + ], + ts: now - 9 * 60 * 1000, + }, + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'file1.ts', 'file2.ts'] Result:\nfile1.tscontent1file2.tscontent2", + }, + ], + ts: now - 8 * 60 * 1000, + }, + ] + + await cline.deduplicateReadFileHistory() + + // Should keep file3.ts read and the multi-file read (which includes new file2.ts) + // The first read of just file1.ts should be removed + expect(cline.apiConversationHistory).toHaveLength(2) + + // Verify file3.ts is still there + const hasFile3 = cline.apiConversationHistory.some((msg) => { + if (Array.isArray(msg.content)) { + return msg.content.some((block) => block.type === "text" && block.text.includes("file3.ts")) + } + return false + }) + expect(hasFile3).toBe(true) + + // Verify multi-file read is still there + const hasMultiFile = cline.apiConversationHistory.some((msg) => { + if (Array.isArray(msg.content)) { + return msg.content.some( + (block) => + block.type === "text" && + block.text.includes("file1.ts") && + block.text.includes("file2.ts"), + ) + } + return false + }) + expect(hasMultiFile).toBe(true) + }) + + it("should deduplicate when multi-file read contains only already-seen files", async () => { + const now = Date.now() + cline.apiConversationHistory = [ + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'file1.ts', 'file2.ts'] Result:\nfile1.tsold1file2.tsold2", + }, + ], + ts: now - 10 * 60 * 1000, + }, + { + role: "user", + content: [ + { + type: "text" as const, + text: "[read_file for 'file1.ts', 'file2.ts'] Result:\nfile1.tsnew1file2.tsnew2", + }, + ], + ts: now - 8 * 60 * 1000, + }, + ] + + await cline.deduplicateReadFileHistory() + + // Should only keep the newer read + expect(cline.apiConversationHistory).toHaveLength(1) + const content = cline.apiConversationHistory[0].content + if (Array.isArray(content) && content[0]?.type === "text") { + expect(content[0].text).toContain("new1") + expect(content[0].text).toContain("new2") + } + }) + }) }) }) diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts index 44be1d3b92..448425c92b 100644 --- a/src/core/tools/__tests__/readFileTool.spec.ts +++ b/src/core/tools/__tests__/readFileTool.spec.ts @@ -127,6 +127,9 @@ describe("read_file tool with maxReadFileLine setting", () => { mockCline.recordToolUsage = vi.fn().mockReturnValue(undefined) mockCline.recordToolError = vi.fn().mockReturnValue(undefined) + // Add the deduplicateReadFileHistory method to the mock + mockCline.deduplicateReadFileHistory = vi.fn().mockReturnValue(undefined) + toolResult = undefined }) @@ -383,6 +386,9 @@ describe("read_file tool XML output structure", () => { mockCline.recordToolError = vi.fn().mockReturnValue(undefined) mockCline.didRejectTool = false + // Add the deduplicateReadFileHistory method to the mock + mockCline.deduplicateReadFileHistory = vi.fn().mockReturnValue(undefined) + toolResult = undefined }) diff --git a/src/core/tools/readFileTool.ts b/src/core/tools/readFileTool.ts index 6de8dd5642..2e62a82be5 100644 --- a/src/core/tools/readFileTool.ts +++ b/src/core/tools/readFileTool.ts @@ -589,6 +589,9 @@ export async function readFileTool( // No status message, just push the files XML pushToolResult(filesXml) } + + // Call deduplication after successful file reads + await cline.deduplicateReadFileHistory() } catch (error) { // Handle all errors using per-file format for consistency const relPath = fileEntries[0]?.path || "unknown" diff --git a/src/shared/__tests__/experiments.spec.ts b/src/shared/__tests__/experiments.spec.ts index 4a8f06d62a..9feec97b4a 100644 --- a/src/shared/__tests__/experiments.spec.ts +++ b/src/shared/__tests__/experiments.spec.ts @@ -23,11 +23,21 @@ describe("experiments", () => { }) }) + describe("READ_FILE_DEDUPLICATION", () => { + it("is configured correctly", () => { + expect(EXPERIMENT_IDS.READ_FILE_DEDUPLICATION).toBe("readFileDeduplication") + expect(experimentConfigsMap.READ_FILE_DEDUPLICATION).toMatchObject({ + enabled: false, + }) + }) + }) + describe("isEnabled", () => { it("returns false when POWER_STEERING experiment is not enabled", () => { const experiments: Record = { powerSteering: false, multiFileApplyDiff: false, + readFileDeduplication: false, } expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false) }) @@ -36,6 +46,7 @@ describe("experiments", () => { const experiments: Record = { powerSteering: true, multiFileApplyDiff: false, + readFileDeduplication: false, } expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true) }) @@ -44,6 +55,7 @@ describe("experiments", () => { const experiments: Record = { powerSteering: false, multiFileApplyDiff: false, + readFileDeduplication: false, } expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false) }) diff --git a/src/shared/experiments.ts b/src/shared/experiments.ts index 1edadf654f..f5f490c228 100644 --- a/src/shared/experiments.ts +++ b/src/shared/experiments.ts @@ -3,6 +3,7 @@ import type { AssertEqual, Equals, Keys, Values, ExperimentId, Experiments } fro export const EXPERIMENT_IDS = { MULTI_FILE_APPLY_DIFF: "multiFileApplyDiff", POWER_STEERING: "powerSteering", + READ_FILE_DEDUPLICATION: "readFileDeduplication", } as const satisfies Record type _AssertExperimentIds = AssertEqual>> @@ -16,6 +17,7 @@ interface ExperimentConfig { export const experimentConfigsMap: Record = { MULTI_FILE_APPLY_DIFF: { enabled: false }, POWER_STEERING: { enabled: false }, + READ_FILE_DEDUPLICATION: { enabled: false }, } export const experimentDefault = Object.fromEntries(