From 5e0c9a71e47b6925d0d6e4a6c0768c3763455e32 Mon Sep 17 00:00:00 2001
From: KJ7LNW <93454819+KJ7LNW@users.noreply.github.com>
Date: Sat, 5 Apr 2025 19:10:11 -0700
Subject: [PATCH] fix: clamp negative line numbers when reading files (#2337)

Improve handling of AI-generated line numbers:
- Clamp negative startLine values to 0 instead of throwing errors
- Convert non-integer line numbers to integers automatically
- Only reject when line numbers aren't numbers at all

These changes prevent unnecessary model retries by handling imperfect
AI-generated line numbers gracefully, making the system more robust
when interacting with AI models.

Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>
Co-authored-by: Eric Wheeler <roo-code@z.ewheeler.org>
---
 .../misc/__tests__/read-lines.test.ts         | 22 ++++++++--------
 src/integrations/misc/read-lines.ts           | 25 ++++++++++++-------
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/src/integrations/misc/__tests__/read-lines.test.ts b/src/integrations/misc/__tests__/read-lines.test.ts
index 5f5997e117..ce0bfc0de3 100644
--- a/src/integrations/misc/__tests__/read-lines.test.ts
+++ b/src/integrations/misc/__tests__/read-lines.test.ts
@@ -33,23 +33,21 @@ describe("nthline", () => {
 
 		it("should throw error for negative to_line", async () => {
 			await expect(readLines(testFile, -3)).rejects.toThrow(
-				"Invalid endLine: -3. Line numbers must be non-negative integers.",
+				"startLine (0) must be less than or equal to endLine (-3)",
 			)
 		})
 
-		it("should throw error for negative from_line", async () => {
-			await expect(readLines(testFile, 3, -1)).rejects.toThrow(
-				"Invalid startLine: -1. Line numbers must be non-negative integers.",
-			)
+		it("should handle negative from_line by clamping to 0", async () => {
+			const lines = await readLines(testFile, 3, -1)
+			expect(lines).toEqual(["Line 1", "Line 2", "Line 3", "Line 4"].join("\n"))
 		})
 
-		it("should throw error for non-integer line numbers", async () => {
-			await expect(readLines(testFile, 3, 1.5)).rejects.toThrow(
-				"Invalid startLine: 1.5. Line numbers must be non-negative integers.",
-			)
-			await expect(readLines(testFile, 3.5)).rejects.toThrow(
-				"Invalid endLine: 3.5. Line numbers must be non-negative integers.",
-			)
+		it("should floor non-integer line numbers", async () => {
+			const linesWithNonIntegerStart = await readLines(testFile, 3, 1.5)
+			expect(linesWithNonIntegerStart).toEqual(["Line 2", "Line 3", "Line 4"].join("\n"))
+
+			const linesWithNonIntegerEnd = await readLines(testFile, 3.5)
+			expect(linesWithNonIntegerEnd).toEqual(["Line 1", "Line 2", "Line 3", "Line 4"].join("\n"))
 		})
 
 		it("should throw error when from_line > to_line", async () => {
diff --git a/src/integrations/misc/read-lines.ts b/src/integrations/misc/read-lines.ts
index 173fdadbdd..2b3780aeed 100644
--- a/src/integrations/misc/read-lines.ts
+++ b/src/integrations/misc/read-lines.ts
@@ -24,17 +24,24 @@ const outOfRangeError = (filepath: string, n: number) => {
  */
 export function readLines(filepath: string, endLine?: number, startLine?: number): Promise<string> {
 	return new Promise((resolve, reject) => {
-		// Validate input parameters
-		// Check startLine validity if provided
-		if (startLine !== undefined && (startLine < 0 || startLine % 1 !== 0)) {
-			return reject(
-				new RangeError(`Invalid startLine: ${startLine}. Line numbers must be non-negative integers.`),
-			)
+		// Reject if startLine is defined but not a number
+		if (startLine !== undefined && typeof startLine !== "number") {
+			return reject(new RangeError(`Invalid startLine: ${startLine}. Line numbers must be numbers.`))
 		}
 
-		// Check endLine validity if provided
-		if (endLine !== undefined && (endLine < 0 || endLine % 1 !== 0)) {
-			return reject(new RangeError(`Invalid endLine: ${endLine}. Line numbers must be non-negative integers.`))
+		// Force startLine to be an integer and clamp to 0 if negative
+		if (startLine !== undefined) {
+			startLine = Math.max(0, Math.floor(startLine))
+		}
+
+		// Reject if endLine is defined but not a number
+		if (endLine !== undefined && typeof endLine !== "number") {
+			return reject(new RangeError(`Invalid endLine: ${endLine}. Line numbers must be numbers.`))
+		}
+
+		// Force endLine to be an integer
+		if (endLine !== undefined) {
+			endLine = Math.floor(endLine)
 		}
 
 		const effectiveStartLine = startLine === undefined ? 0 : startLine