mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-06 08:18:39 +00:00
fix: implement proactive file caching for read_file deduplication
- Changed from reactive to proactive deduplication approach - Added getRecentFileContent method to check cache before reading files - Modified readFileTool to use cached content when available - Added comprehensive tests for the new caching functionality - Fixed legacy format handling in getRecentFileContent - Updated test mocks to include new methods
This commit is contained in:
hannesrudolph
parent
76bca9d97b
commit
44f4bd5194
4 changed files with 450 additions and 1 deletions
|
|
@ -329,6 +329,79 @@ export class Task extends EventEmitter<ClineEvents> {
|
|||
return readApiMessages({ taskId: this.taskId, globalStoragePath: this.globalStoragePath })
|
||||
}
|
||||
|
||||
public async getRecentFileContent(filePath: string): Promise<string | null> {
|
||||
// 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 null
|
||||
}
|
||||
|
||||
// Get the cache window from settings
|
||||
const cacheMinutes = state?.readFileDeduplicationCacheMinutes ?? 5
|
||||
if (cacheMinutes === 0) {
|
||||
// Cache is disabled
|
||||
return null
|
||||
}
|
||||
|
||||
const cacheWindowMs = cacheMinutes * 60 * 1000
|
||||
const now = Date.now()
|
||||
|
||||
// Check recent conversation history for this file
|
||||
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 outside the cache window
|
||||
if (message.ts && now - message.ts > cacheWindowMs) {
|
||||
break
|
||||
}
|
||||
|
||||
// Process content blocks
|
||||
if (Array.isArray(message.content)) {
|
||||
for (const block of message.content) {
|
||||
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>[\s\S]*?<content[^>]*?>([\s\S]*?)<\/content>/g,
|
||||
)
|
||||
for (const match of xmlFileMatches) {
|
||||
const matchedPath = match[1].trim()
|
||||
const content = match[2].trim()
|
||||
if (matchedPath === filePath) {
|
||||
return content
|
||||
}
|
||||
}
|
||||
|
||||
// Handle legacy format (single file)
|
||||
if (
|
||||
readFileMatch[1] &&
|
||||
readFileMatch[1] === filePath &&
|
||||
!resultContent.includes("<files>")
|
||||
) {
|
||||
// For legacy format, the content is directly after "Result:"
|
||||
// Remove any leading/trailing whitespace
|
||||
return resultContent.trim()
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
public async deduplicateReadFileHistory(): Promise<void> {
|
||||
// Check if the experimental feature is enabled
|
||||
const state = await this.providerRef.deref()?.getState()
|
||||
|
|
|
|||
|
|
@ -2105,5 +2105,337 @@ describe("Cline", () => {
|
|||
expect(cline.apiConversationHistory).toHaveLength(2)
|
||||
})
|
||||
})
|
||||
|
||||
describe("getRecentFileContent", () => {
|
||||
let mockProvider: any
|
||||
let mockApiConfig: any
|
||||
let task: 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,
|
||||
},
|
||||
readFileDeduplicationCacheMinutes: 5,
|
||||
}),
|
||||
postStateToWebview: vi.fn().mockResolvedValue(undefined),
|
||||
postMessageToWebview: vi.fn().mockResolvedValue(undefined),
|
||||
updateTaskHistory: vi.fn().mockResolvedValue(undefined),
|
||||
}
|
||||
|
||||
task = new Task({
|
||||
provider: mockProvider,
|
||||
apiConfiguration: mockApiConfig,
|
||||
task: "test task",
|
||||
startTask: false,
|
||||
})
|
||||
})
|
||||
|
||||
it("should return null when feature is disabled", async () => {
|
||||
mockProvider.getState.mockResolvedValue({
|
||||
experiments: {
|
||||
[EXPERIMENT_IDS.READ_FILE_DEDUPLICATION]: false,
|
||||
},
|
||||
})
|
||||
|
||||
const now = Date.now()
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
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: now - 1000, // 1 second ago
|
||||
},
|
||||
]
|
||||
|
||||
const result = await task.getRecentFileContent("test.ts")
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it("should return recent file content within cache window", async () => {
|
||||
const now = Date.now()
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
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 window)
|
||||
},
|
||||
]
|
||||
|
||||
const result = await task.getRecentFileContent("test.ts")
|
||||
expect(result).toBe("recent content")
|
||||
})
|
||||
|
||||
it("should return null for files outside cache window", async () => {
|
||||
const now = Date.now()
|
||||
task.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 (outside 5 minute window)
|
||||
},
|
||||
]
|
||||
|
||||
const result = await task.getRecentFileContent("test.ts")
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it("should return most recent content when multiple reads exist", async () => {
|
||||
const now = Date.now()
|
||||
task.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 - 4 * 60 * 1000, // 4 minutes ago
|
||||
},
|
||||
{
|
||||
role: "assistant",
|
||||
content: [{ type: "text" as const, text: "Processing..." }],
|
||||
ts: now - 3 * 60 * 1000,
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text" as const,
|
||||
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>newer content</content></file></files>",
|
||||
},
|
||||
],
|
||||
ts: now - 2 * 60 * 1000, // 2 minutes ago
|
||||
},
|
||||
]
|
||||
|
||||
const result = await task.getRecentFileContent("test.ts")
|
||||
expect(result).toBe("newer content")
|
||||
})
|
||||
|
||||
it("should handle multi-file reads", async () => {
|
||||
const now = Date.now()
|
||||
task.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 - 2 * 60 * 1000,
|
||||
},
|
||||
]
|
||||
|
||||
const result1 = await task.getRecentFileContent("file1.ts")
|
||||
expect(result1).toBe("content1")
|
||||
|
||||
const result2 = await task.getRecentFileContent("file2.ts")
|
||||
expect(result2).toBe("content2")
|
||||
})
|
||||
|
||||
it("should return null for non-existent files", async () => {
|
||||
const now = Date.now()
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
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: now - 1000,
|
||||
},
|
||||
]
|
||||
|
||||
const result = await task.getRecentFileContent("other.ts")
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it("should handle legacy format", async () => {
|
||||
const now = Date.now()
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text" as const,
|
||||
text: "[read_file for 'legacy.ts'] Result:\nFile content without XML wrapper",
|
||||
},
|
||||
],
|
||||
ts: now - 1000,
|
||||
},
|
||||
]
|
||||
|
||||
const result = await task.getRecentFileContent("legacy.ts")
|
||||
expect(result).toBe("File content without XML wrapper")
|
||||
})
|
||||
|
||||
it("should ignore assistant messages", async () => {
|
||||
const now = Date.now()
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{
|
||||
type: "text" as const,
|
||||
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>assistant content</content></file></files>",
|
||||
},
|
||||
],
|
||||
ts: now - 1000,
|
||||
},
|
||||
]
|
||||
|
||||
const result = await task.getRecentFileContent("test.ts")
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it("should handle messages without timestamps", async () => {
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
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>",
|
||||
},
|
||||
],
|
||||
// No ts property - should be treated as recent
|
||||
},
|
||||
]
|
||||
|
||||
const result = await task.getRecentFileContent("test.ts")
|
||||
expect(result).toBe("test content")
|
||||
})
|
||||
|
||||
it("should use custom cache time from settings", async () => {
|
||||
mockProvider.getState.mockResolvedValue({
|
||||
experiments: {
|
||||
[EXPERIMENT_IDS.READ_FILE_DEDUPLICATION]: true,
|
||||
},
|
||||
readFileDeduplicationCacheMinutes: 10,
|
||||
})
|
||||
|
||||
const now = Date.now()
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text" as const,
|
||||
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>content within 10 min</content></file></files>",
|
||||
},
|
||||
],
|
||||
ts: now - 8 * 60 * 1000, // 8 minutes ago (within 10 minute window)
|
||||
},
|
||||
]
|
||||
|
||||
const result = await task.getRecentFileContent("test.ts")
|
||||
expect(result).toBe("content within 10 min")
|
||||
})
|
||||
|
||||
it("should handle 0 cache time (no caching)", async () => {
|
||||
mockProvider.getState.mockResolvedValue({
|
||||
experiments: {
|
||||
[EXPERIMENT_IDS.READ_FILE_DEDUPLICATION]: true,
|
||||
},
|
||||
readFileDeduplicationCacheMinutes: 0,
|
||||
})
|
||||
|
||||
const now = Date.now()
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text" as const,
|
||||
text: "[read_file for 'test.ts'] Result:\n<files><file><path>test.ts</path><content>very recent content</content></file></files>",
|
||||
},
|
||||
],
|
||||
ts: now - 100, // 0.1 seconds ago
|
||||
},
|
||||
]
|
||||
|
||||
const result = await task.getRecentFileContent("test.ts")
|
||||
expect(result).toBeNull() // With 0 cache time, nothing is cached
|
||||
})
|
||||
|
||||
it("should handle malformed content gracefully", async () => {
|
||||
const now = Date.now()
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
role: "user",
|
||||
content: "string content instead of array", // Invalid format
|
||||
ts: now - 1000,
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "image" as const,
|
||||
source: { type: "base64" as const, media_type: "image/png", data: "..." },
|
||||
},
|
||||
], // Non-text block
|
||||
ts: now - 500,
|
||||
},
|
||||
]
|
||||
|
||||
const result = await task.getRecentFileContent("test.ts")
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it("should handle empty conversation history", async () => {
|
||||
task.apiConversationHistory = []
|
||||
const result = await task.getRecentFileContent("test.ts")
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it("should handle file paths with special characters", async () => {
|
||||
const now = Date.now()
|
||||
task.apiConversationHistory = [
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "text" as const,
|
||||
text: "[read_file for '@scope/package/file.ts'] Result:\n<files><file><path>@scope/package/file.ts</path><content>scoped content</content></file></files>",
|
||||
},
|
||||
],
|
||||
ts: now - 1000,
|
||||
},
|
||||
]
|
||||
|
||||
const result = await task.getRecentFileContent("@scope/package/file.ts")
|
||||
expect(result).toBe("scoped content")
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -130,6 +130,9 @@ describe("read_file tool with maxReadFileLine setting", () => {
|
|||
// Add the deduplicateReadFileHistory method to the mock
|
||||
mockCline.deduplicateReadFileHistory = vi.fn().mockReturnValue(undefined)
|
||||
|
||||
// Add the getRecentFileContent method to the mock
|
||||
mockCline.getRecentFileContent = vi.fn().mockResolvedValue(null)
|
||||
|
||||
toolResult = undefined
|
||||
})
|
||||
|
||||
|
|
@ -389,6 +392,9 @@ describe("read_file tool XML output structure", () => {
|
|||
// Add the deduplicateReadFileHistory method to the mock
|
||||
mockCline.deduplicateReadFileHistory = vi.fn().mockReturnValue(undefined)
|
||||
|
||||
// Add the getRecentFileContent method to the mock
|
||||
mockCline.getRecentFileContent = vi.fn().mockResolvedValue(null)
|
||||
|
||||
toolResult = undefined
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -431,7 +431,45 @@ export async function readFileTool(
|
|||
const fullPath = path.resolve(cline.cwd, relPath)
|
||||
const { maxReadFileLine = -1 } = (await cline.providerRef.deref()?.getState()) ?? {}
|
||||
|
||||
// Process approved files
|
||||
// Check if we have recent content for this file (deduplication)
|
||||
const recentContent = await cline.getRecentFileContent(relPath)
|
||||
if (recentContent !== null) {
|
||||
// We have recent content, use it instead of reading the file again
|
||||
const lines = recentContent.split("\n")
|
||||
const totalLines = lines.length
|
||||
|
||||
// Handle range reads
|
||||
if (fileResult.lineRanges && fileResult.lineRanges.length > 0) {
|
||||
const rangeResults: string[] = []
|
||||
for (const range of fileResult.lineRanges) {
|
||||
const selectedLines = lines.slice(range.start - 1, range.end).join("\n")
|
||||
const content = addLineNumbers(selectedLines, range.start)
|
||||
const lineRangeAttr = ` lines="${range.start}-${range.end}"`
|
||||
rangeResults.push(`<content${lineRangeAttr}>\n${content}</content>`)
|
||||
}
|
||||
updateFileResult(relPath, {
|
||||
xmlContent: `<file><path>${relPath}</path>\n${rangeResults.join("\n")}\n<notice>Using cached content from recent read</notice>\n</file>`,
|
||||
})
|
||||
continue
|
||||
}
|
||||
|
||||
// Handle normal file read with cached content
|
||||
const lineRangeAttr = ` lines="1-${totalLines}"`
|
||||
let xmlInfo = totalLines > 0 ? `<content${lineRangeAttr}>\n${recentContent}</content>\n` : `<content/>`
|
||||
|
||||
if (totalLines === 0) {
|
||||
xmlInfo += `<notice>File is empty</notice>\n`
|
||||
} else {
|
||||
xmlInfo += `<notice>Using cached content from recent read</notice>\n`
|
||||
}
|
||||
|
||||
updateFileResult(relPath, {
|
||||
xmlContent: `<file><path>${relPath}</path>\n${xmlInfo}</file>`,
|
||||
})
|
||||
continue
|
||||
}
|
||||
|
||||
// Process approved files (no cached content available)
|
||||
try {
|
||||
const [totalLines, isBinary] = await Promise.all([countFileLines(fullPath), isBinaryFile(fullPath)])
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue