mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: respect line_ranges parameter in read_file tool with maxReadFileLine
- Add transformNativeFileEntries method to convert snake_case line_ranges to camelCase lineRanges - Transform native protocol format (line_ranges as string arrays) to internal format (lineRanges as LineRange objects) - Add comprehensive tests for native protocol line_ranges handling Fixes #9566
This commit is contained in:
Roo Code
parent
8949c2f6fe
commit
d0773e9bb5
2 changed files with 251 additions and 1 deletions
|
|
@ -107,7 +107,8 @@ export class ReadFileTool extends BaseTool<"read_file"> {
|
|||
|
||||
async execute(params: { files: FileEntry[] }, task: Task, callbacks: ToolCallbacks): Promise<void> {
|
||||
const { handleError, pushToolResult, toolProtocol } = callbacks
|
||||
const fileEntries = params.files
|
||||
// Transform native protocol format if necessary
|
||||
const fileEntries = this.transformNativeFileEntries(params.files)
|
||||
const modelInfo = task.api.getModel().info
|
||||
const protocol = resolveToolProtocol(task.apiConfiguration, modelInfo)
|
||||
const useNative = isNativeProtocol(protocol)
|
||||
|
|
@ -706,6 +707,44 @@ export class ReadFileTool extends BaseTool<"read_file"> {
|
|||
return `[${blockName} with missing path/args/files]`
|
||||
}
|
||||
|
||||
/**
|
||||
* Transform native protocol file entries to the expected format.
|
||||
* Native protocol uses snake_case `line_ranges` as string arrays,
|
||||
* but our internal format uses camelCase `lineRanges` as LineRange objects.
|
||||
*/
|
||||
private transformNativeFileEntries(files: any[]): FileEntry[] {
|
||||
return files.map((file) => {
|
||||
const entry: FileEntry = {
|
||||
path: file.path,
|
||||
}
|
||||
|
||||
// Transform line_ranges (snake_case, string[]) to lineRanges (camelCase, LineRange[])
|
||||
if (file.line_ranges && Array.isArray(file.line_ranges)) {
|
||||
entry.lineRanges = []
|
||||
for (const rangeStr of file.line_ranges) {
|
||||
const match = String(rangeStr).match(/^(\d+)-(\d+)$/)
|
||||
if (match) {
|
||||
const [, startStr, endStr] = match
|
||||
const start = parseInt(startStr, 10)
|
||||
const end = parseInt(endStr, 10)
|
||||
if (!isNaN(start) && !isNaN(end) && start > 0 && end > 0) {
|
||||
entry.lineRanges.push({ start, end })
|
||||
}
|
||||
}
|
||||
}
|
||||
// Only keep lineRanges if we successfully parsed at least one range
|
||||
if (entry.lineRanges.length === 0) {
|
||||
delete entry.lineRanges
|
||||
}
|
||||
} else if (file.lineRanges && Array.isArray(file.lineRanges)) {
|
||||
// Already in the correct format (camelCase with LineRange objects)
|
||||
entry.lineRanges = file.lineRanges
|
||||
}
|
||||
|
||||
return entry
|
||||
})
|
||||
}
|
||||
|
||||
override async handlePartial(task: Task, block: ToolUse<"read_file">): Promise<void> {
|
||||
const argsXmlTag = block.params.args
|
||||
const legacyPath = block.params.path
|
||||
|
|
|
|||
|
|
@ -1726,3 +1726,214 @@ describe("read_file tool with image support", () => {
|
|||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("read_file tool with native protocol line_ranges", () => {
|
||||
// Test that the native protocol format (snake_case line_ranges as strings) works correctly
|
||||
const testFilePath = "test/large-file.txt"
|
||||
const absoluteFilePath = "/test/large-file.txt"
|
||||
|
||||
const mockedCountFileLines = vi.mocked(countFileLines)
|
||||
const mockedReadLines = vi.mocked(readLines)
|
||||
const mockedIsBinaryFile = vi.mocked(isBinaryFile)
|
||||
const mockedPathResolve = vi.mocked(path.resolve)
|
||||
|
||||
let mockCline: any
|
||||
let mockProvider: any
|
||||
let toolResult: ToolResponse | undefined
|
||||
|
||||
beforeEach(() => {
|
||||
// Clear specific mocks
|
||||
mockedCountFileLines.mockClear()
|
||||
mockedReadLines.mockClear()
|
||||
mockedIsBinaryFile.mockClear()
|
||||
mockedPathResolve.mockClear()
|
||||
addLineNumbersMock.mockClear()
|
||||
|
||||
// Use shared mock setup function
|
||||
const mocks = createMockCline()
|
||||
mockCline = mocks.mockCline
|
||||
mockProvider = mocks.mockProvider
|
||||
|
||||
// Set up native protocol support
|
||||
mockCline.api = {
|
||||
getModel: vi.fn().mockReturnValue({
|
||||
info: {
|
||||
supportsImages: false,
|
||||
contextWindow: 200000,
|
||||
maxTokens: 4096,
|
||||
supportsPromptCache: false,
|
||||
supportsNativeTools: true, // Enable native tools support
|
||||
},
|
||||
}),
|
||||
}
|
||||
|
||||
mockedPathResolve.mockReturnValue(absoluteFilePath)
|
||||
mockedIsBinaryFile.mockResolvedValue(false)
|
||||
|
||||
// Mock addLineNumbers to add line numbers to content
|
||||
addLineNumbersMock.mockImplementation((content, startLine = 1) => {
|
||||
if (!content) return ""
|
||||
const lines = content.split("\n")
|
||||
return lines.map((line: string, i: number) => `${startLine + i} | ${line}`).join("\n")
|
||||
})
|
||||
|
||||
toolResult = undefined
|
||||
})
|
||||
|
||||
it("should respect line_ranges in native protocol format even with maxReadFileLine set", async () => {
|
||||
// Setup - file with 472 lines, maxReadFileLine set to 250
|
||||
mockedCountFileLines.mockResolvedValue(472)
|
||||
mockProvider.getState.mockResolvedValue({ maxReadFileLine: 250 })
|
||||
|
||||
// Mock the specific lines we want to read (251-350)
|
||||
const requestedContent = Array.from({ length: 100 }, (_, i) => `Line ${251 + i}`).join("\n")
|
||||
mockedReadLines.mockResolvedValue(requestedContent)
|
||||
|
||||
// Mock addLineNumbers from extract-text module
|
||||
const { addLineNumbers } = await import("../../../integrations/misc/extract-text")
|
||||
vi.mocked(addLineNumbers).mockImplementation((content, startLine = 1) => {
|
||||
if (!content) return ""
|
||||
const lines = content.split("\n")
|
||||
return lines.map((line: string, i: number) => `${startLine + i} | ${line}`).join("\n")
|
||||
})
|
||||
|
||||
// Create a tool use with native protocol format (snake_case line_ranges as string array)
|
||||
const toolUse: ReadFileToolUse = {
|
||||
type: "tool_use",
|
||||
name: "read_file",
|
||||
params: {}, // Native protocol doesn't use params
|
||||
partial: false,
|
||||
nativeArgs: {
|
||||
files: [
|
||||
{
|
||||
path: testFilePath,
|
||||
line_ranges: ["251-350"], // Snake_case, string array format
|
||||
} as any, // Use any to bypass TypeScript for native protocol format
|
||||
],
|
||||
} as any,
|
||||
}
|
||||
|
||||
// Execute the tool
|
||||
await readFileTool.handle(mockCline, toolUse, {
|
||||
askApproval: mockCline.ask,
|
||||
handleError: vi.fn(),
|
||||
pushToolResult: (result: ToolResponse) => {
|
||||
toolResult = result
|
||||
},
|
||||
removeClosingTag: (_: ToolParamName, content?: string) => content ?? "",
|
||||
toolProtocol: "native",
|
||||
})
|
||||
|
||||
// Verify that readLines was called with the correct range
|
||||
expect(mockedReadLines).toHaveBeenCalledWith(absoluteFilePath, 349, 250) // 0-indexed: lines 251-350
|
||||
|
||||
// Verify the result contains the requested lines, not the first 250 lines
|
||||
expect(toolResult).toBeDefined()
|
||||
// For native protocol, the result format is different
|
||||
expect(toolResult).toContain("251-350")
|
||||
expect(toolResult).toContain("251 |") // Check for line number prefix
|
||||
expect(toolResult).not.toContain("Showing only 250 of 472 total lines")
|
||||
})
|
||||
|
||||
it("should handle multiple line_ranges in native protocol format", async () => {
|
||||
// Setup - file with 500 lines
|
||||
mockedCountFileLines.mockResolvedValue(500)
|
||||
mockProvider.getState.mockResolvedValue({ maxReadFileLine: 100 })
|
||||
|
||||
// Create a tool use with multiple ranges
|
||||
const toolUse: ReadFileToolUse = {
|
||||
type: "tool_use",
|
||||
name: "read_file",
|
||||
params: {},
|
||||
partial: false,
|
||||
nativeArgs: {
|
||||
files: [
|
||||
{
|
||||
path: testFilePath,
|
||||
line_ranges: ["10-20", "300-310", "450-460"], // Multiple ranges
|
||||
} as any, // Use any to bypass TypeScript for native protocol format
|
||||
],
|
||||
} as any,
|
||||
}
|
||||
|
||||
// Mock the readLines to return different content for each range
|
||||
mockedReadLines
|
||||
.mockResolvedValueOnce("Lines 10-20 content")
|
||||
.mockResolvedValueOnce("Lines 300-310 content")
|
||||
.mockResolvedValueOnce("Lines 450-460 content")
|
||||
|
||||
// Mock addLineNumbers from extract-text module
|
||||
const { addLineNumbers } = await import("../../../integrations/misc/extract-text")
|
||||
vi.mocked(addLineNumbers).mockImplementation((content, startLine = 1) => {
|
||||
if (!content) return ""
|
||||
return `${startLine} | ${content}`
|
||||
})
|
||||
|
||||
// Execute the tool
|
||||
await readFileTool.handle(mockCline, toolUse, {
|
||||
askApproval: mockCline.ask,
|
||||
handleError: vi.fn(),
|
||||
pushToolResult: (result: ToolResponse) => {
|
||||
toolResult = result
|
||||
},
|
||||
removeClosingTag: (_: ToolParamName, content?: string) => content ?? "",
|
||||
toolProtocol: "native",
|
||||
})
|
||||
|
||||
// Verify that readLines was called for each range
|
||||
expect(mockedReadLines).toHaveBeenCalledTimes(3)
|
||||
expect(mockedReadLines).toHaveBeenNthCalledWith(1, absoluteFilePath, 19, 9) // lines 10-20
|
||||
expect(mockedReadLines).toHaveBeenNthCalledWith(2, absoluteFilePath, 309, 299) // lines 300-310
|
||||
expect(mockedReadLines).toHaveBeenNthCalledWith(3, absoluteFilePath, 459, 449) // lines 450-460
|
||||
|
||||
// Verify the result contains all ranges (native protocol format)
|
||||
expect(toolResult).toContain("10-20")
|
||||
expect(toolResult).toContain("300-310")
|
||||
expect(toolResult).toContain("450-460")
|
||||
})
|
||||
|
||||
it("should handle mixed camelCase and snake_case formats", async () => {
|
||||
// Setup
|
||||
mockedCountFileLines.mockResolvedValue(100)
|
||||
mockProvider.getState.mockResolvedValue({ maxReadFileLine: 50 })
|
||||
mockedReadLines.mockResolvedValue("Lines 60-70 content")
|
||||
|
||||
// Mock addLineNumbers from extract-text module
|
||||
const { addLineNumbers } = await import("../../../integrations/misc/extract-text")
|
||||
vi.mocked(addLineNumbers).mockImplementation((content, startLine = 1) => {
|
||||
if (!content) return ""
|
||||
return `${startLine} | ${content}`
|
||||
})
|
||||
|
||||
// Test with already camelCase format (should still work)
|
||||
const toolUse: ReadFileToolUse = {
|
||||
type: "tool_use",
|
||||
name: "read_file",
|
||||
params: {},
|
||||
partial: false,
|
||||
nativeArgs: {
|
||||
files: [
|
||||
{
|
||||
path: testFilePath,
|
||||
lineRanges: [{ start: 60, end: 70 }], // Already in camelCase with LineRange objects
|
||||
},
|
||||
],
|
||||
},
|
||||
}
|
||||
|
||||
// Execute the tool
|
||||
await readFileTool.handle(mockCline, toolUse, {
|
||||
askApproval: mockCline.ask,
|
||||
handleError: vi.fn(),
|
||||
pushToolResult: (result: ToolResponse) => {
|
||||
toolResult = result
|
||||
},
|
||||
removeClosingTag: (_: ToolParamName, content?: string) => content ?? "",
|
||||
toolProtocol: "native",
|
||||
})
|
||||
|
||||
// Verify the camelCase format still works
|
||||
expect(mockedReadLines).toHaveBeenCalledWith(absoluteFilePath, 69, 59) // lines 60-70
|
||||
expect(toolResult).toContain("60-70")
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue