tweak: make read tool offset 1 indexed instead of 0 to avoid confusion that could be caused by line #s being 1 based (#13198)
This commit is contained in:
@@ -1022,9 +1022,9 @@ export namespace SessionPrompt {
|
||||
}
|
||||
}
|
||||
}
|
||||
offset = Math.max(start - 1, 0)
|
||||
offset = Math.max(start, 1)
|
||||
if (end) {
|
||||
limit = end - offset
|
||||
limit = end - (offset - 1)
|
||||
}
|
||||
}
|
||||
const args = { filePath: filepath, offset, limit }
|
||||
|
||||
@@ -18,10 +18,13 @@ export const ReadTool = Tool.define("read", {
|
||||
description: DESCRIPTION,
|
||||
parameters: z.object({
|
||||
filePath: z.string().describe("The absolute path to the file or directory to read"),
|
||||
offset: z.coerce.number().describe("The 0-based line offset to start reading from").optional(),
|
||||
offset: z.coerce.number().describe("The line number to start reading from (1-indexed)").optional(),
|
||||
limit: z.coerce.number().describe("The maximum number of lines to read (defaults to 2000)").optional(),
|
||||
}),
|
||||
async execute(params, ctx) {
|
||||
if (params.offset !== undefined && params.offset < 1) {
|
||||
throw new Error("offset must be greater than or equal to 1")
|
||||
}
|
||||
let filepath = params.filePath
|
||||
if (!path.isAbsolute(filepath)) {
|
||||
filepath = path.resolve(Instance.directory, filepath)
|
||||
@@ -78,9 +81,10 @@ export const ReadTool = Tool.define("read", {
|
||||
entries.sort((a, b) => a.localeCompare(b))
|
||||
|
||||
const limit = params.limit ?? DEFAULT_READ_LIMIT
|
||||
const offset = params.offset || 0
|
||||
const sliced = entries.slice(offset, offset + limit)
|
||||
const truncated = offset + sliced.length < entries.length
|
||||
const offset = params.offset ?? 1
|
||||
const start = offset - 1
|
||||
const sliced = entries.slice(start, start + limit)
|
||||
const truncated = start + sliced.length < entries.length
|
||||
|
||||
const output = [
|
||||
`<path>${filepath}</path>`,
|
||||
@@ -138,13 +142,15 @@ export const ReadTool = Tool.define("read", {
|
||||
if (isBinary) throw new Error(`Cannot read binary file: ${filepath}`)
|
||||
|
||||
const limit = params.limit ?? DEFAULT_READ_LIMIT
|
||||
const offset = params.offset || 0
|
||||
const offset = params.offset ?? 1
|
||||
const start = offset - 1
|
||||
const lines = await file.text().then((text) => text.split("\n"))
|
||||
if (start >= lines.length) throw new Error(`Offset ${offset} is out of range for this file (${lines.length} lines)`)
|
||||
|
||||
const raw: string[] = []
|
||||
let bytes = 0
|
||||
let truncatedByBytes = false
|
||||
for (let i = offset; i < Math.min(lines.length, offset + limit); i++) {
|
||||
for (let i = start; i < Math.min(lines.length, start + limit); i++) {
|
||||
const line = lines[i].length > MAX_LINE_LENGTH ? lines[i].substring(0, MAX_LINE_LENGTH) + "..." : lines[i]
|
||||
const size = Buffer.byteLength(line, "utf-8") + (raw.length > 0 ? 1 : 0)
|
||||
if (bytes + size > MAX_BYTES) {
|
||||
@@ -156,7 +162,7 @@ export const ReadTool = Tool.define("read", {
|
||||
}
|
||||
|
||||
const content = raw.map((line, index) => {
|
||||
return `${index + offset + 1}: ${line}`
|
||||
return `${index + offset}: ${line}`
|
||||
})
|
||||
const preview = raw.slice(0, 20).join("\n")
|
||||
|
||||
@@ -164,7 +170,7 @@ export const ReadTool = Tool.define("read", {
|
||||
output += content.join("\n")
|
||||
|
||||
const totalLines = lines.length
|
||||
const lastReadLine = offset + raw.length
|
||||
const lastReadLine = offset + raw.length - 1
|
||||
const hasMoreLines = totalLines > lastReadLine
|
||||
const truncated = hasMoreLines || truncatedByBytes
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ Read a file or directory from the local filesystem. If the path does not exist,
|
||||
Usage:
|
||||
- The filePath parameter should be an absolute path.
|
||||
- By default, this tool returns up to 2000 lines from the start of the file.
|
||||
- The offset parameter is the line number to start from (1-indexed).
|
||||
- To read later sections, call this tool again with a larger offset.
|
||||
- Use the grep tool to find specific content in large files or files with long lines.
|
||||
- If you are unsure of the correct file path, use the glob tool to look up filenames by glob pattern.
|
||||
|
||||
@@ -258,7 +258,7 @@ describe("tool.read truncation", () => {
|
||||
test("respects offset parameter", async () => {
|
||||
await using tmp = await tmpdir({
|
||||
init: async (dir) => {
|
||||
const lines = Array.from({ length: 20 }, (_, i) => `line${i}`).join("\n")
|
||||
const lines = Array.from({ length: 20 }, (_, i) => `line${i + 1}`).join("\n")
|
||||
await Bun.write(path.join(dir, "offset.txt"), lines)
|
||||
},
|
||||
})
|
||||
@@ -275,11 +275,29 @@ describe("tool.read truncation", () => {
|
||||
})
|
||||
})
|
||||
|
||||
test("throws when offset is beyond end of file", async () => {
|
||||
await using tmp = await tmpdir({
|
||||
init: async (dir) => {
|
||||
const lines = Array.from({ length: 3 }, (_, i) => `line${i + 1}`).join("\n")
|
||||
await Bun.write(path.join(dir, "short.txt"), lines)
|
||||
},
|
||||
})
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
const read = await ReadTool.init()
|
||||
await expect(
|
||||
read.execute({ filePath: path.join(tmp.path, "short.txt"), offset: 4, limit: 5 }, ctx),
|
||||
).rejects.toThrow("Offset 4 is out of range for this file (3 lines)")
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
test("does not mark final directory page as truncated", async () => {
|
||||
await using tmp = await tmpdir({
|
||||
init: async (dir) => {
|
||||
await Promise.all(
|
||||
Array.from({ length: 10 }, (_, i) => Bun.write(path.join(dir, "dir", `file-${i}.txt`), `line${i}`)),
|
||||
Array.from({ length: 10 }, (_, i) => Bun.write(path.join(dir, "dir", `file-${i + 1}.txt`), `line${i}`)),
|
||||
)
|
||||
},
|
||||
})
|
||||
@@ -287,7 +305,7 @@ describe("tool.read truncation", () => {
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
const read = await ReadTool.init()
|
||||
const result = await read.execute({ filePath: path.join(tmp.path, "dir"), offset: 5, limit: 5 }, ctx)
|
||||
const result = await read.execute({ filePath: path.join(tmp.path, "dir"), offset: 6, limit: 5 }, ctx)
|
||||
expect(result.metadata.truncated).toBe(false)
|
||||
expect(result.output).not.toContain("Showing 5 of 10 entries")
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user