fix: implement read_file history deduplication (#6279)

- Add READ_FILE_DEDUPLICATION experimental feature flag
- Implement deduplicateReadFileHistory method in Task class
- Integrate deduplication into readFileTool after successful reads
- Add comprehensive unit tests for deduplication logic
- Update readFileTool tests to include mock deduplication method

This feature removes duplicate read_file entries from conversation history
while preserving the most recent content for each file. It respects a 5-minute
cache window and handles single files, multi-file reads, and legacy formats.
This commit is contained in:
hannesrudolph 2025-07-28 12:42:39 -06:00
parent 342ee70fb4
commit 311ef2088d
7 changed files with 581 additions and 1 deletions

View file

@ -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<typeof experimentIdsSchema>
export const experimentsSchema = z.object({
powerSteering: z.boolean().optional(),
multiFileApplyDiff: z.boolean().optional(),
readFileDeduplication: z.boolean().optional(),
})
export type Experiments = z.infer<typeof experimentsSchema>

View file

@ -329,6 +329,110 @@ export class Task extends EventEmitter<ClineEvents> {
return readApiMessages({ taskId: this.taskId, globalStoragePath: this.globalStoragePath })
}
public async deduplicateReadFileHistory(): Promise<void> {
// 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<string, { messageIndex: number; blockIndex: number }>()
const blocksToRemove = new Map<number, Set<number>>() // 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(/<file>\s*<path>([^<]+)<\/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)

View file

@ -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:\n<files><file><path>test.ts</path><content>test content</content></file></files>",
},
],
ts: Date.now() - 10 * 60 * 1000, // 10 minutes ago
},
{
role: "user",
content: [
{
type: "text" as const,
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>test content</content></file></files>",
},
],
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:\n<files><file><path>test.ts</path><content>old content</content></file></files>",
},
],
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:\n<files><file><path>test.ts</path><content>new content</content></file></files>",
},
],
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:\n<files><file><path>test.ts</path><content>old content</content></file></files>",
},
],
ts: now - 10 * 60 * 1000, // 10 minutes ago
},
{
role: "user",
content: [
{
type: "text" as const,
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>recent content</content></file></files>",
},
],
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:\n<files><file><path>file1.ts</path><content>content1</content></file><file><path>file2.ts</path><content>content2</content></file></files>",
},
],
ts: now - 10 * 60 * 1000,
},
{
role: "user",
content: [
{
type: "text" as const,
text: "[read_file for 'file1.ts', 'file2.ts'] Result:\n<files><file><path>file1.ts</path><content>new content1</content></file><file><path>file2.ts</path><content>new content2</content></file></files>",
},
],
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:\n<files><file><path>test.ts</path><content>content</content></file></files>",
},
{ 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:\n<files><file><path>test.ts</path><content>new content</content></file></files>",
},
],
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:\n<files><file><path>test.ts</path><content>content</content></file></files>",
},
],
// 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:\n<files><file><path>test.ts</path><content>content</content></file></files>",
},
],
ts: now - 10 * 60 * 1000,
},
{
role: "user",
content: [
{
type: "text" as const,
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>content</content></file></files>",
},
],
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:\n<files><file><path>file1.ts</path><content>content1</content></file></files>",
},
],
ts: now - 10 * 60 * 1000,
},
{
role: "user",
content: [
{
type: "text" as const,
text: "[read_file for 'file3.ts'] Result:\n<files><file><path>file3.ts</path><content>content3</content></file></files>",
},
],
ts: now - 9 * 60 * 1000,
},
{
role: "user",
content: [
{
type: "text" as const,
text: "[read_file for 'file1.ts', 'file2.ts'] Result:\n<files><file><path>file1.ts</path><content>content1</content></file><file><path>file2.ts</path><content>content2</content></file></files>",
},
],
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:\n<files><file><path>file1.ts</path><content>old1</content></file><file><path>file2.ts</path><content>old2</content></file></files>",
},
],
ts: now - 10 * 60 * 1000,
},
{
role: "user",
content: [
{
type: "text" as const,
text: "[read_file for 'file1.ts', 'file2.ts'] Result:\n<files><file><path>file1.ts</path><content>new1</content></file><file><path>file2.ts</path><content>new2</content></file></files>",
},
],
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")
}
})
})
})
})

View file

@ -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
})

View file

@ -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"

View file

@ -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<ExperimentId, boolean> = {
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<ExperimentId, boolean> = {
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<ExperimentId, boolean> = {
powerSteering: false,
multiFileApplyDiff: false,
readFileDeduplication: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
})

View file

@ -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<string, ExperimentId>
type _AssertExperimentIds = AssertEqual<Equals<ExperimentId, Values<typeof EXPERIMENT_IDS>>>
@ -16,6 +17,7 @@ interface ExperimentConfig {
export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
MULTI_FILE_APPLY_DIFF: { enabled: false },
POWER_STEERING: { enabled: false },
READ_FILE_DEDUPLICATION: { enabled: false },
}
export const experimentDefault = Object.fromEntries(