From a9d686f3899d696b10fb840b51d0abb4220d5a68 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sun, 21 Dec 2025 16:29:16 +0000 Subject: [PATCH] feat: unify line_range parameter for read_file tool in native mode This PR addresses Issue #10239 by unifying the line_range parameter format between XML and Native protocols: - XML mode: line_range (singular) with format "1-50" - Native mode: NOW uses line_range (singular) with format "1-50" or "1-50,100-150" Previously native mode used line_ranges (plural) with JSON array format [[1,50],[100,150]]. Changes: - Update native tool definition to use line_range (string) instead of line_ranges (array) - Update NativeToolCallParser to handle both new and legacy formats for backward compatibility - Update all related tests The unified format makes it easier for models to understand and use incremental file reading, especially for continuation after truncation. --- .../__tests__/bedrock-native-tools.spec.ts | 21 ++- .../assistant-message/NativeToolCallParser.ts | 39 +++++- .../__tests__/NativeToolCallParser.spec.ts | 128 +++++++++++++++++- .../native-tools/__tests__/converters.spec.ts | 9 +- src/core/prompts/tools/native-tools/index.ts | 2 +- .../prompts/tools/native-tools/read_file.ts | 32 ++--- src/utils/__tests__/json-schema.spec.ts | 43 ++---- 7 files changed, 203 insertions(+), 71 deletions(-) diff --git a/src/api/providers/__tests__/bedrock-native-tools.spec.ts b/src/api/providers/__tests__/bedrock-native-tools.spec.ts index 8325f94bfb6..f2318fa310b 100644 --- a/src/api/providers/__tests__/bedrock-native-tools.spec.ts +++ b/src/api/providers/__tests__/bedrock-native-tools.spec.ts @@ -141,13 +141,12 @@ describe("AwsBedrockHandler Native Tool Calling", () => { type: "object", properties: { path: { type: "string" }, - line_ranges: { - type: ["array", "null"], - items: { type: "integer" }, - description: "Optional line ranges", + line_range: { + type: ["string", "null"], + description: "Optional line range", }, }, - required: ["path", "line_ranges"], + required: ["path", "line_range"], }, }, }, @@ -167,14 +166,12 @@ describe("AwsBedrockHandler Native Tool Calling", () => { expect(executeCommandSchema.properties.cwd.type).toBeUndefined() expect(executeCommandSchema.properties.cwd.description).toBe("Working directory (optional)") - // Second tool: line_ranges should be transformed from type: ["array", "null"] to anyOf + // Second tool: line_range should be transformed from type: ["string", "null"] to anyOf const readFileSchema = bedrockTools[1].toolSpec.inputSchema.json as any - const lineRanges = readFileSchema.properties.files.items.properties.line_ranges - expect(lineRanges.anyOf).toEqual([{ type: "array" }, { type: "null" }]) - expect(lineRanges.type).toBeUndefined() - // items also gets additionalProperties: false from normalization - expect(lineRanges.items.type).toBe("integer") - expect(lineRanges.description).toBe("Optional line ranges") + const lineRange = readFileSchema.properties.files.items.properties.line_range + expect(lineRange.anyOf).toEqual([{ type: "string" }, { type: "null" }]) + expect(lineRange.type).toBeUndefined() + expect(lineRange.description).toBe("Optional line range") }) it("should filter non-function tools", () => { diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index 89bc1a9c340..63c1b12d7ff 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -295,18 +295,47 @@ export class NativeToolCallParser { } /** - * Convert raw file entries from API (with line_ranges) to FileEntry objects - * (with lineRanges). Handles multiple formats for compatibility: + * Parse a line range string in the format "start-end" or "start-end,start-end" + * Returns an array of { start, end } objects. + */ + private static parseLineRangeString(lineRange: string): Array<{ start: number; end: number }> { + const ranges: Array<{ start: number; end: number }> = [] + // Split by comma for multiple ranges (e.g., "1-50,100-150") + const parts = lineRange.split(",") + for (const part of parts) { + const match = part.trim().match(/^(\d+)-(\d+)$/) + if (match) { + ranges.push({ start: parseInt(match[1], 10), end: parseInt(match[2], 10) }) + } + } + return ranges + } + + /** + * Convert raw file entries from API to FileEntry objects (with lineRanges). + * Handles multiple formats for backward 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"] } + * New unified format: { path: string, line_range: "1-50" } or "1-50,100-150" + * Legacy tuple format: { path: string, line_ranges: [[1, 50], [100, 150]] } + * Legacy object format: { path: string, line_ranges: [{ start: 1, end: 50 }] } + * Legacy string array 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 } + + // New unified format: line_range as string "1-50" or "1-50,100-150" + if (file.line_range && typeof file.line_range === "string") { + const ranges = this.parseLineRangeString(file.line_range) + if (ranges.length > 0) { + entry.lineRanges = ranges + } + return entry + } + + // Legacy format: line_ranges as array if (file.line_ranges && Array.isArray(file.line_ranges)) { entry.lineRanges = file.line_ranges .map((range: any) => { diff --git a/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts b/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts index 0e81671cc15..37a66f3da7b 100644 --- a/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts +++ b/src/core/assistant-message/__tests__/NativeToolCallParser.spec.ts @@ -8,7 +8,68 @@ describe("NativeToolCallParser", () => { describe("parseToolCall", () => { describe("read_file tool", () => { - it("should handle line_ranges as tuples (new format)", () => { + it("should handle line_range as a single range string (unified format)", () => { + const toolCall = { + id: "toolu_123", + name: "read_file" as const, + arguments: JSON.stringify({ + files: [ + { + path: "src/core/task/Task.ts", + line_range: "1920-1990", + }, + ], + }), + } + + 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 }]) + } + }) + + it("should handle line_range as a comma-separated string for multiple ranges (unified format)", () => { + const toolCall = { + id: "toolu_123", + name: "read_file" as const, + arguments: JSON.stringify({ + files: [ + { + path: "src/core/task/Task.ts", + line_range: "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 tuples (legacy format)", () => { const toolCall = { id: "toolu_123", name: "read_file" as const, @@ -43,7 +104,7 @@ describe("NativeToolCallParser", () => { } }) - it("should handle line_ranges as strings (legacy format)", () => { + it("should handle line_ranges as strings array (legacy format)", () => { const toolCall = { id: "toolu_123", name: "read_file" as const, @@ -174,7 +235,36 @@ describe("NativeToolCallParser", () => { describe("processStreamingChunk", () => { describe("read_file tool", () => { - it("should convert line_ranges strings to lineRanges objects during streaming", () => { + it("should convert line_range string to lineRanges objects during streaming (unified format)", () => { + const id = "toolu_streaming_unified" + NativeToolCallParser.startStreamingToolCall(id, "read_file") + + // Simulate streaming chunks using the new unified format + const fullArgs = JSON.stringify({ + files: [ + { + path: "src/test.ts", + line_range: "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 }, + ]) + }) + + it("should convert line_ranges strings to lineRanges objects during streaming (legacy format)", () => { const id = "toolu_streaming_123" NativeToolCallParser.startStreamingToolCall(id, "read_file") @@ -207,7 +297,37 @@ describe("NativeToolCallParser", () => { describe("finalizeStreamingToolCall", () => { describe("read_file tool", () => { - it("should convert line_ranges strings to lineRanges objects on finalize", () => { + it("should convert line_range string to lineRanges objects on finalize (unified format)", () => { + const id = "toolu_finalize_unified" + NativeToolCallParser.startStreamingToolCall(id, "read_file") + + // Add the complete arguments using new unified format + NativeToolCallParser.processStreamingChunk( + id, + JSON.stringify({ + files: [ + { + path: "finalized.ts", + line_range: "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 }]) + } + }) + + it("should convert line_ranges strings to lineRanges objects on finalize (legacy format)", () => { const id = "toolu_finalize_123" NativeToolCallParser.startStreamingToolCall(id, "read_file") diff --git a/src/core/prompts/tools/native-tools/__tests__/converters.spec.ts b/src/core/prompts/tools/native-tools/__tests__/converters.spec.ts index 4c1f606754d..d16d77cd0d1 100644 --- a/src/core/prompts/tools/native-tools/__tests__/converters.spec.ts +++ b/src/core/prompts/tools/native-tools/__tests__/converters.spec.ts @@ -91,12 +91,13 @@ describe("converters", () => { type: "object", properties: { path: { type: "string" }, - line_ranges: { - type: ["array", "null"], - items: { type: "string", pattern: "^[0-9]+-[0-9]+$" }, + line_range: { + type: ["string", "null"], + description: + "Optional line range to read. Format: 'start-end' with 1-based inclusive line numbers. For multiple ranges, use comma-separated format like '1-50,100-150'.", }, }, - required: ["path", "line_ranges"], + required: ["path", "line_range"], }, }, }, diff --git a/src/core/prompts/tools/native-tools/index.ts b/src/core/prompts/tools/native-tools/index.ts index 79302a39f31..ff015e675c1 100644 --- a/src/core/prompts/tools/native-tools/index.ts +++ b/src/core/prompts/tools/native-tools/index.ts @@ -27,7 +27,7 @@ export { convertOpenAIToolToAnthropic, convertOpenAIToolsToAnthropic } from "./c /** * Get native tools array, optionally customizing based on settings. * - * @param partialReadsEnabled - Whether to include line_ranges support in read_file tool (default: true) + * @param partialReadsEnabled - Whether to include line_range support in read_file tool (default: true) * @returns Array of native tool definitions */ export function getNativeTools(partialReadsEnabled: boolean = true): OpenAI.Chat.ChatCompletionTool[] { diff --git a/src/core/prompts/tools/native-tools/read_file.ts b/src/core/prompts/tools/native-tools/read_file.ts index bf43f26c8af..037d408e03f 100644 --- a/src/core/prompts/tools/native-tools/read_file.ts +++ b/src/core/prompts/tools/native-tools/read_file.ts @@ -5,28 +5,32 @@ const READ_FILE_BASE_DESCRIPTION = `Read one or more files and return their cont const READ_FILE_SUPPORTS_NOTE = `Supports text extraction from PDF and DOCX files, but may not handle other binary files properly.` /** - * Creates the read_file tool definition, optionally including line_ranges support + * Creates the read_file tool definition, optionally including line_range support * based on whether partial reads are enabled. * - * @param partialReadsEnabled - Whether to include line_ranges parameter + * Uses `line_range` (singular) with a simple string format like "1-50" or "1-50,100-150" + * for multiple ranges. This unified format matches the XML protocol for consistency. + * + * @param partialReadsEnabled - Whether to include line_range parameter * @returns Native tool definition for read_file */ export function createReadFileTool(partialReadsEnabled: boolean = true): OpenAI.Chat.ChatCompletionTool { const baseDescription = READ_FILE_BASE_DESCRIPTION + " Structure: { files: [{ path: 'relative/path.ts'" + - (partialReadsEnabled ? ", line_ranges: [[1, 50], [100, 150]]" : "") + + (partialReadsEnabled ? ", line_range: '1-50'" : "") + " }] }. " + "The 'path' is required and relative to workspace. " const optionalRangesDescription = partialReadsEnabled - ? "The 'line_ranges' is optional for reading specific sections. Each range is a [start, end] tuple (1-based inclusive). " + ? "The 'line_range' is optional for reading specific sections. Format: 'start-end' (1-based inclusive). For multiple ranges, use comma-separated format like '1-50,100-150'. " : "" 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 range: { files: [{ path: 'src/app.ts', line_range: '1-50' }] }. " + + "Example with multiple ranges: { files: [{ path: 'src/app.ts', line_range: '1-50,100-150' }] }. " + + "Example multiple files: { files: [{ path: 'file1.ts', line_range: '1-50' }, { path: 'file2.ts' }] }" : "Example single file: { files: [{ path: 'src/app.ts' }] }. " + "Example multiple files: { files: [{ path: 'file1.ts' }, { path: 'file2.ts' }] }" @@ -40,24 +44,18 @@ export function createReadFileTool(partialReadsEnabled: boolean = true): OpenAI. }, } - // Only include line_ranges if partial reads are enabled + // Only include line_range if partial reads are enabled if (partialReadsEnabled) { - fileProperties.line_ranges = { - type: ["array", "null"], + fileProperties.line_range = { + type: ["string", "null"], description: - "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: "array", - items: { type: "integer" }, - minItems: 2, - maxItems: 2, - }, + "Optional line range to read. Format: 'start-end' with 1-based inclusive line numbers. For multiple ranges, use comma-separated format like '1-50,100-150'.", } } // When using strict mode, ALL properties must be in the required array // Optional properties are handled by having type: ["...", "null"] - const fileRequiredProperties = partialReadsEnabled ? ["path", "line_ranges"] : ["path"] + const fileRequiredProperties = partialReadsEnabled ? ["path", "line_range"] : ["path"] return { type: "function", diff --git a/src/utils/__tests__/json-schema.spec.ts b/src/utils/__tests__/json-schema.spec.ts index c53e0d7b869..555bcb02e5b 100644 --- a/src/utils/__tests__/json-schema.spec.ts +++ b/src/utils/__tests__/json-schema.spec.ts @@ -86,9 +86,8 @@ describe("normalizeToolSchema", () => { type: "object", properties: { path: { type: "string" }, - line_ranges: { - type: ["array", "null"], - items: { type: "integer" }, + line_range: { + type: ["string", "null"], }, }, }, @@ -103,9 +102,8 @@ describe("normalizeToolSchema", () => { type: "object", properties: { path: { type: "string" }, - line_ranges: { - anyOf: [{ type: "array" }, { type: "null" }], - items: { type: "integer" }, + line_range: { + anyOf: [{ type: "string" }, { type: "null" }], }, }, additionalProperties: false, @@ -123,15 +121,11 @@ describe("normalizeToolSchema", () => { type: "object", properties: { path: { type: "string" }, - line_ranges: { - type: ["array", "null"], - items: { - type: "array", - items: { type: "integer" }, - }, + line_range: { + type: ["string", "null"], }, }, - required: ["path", "line_ranges"], + required: ["path", "line_range"], }, }, }, @@ -143,7 +137,7 @@ describe("normalizeToolSchema", () => { const properties = result.properties as Record> const filesItems = properties.files.items as Record const filesItemsProps = filesItems.properties as Record> - expect(filesItemsProps.line_ranges.anyOf).toEqual([{ type: "array" }, { type: "null" }]) + expect(filesItemsProps.line_range.anyOf).toEqual([{ type: "string" }, { type: "null" }]) }) it("should recursively transform anyOf arrays", () => { @@ -232,18 +226,12 @@ describe("normalizeToolSchema", () => { type: "string", description: "Path to the file", }, - line_ranges: { - type: ["array", "null"], - description: "Optional line ranges", - items: { - type: "array", - items: { type: "integer" }, - minItems: 2, - maxItems: 2, - }, + line_range: { + type: ["string", "null"], + description: "Optional line range", }, }, - required: ["path", "line_ranges"], + required: ["path", "line_range"], additionalProperties: false, }, minItems: 1, @@ -255,14 +243,13 @@ describe("normalizeToolSchema", () => { const result = normalizeToolSchema(input) - // Verify the line_ranges was transformed + // Verify the line_range was transformed const files = (result.properties as Record).files as Record const items = files.items as Record const props = items.properties as Record> - expect(props.line_ranges.anyOf).toEqual([{ type: "array" }, { type: "null" }]) + expect(props.line_range.anyOf).toEqual([{ type: "string" }, { type: "null" }]) // Verify other properties are preserved - expect(props.line_ranges.items).toBeDefined() - expect(props.line_ranges.description).toBe("Optional line ranges") + expect(props.line_range.description).toBe("Optional line range") }) describe("format field handling", () => {