mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
feat: implement read_file history deduplication
- 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 functionality - Preserve cache window (5 minutes) to avoid modifying recent messages - Handle both single-file and multi-file read operations - Support legacy read_file format for backward compatibility Fixes #6279
This commit is contained in:
parent
b73c17a375
commit
08c76d704f
6 changed files with 428 additions and 4 deletions
|
|
@ -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>
|
||||
|
|
|
|||
|
|
@ -353,6 +353,100 @@ export class Task extends EventEmitter<ClineEvents> {
|
|||
}
|
||||
}
|
||||
|
||||
public async deduplicateReadFileHistory(): Promise<void> {
|
||||
// Check if the experimental feature is enabled
|
||||
const state = await this.providerRef.deref()?.getState()
|
||||
const isDeduplicationEnabled = experiments.isEnabled(
|
||||
state?.experiments ?? {},
|
||||
EXPERIMENT_IDS.READ_FILE_DEDUPLICATION,
|
||||
)
|
||||
|
||||
if (!isDeduplicationEnabled) {
|
||||
return
|
||||
}
|
||||
|
||||
// Track which files have been seen (most recent occurrence)
|
||||
const seenFiles = new Set<string>()
|
||||
const cacheWindowMs = 5 * 60 * 1000 // 5 minutes
|
||||
const now = Date.now()
|
||||
|
||||
// Iterate through conversation history in reverse order (newest to oldest)
|
||||
for (let i = this.apiConversationHistory.length - 1; i >= 0; i--) {
|
||||
const message = this.apiConversationHistory[i]
|
||||
|
||||
// Skip if message is within cache window
|
||||
if (message.ts && now - message.ts < cacheWindowMs) {
|
||||
continue
|
||||
}
|
||||
|
||||
// Only process user messages
|
||||
if (message.role !== "user") {
|
||||
continue
|
||||
}
|
||||
|
||||
// Process content blocks
|
||||
if (Array.isArray(message.content)) {
|
||||
const newContent = message.content.filter((block) => {
|
||||
if (block.type !== "text") {
|
||||
return true // Keep non-text blocks
|
||||
}
|
||||
|
||||
// Check if this is a read_file result
|
||||
const readFileMatch = block.text.match(/^\[read_file.*?\] Result:/)
|
||||
if (!readFileMatch) {
|
||||
return true // Keep non-read_file blocks
|
||||
}
|
||||
|
||||
// Extract file paths from the read_file result
|
||||
// Handle both single file and multi-file formats
|
||||
const filePaths: string[] = []
|
||||
|
||||
// Try to match file paths in XML format
|
||||
const filePathMatches = block.text.matchAll(/<file><path>([^<]+)<\/path>/g)
|
||||
for (const match of filePathMatches) {
|
||||
filePaths.push(match[1])
|
||||
}
|
||||
|
||||
// If no paths found in XML, try legacy format
|
||||
if (filePaths.length === 0) {
|
||||
const legacyMatch = block.text.match(/\[read_file for '([^']+)'/)
|
||||
if (legacyMatch) {
|
||||
filePaths.push(legacyMatch[1])
|
||||
}
|
||||
}
|
||||
|
||||
// Check if all files in this result have been seen more recently
|
||||
if (filePaths.length > 0) {
|
||||
const allFilesSeen = filePaths.every((path) => seenFiles.has(path))
|
||||
|
||||
if (allFilesSeen) {
|
||||
// Remove this duplicate read_file result
|
||||
return false
|
||||
} else {
|
||||
// Mark these files as seen
|
||||
filePaths.forEach((path) => seenFiles.add(path))
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
// Keep blocks we couldn't parse
|
||||
return true
|
||||
})
|
||||
|
||||
// Update message content if any blocks were removed
|
||||
if (newContent.length !== message.content.length) {
|
||||
this.apiConversationHistory[i] = {
|
||||
...message,
|
||||
content: newContent,
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Save the updated conversation history
|
||||
await this.saveApiConversationHistory()
|
||||
}
|
||||
|
||||
// Cline Messages
|
||||
|
||||
private async getSavedClineMessages(): Promise<ClineMessage[]> {
|
||||
|
|
|
|||
|
|
@ -1493,5 +1493,331 @@ describe("Cline", () => {
|
|||
expect(noModelTask.apiConfiguration.apiProvider).toBe("openai")
|
||||
})
|
||||
})
|
||||
|
||||
describe("deduplicateReadFileHistory", () => {
|
||||
let mockProvider: any
|
||||
let mockApiConfig: any
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
|
||||
mockApiConfig = {
|
||||
apiProvider: "anthropic",
|
||||
apiKey: "test-key",
|
||||
}
|
||||
|
||||
mockProvider = {
|
||||
context: {
|
||||
globalStorageUri: { fsPath: "/test/storage" },
|
||||
},
|
||||
getState: vi.fn(),
|
||||
postStateToWebview: vi.fn().mockResolvedValue(undefined),
|
||||
postMessageToWebview: vi.fn().mockResolvedValue(undefined),
|
||||
updateTaskHistory: vi.fn().mockResolvedValue(undefined),
|
||||
}
|
||||
})
|
||||
|
||||
it("should not deduplicate when feature is disabled", async () => {
|
||||
mockProvider.getState.mockResolvedValue({
|
||||
experiments: {
|
||||
[EXPERIMENT_IDS.READ_FILE_DEDUPLICATION]: false,
|
||||
},
|
||||
})
|
||||
|
||||
const task = new Task({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: mockApiConfig,
|
||||
task: "test task",
|
||||
startTask: false,
|
||||
})
|
||||
|
||||
// Add duplicate read_file results
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>content1</content></file></files>",
|
||||
},
|
||||
],
|
||||
ts: Date.now() - 10 * 60 * 1000, // 10 minutes ago
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>content2</content></file></files>",
|
||||
},
|
||||
],
|
||||
ts: Date.now() - 8 * 60 * 1000, // 8 minutes ago
|
||||
},
|
||||
]
|
||||
|
||||
const originalLength = task.apiConversationHistory.length
|
||||
|
||||
await task.deduplicateReadFileHistory()
|
||||
|
||||
// Should not remove anything when feature is disabled
|
||||
expect(task.apiConversationHistory.length).toBe(originalLength)
|
||||
})
|
||||
|
||||
it("should deduplicate when feature is enabled", async () => {
|
||||
mockProvider.getState.mockResolvedValue({
|
||||
experiments: {
|
||||
[EXPERIMENT_IDS.READ_FILE_DEDUPLICATION]: true,
|
||||
},
|
||||
})
|
||||
|
||||
const task = new Task({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: mockApiConfig,
|
||||
task: "test task",
|
||||
startTask: false,
|
||||
})
|
||||
|
||||
// Add duplicate read_file results
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>content1</content></file></files>",
|
||||
},
|
||||
],
|
||||
ts: Date.now() - 10 * 60 * 1000, // 10 minutes ago
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>content2</content></file></files>",
|
||||
},
|
||||
],
|
||||
ts: Date.now() - 8 * 60 * 1000, // 8 minutes ago
|
||||
},
|
||||
]
|
||||
|
||||
await task.deduplicateReadFileHistory()
|
||||
|
||||
// Should keep only the most recent read_file result
|
||||
expect(task.apiConversationHistory.length).toBe(2)
|
||||
expect(task.apiConversationHistory[0].content).toHaveLength(0) // First one should be empty
|
||||
expect(task.apiConversationHistory[1].content).toHaveLength(1) // Second one should remain
|
||||
})
|
||||
|
||||
it("should not deduplicate messages within cache window", async () => {
|
||||
mockProvider.getState.mockResolvedValue({
|
||||
experiments: {
|
||||
[EXPERIMENT_IDS.READ_FILE_DEDUPLICATION]: true,
|
||||
},
|
||||
})
|
||||
|
||||
const task = new Task({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: mockApiConfig,
|
||||
task: "test task",
|
||||
startTask: false,
|
||||
})
|
||||
|
||||
const now = Date.now()
|
||||
|
||||
// Add duplicate read_file results
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>content1</content></file></files>",
|
||||
},
|
||||
],
|
||||
ts: now - 10 * 60 * 1000, // 10 minutes ago (outside cache window)
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>content2</content></file></files>",
|
||||
},
|
||||
],
|
||||
ts: now - 2 * 60 * 1000, // 2 minutes ago (inside cache window)
|
||||
},
|
||||
]
|
||||
|
||||
await task.deduplicateReadFileHistory()
|
||||
|
||||
// Both should remain because the second one is within cache window
|
||||
expect(task.apiConversationHistory.length).toBe(2)
|
||||
expect(task.apiConversationHistory[0].content).toHaveLength(1)
|
||||
expect(task.apiConversationHistory[1].content).toHaveLength(1)
|
||||
})
|
||||
|
||||
it("should handle multi-file read_file results", async () => {
|
||||
mockProvider.getState.mockResolvedValue({
|
||||
experiments: {
|
||||
[EXPERIMENT_IDS.READ_FILE_DEDUPLICATION]: true,
|
||||
},
|
||||
})
|
||||
|
||||
const task = new Task({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: mockApiConfig,
|
||||
task: "test task",
|
||||
startTask: false,
|
||||
})
|
||||
|
||||
// Add multi-file read_file results
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "[read_file] Result:\n<files><file><path>a.ts</path></file><file><path>b.ts</path></file></files>",
|
||||
},
|
||||
],
|
||||
ts: Date.now() - 10 * 60 * 1000,
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "[read_file] Result:\n<files><file><path>a.ts</path></file></files>",
|
||||
},
|
||||
],
|
||||
ts: Date.now() - 8 * 60 * 1000,
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "[read_file] Result:\n<files><file><path>b.ts</path></file><file><path>c.ts</path></file></files>",
|
||||
},
|
||||
],
|
||||
ts: Date.now() - 6 * 60 * 1000,
|
||||
},
|
||||
]
|
||||
|
||||
await task.deduplicateReadFileHistory()
|
||||
|
||||
// First result should be removed (both a.ts and b.ts have been read more recently)
|
||||
expect(task.apiConversationHistory[0].content).toHaveLength(0)
|
||||
// Second result should remain (most recent read of a.ts)
|
||||
expect(task.apiConversationHistory[1].content).toHaveLength(1)
|
||||
// Third result should remain (most recent read of b.ts and c.ts)
|
||||
expect(task.apiConversationHistory[2].content).toHaveLength(1)
|
||||
})
|
||||
|
||||
it("should preserve non-read_file content blocks", async () => {
|
||||
mockProvider.getState.mockResolvedValue({
|
||||
experiments: {
|
||||
[EXPERIMENT_IDS.READ_FILE_DEDUPLICATION]: true,
|
||||
},
|
||||
})
|
||||
|
||||
const task = new Task({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: mockApiConfig,
|
||||
task: "test task",
|
||||
startTask: false,
|
||||
})
|
||||
|
||||
// Add mixed content
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{ type: "text", text: "Regular user message" },
|
||||
{
|
||||
type: "text",
|
||||
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path></file></files>",
|
||||
},
|
||||
{ type: "image", source: { type: "base64", media_type: "image/png", data: "base64data" } },
|
||||
],
|
||||
ts: Date.now() - 10 * 60 * 1000,
|
||||
},
|
||||
{
|
||||
role: "assistant",
|
||||
content: [{ type: "text", text: "Assistant response" }],
|
||||
ts: Date.now() - 9 * 60 * 1000,
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path></file></files>",
|
||||
},
|
||||
],
|
||||
ts: Date.now() - 8 * 60 * 1000,
|
||||
},
|
||||
]
|
||||
|
||||
await task.deduplicateReadFileHistory()
|
||||
|
||||
// First message should have read_file removed but other content preserved
|
||||
expect(task.apiConversationHistory[0].content).toHaveLength(2)
|
||||
const content0 = task.apiConversationHistory[0].content[0]
|
||||
const content1 = task.apiConversationHistory[0].content[1]
|
||||
expect(typeof content0 === "object" && content0 !== null && "type" in content0 && content0.type).toBe(
|
||||
"text",
|
||||
)
|
||||
expect(typeof content0 === "object" && content0 !== null && "text" in content0 && content0.text).toBe(
|
||||
"Regular user message",
|
||||
)
|
||||
expect(typeof content1 === "object" && content1 !== null && "type" in content1 && content1.type).toBe(
|
||||
"image",
|
||||
)
|
||||
|
||||
// Assistant message should remain unchanged
|
||||
expect(task.apiConversationHistory[1].content).toHaveLength(1)
|
||||
|
||||
// Last read_file should remain
|
||||
expect(task.apiConversationHistory[2].content).toHaveLength(1)
|
||||
})
|
||||
|
||||
it("should handle legacy single-file format", async () => {
|
||||
mockProvider.getState.mockResolvedValue({
|
||||
experiments: {
|
||||
[EXPERIMENT_IDS.READ_FILE_DEDUPLICATION]: true,
|
||||
},
|
||||
})
|
||||
|
||||
const task = new Task({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: mockApiConfig,
|
||||
task: "test task",
|
||||
startTask: false,
|
||||
})
|
||||
|
||||
// Add legacy format read_file results
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
role: "user",
|
||||
content: [{ type: "text", text: "[read_file for 'test.ts'] Result:\nFile content here" }],
|
||||
ts: Date.now() - 10 * 60 * 1000,
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [{ type: "text", text: "[read_file for 'test.ts'] Result:\nUpdated file content" }],
|
||||
ts: Date.now() - 8 * 60 * 1000,
|
||||
},
|
||||
]
|
||||
|
||||
await task.deduplicateReadFileHistory()
|
||||
|
||||
// First one should be removed
|
||||
expect(task.apiConversationHistory[0].content).toHaveLength(0)
|
||||
// Second one should remain
|
||||
expect(task.apiConversationHistory[1].content).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -609,5 +609,8 @@ export async function readFileTool(
|
|||
const xmlResults = fileResults.filter((result) => result.xmlContent).map((result) => result.xmlContent)
|
||||
|
||||
pushToolResult(`<files>\n${xmlResults.join("\n")}\n</files>`)
|
||||
|
||||
// Deduplicate read_file history after successful reads
|
||||
await cline.deduplicateReadFileHistory()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -222,10 +222,8 @@ describe("mergeExtensionState", () => {
|
|||
apiConfiguration: { modelMaxThinkingTokens: 456, modelTemperature: 0.3 },
|
||||
experiments: {
|
||||
powerSteering: true,
|
||||
marketplace: false,
|
||||
disableCompletionCommand: false,
|
||||
concurrentFileReads: true,
|
||||
multiFileApplyDiff: true,
|
||||
readFileDeduplication: false,
|
||||
} as Record<ExperimentId, boolean>,
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue