mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-08 03:07:53 +00:00
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.
This commit is contained in:
parent
78dc34498b
commit
a9d686f389
7 changed files with 203 additions and 71 deletions
|
|
@ -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", () => {
|
||||
|
|
|
|||
|
|
@ -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) => {
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
||||
|
|
|
|||
|
|
@ -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"],
|
||||
},
|
||||
},
|
||||
},
|
||||
|
|
|
|||
|
|
@ -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[] {
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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<string, Record<string, unknown>>
|
||||
const filesItems = properties.files.items as Record<string, unknown>
|
||||
const filesItemsProps = filesItems.properties as Record<string, Record<string, unknown>>
|
||||
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<string, unknown>).files as Record<string, unknown>
|
||||
const items = files.items as Record<string, unknown>
|
||||
const props = items.properties as Record<string, Record<string, unknown>>
|
||||
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", () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue