feat: improve readFileTool safeguard with character-based limiting and i18n

- Replace line-based limiting with character-based limiting to handle files with very long lines
- Move threshold constants to packages/types/src/file-limits.ts for better organization
- Add readLinesWithCharLimit function that truncates at complete line boundaries
- Optimize file reading to avoid double reads when checking token count
- Add i18n support for safeguard notice messages in all 18 supported languages
- Update tests to match new character-based implementation
- Safeguard now limits by character count (200KB default) instead of line count
- Ensures files are never truncated in the middle of a line

Author: daniel-lxs

packages/types/src/file-limits.ts

@@ -0,0 +1,39 @@
+/**
+ * File size and limit constants used across the application
+ */
+
+/**
+ * Files larger than this threshold will be checked for token count
+ * to prevent consuming too much of the context window
+ */
+export const LARGE_FILE_SIZE_THRESHOLD = 100 * 1024 // 100KB
+
+/**
+ * Files larger than this size will have the safeguard applied automatically
+ * without token counting
+ */
+export const VERY_LARGE_FILE_SIZE = 1024 * 1024 // 1MB
+
+/**
+ * Default number of lines to read when applying the large file safeguard
+ */
+export const FALLBACK_MAX_LINES = 2000
+
+/**
+ * Maximum character count for file reading when safeguard is applied.
+ * Based on typical token-to-character ratio (1 token ≈ 4 characters),
+ * this ensures we don't consume too much of the context window.
+ * For a 100k token context window at 50%, this would be ~200k characters.
+ */
+export const MAX_CHAR_LIMIT = 200_000 // 200k characters
+
+/**
+ * Percentage of the context window to use as the maximum token threshold
+ * for file reading operations
+ */
+export const CONTEXT_WINDOW_PERCENTAGE = 0.5 // 50%
+
+/**
+ * Average characters per token ratio used for estimation
+ */
+export const CHARS_PER_TOKEN_RATIO = 4

packages/types/src/index.ts

@@ -4,6 +4,7 @@ export * from "./api.js"
 export * from "./codebase-index.js"
 export * from "./cloud.js"
 export * from "./experiment.js"
+export * from "./file-limits.js"
 export * from "./followup.js"
 export * from "./global-settings.js"
 export * from "./history.js"

src/core/tools/__tests__/readFileTool.spec.ts

@@ -5,6 +5,7 @@ import { stat } from "fs/promises"
 
 import { countFileLines } from "../../../integrations/misc/line-counter"
 import { readLines } from "../../../integrations/misc/read-lines"
+import { readLinesWithCharLimit } from "../../../integrations/misc/read-lines-char-limit"
 import { extractTextFromFile } from "../../../integrations/misc/extract-text"
 import { parseSourceCodeDefinitionsForFile } from "../../../services/tree-sitter"
 import { isBinaryFile } from "isbinaryfile"
@@ -33,6 +34,7 @@ vi.mock("isbinaryfile")
 
 vi.mock("../../../integrations/misc/line-counter")
 vi.mock("../../../integrations/misc/read-lines")
+vi.mock("../../../integrations/misc/read-lines-char-limit")
 
 // Mock input content for tests
 let mockInputContent = ""
