mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): take grep context lines from ripgrep instead of reading matched files (#11705)
When `context` is set, the grep tool read every matched file whole and split it into lines, only to print the lines around each match. Memory grew with the size of the matched files, not the output. In the issue's reproduction (8 log files of about 95 MB, one match each, `context: 2`, `--max-old-space-size=1024`), the process ran out of heap after about 10 s. ripgrep already supports context. This change passes `--context N` to rg and formats the `context` events from its JSON output. The tool no longer opens matched files. Details: - After the last allowed match (`limit`), rg still sends that match's trailing context. The tool keeps those lines, then stops rg. A match inside that window is shown as a context line, as before. - Lines past the 50 KB output cap are counted but not stored, since truncation drops them anyway. Match counts, the match-limit notice, and `details.matchLimitReached` stay as before. - rg merges overlapping context, so lines shared by nearby matches are printed once instead of repeated per match. This is the one intended output change. - Context lines are decoded by rg, so UTF-16 files with a BOM show correctly with context (#11423, closed as no-action). Non-UTF-8 lines and paths, which rg sends base64-encoded, are decoded too. Two related grep fixes in the same code: - **Exact match limit:** with exactly N matches and `limit: N`, grep said "N matches limit reached. Use limit=2N". It now sets the notice and `details.matchLimitReached` only after rg sends one more match. Cost: when no further match exists, rg finishes the search instead of being stopped at the Nth match, the same as a search with fewer than N matches. - **Lone CR:** grep removed every `\r` in a line, so `one\rtwo needle` showed as `onetwo needle` and text copied from grep failed to match in edit. It now strips only the line terminator (`\n` or `\r\n`), like the read tool. The TUI still hides CRs when rendering. This updates the expectation in the #11398 test from `beforeprogress` to `before\rprogress`. Breaking: `GrepOperations.readFile` is removed because nothing uses it anymore. rg always ran locally, so this hook never made grep work on a remote backend. Docs now say to override `execute()` for remote search, as the Gondolin example does. Code that passes `readFile` still runs, and the value is ignored. Only code that type-checks an object literal against `GrepOperations` fails to compile. Known remaining differences and limits: - Memory no longer grows with file size or match count, but a single huge matching line (tens of MB) is still buffered whole before being cut to 500 characters. That was already true for `context: 0` on main. Bounding it would mean discarding oversized rg JSON records, which is a separate change. - In a directory search, rg stops reading a file once it finds a NUL byte, so context past that point in a text-looking file that turns binary later is omitted. main reread the file and printed it. Explicit file paths are unaffected. - `details.truncation.totalLines`/`totalBytes` count formatted rows. With deduplicated context, or file names containing newlines, they can differ from main. No renderer or notice displays them. **Testing** - Issue reproduction: before, `FATAL ERROR ... JavaScript heap out of memory`. After, correct output in about 60 ms with 135 MB RSS. - `pattern: "line", context: 3, limit: 100000` over the same 770 MB of logs reports both limit notices and peaks at 154 MB RSS. - New tests in `test/tools.test.ts`: - Merged overlapping context, plus trailing context at the limit including a match inside it. Fails on main. - UTF-16 with context. Fails on main. - Truncation reported when stored output ends exactly at 50 KB. Guards the counting logic. - Matches still counted past the byte cap. Passes on main too; it guards against stopping rg early at the cap. - Huge `context` value accepted. - No limit notice with exactly `limit` matches (with and without context), notice with more. Fails before this change. - Lone CR kept and CRLF stripped, with and without context. Fails before this change. - All 94 tests in the file pass. `npm run check` passes. fixes #9276 This PR is AI-generated.
This commit is contained in:
parent
2bd2c2d0a6
commit
8da141c2ad
4 changed files with 215 additions and 69 deletions
|
|
@ -920,11 +920,153 @@ describe("Coding Agent Tools", () => {
|
|||
});
|
||||
|
||||
const output = getTextOutput(result);
|
||||
expect(output).toContain("lone-cr.txt-1- beforeprogress");
|
||||
// A lone CR inside a line is kept, as the read tool shows it, so the text can be copied into edit.
|
||||
expect(output).toContain("lone-cr.txt-1- before\rprogress");
|
||||
expect(output).toContain("lone-cr.txt:2: needle");
|
||||
expect(output).toContain("lone-cr.txt-3- after");
|
||||
});
|
||||
|
||||
// https://github.com/earendil-works/pi/issues/9276
|
||||
it("should print overlapping context once and keep the last match's trailing context", async () => {
|
||||
const testFile = join(testDir, "rg-context.txt");
|
||||
writeFileSync(testFile, ["a", "match one", "b", "match two", "match three", "c", "match four"].join("\n"));
|
||||
|
||||
const result = await grepTool.execute("test-call-rg-context", {
|
||||
pattern: "match",
|
||||
path: testFile,
|
||||
context: 1,
|
||||
limit: 2,
|
||||
});
|
||||
expect(getTextOutput(result).split("\n\n")[0]).toBe(
|
||||
[
|
||||
"rg-context.txt-1- a",
|
||||
"rg-context.txt:2: match one",
|
||||
"rg-context.txt-3- b",
|
||||
"rg-context.txt:4: match two",
|
||||
// A match inside the last allowed match's trailing context is shown as context.
|
||||
"rg-context.txt-5- match three",
|
||||
].join("\n"),
|
||||
);
|
||||
});
|
||||
|
||||
// https://github.com/earendil-works/pi/issues/9276
|
||||
it("should keep counting matches past the byte cap", async () => {
|
||||
const testFile = join(testDir, "byte-cap.txt");
|
||||
const line = `match ${"x".repeat(400)}`;
|
||||
writeFileSync(testFile, Array.from({ length: 200 }, () => line).join("\n"));
|
||||
|
||||
const result = await grepTool.execute("test-call-byte-cap", {
|
||||
pattern: "match",
|
||||
path: testFile,
|
||||
limit: 150,
|
||||
});
|
||||
|
||||
const output = getTextOutput(result);
|
||||
expect(output).toContain("150 matches limit reached");
|
||||
expect(output).toContain("50.0KB limit reached");
|
||||
expect(result.details?.matchLimitReached).toBe(150);
|
||||
expect(result.details?.truncation?.totalLines).toBe(150);
|
||||
});
|
||||
|
||||
// https://github.com/earendil-works/pi/issues/9276
|
||||
it("should report truncation when stored output ends exactly at the byte cap", async () => {
|
||||
const testFile = join(testDir, "x");
|
||||
// 103 rows of 500 bytes plus one of 12 bytes, with newlines, join to exactly 50KB; one more match follows.
|
||||
writeFileSync(testFile, [...Array(103).fill("x".repeat(490)), "x".repeat(7), "extra x"].join("\n"));
|
||||
|
||||
const result = await grepTool.execute("test-call-byte-cap-boundary", {
|
||||
pattern: "x",
|
||||
path: testFile,
|
||||
limit: 1000,
|
||||
});
|
||||
|
||||
expect(getTextOutput(result)).toContain("[50.0KB limit reached]");
|
||||
expect(result.details?.truncation?.totalLines).toBe(105);
|
||||
});
|
||||
|
||||
// https://github.com/earendil-works/pi/issues/9276
|
||||
it("should accept a huge context value", async () => {
|
||||
const testFile = join(testDir, "huge-context.txt");
|
||||
writeFileSync(testFile, "before\nneedle\nafter");
|
||||
|
||||
const result = await grepTool.execute("test-call-huge-context", {
|
||||
pattern: "needle",
|
||||
path: testFile,
|
||||
context: 1e30,
|
||||
});
|
||||
|
||||
expect(getTextOutput(result)).toBe(
|
||||
["huge-context.txt-1- before", "huge-context.txt:2: needle", "huge-context.txt-3- after"].join("\n"),
|
||||
);
|
||||
});
|
||||
|
||||
// Context lines come from ripgrep, which decodes UTF-16; rereading the file as UTF-8 corrupted them (#11423).
|
||||
it("should decode UTF-16 files with context", async () => {
|
||||
const testFile = join(testDir, "utf16.txt");
|
||||
writeFileSync(testFile, Buffer.from("\uFEFFbefore\nneedle 中文\nafter\n", "utf16le"));
|
||||
|
||||
const result = await grepTool.execute("test-call-utf16-context", {
|
||||
pattern: "needle",
|
||||
path: testFile,
|
||||
context: 1,
|
||||
});
|
||||
|
||||
expect(getTextOutput(result)).toBe(
|
||||
["utf16.txt-1- before", "utf16.txt:2: needle 中文", "utf16.txt-3- after"].join("\n"),
|
||||
);
|
||||
});
|
||||
|
||||
it("should keep a lone CR inside a line and strip CRLF terminators", async () => {
|
||||
const testFile = join(testDir, "mixed-cr.txt");
|
||||
writeFileSync(testFile, "one\rtwo needle\r\nnext\r\n");
|
||||
|
||||
const plain = await grepTool.execute("test-call-cr-plain", { pattern: "needle", path: testFile });
|
||||
expect(getTextOutput(plain)).toBe("mixed-cr.txt:1: one\rtwo needle");
|
||||
|
||||
const withContext = await grepTool.execute("test-call-cr-context", {
|
||||
pattern: "needle",
|
||||
path: testFile,
|
||||
context: 1,
|
||||
});
|
||||
expect(getTextOutput(withContext)).toBe(
|
||||
["mixed-cr.txt:1: one\rtwo needle", "mixed-cr.txt-2- next"].join("\n"),
|
||||
);
|
||||
});
|
||||
|
||||
it("should not report the match limit when exactly limit matches exist", async () => {
|
||||
const testFile = join(testDir, "exact-limit.txt");
|
||||
writeFileSync(testFile, ["match one", "x", "match two", "y"].join("\n"));
|
||||
|
||||
for (const context of [0, 1]) {
|
||||
const exact = await grepTool.execute("test-call-exact-limit", {
|
||||
pattern: "match",
|
||||
path: testFile,
|
||||
limit: 2,
|
||||
context,
|
||||
});
|
||||
expect(getTextOutput(exact)).not.toContain("limit reached");
|
||||
expect(exact.details?.matchLimitReached).toBeUndefined();
|
||||
|
||||
const over = await grepTool.execute("test-call-over-limit", {
|
||||
pattern: "match",
|
||||
path: testFile,
|
||||
limit: 1,
|
||||
context,
|
||||
});
|
||||
expect(getTextOutput(over)).toContain("[1 matches limit reached. Use limit=2 for more, or refine pattern]");
|
||||
expect(over.details?.matchLimitReached).toBe(1);
|
||||
}
|
||||
|
||||
const fractional = await grepTool.execute("test-call-fractional-limit", {
|
||||
pattern: "match",
|
||||
path: testFile,
|
||||
limit: 1.5,
|
||||
});
|
||||
expect(getTextOutput(fractional)).toContain(
|
||||
"[1 matches limit reached. Use limit=2 for more, or refine pattern]",
|
||||
);
|
||||
});
|
||||
|
||||
// https://github.com/earendil-works/pi/issues/11203
|
||||
it("should ignore RIPGREP_CONFIG_PATH", async () => {
|
||||
const configFile = join(testDir, "rg-config");
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue