Jelajahi Sumber

fix: convert line_ranges strings to lineRanges objects in native tool calls (#9627)

Daniel 2 bulan lalu
induk
melakukan
87d646316e

+ 41 - 2
src/core/assistant-message/NativeToolCallParser.ts

@@ -286,6 +286,45 @@ export class NativeToolCallParser {
 		return finalToolUse
 	}
 
+	/**
+	 * Convert raw file entries from API (with line_ranges) to FileEntry objects
+	 * (with lineRanges). Handles multiple formats for compatibility:
+	 *
+	 * New tuple format: { path: string, line_ranges: [[1, 50], [100, 150]] }
+	 * Object format: { path: string, line_ranges: [{ start: 1, end: 50 }] }
+	 * Legacy string format: { path: string, line_ranges: ["1-50"] }
+	 *
+	 * Returns: { path: string, lineRanges: [{ start: 1, end: 50 }] }
+	 */
+	private static convertFileEntries(files: any[]): FileEntry[] {
+		return files.map((file: any) => {
+			const entry: FileEntry = { path: file.path }
+			if (file.line_ranges && Array.isArray(file.line_ranges)) {
+				entry.lineRanges = file.line_ranges
+					.map((range: any) => {
+						// Handle tuple format: [start, end]
+						if (Array.isArray(range) && range.length >= 2) {
+							return { start: Number(range[0]), end: Number(range[1]) }
+						}
+						// Handle object format: { start: number, end: number }
+						if (typeof range === "object" && range !== null && "start" in range && "end" in range) {
+							return { start: Number(range.start), end: Number(range.end) }
+						}
+						// Handle legacy string format: "1-50"
+						if (typeof range === "string") {
+							const match = range.match(/^(\d+)-(\d+)$/)
+							if (match) {
+								return { start: parseInt(match[1], 10), end: parseInt(match[2], 10) }
+							}
+						}
+						return null
+					})
+					.filter(Boolean)
+			}
+			return entry
+		})
+	}
+
 	/**
 	 * Create a partial ToolUse from currently parsed arguments.
 	 * Used during streaming to show progress.
@@ -313,7 +352,7 @@ export class NativeToolCallParser {
 		switch (name) {
 			case "read_file":
 				if (partialArgs.files && Array.isArray(partialArgs.files)) {
-					nativeArgs = { files: partialArgs.files }
+					nativeArgs = { files: this.convertFileEntries(partialArgs.files) }
 				}
 				break
 
@@ -558,7 +597,7 @@ export class NativeToolCallParser {
 			switch (toolCall.name) {
 				case "read_file":
 					if (args.files && Array.isArray(args.files)) {
-						nativeArgs = { files: args.files } as NativeArgsFor<TName>
+						nativeArgs = { files: this.convertFileEntries(args.files) } as NativeArgsFor<TName>
 					}
 					break
 

+ 241 - 0
src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts

@@ -0,0 +1,241 @@
+import { NativeToolCallParser } from "../NativeToolCallParser"
+
+describe("NativeToolCallParser", () => {
+	beforeEach(() => {
+		NativeToolCallParser.clearAllStreamingToolCalls()
+		NativeToolCallParser.clearRawChunkState()
+	})
+
+	describe("parseToolCall", () => {
+		describe("read_file tool", () => {
+			it("should handle line_ranges as tuples (new format)", () => {
+				const toolCall = {
+					id: "toolu_123",
+					name: "read_file" as const,
+					arguments: JSON.stringify({
+						files: [
+							{
+								path: "src/core/task/Task.ts",
+								line_ranges: [
+									[1920, 1990],
+									[2060, 2120],
+								],
+							},
+						],
+					}),
+				}
+
+				const result = NativeToolCallParser.parseToolCall(toolCall)
+
+				expect(result).not.toBeNull()
+				expect(result?.type).toBe("tool_use")
+				if (result?.type === "tool_use") {
+					expect(result.nativeArgs).toBeDefined()
+					const nativeArgs = result.nativeArgs as {
+						files: Array<{ path: string; lineRanges?: Array<{ start: number; end: number }> }>
+					}
+					expect(nativeArgs.files).toHaveLength(1)
+					expect(nativeArgs.files[0].path).toBe("src/core/task/Task.ts")
+					expect(nativeArgs.files[0].lineRanges).toEqual([
+						{ start: 1920, end: 1990 },
+						{ start: 2060, end: 2120 },
+					])
+				}
+			})
+
+			it("should handle line_ranges as strings (legacy format)", () => {
+				const toolCall = {
+					id: "toolu_123",
+					name: "read_file" as const,
+					arguments: JSON.stringify({
+						files: [
+							{
+								path: "src/core/task/Task.ts",
+								line_ranges: ["1920-1990", "2060-2120"],
+							},
+						],
+					}),
+				}
+
+				const result = NativeToolCallParser.parseToolCall(toolCall)
+
+				expect(result).not.toBeNull()
+				expect(result?.type).toBe("tool_use")
+				if (result?.type === "tool_use") {
+					expect(result.nativeArgs).toBeDefined()
+					const nativeArgs = result.nativeArgs as {
+						files: Array<{ path: string; lineRanges?: Array<{ start: number; end: number }> }>
+					}
+					expect(nativeArgs.files).toHaveLength(1)
+					expect(nativeArgs.files[0].path).toBe("src/core/task/Task.ts")
+					expect(nativeArgs.files[0].lineRanges).toEqual([
+						{ start: 1920, end: 1990 },
+						{ start: 2060, end: 2120 },
+					])
+				}
+			})
+
+			it("should handle files without line_ranges", () => {
+				const toolCall = {
+					id: "toolu_123",
+					name: "read_file" as const,
+					arguments: JSON.stringify({
+						files: [
+							{
+								path: "src/utils.ts",
+							},
+						],
+					}),
+				}
+
+				const result = NativeToolCallParser.parseToolCall(toolCall)
+
+				expect(result).not.toBeNull()
+				expect(result?.type).toBe("tool_use")
+				if (result?.type === "tool_use") {
+					const nativeArgs = result.nativeArgs as {
+						files: Array<{ path: string; lineRanges?: Array<{ start: number; end: number }> }>
+					}
+					expect(nativeArgs.files).toHaveLength(1)
+					expect(nativeArgs.files[0].path).toBe("src/utils.ts")
+					expect(nativeArgs.files[0].lineRanges).toBeUndefined()
+				}
+			})
+
+			it("should handle multiple files with different line_ranges", () => {
+				const toolCall = {
+					id: "toolu_123",
+					name: "read_file" as const,
+					arguments: JSON.stringify({
+						files: [
+							{
+								path: "file1.ts",
+								line_ranges: ["1-50"],
+							},
+							{
+								path: "file2.ts",
+								line_ranges: ["100-150", "200-250"],
+							},
+							{
+								path: "file3.ts",
+							},
+						],
+					}),
+				}
+
+				const result = NativeToolCallParser.parseToolCall(toolCall)
+
+				expect(result).not.toBeNull()
+				expect(result?.type).toBe("tool_use")
+				if (result?.type === "tool_use") {
+					const nativeArgs = result.nativeArgs as {
+						files: Array<{ path: string; lineRanges?: Array<{ start: number; end: number }> }>
+					}
+					expect(nativeArgs.files).toHaveLength(3)
+					expect(nativeArgs.files[0].lineRanges).toEqual([{ start: 1, end: 50 }])
+					expect(nativeArgs.files[1].lineRanges).toEqual([
+						{ start: 100, end: 150 },
+						{ start: 200, end: 250 },
+					])
+					expect(nativeArgs.files[2].lineRanges).toBeUndefined()
+				}
+			})
+
+			it("should filter out invalid line_range strings", () => {
+				const toolCall = {
+					id: "toolu_123",
+					name: "read_file" as const,
+					arguments: JSON.stringify({
+						files: [
+							{
+								path: "file.ts",
+								line_ranges: ["1-50", "invalid", "100-200", "abc-def"],
+							},
+						],
+					}),
+				}
+
+				const result = NativeToolCallParser.parseToolCall(toolCall)
+
+				expect(result).not.toBeNull()
+				expect(result?.type).toBe("tool_use")
+				if (result?.type === "tool_use") {
+					const nativeArgs = result.nativeArgs as {
+						files: Array<{ path: string; lineRanges?: Array<{ start: number; end: number }> }>
+					}
+					expect(nativeArgs.files[0].lineRanges).toEqual([
+						{ start: 1, end: 50 },
+						{ start: 100, end: 200 },
+					])
+				}
+			})
+		})
+	})
+
+	describe("processStreamingChunk", () => {
+		describe("read_file tool", () => {
+			it("should convert line_ranges strings to lineRanges objects during streaming", () => {
+				const id = "toolu_streaming_123"
+				NativeToolCallParser.startStreamingToolCall(id, "read_file")
+
+				// Simulate streaming chunks
+				const fullArgs = JSON.stringify({
+					files: [
+						{
+							path: "src/test.ts",
+							line_ranges: ["10-20", "30-40"],
+						},
+					],
+				})
+
+				// Process the complete args as a single chunk for simplicity
+				const result = NativeToolCallParser.processStreamingChunk(id, fullArgs)
+
+				expect(result).not.toBeNull()
+				expect(result?.nativeArgs).toBeDefined()
+				const nativeArgs = result?.nativeArgs as {
+					files: Array<{ path: string; lineRanges?: Array<{ start: number; end: number }> }>
+				}
+				expect(nativeArgs.files).toHaveLength(1)
+				expect(nativeArgs.files[0].lineRanges).toEqual([
+					{ start: 10, end: 20 },
+					{ start: 30, end: 40 },
+				])
+			})
+		})
+	})
+
+	describe("finalizeStreamingToolCall", () => {
+		describe("read_file tool", () => {
+			it("should convert line_ranges strings to lineRanges objects on finalize", () => {
+				const id = "toolu_finalize_123"
+				NativeToolCallParser.startStreamingToolCall(id, "read_file")
+
+				// Add the complete arguments
+				NativeToolCallParser.processStreamingChunk(
+					id,
+					JSON.stringify({
+						files: [
+							{
+								path: "finalized.ts",
+								line_ranges: ["500-600"],
+							},
+						],
+					}),
+				)
+
+				const result = NativeToolCallParser.finalizeStreamingToolCall(id)
+
+				expect(result).not.toBeNull()
+				expect(result?.type).toBe("tool_use")
+				if (result?.type === "tool_use") {
+					const nativeArgs = result.nativeArgs as {
+						files: Array<{ path: string; lineRanges?: Array<{ start: number; end: number }> }>
+					}
+					expect(nativeArgs.files[0].path).toBe("finalized.ts")
+					expect(nativeArgs.files[0].lineRanges).toEqual([{ start: 500, end: 600 }])
+				}
+			})
+		})
+	})
+})

+ 9 - 7
src/core/prompts/tools/native-tools/read_file.ts

@@ -15,18 +15,18 @@ export function createReadFileTool(partialReadsEnabled: boolean = true): OpenAI.
 	const baseDescription =
 		READ_FILE_BASE_DESCRIPTION +
 		" Structure: { files: [{ path: 'relative/path.ts'" +
-		(partialReadsEnabled ? ", line_ranges: ['1-50', '100-150']" : "") +
+		(partialReadsEnabled ? ", line_ranges: [[1, 50], [100, 150]]" : "") +
 		" }] }. " +
 		"The 'path' is required and relative to workspace. "
 
 	const optionalRangesDescription = partialReadsEnabled
-		? "The 'line_ranges' is optional for reading specific sections (format: 'start-end', 1-based inclusive). "
+		? "The 'line_ranges' is optional for reading specific sections. Each range is a [start, end] tuple (1-based inclusive). "
 		: ""
 
 	const examples = partialReadsEnabled
 		? "Example single file: { files: [{ path: 'src/app.ts' }] }. " +
-			"Example with line ranges: { files: [{ path: 'src/app.ts', line_ranges: ['1-50', '100-150'] }] }. " +
-			"Example multiple files: { files: [{ path: 'file1.ts', line_ranges: ['1-50'] }, { path: 'file2.ts' }] }"
+			"Example with line ranges: { files: [{ path: 'src/app.ts', line_ranges: [[1, 50], [100, 150]] }] }. " +
+			"Example multiple files: { files: [{ path: 'file1.ts', line_ranges: [[1, 50]] }, { path: 'file2.ts' }] }"
 		: "Example single file: { files: [{ path: 'src/app.ts' }] }. " +
 			"Example multiple files: { files: [{ path: 'file1.ts' }, { path: 'file2.ts' }] }"
 
@@ -45,10 +45,12 @@ export function createReadFileTool(partialReadsEnabled: boolean = true): OpenAI.
 		fileProperties.line_ranges = {
 			type: ["array", "null"],
 			description:
-				"Optional 1-based inclusive ranges to read (format: start-end). Use multiple ranges for non-contiguous sections and keep ranges tight to the needed context.",
+				"Optional line ranges to read. Each range is a [start, end] tuple with 1-based inclusive line numbers. Use multiple ranges for non-contiguous sections.",
 			items: {
-				type: "string",
-				pattern: "^[0-9]+-[0-9]+$",
+				type: "array",
+				items: { type: "integer" },
+				minItems: 2,
+				maxItems: 2,
 			},
 		}
 	}