Skip to content

Commit e16c36e

Browse files
committed
Consolidate the four grep-output truncation implementations into one
Grep results passed through both ripgrep-plugin's line cap and rg-output's byte cap on the way in, then result-truncation-plugin's character cap on the way out, each with its own wording. Whenever more than one of those caps actually fired on the same result, the notices concatenated (or one silently clobbered another), so the model would sometimes see two differently-worded "truncated" notices, or a mangled fragment of one. result-truncation-plugin.ts's truncateToolResultContent is now the only place that attaches a truncation notice; it takes an optional threshold so other callers can reuse the same wording at a different cap size. The grep-specific cappers (rg-output's byte-cap breach, ripgrep-plugin's line-count cap) now trim silently and rely on that final pass to report the truncation once. bounded-grep-fallback's own byte-cap loop is removed outright — ripgrep-plugin's boundedContent already re-checks the fallback walker's output against the same byte-cap primitive downstream, making the fallback's own pass redundant. rg-output's timeout notice is untouched, since a run timing out is a different, non-redundant fact from output being oversized.
1 parent 4336578 commit e16c36e

7 files changed

Lines changed: 124 additions & 40 deletions

File tree

src/plugins/bounded-grep-fallback.ts

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,12 @@ export const BOUNDED_GREP_MAX_DIRECTORY_ENTRIES = 25_000;
1212
/** Max bytes read from any single file during content search. */
1313
export const BOUNDED_GREP_MAX_PER_FILE_BYTES = 512_000;
1414

15-
/** Max bytes in the formatted result string. */
16-
export const BOUNDED_GREP_MAX_OUTPUT_BYTES = 512_000;
17-
1815
export const BOUNDED_GREP_DEFAULT_MAX_RESULTS = 500;
1916
export const BOUNDED_SEARCH_DEFAULT_MAX_RESULTS = 1000;
2017

2118
export type BoundedGrepLimits = {
2219
maxDirectoryEntries?: number;
2320
maxPerFileBytes?: number;
24-
maxOutputBytes?: number;
2521
};
2622

2723
export type BoundedGrepArgs = {
@@ -97,15 +93,6 @@ function isBinary(buf: Buffer): boolean {
9793
return buf.includes(0);
9894
}
9995

100-
function capOutput(text: string, maxBytes: number): string {
101-
if (Buffer.byteLength(text, "utf8") <= maxBytes) return text;
102-
let cut = text;
103-
while (cut.length > 0 && Buffer.byteLength(cut, "utf8") > maxBytes) {
104-
cut = cut.slice(0, Math.floor(cut.length * 0.9));
105-
}
106-
return `${cut}\n... (output truncated at ${maxBytes} bytes; narrow path/glob or pattern)`;
107-
}
108-
10996
async function collectFilePaths(
11097
basePath: string,
11198
globFilter: RegExp | null,
@@ -270,7 +257,6 @@ export async function runBoundedGrep(
270257

271258
const maxDirectoryEntries = limits.maxDirectoryEntries ?? BOUNDED_GREP_MAX_DIRECTORY_ENTRIES;
272259
const maxPerFileBytes = limits.maxPerFileBytes ?? BOUNDED_GREP_MAX_PER_FILE_BYTES;
273-
const maxOutputBytes = limits.maxOutputBytes ?? BOUNDED_GREP_MAX_OUTPUT_BYTES;
274260

275261
const basePath = resolve(baseCwd, args.path ?? ".");
276262
const contextLines = args.context ?? 0;
@@ -329,7 +315,7 @@ export async function runBoundedGrep(
329315
if (walkTruncated) {
330316
output += `\n... (directory walk capped at ${maxDirectoryEntries} files; narrow path/glob)`;
331317
}
332-
return capOutput(output, maxOutputBytes);
318+
return output;
333319
}
334320

335321
export async function runBoundedSearchFiles(
@@ -341,7 +327,6 @@ export async function runBoundedSearchFiles(
341327
signal.throwIfAborted();
342328

343329
const maxDirectoryEntries = limits.maxDirectoryEntries ?? BOUNDED_GREP_MAX_DIRECTORY_ENTRIES;
344-
const maxOutputBytes = limits.maxOutputBytes ?? BOUNDED_GREP_MAX_OUTPUT_BYTES;
345330

346331
const basePath = resolve(baseCwd, args.path ?? ".");
347332
const maxResults = args.max_results ?? BOUNDED_SEARCH_DEFAULT_MAX_RESULTS;
@@ -391,6 +376,6 @@ export async function runBoundedSearchFiles(
391376
if (walkTruncated) {
392377
result += `\n... (directory walk capped at ${maxDirectoryEntries} files; narrow path/glob)`;
393378
}
394-
return capOutput(result, maxOutputBytes);
379+
return result;
395380
}
396381

src/plugins/result-truncation-plugin.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,23 @@ const TRUNCATABLE_TOOLS = new Set(["read_file", "grep", "run_shell", "search_fil
66
// 80 000 chars ≈ 20 000 tokens. Keeps a single result from dominating context.
77
const MAX_RESULT_CHARS = 80_000;
88

9-
// Shared with the MCP tool runner (src/mcp/plugin.ts), which is not part of the
10-
// posix runner this middleware wraps and so applies the same truncation directly.
11-
export function truncateToolResultContent(content: string): string {
12-
if (content.length <= MAX_RESULT_CHARS) return content;
9+
// Shared with the MCP tool runner (src/mcp/plugin.ts), which is not part of
10+
// the posix runner this middleware wraps and so applies the same truncation
11+
// directly. This is the single primitive that produces a truncation notice —
12+
// callers may pass their own threshold but never invent their own wording, so
13+
// a result can never carry two differently-worded "truncated" notices. The
14+
// grep-specific caps in rg-output.ts and ripgrep-plugin.ts deliberately don't
15+
// call this: they trim silently and leave notice duty to this middleware,
16+
// which runs after them in the plugin chain and sees the final content.
17+
export function truncateToolResultContent(
18+
content: string,
19+
maxChars: number = MAX_RESULT_CHARS,
20+
): string {
21+
if (content.length <= maxChars) return content;
1322

14-
const remaining = content.length - MAX_RESULT_CHARS;
23+
const remaining = content.length - maxChars;
1524
return (
16-
content.slice(0, MAX_RESULT_CHARS) +
25+
content.slice(0, maxChars) +
1726
`\n[output truncated — ${remaining.toLocaleString()} characters omitted. ` +
1827
`Use offset/limit params or a more targeted query to see the rest.]`
1928
);

src/plugins/rg-output.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,11 @@ const line = "big.txt:1:match line here\n";
77
test("the cap fires on the chunk that breaches it", () => {
88
const collector = createRgCollector(200);
99
expect(collector.push(line.repeat(4))).toBeUndefined();
10-
expect(collector.push(line.repeat(20))).toMatchObject({
11-
kind: "partial",
12-
notice: expect.stringContaining("exceeded 200 bytes"),
13-
});
10+
const outcome = collector.push(line.repeat(20));
11+
expect(outcome).toMatchObject({ kind: "partial" });
12+
// No notice of its own: the final tool result gets exactly one truncation
13+
// notice, from result-truncation-plugin.ts, not one per cap that fired.
14+
expect(outcome?.kind === "partial" ? outcome.notice : "defined").toBeUndefined();
1415
});
1516

1617
test("an over-cap run reports no more than the cap, cut at a line boundary", () => {

src/plugins/rg-output.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export type RgOutcome =
99
| { kind: "output"; stdout: string }
1010
| { kind: "no-match" }
1111
| { kind: "error"; message: string }
12-
| { kind: "partial"; stdout: string; notice: string };
12+
| { kind: "partial"; stdout: string; notice?: string };
1313

1414
export type RgCollector = {
1515
/** Returns an outcome once the cap is breached, otherwise undefined. */
@@ -37,12 +37,15 @@ export function createRgCollector(maxOutputBytes: number): RgCollector {
3737
return outcome;
3838
};
3939

40+
// No notice here: the cap only stops collection early to bound memory
41+
// while the stream is still live. The result-truncation-plugin.ts pass
42+
// that runs over the final tool result is the single place a "truncated"
43+
// notice gets attached, so this cap does not add one of its own.
4044
const overCap = (): RgOutcome | undefined => {
4145
if (stdout.length <= maxOutputBytes) return undefined;
4246
return settle({
4347
kind: "partial",
4448
stdout: truncateToWholeLines(stdout, maxOutputBytes),
45-
notice: `search output exceeded ${maxOutputBytes} bytes — showing partial results; narrow path/glob or pattern`,
4649
});
4750
};
4851

src/plugins/rg-run.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ test("an over-cap run is capped regardless of how stdout is chunked", async () =
4848
expect(result.kind).toBe("partial");
4949
if (result.kind !== "partial") continue;
5050
expect(result.stdout.length).toBeLessThanOrEqual(200);
51-
expect(result.notice).toContain("exceeded 200 bytes");
51+
// No notice of its own: the final tool result gets exactly one
52+
// truncation notice, from result-truncation-plugin.ts.
53+
expect(result.notice).toBeUndefined();
5254
}
5355
});
5456

src/plugins/ripgrep-plugin.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,25 @@ import { MAX_OUTPUT_BYTES, runRg, type RgLimits, type SpawnRg } from "./rg-run.j
2121
const DEFAULT_GREP_MAX = 500;
2222
const DEFAULT_SEARCH_MAX = 1000;
2323

24+
// Caps the number of matches shown; carries no notice of its own. The final
25+
// tool result still passes through result-truncation-plugin.ts, which is the
26+
// single place a "truncated" notice gets attached — a count-based notice
27+
// here would double up with that pass whenever both conditions are true.
2428
function capLines(text: string, max: number): string {
2529
const lines = text.split("\n").filter((line) => line.length > 0);
26-
if (lines.length <= max) return lines.join("\n");
27-
return `${lines.slice(0, max).join("\n")}\n... (showing first ${max} of ${lines.length}+ lines; narrow path/glob)`;
30+
return lines.slice(0, max).join("\n");
2831
}
2932

3033
// Mirrors read_file's truncate-and-offer behavior: a cap or timeout still
3134
// surfaces whatever matches were collected before it fired, instead of
32-
// discarding them behind a bare error.
33-
function partialContent(stdout: string, maxResults: number, notice: string): string {
35+
// discarding them behind a bare error. `notice` is only set for conditions
36+
// result-truncation-plugin.ts can't see, like a run timing out.
37+
function partialContent(stdout: string, maxResults: number, notice?: string): string {
3438
const capped = capLines(stdout, maxResults);
35-
if (capped.length === 0) return `no matches collected before ${notice}`;
36-
return `${capped}\n... ${notice}`;
39+
if (capped.length === 0) {
40+
return notice === undefined ? "no matches collected" : `no matches collected before ${notice}`;
41+
}
42+
return notice === undefined ? capped : `${capped}\n... ${notice}`;
3743
}
3844

3945
// The fallback walker collects its whole result in memory before returning, so

tests/unit/ripgrep-plugin.test.ts

Lines changed: 82 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { tmpdir } from "node:os";
55
import type { ToolCall, ToolResult } from "@intx/types/runtime";
66

77
import { ripgrepPlugin } from "../../src/plugins/ripgrep-plugin.js";
8+
import { resultTruncationPlugin } from "../../src/plugins/result-truncation-plugin.js";
89
import type { RgChild, SpawnRg } from "../../src/plugins/rg-run.js";
910

1011
// Repo root derived from this file, not process.cwd(): these cases search real
@@ -32,6 +33,34 @@ const stalledSpawn: SpawnRg = (): RgChild => ({
3233
kill: () => undefined,
3334
});
3435

36+
// A child whose stdout is scripted directly, bypassing a real `rg` process
37+
// (and its own --max-count filtering) so the byte cap and the line-count cap
38+
// can both be forced to fire on the same run.
39+
function scriptedSpawn(stdout: string, code: number | null): SpawnRg {
40+
return () => {
41+
let onData: ((chunk: unknown) => void) | undefined;
42+
let onClose: ((code: number | null) => void) | undefined;
43+
const child: RgChild = {
44+
pid: undefined,
45+
stdout: {
46+
on: (_event, listener) => {
47+
onData = listener;
48+
},
49+
},
50+
stderr: { on: () => undefined },
51+
on: ((event: string, listener: (arg: never) => void) => {
52+
if (event === "close") onClose = listener as (code: number | null) => void;
53+
}) as RgChild["on"],
54+
kill: () => undefined,
55+
};
56+
queueMicrotask(() => {
57+
onData?.(stdout);
58+
onClose?.(code);
59+
});
60+
return child;
61+
};
62+
}
63+
3564
async function withTempDir(run: (dir: string) => Promise<void>): Promise<void> {
3665
const dir = await mkdtemp(join(tmpdir(), "ripgrep-plugin-"));
3766
try {
@@ -90,8 +119,7 @@ test("grep returns partial matches when the output byte cap is hit", async () =>
90119
);
91120
expect(result.isError).toBeUndefined();
92121
expect(result.content).toContain("match line here");
93-
expect(result.content).toContain("exceeded 200 bytes");
94-
expect(result.content).toContain("narrow path/glob or pattern");
122+
expect(String(result.content).length).toBeLessThanOrEqual(200);
95123
});
96124
});
97125

@@ -117,12 +145,62 @@ test("the output byte cap holds when ripgrep is unavailable", async () => {
117145
);
118146
expect(result.isError).toBeUndefined();
119147
expect(result.content).toContain("match line here");
120-
expect(result.content).toContain("exceeded 200 bytes");
121-
expect(result.content.length).toBeLessThan(400);
148+
expect(String(result.content).length).toBeLessThan(400);
122149
});
123150
});
124151
});
125152

153+
// A grep run that both breaches the byte cap (rg-output.ts) and matches more
154+
// lines than max_results (ripgrep-plugin.ts's own count cap) used to carry
155+
// two notices: capLines added its own "(showing first N of M+ lines...)"
156+
// text on top of whatever the byte-cap breach had already reported, because
157+
// partialContent concatenated both unconditionally. It must report the
158+
// truncation exactly once.
159+
test("a grep result that hits both the byte cap and the match-count cap carries exactly one truncation notice", async () => {
160+
// 400 matched lines emitted directly, bypassing a real `rg` process so
161+
// nothing upstream of ripgrep-plugin.ts pre-limits the line count.
162+
const result = await run(
163+
{ id: "c", name: "grep", arguments: { pattern: "match", path: cwd, max_results: 3 } },
164+
{ maxOutputBytes: 200 },
165+
scriptedSpawn("big.txt:1:match line here\n".repeat(400), 0),
166+
);
167+
168+
expect(result.isError).toBeUndefined();
169+
const content = String(result.content);
170+
// Both grep-specific caps fired (byte cap at 200 bytes, line cap at 3
171+
// matches) but neither attaches its own notice — ripgrep-plugin.ts leaves
172+
// that to result-truncation-plugin.ts, which runs later in the real chain
173+
// and sees the final content. A regression that reintroduces either cap's
174+
// own notice text would fail this.
175+
expect(content.split("\n").length).toBeLessThanOrEqual(3);
176+
expect(content).not.toMatch(/showing first|exceeded \d+ bytes|timed out/);
177+
});
178+
179+
// The same result, run through the full chain (ripgrepPlugin then
180+
// result-truncation-plugin, matching buildCorePosixToolPlugins in
181+
// src/agent/posix-tool-plugins.ts), still carries at most one notice — the
182+
// grep-specific caps stay silent and result-truncation-plugin.ts's char cap
183+
// is the backstop for content that's still oversized after them.
184+
test("a large grep result carries at most one truncation notice through the plugin chain", async () => {
185+
await withTempDir(async (dir) => {
186+
const lines = Array.from({ length: 1000 }, (_, i) => `line ${i} ${"x".repeat(300)}`);
187+
await writeFile(join(dir, "big.txt"), lines.join("\n") + "\n");
188+
189+
const grepHandler = ripgrepPlugin(dir).middleware!(fallback);
190+
const handler = resultTruncationPlugin().middleware!(grepHandler);
191+
const result = await handler(
192+
{ id: "c", name: "grep", arguments: { pattern: "line", path: dir } },
193+
new AbortController().signal,
194+
);
195+
196+
expect(result.isError).toBeUndefined();
197+
const content = String(result.content);
198+
expect(content).toContain("output truncated");
199+
expect(content).not.toContain("showing first");
200+
expect((content.match(/\[output truncated/g) ?? []).length).toBe(1);
201+
});
202+
});
203+
126204
test("grep returns partial matches when the timeout fires", async () => {
127205
const result = await run(
128206
{ id: "c", name: "grep", arguments: { pattern: "e", path: "src" } },

0 commit comments

Comments
 (0)