From 3323b97aad71f765de8063811f4748d250b396c2 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sat, 17 Jan 2026 20:45:51 +0000 Subject: [PATCH 1/2] feat: add file context tracking to skip redundant re-reads - Add content_hash field to FileMetadataEntry schema for tracking file content - Add isFileUnchangedInContext() method to FileContextTracker to detect unchanged files - Add computeContentHash() and computeFileHash() methods for MD5 hashing - Modify trackFileContext() to store content hash on read/edit operations - Update ReadFileTool to skip re-reading files that are unchanged and in context - Add comprehensive tests for the new functionality - Update existing ReadFileTool tests to mock new isFileUnchangedInContext method This reduces token usage by skipping redundant file re-reads when: 1. The file has an active read entry in context tracking 2. The file has not been edited since the last read 3. The file content hash matches the stored hash Addresses issue #10653 --- .../context-tracking/FileContextTracker.ts | 91 +++- .../FileContextTrackerTypes.ts | 2 + .../__tests__/FileContextTracker.spec.ts | 472 ++++++++++++++++++ src/core/tools/ReadFileTool.ts | 48 +- src/core/tools/__tests__/readFileTool.spec.ts | 7 + 5 files changed, 614 insertions(+), 6 deletions(-) create mode 100644 src/core/context-tracking/__tests__/FileContextTracker.spec.ts diff --git a/src/core/context-tracking/FileContextTracker.ts b/src/core/context-tracking/FileContextTracker.ts index 5741b62cfc2..854870743d6 100644 --- a/src/core/context-tracking/FileContextTracker.ts +++ b/src/core/context-tracking/FileContextTracker.ts @@ -1,6 +1,7 @@ import { safeWriteJson } from "../../utils/safeWriteJson" import * as path from "path" import * as vscode from "vscode" +import * as crypto from "crypto" import { getTaskDirectoryPath } from "../../utils/storage" import { GlobalFileNames } from "../../shared/globalFileNames" import { fileExistsAtPath } from "../../utils/fs" @@ -78,14 +79,15 @@ export class FileContextTracker { // Tracks a file operation in metadata and sets up a watcher for the file // This is the main entry point for FileContextTracker and is called when a file is passed to Roo via a tool, mention, or edit. - async trackFileContext(filePath: string, operation: RecordSource) { + // When contentHash is provided, it's stored to enable skip-redundant-reads optimization + async trackFileContext(filePath: string, operation: RecordSource, contentHash?: string) { try { const cwd = this.getCwd() if (!cwd) { return } - await this.addFileToFileContextTracker(this.taskId, filePath, operation) + await this.addFileToFileContextTracker(this.taskId, filePath, operation, contentHash) // Set up file watcher for this file await this.setupFileWatcher(filePath) @@ -94,6 +96,84 @@ export class FileContextTracker { } } + /** + * Computes a hash of file content for detecting unchanged files. + * Uses MD5 for speed - we only need to detect changes, not cryptographic security. + */ + static computeContentHash(content: string): string { + return crypto.createHash("md5").update(content).digest("hex") + } + + /** + * Computes the hash of a file on disk. + * Returns null if the file cannot be read. + */ + async computeFileHash(filePath: string): Promise { + try { + const cwd = this.getCwd() + if (!cwd) { + return null + } + const fullPath = path.resolve(cwd, filePath) + const content = await fs.readFile(fullPath, "utf-8") + return FileContextTracker.computeContentHash(content) + } catch (error) { + console.error("Failed to compute file hash:", error) + return null + } + } + + /** + * Checks if a file's content is still valid in the current context. + * Returns true if: + * - The file was previously read (has an active entry with roo_read_date) + * - No user edits occurred after the last read + * - The content hash matches the current file on disk + * + * This enables the "skip redundant re-reads" optimization to reduce token usage. + */ + async isFileUnchangedInContext(filePath: string): Promise { + try { + const metadata = await this.getTaskMetadata(this.taskId) + + // Find the most recent active entry for this file that was read by Roo + const activeEntries = metadata.files_in_context.filter( + (entry) => + entry.path === filePath && + entry.record_state === "active" && + entry.roo_read_date !== null && + entry.content_hash, + ) + + if (activeEntries.length === 0) { + return false // No active read entry with hash found + } + + // Get the most recent entry (highest roo_read_date) + const latestEntry = activeEntries.reduce((latest, current) => + (current.roo_read_date ?? 0) > (latest.roo_read_date ?? 0) ? current : latest, + ) + + // Check if user edited the file after Roo's last read + if (latestEntry.user_edit_date && latestEntry.roo_read_date) { + if (latestEntry.user_edit_date > latestEntry.roo_read_date) { + return false // User edited after last read + } + } + + // Compute current file hash and compare + const currentHash = await this.computeFileHash(filePath) + if (!currentHash) { + return false // Couldn't read file + } + + return currentHash === latestEntry.content_hash + } catch (error) { + console.error("Failed to check if file is unchanged in context:", error) + return false + } + } + public getContextProxy(): ContextProxy | undefined { const provider = this.providerRef.deref() if (!provider) { @@ -140,7 +220,8 @@ export class FileContextTracker { // Adds a file to the metadata tracker // This handles the business logic of determining if the file is new, stale, or active. // It also updates the metadata with the latest read/edit dates. - async addFileToFileContextTracker(taskId: string, filePath: string, source: RecordSource) { + // When contentHash is provided (for read_tool operations), it's stored to enable skip-redundant-reads. + async addFileToFileContextTracker(taskId: string, filePath: string, source: RecordSource, contentHash?: string) { try { const metadata = await this.getTaskMetadata(taskId) const now = Date.now() @@ -168,12 +249,14 @@ export class FileContextTracker { roo_read_date: getLatestDateForField(filePath, "roo_read_date"), roo_edit_date: getLatestDateForField(filePath, "roo_edit_date"), user_edit_date: getLatestDateForField(filePath, "user_edit_date"), + content_hash: null, // Will be set for read operations if provided } switch (source) { // user_edited: The user has edited the file case "user_edited": newEntry.user_edit_date = now + newEntry.content_hash = null // Invalidate hash on user edit this.recentlyModifiedFiles.add(filePath) break @@ -181,6 +264,7 @@ export class FileContextTracker { case "roo_edited": newEntry.roo_read_date = now newEntry.roo_edit_date = now + newEntry.content_hash = contentHash ?? null // Store new hash if provided this.checkpointPossibleFiles.add(filePath) this.markFileAsEditedByRoo(filePath) break @@ -189,6 +273,7 @@ export class FileContextTracker { case "read_tool": case "file_mentioned": newEntry.roo_read_date = now + newEntry.content_hash = contentHash ?? null // Store hash for skip-redundant-reads break } diff --git a/src/core/context-tracking/FileContextTrackerTypes.ts b/src/core/context-tracking/FileContextTrackerTypes.ts index 7a761a1d39c..1ee9e4b5cb2 100644 --- a/src/core/context-tracking/FileContextTrackerTypes.ts +++ b/src/core/context-tracking/FileContextTrackerTypes.ts @@ -14,6 +14,8 @@ export const fileMetadataEntrySchema = z.object({ roo_read_date: z.number().nullable(), roo_edit_date: z.number().nullable(), user_edit_date: z.number().nullable().optional(), + // Hash of file content when last read (for detecting unchanged files) + content_hash: z.string().nullable().optional(), }) // TypeScript type derived from the Zod schema diff --git a/src/core/context-tracking/__tests__/FileContextTracker.spec.ts b/src/core/context-tracking/__tests__/FileContextTracker.spec.ts new file mode 100644 index 00000000000..979e066ec70 --- /dev/null +++ b/src/core/context-tracking/__tests__/FileContextTracker.spec.ts @@ -0,0 +1,472 @@ +// npx vitest run core/context-tracking/__tests__/FileContextTracker.spec.ts + +import * as crypto from "crypto" +import { FileContextTracker } from "../FileContextTracker" +import type { TaskMetadata } from "../FileContextTrackerTypes" + +// Mock vscode module +vi.mock("vscode", () => ({ + workspace: { + workspaceFolders: [{ uri: { fsPath: "/test/workspace" } }], + createFileSystemWatcher: vi.fn().mockReturnValue({ + onDidChange: vi.fn(), + onDidCreate: vi.fn(), + onDidDelete: vi.fn(), + dispose: vi.fn(), + }), + }, + RelativePattern: vi.fn(), + Uri: { + file: vi.fn((path: string) => ({ fsPath: path })), + }, +})) + +// Mock fs/promises +const mockFsReadFile = vi.fn() +vi.mock("fs/promises", () => ({ + default: { + readFile: (...args: unknown[]) => mockFsReadFile(...args), + writeFile: vi.fn().mockResolvedValue(undefined), + mkdir: vi.fn().mockResolvedValue(undefined), + }, + readFile: (...args: unknown[]) => mockFsReadFile(...args), + writeFile: vi.fn().mockResolvedValue(undefined), + mkdir: vi.fn().mockResolvedValue(undefined), +})) + +// Mock fileExistsAtPath +vi.mock("../../../utils/fs", () => ({ + fileExistsAtPath: vi.fn().mockResolvedValue(false), +})) + +// Mock storage utils +vi.mock("../../../utils/storage", () => ({ + getTaskDirectoryPath: vi.fn().mockResolvedValue("/test/storage/task123"), +})) + +// Mock safeWriteJson +vi.mock("../../../utils/safeWriteJson", () => ({ + safeWriteJson: vi.fn().mockResolvedValue(undefined), +})) + +// Mock ClineProvider +vi.mock("../../webview/ClineProvider", () => ({ + ClineProvider: vi.fn(), +})) + +describe("FileContextTracker", () => { + describe("computeContentHash", () => { + it("should compute MD5 hash of content", () => { + const content = "Hello, World!" + const expectedHash = crypto.createHash("md5").update(content).digest("hex") + + const hash = FileContextTracker.computeContentHash(content) + + expect(hash).toBe(expectedHash) + }) + + it("should return different hashes for different content", () => { + const content1 = "Hello, World!" + const content2 = "Hello, World!!" + + const hash1 = FileContextTracker.computeContentHash(content1) + const hash2 = FileContextTracker.computeContentHash(content2) + + expect(hash1).not.toBe(hash2) + }) + + it("should return same hash for same content", () => { + const content = "Test content for hashing" + + const hash1 = FileContextTracker.computeContentHash(content) + const hash2 = FileContextTracker.computeContentHash(content) + + expect(hash1).toBe(hash2) + }) + + it("should handle empty string", () => { + const content = "" + const expectedHash = crypto.createHash("md5").update(content).digest("hex") + + const hash = FileContextTracker.computeContentHash(content) + + expect(hash).toBe(expectedHash) + }) + + it("should handle multi-line content", () => { + const content = "Line 1\nLine 2\nLine 3" + const expectedHash = crypto.createHash("md5").update(content).digest("hex") + + const hash = FileContextTracker.computeContentHash(content) + + expect(hash).toBe(expectedHash) + }) + }) + + describe("isFileUnchangedInContext", () => { + let tracker: FileContextTracker + let mockProvider: any + + beforeEach(() => { + mockProvider = { + contextProxy: { + globalStorageUri: { fsPath: "/test/storage" }, + }, + } + tracker = new FileContextTracker(mockProvider, "task123") + }) + + it("should return false when no active read entry exists", async () => { + // Mock getTaskMetadata to return empty metadata + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [], + }) + + const result = await tracker.isFileUnchangedInContext("test.ts") + + expect(result).toBe(false) + }) + + it("should return false when entry has no content_hash", async () => { + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [ + { + path: "test.ts", + record_state: "active", + record_source: "read_tool", + roo_read_date: Date.now(), + roo_edit_date: null, + user_edit_date: null, + // No content_hash + }, + ], + }) + + const result = await tracker.isFileUnchangedInContext("test.ts") + + expect(result).toBe(false) + }) + + it("should return false when user edited file after read", async () => { + const readTime = Date.now() - 1000 + const editTime = Date.now() + const contentHash = FileContextTracker.computeContentHash("original content") + + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [ + { + path: "test.ts", + record_state: "active", + record_source: "read_tool", + roo_read_date: readTime, + roo_edit_date: null, + user_edit_date: editTime, // User edited after read + content_hash: contentHash, + }, + ], + }) + + const result = await tracker.isFileUnchangedInContext("test.ts") + + expect(result).toBe(false) + }) + + it("should return false when file hash does not match", async () => { + const contentHash = FileContextTracker.computeContentHash("original content") + const newContentHash = FileContextTracker.computeContentHash("modified content") + + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [ + { + path: "test.ts", + record_state: "active", + record_source: "read_tool", + roo_read_date: Date.now(), + roo_edit_date: null, + user_edit_date: null, + content_hash: contentHash, + }, + ], + }) + + // Mock computeFileHash to return different hash + vi.spyOn(tracker, "computeFileHash").mockResolvedValue(newContentHash) + + const result = await tracker.isFileUnchangedInContext("test.ts") + + expect(result).toBe(false) + }) + + it("should return true when file is unchanged in context", async () => { + const contentHash = FileContextTracker.computeContentHash("same content") + + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [ + { + path: "test.ts", + record_state: "active", + record_source: "read_tool", + roo_read_date: Date.now(), + roo_edit_date: null, + user_edit_date: null, + content_hash: contentHash, + }, + ], + }) + + // Mock computeFileHash to return same hash + vi.spyOn(tracker, "computeFileHash").mockResolvedValue(contentHash) + + const result = await tracker.isFileUnchangedInContext("test.ts") + + expect(result).toBe(true) + }) + + it("should return true when user_edit_date is before roo_read_date", async () => { + const editTime = Date.now() - 2000 + const readTime = Date.now() - 1000 // Read after edit + const contentHash = FileContextTracker.computeContentHash("content") + + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [ + { + path: "test.ts", + record_state: "active", + record_source: "read_tool", + roo_read_date: readTime, + roo_edit_date: null, + user_edit_date: editTime, + content_hash: contentHash, + }, + ], + }) + + vi.spyOn(tracker, "computeFileHash").mockResolvedValue(contentHash) + + const result = await tracker.isFileUnchangedInContext("test.ts") + + expect(result).toBe(true) + }) + + it("should use most recent active entry when multiple exist", async () => { + const olderHash = FileContextTracker.computeContentHash("old content") + const newerHash = FileContextTracker.computeContentHash("new content") + const currentHash = newerHash + + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [ + { + path: "test.ts", + record_state: "active", + record_source: "read_tool", + roo_read_date: Date.now() - 2000, + roo_edit_date: null, + user_edit_date: null, + content_hash: olderHash, + }, + { + path: "test.ts", + record_state: "active", + record_source: "read_tool", + roo_read_date: Date.now() - 1000, // More recent + roo_edit_date: null, + user_edit_date: null, + content_hash: newerHash, + }, + ], + }) + + vi.spyOn(tracker, "computeFileHash").mockResolvedValue(currentHash) + + const result = await tracker.isFileUnchangedInContext("test.ts") + + expect(result).toBe(true) + }) + + it("should return false when file cannot be read", async () => { + const contentHash = FileContextTracker.computeContentHash("content") + + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [ + { + path: "test.ts", + record_state: "active", + record_source: "read_tool", + roo_read_date: Date.now(), + roo_edit_date: null, + user_edit_date: null, + content_hash: contentHash, + }, + ], + }) + + // Mock computeFileHash to return null (file read failed) + vi.spyOn(tracker, "computeFileHash").mockResolvedValue(null) + + const result = await tracker.isFileUnchangedInContext("test.ts") + + expect(result).toBe(false) + }) + + it("should only consider active entries, not stale ones", async () => { + const contentHash = FileContextTracker.computeContentHash("content") + + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [ + { + path: "test.ts", + record_state: "stale", // Not active + record_source: "read_tool", + roo_read_date: Date.now(), + roo_edit_date: null, + user_edit_date: null, + content_hash: contentHash, + }, + ], + }) + + const result = await tracker.isFileUnchangedInContext("test.ts") + + expect(result).toBe(false) + }) + }) + + describe("trackFileContext with content hash", () => { + let tracker: FileContextTracker + let mockProvider: any + let savedMetadata: TaskMetadata | null + + beforeEach(() => { + savedMetadata = null + mockProvider = { + contextProxy: { + globalStorageUri: { fsPath: "/test/storage" }, + }, + } + tracker = new FileContextTracker(mockProvider, "task123") + + // Spy on saveTaskMetadata to capture what's being saved + vi.spyOn(tracker, "saveTaskMetadata").mockImplementation(async (_taskId, metadata) => { + savedMetadata = metadata + }) + }) + + it("should store content_hash when provided for read_tool", async () => { + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [], + }) + + const contentHash = FileContextTracker.computeContentHash("file content") + await tracker.trackFileContext("test.ts", "read_tool", contentHash) + + expect(savedMetadata).not.toBeNull() + expect(savedMetadata!.files_in_context).toHaveLength(1) + expect(savedMetadata!.files_in_context[0].content_hash).toBe(contentHash) + expect(savedMetadata!.files_in_context[0].record_source).toBe("read_tool") + }) + + it("should store content_hash when provided for file_mentioned", async () => { + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [], + }) + + const contentHash = FileContextTracker.computeContentHash("mentioned file content") + await tracker.trackFileContext("test.ts", "file_mentioned", contentHash) + + expect(savedMetadata).not.toBeNull() + expect(savedMetadata!.files_in_context[0].content_hash).toBe(contentHash) + }) + + it("should set content_hash to null for user_edited", async () => { + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [], + }) + + // Even if hash is provided, user_edited should invalidate it + await tracker.trackFileContext("test.ts", "user_edited", "some_hash") + + expect(savedMetadata).not.toBeNull() + expect(savedMetadata!.files_in_context[0].content_hash).toBeNull() + }) + + it("should store content_hash for roo_edited when provided", async () => { + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [], + }) + + const contentHash = FileContextTracker.computeContentHash("edited content") + await tracker.trackFileContext("test.ts", "roo_edited", contentHash) + + expect(savedMetadata).not.toBeNull() + expect(savedMetadata!.files_in_context[0].content_hash).toBe(contentHash) + }) + + it("should mark existing entries as stale when tracking new read", async () => { + const oldHash = FileContextTracker.computeContentHash("old content") + const newHash = FileContextTracker.computeContentHash("new content") + + vi.spyOn(tracker, "getTaskMetadata").mockResolvedValue({ + files_in_context: [ + { + path: "test.ts", + record_state: "active", + record_source: "read_tool", + roo_read_date: Date.now() - 1000, + roo_edit_date: null, + user_edit_date: null, + content_hash: oldHash, + }, + ], + }) + + await tracker.trackFileContext("test.ts", "read_tool", newHash) + + expect(savedMetadata).not.toBeNull() + // Should have 2 entries - one stale, one new active + expect(savedMetadata!.files_in_context).toHaveLength(2) + expect(savedMetadata!.files_in_context[0].record_state).toBe("stale") + expect(savedMetadata!.files_in_context[1].record_state).toBe("active") + expect(savedMetadata!.files_in_context[1].content_hash).toBe(newHash) + }) + }) + + describe("computeFileHash", () => { + let tracker: FileContextTracker + let mockProvider: any + + beforeEach(() => { + mockProvider = { + contextProxy: { + globalStorageUri: { fsPath: "/test/storage" }, + }, + } + tracker = new FileContextTracker(mockProvider, "task123") + }) + + it("should return hash of file content", async () => { + const fileContent = "file content here" + mockFsReadFile.mockResolvedValue(fileContent) + + const hash = await tracker.computeFileHash("test.ts") + + expect(hash).toBe(FileContextTracker.computeContentHash(fileContent)) + }) + + it("should return null when file read fails", async () => { + mockFsReadFile.mockRejectedValue(new Error("File not found")) + + const hash = await tracker.computeFileHash("nonexistent.ts") + + expect(hash).toBeNull() + }) + + it("should return null when no workspace folder", async () => { + // Override getCwd to return undefined + vi.spyOn(tracker as any, "getCwd").mockReturnValue(undefined) + + const hash = await tracker.computeFileHash("test.ts") + + expect(hash).toBeNull() + }) + }) +}) diff --git a/src/core/tools/ReadFileTool.ts b/src/core/tools/ReadFileTool.ts index 2bba6bc6cd9..625654817a3 100644 --- a/src/core/tools/ReadFileTool.ts +++ b/src/core/tools/ReadFileTool.ts @@ -10,6 +10,7 @@ import { formatResponse } from "../prompts/responses" import { getModelMaxOutputTokens } from "../../shared/api" import { t } from "../../i18n" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" +import { FileContextTracker } from "../context-tracking/FileContextTracker" import { isPathOutsideWorkspace } from "../../utils/pathUtils" import { getReadablePath } from "../../utils/path" import { countFileLines } from "../../integrations/misc/line-counter" @@ -34,7 +35,7 @@ import { BaseTool, ToolCallbacks } from "./BaseTool" interface FileResult { path: string - status: "approved" | "denied" | "blocked" | "error" | "pending" + status: "approved" | "denied" | "blocked" | "error" | "pending" | "unchanged" content?: string error?: string notice?: string @@ -352,6 +353,22 @@ export class ReadFileTool extends BaseTool<"read_file"> { const fullPath = path.resolve(task.cwd, relPath) try { + // Skip redundant re-reads optimization: check if file is unchanged in context + // Only applies to full file reads (no line ranges specified) + if (!fileResult.lineRanges || fileResult.lineRanges.length === 0) { + const isUnchanged = await task.fileContextTracker.isFileUnchangedInContext(relPath) + if (isUnchanged) { + const notice = "File unchanged since last read. Content is already in context." + updateFileResult(relPath, { + status: "unchanged", + notice, + xmlContent: `${relPath}\n${notice}\n`, + nativeContent: `File: ${relPath}\nNote: ${notice}`, + }) + continue + } + } + // Check if the path is a directory before attempting to read it const stats = await fs.stat(fullPath) if (stats.isDirectory()) { @@ -423,7 +440,13 @@ export class ReadFileTool extends BaseTool<"read_file"> { const lineCount = lines.length const lineRangeAttr = lineCount > 0 ? ` lines="1-${lineCount}"` : "" - await task.fileContextTracker.trackFileContext(relPath, "read_tool" as RecordSource) + // Compute content hash for skip-redundant-reads optimization + const contentHash = FileContextTracker.computeContentHash(content) + await task.fileContextTracker.trackFileContext( + relPath, + "read_tool" as RecordSource, + contentHash, + ) updateFileResult(relPath, { xmlContent: @@ -603,7 +626,26 @@ export class ReadFileTool extends BaseTool<"read_file"> { } } - await task.fileContextTracker.trackFileContext(relPath, "read_tool" as RecordSource) + // Compute content hash for skip-redundant-reads optimization + // Only store hash for complete reads (not truncated) + let contentHash: string | undefined + if (safeReadBudget > 0) { + const readResult = await readFileWithTokenBudget(fullPath, { budgetTokens: safeReadBudget }) + // Re-read to get content for hash (already done above, use same result) + // Actually we need to use the result.content from above, so we compute hash here + // Note: content variable above is already the numbered content, we need raw + // For simplicity, compute hash from the result we already have (pre-addLineNumbers) + } + // For full reads, compute hash from raw content + if (safeReadBudget > 0) { + try { + const rawContent = await fs.readFile(fullPath, "utf-8") + contentHash = FileContextTracker.computeContentHash(rawContent) + } catch { + // If we can't read for hash, that's ok - just don't store hash + } + } + await task.fileContextTracker.trackFileContext(relPath, "read_tool" as RecordSource, contentHash) updateFileResult(relPath, { xmlContent: `${relPath}\n${xmlInfo}`, diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts index f178e38026c..b068de9d411 100644 --- a/src/core/tools/__tests__/readFileTool.spec.ts +++ b/src/core/tools/__tests__/readFileTool.spec.ts @@ -225,6 +225,7 @@ function createMockCline(): any { removeClosingTag: vi.fn((tag, content) => content), fileContextTracker: { trackFileContext: vi.fn().mockResolvedValue(undefined), + isFileUnchangedInContext: vi.fn().mockResolvedValue(false), }, recordToolUsage: vi.fn().mockReturnValue(undefined), recordToolError: vi.fn().mockReturnValue(undefined), @@ -848,6 +849,7 @@ describe("read_file tool output structure", () => { mockCline.removeClosingTag = vi.fn((tag, content) => content) mockCline.fileContextTracker = { trackFileContext: vi.fn().mockResolvedValue(undefined), + isFileUnchangedInContext: vi.fn().mockResolvedValue(false), } mockCline.recordToolUsage = vi.fn().mockReturnValue(undefined) mockCline.recordToolError = vi.fn().mockReturnValue(undefined) @@ -921,6 +923,7 @@ describe("read_file tool output structure", () => { mockCline.removeClosingTag = vi.fn((tag, content) => content) mockCline.fileContextTracker = { trackFileContext: vi.fn().mockResolvedValue(undefined), + isFileUnchangedInContext: vi.fn().mockResolvedValue(false), } mockCline.recordToolUsage = vi.fn().mockReturnValue(undefined) mockCline.recordToolError = vi.fn().mockReturnValue(undefined) @@ -1007,6 +1010,7 @@ describe("read_file tool output structure", () => { mockCline.removeClosingTag = vi.fn((tag, content) => content) mockCline.fileContextTracker = { trackFileContext: vi.fn().mockResolvedValue(undefined), + isFileUnchangedInContext: vi.fn().mockResolvedValue(false), } mockCline.recordToolUsage = vi.fn().mockReturnValue(undefined) mockCline.recordToolError = vi.fn().mockReturnValue(undefined) @@ -1080,6 +1084,7 @@ describe("read_file tool output structure", () => { mockCline.removeClosingTag = vi.fn((tag, content) => content) mockCline.fileContextTracker = { trackFileContext: vi.fn().mockResolvedValue(undefined), + isFileUnchangedInContext: vi.fn().mockResolvedValue(false), } mockCline.recordToolUsage = vi.fn().mockReturnValue(undefined) mockCline.recordToolError = vi.fn().mockReturnValue(undefined) @@ -1205,6 +1210,7 @@ describe("read_file tool output structure", () => { mockCline.removeClosingTag = vi.fn((tag, content) => content) mockCline.fileContextTracker = { trackFileContext: vi.fn().mockResolvedValue(undefined), + isFileUnchangedInContext: vi.fn().mockResolvedValue(false), } mockCline.recordToolUsage = vi.fn().mockReturnValue(undefined) mockCline.recordToolError = vi.fn().mockReturnValue(undefined) @@ -1252,6 +1258,7 @@ describe("read_file tool output structure", () => { mockCline.removeClosingTag = vi.fn((tag, content) => content) mockCline.fileContextTracker = { trackFileContext: vi.fn().mockResolvedValue(undefined), + isFileUnchangedInContext: vi.fn().mockResolvedValue(false), } mockCline.recordToolUsage = vi.fn().mockReturnValue(undefined) mockCline.recordToolError = vi.fn().mockReturnValue(undefined) From ea696ce6afcfd4a007b6dca098550be544faf74b Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sat, 17 Jan 2026 21:10:07 +0000 Subject: [PATCH 2/2] fix: only store content hash for complete file reads - Remove dead code that called readFileWithTokenBudget but never used result - Remove redundant fs.readFile call (content was already read earlier) - Only compute and store contentHash when result.complete is true - Fixes incorrect hash storage for truncated files --- src/core/tools/ReadFileTool.ts | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/src/core/tools/ReadFileTool.ts b/src/core/tools/ReadFileTool.ts index 625654817a3..ba704aaa1cb 100644 --- a/src/core/tools/ReadFileTool.ts +++ b/src/core/tools/ReadFileTool.ts @@ -580,6 +580,7 @@ export class ReadFileTool extends BaseTool<"read_file"> { const safeReadBudget = Math.floor(remainingTokens * FILE_READ_BUDGET_PERCENT) let content: string + let contentHash: string | undefined let xmlInfo = "" let nativeInfo = "" @@ -597,6 +598,11 @@ export class ReadFileTool extends BaseTool<"read_file"> { content = addLineNumbers(result.content) + // Only compute hash for complete reads (not truncated) + if (result.complete) { + contentHash = FileContextTracker.computeContentHash(result.content) + } + if (!result.complete) { // File was truncated const notice = `File truncated: showing ${result.lineCount} lines (${result.tokenCount} tokens) due to context budget. Use line_range to read specific sections.` @@ -626,25 +632,6 @@ export class ReadFileTool extends BaseTool<"read_file"> { } } - // Compute content hash for skip-redundant-reads optimization - // Only store hash for complete reads (not truncated) - let contentHash: string | undefined - if (safeReadBudget > 0) { - const readResult = await readFileWithTokenBudget(fullPath, { budgetTokens: safeReadBudget }) - // Re-read to get content for hash (already done above, use same result) - // Actually we need to use the result.content from above, so we compute hash here - // Note: content variable above is already the numbered content, we need raw - // For simplicity, compute hash from the result we already have (pre-addLineNumbers) - } - // For full reads, compute hash from raw content - if (safeReadBudget > 0) { - try { - const rawContent = await fs.readFile(fullPath, "utf-8") - contentHash = FileContextTracker.computeContentHash(rawContent) - } catch { - // If we can't read for hash, that's ok - just don't store hash - } - } await task.fileContextTracker.trackFileContext(relPath, "read_tool" as RecordSource, contentHash) updateFileResult(relPath, {