@@ -655,7 +657,15 @@ describe("read_file tool with large file safeguard", () => {
 			const partialContent = Array(2000).fill("This is a line of text").join("\n")
 
 			mockedExtractTextFromFile.mockResolvedValue(largeFileContent)
-			mockedReadLines.mockResolvedValue(partialContent)
+
+			// Mock readLinesWithCharLimit
+			const mockedReadLinesWithCharLimit = vi.mocked(readLinesWithCharLimit)
+			mockedReadLinesWithCharLimit.mockResolvedValue({
+				content: partialContent,
+				linesRead: 2000,
+				charactersRead: partialContent.length,
+				wasTruncated: true,
+			})
 
 			// Setup addLineNumbers mock for this test
 			addLineNumbersMock.mockImplementation((text: string) => {
@@ -685,11 +695,10 @@ describe("read_file tool with large file safeguard", () => {
 
 			// Verify safeguard was applied
 			expect(mockedTiktoken).toHaveBeenCalled()
-			expect(mockedReadLines).toHaveBeenCalledWith(absoluteFilePath, 1999, 0)
+			expect(mockedReadLinesWithCharLimit).toHaveBeenCalled()
 
 			// Verify the result contains the safeguard notice
-			expect(result).toContain("<notice>This file is 200KB and contains approximately 60,000 tokens")
-			expect(result).toContain("Showing only the first 2000 lines to preserve context space")
+			expect(result).toContain("<notice>readFile.safeguardNotice</notice>")
 			expect(result).toContain(`<content lines="1-2000">`)
 		})
 
@@ -725,7 +734,8 @@ describe("read_file tool with large file safeguard", () => {
 
 			// Verify safeguard was NOT applied
 			expect(mockedTiktoken).toHaveBeenCalled()
-			expect(mockedReadLines).not.toHaveBeenCalled()
+			const mockedReadLinesWithCharLimit = vi.mocked(readLinesWithCharLimit)
+			expect(mockedReadLinesWithCharLimit).not.toHaveBeenCalled()
 			expect(mockedExtractTextFromFile).toHaveBeenCalled()
 
 			// Verify no safeguard notice
@@ -765,7 +775,8 @@ describe("read_file tool with large file safeguard", () => {
 
 			// Verify tiktoken was NOT called (optimization)
 			expect(mockedTiktoken).not.toHaveBeenCalled()
-			expect(mockedReadLines).not.toHaveBeenCalled()
+			const mockedReadLinesWithCharLimit = vi.mocked(readLinesWithCharLimit)
+			expect(mockedReadLinesWithCharLimit).not.toHaveBeenCalled()
 			expect(mockedExtractTextFromFile).toHaveBeenCalled()
 
 			// Verify no safeguard notice
@@ -778,7 +789,15 @@ describe("read_file tool with large file safeguard", () => {
 			const partialContent = Array(2000).fill("This is a line of text").join("\n")
 
 			mockedExtractTextFromFile.mockResolvedValue("Large content")
-			mockedReadLines.mockResolvedValue(partialContent)
+
+			// Mock readLinesWithCharLimit
+			const mockedReadLinesWithCharLimit = vi.mocked(readLinesWithCharLimit)
+			mockedReadLinesWithCharLimit.mockResolvedValue({
+				content: partialContent,
+				linesRead: 2000,
+				charactersRead: partialContent.length,
+				wasTruncated: true,
+			})
 
 			// Setup addLineNumbers mock for partial content
 			addLineNumbersMock.mockImplementation((text: string) => {
@@ -825,11 +844,10 @@ describe("read_file tool with large file safeguard", () => {
 
 			// Verify safeguard was applied despite token counting failure
 			expect(mockedTiktoken).toHaveBeenCalled()
-			expect(mockedReadLines).toHaveBeenCalledWith(absoluteFilePath, 1999, 0)
+			expect(mockedReadLinesWithCharLimit).toHaveBeenCalled()
 
-			// Verify the result contains the safeguard notice (without token count)
-			expect(toolResult).toContain("<notice>This file is 2048KB")
-			expect(toolResult).toContain("Showing only the first 2000 lines to preserve context space")
+			// Verify the result contains the safeguard notice
+			expect(toolResult).toContain("<notice>readFile.safeguardNotice</notice>")
 			expect(toolResult).toContain(`<content lines="1-2000">`)
 		})
 
@@ -861,8 +879,10 @@ describe("read_file tool with large file safeguard", () => {
 			// Verify tiktoken was NOT called
 			expect(mockedTiktoken).not.toHaveBeenCalled()
 
-			// The normal maxReadFileLine logic should apply
+			// The normal maxReadFileLine logic should apply (using readLines, not readLinesWithCharLimit)
 			expect(mockedReadLines).toHaveBeenCalled()
+			const mockedReadLinesWithCharLimit = vi.mocked(readLinesWithCharLimit)
+			expect(mockedReadLinesWithCharLimit).not.toHaveBeenCalled()
 		})
 
 		it("should handle line ranges correctly with safeguard", async () => {
@@ -955,10 +975,16 @@ describe("read_file tool with large file safeguard", () => {
 				}),
 			}
 			mockedExtractTextFromFile.mockResolvedValue("content")
-			mockedReadLines.mockResolvedValue("partial content")
+			const mockedReadLinesWithCharLimit = vi.mocked(readLinesWithCharLimit)
+			mockedReadLinesWithCharLimit.mockResolvedValue({
+				content: "partial content",
+				linesRead: 2000,
+				charactersRead: 50000,
+				wasTruncated: true,
+			})
 			await executeReadFileTool({}, { fileSize: 100 * 1024 + 1, maxReadFileLine: -1, tokenCount: 50001 })
-			expect(mockedReadLines).toHaveBeenCalled()
-			expect(toolResult).toContain("preserve context space")
+			expect(mockedReadLinesWithCharLimit).toHaveBeenCalled()
+			expect(toolResult).toContain("<notice>readFile.safeguardNotice</notice>")
 		})
 	})
 })

src/core/tools/readFileTool.ts

@@ -16,6 +16,15 @@ import { extractTextFromFile, addLineNumbers, getSupportedBinaryFormats } from "
 import { parseSourceCodeDefinitionsForFile } from "../../services/tree-sitter"
 import { parseXml } from "../../utils/xml"
 import { tiktoken } from "../../utils/tiktoken"
+import {
+	LARGE_FILE_SIZE_THRESHOLD,
+	VERY_LARGE_FILE_SIZE,
+	FALLBACK_MAX_LINES,
+	CONTEXT_WINDOW_PERCENTAGE,
+	MAX_CHAR_LIMIT,
+	CHARS_PER_TOKEN_RATIO,
+} from "@roo-code/types"
+import { readLinesWithCharLimit } from "../../integrations/misc/read-lines-char-limit"
 
 export function getReadFileToolDescription(blockName: string, blockParams: any): string {
 	// Handle both single path and multiple files via args
@@ -519,19 +528,16 @@ export async function readFileTool(
 				}
 
 				// Handle normal file read with safeguard for large files
-				// Define thresholds for the safeguard
-				const LARGE_FILE_SIZE_THRESHOLD = 100 * 1024 // 100KB - files larger than this will be checked for token count
-				const VERY_LARGE_FILE_SIZE = 1024 * 1024 // 1MB - apply safeguard automatically
-				const FALLBACK_MAX_LINES = 2000 // Default number of lines to read when applying safeguard
-
 				// Get the actual context window size from the model
 				const contextWindow = cline.api.getModel().info.contextWindow || 100000 // Default to 100k if not available
-				const MAX_TOKEN_THRESHOLD = Math.floor(contextWindow * 0.5) // Use 50% of the actual context window
+				const MAX_TOKEN_THRESHOLD = Math.floor(contextWindow * CONTEXT_WINDOW_PERCENTAGE)
+				const MAX_CHAR_THRESHOLD = MAX_TOKEN_THRESHOLD * CHARS_PER_TOKEN_RATIO
 
 				// Check if we should apply the safeguard
 				let shouldApplySafeguard = false
 				let safeguardNotice = ""
-				let linesToRead = totalLines
+				let fullContent: string | null = null
+				let actualLinesRead = totalLines
 
 				if (maxReadFileLine === -1) {
 					// Get file size
@@ -541,22 +547,22 @@ export async function readFileTool(
 					if (fileStats.size > LARGE_FILE_SIZE_THRESHOLD) {
 						// File is large enough to warrant token count check
 						try {
-							const fullContent = await extractTextFromFile(fullPath)
+							// Read the full content once
+							fullContent = await extractTextFromFile(fullPath)
 							const tokenCount = await tiktoken([{ type: "text", text: fullContent }])
 
 							if (tokenCount > MAX_TOKEN_THRESHOLD) {
 								shouldApplySafeguard = true
-								linesToRead = FALLBACK_MAX_LINES
-								safeguardNotice = `<notice>This file is ${fileSizeKB}KB and contains approximately ${tokenCount.toLocaleString()} tokens, which could consume a significant portion of the context window. Showing only the first ${FALLBACK_MAX_LINES} lines to preserve context space. Use line_range if you need to read specific sections.</notice>\n`
+								// Clear fullContent to avoid using it when we need partial content
+								fullContent = null
 							}
+							// If tokenCount <= MAX_TOKEN_THRESHOLD, we keep fullContent to reuse it
 						} catch (error) {
 							// If token counting fails, apply safeguard based on file size alone
 							console.warn(`Failed to count tokens for large file ${relPath}:`, error)
 							if (fileStats.size > VERY_LARGE_FILE_SIZE) {
 								// For very large files (>1MB), apply safeguard anyway
 								shouldApplySafeguard = true
-								linesToRead = FALLBACK_MAX_LINES
-								safeguardNotice = `<notice>This file is ${fileSizeKB}KB, which could consume a significant portion of the context window. Showing only the first ${FALLBACK_MAX_LINES} lines to preserve context space. Use line_range if you need to read specific sections.</notice>\n`
 							}
 						}
 					}
@@ -566,12 +572,32 @@ export async function readFileTool(
 				let lineRangeAttr: string
 
 				if (shouldApplySafeguard) {
-					// Read partial file with safeguard
-					content = addLineNumbers(await readLines(fullPath, linesToRead - 1, 0))
-					lineRangeAttr = ` lines="1-${linesToRead}"`
+					// Read partial file with character-based safeguard
+					// Use the smaller of MAX_CHAR_LIMIT or the calculated character threshold
+					const charLimit = Math.min(MAX_CHAR_LIMIT, MAX_CHAR_THRESHOLD)
+					const result = await readLinesWithCharLimit(fullPath, charLimit)
+
+					content = addLineNumbers(result.content, 1)
+					actualLinesRead = result.linesRead
+					lineRangeAttr = ` lines="1-${actualLinesRead}"`
+
+					const fileStats = await stat(fullPath)
+					const fileSizeKB = Math.round(fileStats.size / 1024)
+
+					if (result.wasTruncated) {
+						safeguardNotice = `<notice>${t("tools:readFile.safeguardNotice", {
+							fileSizeKB,
+							actualLinesRead,
+							charactersRead: result.charactersRead.toLocaleString(),
+						})}</notice>\n`
+					}
 				} else {
-					// Read full file as normal
-					content = await extractTextFromFile(fullPath)
+					// Read full file - reuse fullContent if we already have it
+					if (fullContent !== null) {
+						content = fullContent
+					} else {
+						content = await extractTextFromFile(fullPath)
+					}
 					lineRangeAttr = ` lines="1-${totalLines}"`
 				}
 

src/i18n/locales/ca/tools.json

@@ -2,7 +2,8 @@
 	"readFile": {
 		"linesRange": " (lines {{start}}-{{end}})",
 		"definitionsOnly": " (definitions only)",
-		"maxLines": " (max {{max}} lines)"
+		"maxLines": " (max {{max}} lines)",
+		"safeguardNotice": "This file is {{fileSizeKB}}KB and would consume a significant portion of the context window. Showing only the first {{actualLinesRead}} complete lines ({{charactersRead}} characters) to preserve context space. Use line_range if you need to read specific sections."
 	},
 	"toolRepetitionLimitReached": "Roo appears to be stuck in a loop, attempting the same action ({{toolName}}) repeatedly. This might indicate a problem with its current strategy. Consider rephrasing the task, providing more specific instructions, or guiding it towards a different approach.",
 	"codebaseSearch": {