feat: Add file read caching to prevent redundant reads in conversation history

src/core/config/envConfig.ts

@@ -0,0 +1,8 @@
+function getFromEnv(key: string, defaultValue: string): string {
+	const value = process.env[key]
+	return value === undefined ? defaultValue : value
+}
+
+export const ROO_AGENT_CONFIG = {
+	fileReadCacheSize: () => parseInt(getFromEnv("ROO_FILE_READ_CACHE_SIZE", "100")),
+}

src/core/prompts/tools/read-file.ts

@@ -5,22 +5,22 @@ export function getReadFileDescription(args: ToolArgs): string {
 	const isMultipleReadsEnabled = maxConcurrentReads > 1
 
 	return `## read_file
-Description: Request to read the contents of ${isMultipleReadsEnabled ? "one or more files" : "a file"}. The tool outputs line-numbered content (e.g. "1 | const x = 1") for easy reference when creating diffs or discussing code.${args.partialReadsEnabled ? " Use line ranges to efficiently read specific portions of large files." : ""} Supports text extraction from PDF and DOCX files, but may not handle other binary files properly.
+Description: Request to read the contents of ${isMultipleReadsEnabled ? "one or more files" : "a file"}. The tool outputs line-numbered content (e.g. "1 | const x = 1") for easy reference when creating diffs or discussing code. Use line ranges to efficiently read specific portions of large files. Supports text extraction from PDF and DOCX files, but may not handle other binary files properly.
 
 ${isMultipleReadsEnabled ? `**IMPORTANT: You can read a maximum of ${maxConcurrentReads} files in a single request.** If you need to read more files, use multiple sequential read_file requests.` : "**IMPORTANT: Multiple file reads are currently disabled. You can only read one file at a time.**"}
 
-${args.partialReadsEnabled ? `By specifying line ranges, you can efficiently read specific portions of large files without loading the entire file into memory.` : ""}
+By specifying line ranges, you can efficiently read specific portions of large files without loading the entire file into memory.
 Parameters:
 - args: Contains one or more file elements, where each file contains:
   - path: (required) File path (relative to workspace directory ${args.cwd})
-  ${args.partialReadsEnabled ? `- line_range: (optional) One or more line range elements in format "start-end" (1-based, inclusive)` : ""}
+  - line_range: (optional) One or more line range elements in format "start-end" (1-based, inclusive)
 
 Usage:
 <read_file>
 <args>
   <file>
     <path>path/to/file</path>
-    ${args.partialReadsEnabled ? `<line_range>start-end</line_range>` : ""}
+    <line_range>start-end</line_range>
   </file>
 </args>
 </read_file>
@@ -32,7 +32,7 @@ Examples:
 <args>
   <file>
     <path>src/app.ts</path>
-    ${args.partialReadsEnabled ? `<line_range>1-1000</line_range>` : ""}
+    <line_range>1-1000</line_range>
   </file>
 </args>
 </read_file>
@@ -44,16 +44,12 @@ ${isMultipleReadsEnabled ? `2. Reading multiple files (within the ${maxConcurren
 <args>
   <file>
     <path>src/app.ts</path>
-    ${
-		args.partialReadsEnabled
-			? `<line_range>1-50</line_range>
-    <line_range>100-150</line_range>`
-			: ""
-	}
+    <line_range>1-50</line_range>
+    <line_range>100-150</line_range>
   </file>
   <file>
     <path>src/utils.ts</path>
-    ${args.partialReadsEnabled ? `<line_range>10-20</line_range>` : ""}
+    <line_range>10-20</line_range>
   </file>
 </args>
 </read_file>`
@@ -72,14 +68,10 @@ ${isMultipleReadsEnabled ? "3. " : "2. "}Reading an entire file:
 IMPORTANT: You MUST use this Efficient Reading Strategy:
 - ${isMultipleReadsEnabled ? `You MUST read all related files and implementations together in a single operation (up to ${maxConcurrentReads} files at once)` : "You MUST read files one at a time, as multiple file reads are currently disabled"}
 - You MUST obtain all necessary context before proceeding with changes
-${
-	args.partialReadsEnabled
-		? `- You MUST use line ranges to read specific portions of large files, rather than reading entire files when not needed
+${`- You MUST use line ranges to read specific portions of large files, rather than reading entire files when not needed
 - You MUST combine adjacent line ranges (<10 lines apart)
 - You MUST use multiple ranges for content separated by >10 lines
 - You MUST include sufficient line context for planned modifications while keeping ranges minimal
-`
-		: ""
-}
+`}
 ${isMultipleReadsEnabled ? `- When you need to read more than ${maxConcurrentReads} files, prioritize the most critical files first, then use subsequent read_file requests for additional files` : ""}`
 }

src/core/services/__tests__/fileReadCacheService.spec.ts

@@ -0,0 +1,161 @@
+import { vi, describe, it, expect, beforeEach } from "vitest"
+import {
+	processAndFilterReadRequest,
+	subtractRange,
+	subtractRanges,
+	ConversationMessage,
+	mtimeCache,
+} from "../fileReadCacheService"
+import { stat } from "fs/promises"
+import { lruCache } from "../../utils/lruCache"
+vi.mock("fs/promises", () => ({
+	stat: vi.fn(),
+}))
+vi.mock("../../utils/lruCache")
+vi.mock("../../config/envConfig", () => ({
+	ROO_AGENT_CONFIG: {
+		fileReadCacheSize: () => 10,
+	},
+}))
+const mockedStat = vi.mocked(stat)
+describe("fileReadCacheService", () => {
+	describe("subtractRange", () => {
+		it("should return the original range if there is no overlap", () => {
+			const original = { start: 1, end: 10 }
+			const toRemove = { start: 11, end: 20 }
+			expect(subtractRange(original, toRemove)).toEqual([original])
+		})
+		it("should return an empty array if the range is completely removed", () => {
+			const original = { start: 1, end: 10 }
+			const toRemove = { start: 1, end: 10 }
+			expect(subtractRange(original, toRemove)).toEqual([])
+		})
+		it("should subtract from the beginning", () => {
+			const original = { start: 1, end: 10 }
+			const toRemove = { start: 1, end: 5 }
+			expect(subtractRange(original, toRemove)).toEqual([{ start: 6, end: 10 }])
+		})
+		it("should subtract from the end", () => {
+			const original = { start: 1, end: 10 }
+			const toRemove = { start: 6, end: 10 }
+			expect(subtractRange(original, toRemove)).toEqual([{ start: 1, end: 5 }])
+		})
+		it("should subtract from the middle, creating two new ranges", () => {
+			const original = { start: 1, end: 10 }
+			const toRemove = { start: 4, end: 6 }
+			expect(subtractRange(original, toRemove)).toEqual([
+				{ start: 1, end: 3 },
+				{ start: 7, end: 10 },
+			])
+		})
+	})
+	describe("subtractRanges", () => {
+		it("should subtract multiple ranges from a single original range", () => {
+			const originals = [{ start: 1, end: 20 }]
+			const toRemoves = [
+				{ start: 1, end: 5 },
+				{ start: 15, end: 20 },
+			]
+			expect(subtractRanges(originals, toRemoves)).toEqual([{ start: 6, end: 14 }])
+		})
+	})
+	describe("processAndFilterReadRequest", () => {
+		const MOCK_FILE_PATH = "/test/file.txt"
+		const CURRENT_MTIME = new Date("2025-01-01T12:00:00.000Z")
+
+		beforeEach(() => {
+			vi.clearAllMocks()
+			mtimeCache.clear()
+			vi.useFakeTimers().setSystemTime(CURRENT_MTIME)
+			mockedStat.mockResolvedValue({
+				mtime: CURRENT_MTIME,
+				size: 1024, // Add size for the new cache implementation
+			} as any)
+		})
+
+		afterEach(() => {
+			vi.useRealTimers()
+		})
+
+		afterAll(() => {
+			vi.clearAllMocks()
+		})
+
+		it("should allow all when history is empty", async () => {
+			const requestedRanges = [{ start: 1, end: 10 }]
+			const result = await processAndFilterReadRequest(MOCK_FILE_PATH, requestedRanges, [])
+			expect(result.status).toBe("ALLOW_ALL")
+			expect(result.rangesToRead).toEqual(requestedRanges)
+		})
+
+		it("should reject all when a full cache hit occurs", async () => {
+			const requestedRanges = [{ start: 1, end: 10 }]
+			const conversationHistory: ConversationMessage[] = [
+				{
+					files: [
+						{
+							fileName: MOCK_FILE_PATH,
+							mtime: CURRENT_MTIME.getTime(),
+							lineRanges: [{ start: 1, end: 10 }],
+						},
+					],
+				} as any,
+			]
+			const result = await processAndFilterReadRequest(MOCK_FILE_PATH, requestedRanges, conversationHistory)
+			expect(result.status).toBe("REJECT_ALL")
+			expect(result.rangesToRead).toEqual([])
+		})
+
+		it("should allow partial when a partial cache hit occurs", async () => {
+			const requestedRanges = [{ start: 1, end: 20 }]
+			const conversationHistory: ConversationMessage[] = [
+				{
+					files: [
+						{
+							fileName: MOCK_FILE_PATH,
+							mtime: CURRENT_MTIME.getTime(),
+							lineRanges: [{ start: 1, end: 10 }],
+						},
+					],
+				} as any,
+			]
+			const result = await processAndFilterReadRequest(MOCK_FILE_PATH, requestedRanges, conversationHistory)
+			expect(result.status).toBe("ALLOW_PARTIAL")
+			expect(result.rangesToRead).toEqual([{ start: 11, end: 20 }])
+		})
+
+		it("should allow all when mtime is older in history", async () => {
+			const requestedRanges = [{ start: 1, end: 10 }]
+			const conversationHistory: ConversationMessage[] = [
+				{
+					files: [
+						{
+							fileName: MOCK_FILE_PATH,
+							mtime: CURRENT_MTIME.getTime() - 100, // Older mtime
+							lineRanges: [{ start: 1, end: 10 }],
+						},
+					],
+				} as any,
+			]
+			const result = await processAndFilterReadRequest(MOCK_FILE_PATH, requestedRanges, conversationHistory)
+			expect(result.status).toBe("ALLOW_ALL")
+			expect(result.rangesToRead).toEqual(requestedRanges)
+		})
+
+		it("should allow all when file does not exist", async () => {
+			mockedStat.mockRejectedValue({ code: "ENOENT" })
+			const requestedRanges = [{ start: 1, end: 10 }]
+			const result = await processAndFilterReadRequest(MOCK_FILE_PATH, requestedRanges, [])
+			expect(result.status).toBe("ALLOW_ALL")
+			expect(result.rangesToRead).toEqual(requestedRanges)
+		})
+
+		it("should throw an error for non-ENOENT stat errors", async () => {
+			const error = new Error("EPERM")
+			mockedStat.mockRejectedValue(error)
+			const requestedRanges = [{ start: 1, end: 10 }]
+			const result = await processAndFilterReadRequest(MOCK_FILE_PATH, requestedRanges, [])
+			expect(result.status).toBe("ALLOW_ALL") // Fallback to allow all
+		})
+	})
+})