smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(read): enforced exact line selector bounds

Browse Source 
- Removed implicit padding and syntactic boundary expansion from explicit ranges.
- Covered local, converted, multi-range, and artifact reads.

Fixes #5802
This commit is contained in:
roboomp
2026-07-17 07:45:39 +00:00
parent 0f9fceeea4
commit e5d5aec189
7 changed files with 93 additions and 186 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/test/read-multi-range.test.ts
+13 -14
View File
@@ -79,6 +79,8 @@ describe("read tool multi-range selector", () => {
expect(text).toContain("line 20");
expect(text).toContain("line 21");
expect(text).toContain("line 22");
expect(text).not.toMatch(/^2:line 2$/m);
expect(text).not.toMatch(/^6:line 6$/m);
// Lines between the ranges must be elided
expect(text).not.toContain("line 10");
expect(text).not.toContain("line 19");
@@ -86,7 +88,7 @@ describe("read tool multi-range selector", () => {
expect(text).toContain("…");
});
it("includes the matching closing bracket line outside a forward range", async () => {
it("does not add a closing bracket outside a forward range", async () => {
const filePath = path.join(tmpDir, "brackets.ts");
await fs.writeFile(
filePath,
@@ -106,13 +108,13 @@ describe("read tool multi-range selector", () => {
const text = textOutput(await tool.execute("call-bracket-close", { path: `${filePath}:1-1` }));
expect(text).toContain("function outer() {");
expect(text).toContain("…");
expect(text).toContain("}");
expect(text).not.toContain("…");
expect(text).not.toMatch(/^7:}$/m);
expect(text).not.toContain("const four");
expect(text).not.toContain("return one + two");
});
it("includes the matching opening bracket line outside a reverse range", async () => {
it("does not add an opening bracket outside a reverse range", async () => {
const filePath = path.join(tmpDir, "brackets.ts");
await fs.writeFile(
filePath,
@@ -131,13 +133,14 @@ describe("read tool multi-range selector", () => {
const tool = new ReadTool(createSession(tmpDir));
const text = textOutput(await tool.execute("call-bracket-open", { path: `${filePath}:7-7` }));
expect(text.indexOf("function outer() {")).toBeLessThan(text.indexOf("}"));
expect(text).toContain("…");
expect(text).toMatch(/^7:}$/m);
expect(text).not.toContain("function outer() {");
expect(text).not.toContain("…");
expect(text).not.toContain("const one = 1");
expect(text).not.toContain("const four = 4");
});
it("uses tree-sitter syntactic spans for indentation languages (Python)", async () => {
it("does not add Python syntactic boundaries outside a range", async () => {
const filePath = path.join(tmpDir, "module.py");
await fs.writeFile(
filePath,
@@ -156,15 +159,11 @@ describe("read tool multi-range selector", () => {
);
const tool = new ReadTool(createSession(tmpDir));
// Read only the `def` header (expands by a few trailing context lines).
// Python has no closing delimiter, so a bracket scan would surface
// nothing; tree-sitter surfaces the def's last body line (9) as the
// block boundary, behind an ellipsis for the skipped middle.
const text = textOutput(await tool.execute("call-py-def", { path: `${filePath}:1-1` }));
expect(text).toContain("def greet(name):");
expect(text).toContain("…");
expect(text).toContain("return a + b + c + d + e + f + g + len(name)");
expect(text).not.toContain("…");
expect(text).not.toContain("return a + b + c + d + e + f + g + len(name)");
expect(text).not.toContain("trailing = 1");
});
@@ -179,7 +178,7 @@ describe("read tool multi-range selector", () => {
// All lines from the merged range present
for (const i of [3, 4, 5, 6, 7, 8, 9]) {
expect(text).toContain(`line ${i}\n`);
expect(text).toMatch(new RegExp(`^${i}:line ${i}$`, "m"));
}
// No separator because ranges merged into one contiguous block
expect(text).not.toContain("…");