From aa8fa00c4e1ebb969963fd2ff1711743035320d0 Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 19 May 2026 05:30:59 +0200 Subject: [PATCH] fix(coding-agent/tools): capped AST parse errors and preserved total counts - Added capParseErrors in shared render utilities and updated ast_grep and ast_edit to return capped parseErrors plus parseErrorsTotal. - Threaded the preserved totals into parse-error formatting and renderer output so labels and overflow counts report the full number of issues. - Added an ast_grep test asserting parse errors are capped at PARSE_ERRORS_LIMIT while parseErrorsTotal retains the original count. --- packages/coding-agent/CHANGELOG.md | 4 ++ packages/coding-agent/src/tools/ast-edit.ts | 30 +++++++++------ packages/coding-agent/src/tools/ast-grep.ts | 24 +++++++----- .../coding-agent/src/tools/render-utils.ts | 38 +++++++++++++------ .../coding-agent/test/tools/ast-grep.test.ts | 30 +++++++++++++++ 5 files changed, 93 insertions(+), 33 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index dabf0fd39..a6bb283fe 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `ast_grep` and `ast_edit` tool details retaining every per-file parse error — a scan over hundreds of files with syntax-error nodes inflated `details.parseErrors` to one entry per file, leaking into traces and the renderer's "X more" overflow. Errors are now capped at `PARSE_ERRORS_LIMIT` (20) at the source, with the original total preserved in a new `parseErrorsTotal` field for accurate count labels. + ## [15.1.4] - 2026-05-19 ### Fixed diff --git a/packages/coding-agent/src/tools/ast-edit.ts b/packages/coding-agent/src/tools/ast-edit.ts index fdaa0fa7e..7e27253d1 100644 --- a/packages/coding-agent/src/tools/ast-edit.ts +++ b/packages/coding-agent/src/tools/ast-edit.ts @@ -18,8 +18,8 @@ import type { OutputMeta } from "./output-meta"; import { resolveToolSearchScope } from "./path-utils"; import { appendParseErrorsBulletList, + capParseErrors, createCachedComponent, - dedupeParseErrors, formatCodeFrameLine, formatCount, formatEmptyMessage, @@ -146,6 +146,8 @@ export interface AstEditToolDetails { applied: boolean; limitReached: boolean; parseErrors?: string[]; + /** Total parse error count before {@link PARSE_ERRORS_LIMIT} capping. Omitted when no errors. */ + parseErrorsTotal?: number; scopePath?: string; files?: string[]; fileReplacements?: Array<{ path: string; count: number }>; @@ -210,7 +212,7 @@ export class AstEditTool implements AgentTool formatResultPath(filePath, isDirectory, resolvedSearchPath, this.session.cwd); @@ -237,15 +239,15 @@ export class AstEditTool implements AgentTool 0 ? { parseErrors: dedupedParseErrors } : {}), + ...(cappedParseErrors.length > 0 ? { parseErrors: cappedParseErrors, parseErrorsTotal } : {}), scopePath, files: fileList, fileReplacements: [], }; if (result.totalReplacements === 0) { - const parseMessage = dedupedParseErrors.length - ? `\n${formatParseErrors(dedupedParseErrors).join("\n")}` + const parseMessage = cappedParseErrors.length + ? `\n${formatParseErrors(cappedParseErrors, parseErrorsTotal).join("\n")}` : ""; return toolResult(baseDetails).text(`No replacements made${parseMessage}`).done(); } @@ -308,8 +310,8 @@ export class AstEditTool implements AgentTool(); for (const fileChange of applyResult.fileChanges) { @@ -350,7 +354,9 @@ export class AstEditTool implements AgentTool 0 ? { parseErrors: dedupedApplyParseErrors } : {}), + ...(cappedApplyParseErrors.length > 0 + ? { parseErrors: cappedApplyParseErrors, parseErrorsTotal: applyParseErrorsTotal } + : {}), scopePath, files: appliedFileList, fileReplacements: appliedFileReplacements, @@ -441,7 +447,7 @@ export const astEditToolRenderer = { if (filesSearched > 0) meta.push(`searched ${filesSearched}`); const header = renderStatusLine({ icon: "warning", title: "AST Edit", description, meta }, uiTheme); const lines = [header, formatEmptyMessage("No replacements made", uiTheme)]; - appendParseErrorsBulletList(lines, details?.parseErrors, uiTheme); + appendParseErrorsBulletList(lines, details?.parseErrors, uiTheme, details?.parseErrorsTotal); return new Text(lines.join("\n"), 0, 0); } @@ -470,7 +476,9 @@ export const astEditToolRenderer = { extraLines.push(uiTheme.fg("warning", "limit reached; narrow path")); } if (details?.parseErrors?.length) { - extraLines.push(uiTheme.fg("warning", formatParseErrorsCountLabel(details.parseErrors))); + extraLines.push( + uiTheme.fg("warning", formatParseErrorsCountLabel(details.parseErrors, details.parseErrorsTotal)), + ); } return createCachedComponent( () => options.expanded, diff --git a/packages/coding-agent/src/tools/ast-grep.ts b/packages/coding-agent/src/tools/ast-grep.ts index 01914cad1..0c87b4f47 100644 --- a/packages/coding-agent/src/tools/ast-grep.ts +++ b/packages/coding-agent/src/tools/ast-grep.ts @@ -18,8 +18,8 @@ import type { OutputMeta } from "./output-meta"; import { resolveToolSearchScope } from "./path-utils"; import { appendParseErrorsBulletList, + capParseErrors, createCachedComponent, - dedupeParseErrors, formatCodeFrameLine, formatCount, formatEmptyMessage, @@ -104,6 +104,8 @@ export interface AstGrepToolDetails { filesSearched: number; limitReached: boolean; parseErrors?: string[]; + /** Total parse error count before {@link PARSE_ERRORS_LIMIT} capping. Omitted when no errors. */ + parseErrorsTotal?: number; scopePath?: string; files?: string[]; fileMatches?: Array<{ path: string; count: number }>; @@ -172,7 +174,7 @@ export class AstGrepTool implements AgentTool formatResultPath(filePath, isDirectory, resolvedSearchPath, this.session.cwd); @@ -193,18 +195,18 @@ export class AstGrepTool implements AgentTool 0 ? { parseErrors: dedupedParseErrors } : {}), + ...(cappedParseErrors.length > 0 ? { parseErrors: cappedParseErrors, parseErrorsTotal } : {}), scopePath, files: fileList, fileMatches: [], }; if (result.matches.length === 0) { - const noMatchMessage = dedupedParseErrors.length + const noMatchMessage = cappedParseErrors.length ? "No matches found. Parse issues mean the query may be mis-scoped; narrow `paths` before concluding absence." : "No matches found"; - const parseMessage = dedupedParseErrors.length - ? `\n${formatParseErrors(dedupedParseErrors).join("\n")}` + const parseMessage = cappedParseErrors.length + ? `\n${formatParseErrors(cappedParseErrors, parseErrorsTotal).join("\n")}` : ""; return toolResult(baseDetails).text(`${noMatchMessage}${parseMessage}`).done(); } @@ -269,8 +271,8 @@ export class AstGrepTool implements AgentTool PARSE_ERRORS_LIMIT - ? `Parse issues (${PARSE_ERRORS_LIMIT} / ${deduped.length}):` - : "Parse issues:"; + const header = fullCount > capped.length ? `Parse issues (${capped.length} / ${fullCount}):` : "Parse issues:"; return [header, ...capped.map(err => `- ${err}`)]; } +/** + * Cap an upstream parse-error list to {@link PARSE_ERRORS_LIMIT} unique entries, + * preserving the original deduplicated total. Use this at the source so tool + * details never carry thousands of per-file parse errors into traces or + * renderers. + */ +export function capParseErrors( + errors: string[] | undefined, + limit: number = PARSE_ERRORS_LIMIT, +): { errors: string[]; total: number } { + const deduped = dedupeParseErrors(errors); + return { errors: deduped.slice(0, limit), total: deduped.length }; +} + // ============================================================================= // Renderer helpers shared by search / find / ast tools // ============================================================================= @@ -712,14 +724,16 @@ export function appendParseErrorsBulletList( lines: string[], parseErrors: readonly string[] | undefined, theme: Theme, + total?: number, ): void { if (!parseErrors || parseErrors.length === 0) return; + const fullCount = total ?? parseErrors.length; const capped = parseErrors.slice(0, PARSE_ERRORS_LIMIT); for (const err of capped) { lines.push(theme.fg("warning", ` - ${err}`)); } - if (parseErrors.length > PARSE_ERRORS_LIMIT) { - lines.push(theme.fg("dim", ` … ${parseErrors.length - PARSE_ERRORS_LIMIT} more`)); + if (fullCount > capped.length) { + lines.push(theme.fg("dim", ` … ${fullCount - capped.length} more`)); } } @@ -727,11 +741,11 @@ export function appendParseErrorsBulletList( * Human-readable summary string for the parse-issues count, capped by * {@link PARSE_ERRORS_LIMIT}. */ -export function formatParseErrorsCountLabel(parseErrors: readonly string[]): string { - const total = parseErrors.length; - return total > PARSE_ERRORS_LIMIT - ? `${PARSE_ERRORS_LIMIT} / ${total} parse issues` - : `${total} parse issue${total !== 1 ? "s" : ""}`; +export function formatParseErrorsCountLabel(parseErrors: readonly string[], total?: number): string { + const fullCount = total ?? parseErrors.length; + return fullCount > PARSE_ERRORS_LIMIT + ? `${PARSE_ERRORS_LIMIT} / ${fullCount} parse issues` + : `${fullCount} parse issue${fullCount !== 1 ? "s" : ""}`; } // ============================================================================= diff --git a/packages/coding-agent/test/tools/ast-grep.test.ts b/packages/coding-agent/test/tools/ast-grep.test.ts index d66cc7aa2..49b4cbc6e 100644 --- a/packages/coding-agent/test/tools/ast-grep.test.ts +++ b/packages/coding-agent/test/tools/ast-grep.test.ts @@ -46,6 +46,36 @@ describe("ast_grep parse errors", () => { await fs.rm(tempDir, { recursive: true, force: true }); } }); + it("caps parseErrors at PARSE_ERRORS_LIMIT and records the original total", async () => { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "ast-grep-parse-cap-")); + try { + const fileCount = 35; + for (let i = 0; i < fileCount; i++) { + await Bun.write(path.join(tempDir, `broken-${i}.ts`), "export function broken( { return 1; }"); + } + + const tools = await createTools(createTestSession(tempDir)); + const tool = tools.find(entry => entry.name === "ast_grep"); + expect(tool).toBeDefined(); + + const result = await tool!.execute("ast-grep-parse-cap", { + pat: "someUnlikelyCall($A)", + paths: [tempDir], + }); + + const text = result.content.find(content => content.type === "text")?.text ?? ""; + const details = result.details as + | { parseErrors?: string[]; parseErrorsTotal?: number; matchCount?: number } + | undefined; + + expect(details?.matchCount).toBe(0); + expect(details?.parseErrors?.length).toBe(20); + expect(details?.parseErrorsTotal).toBe(fileCount); + expect(text).toContain(`Parse issues (20 / ${fileCount}):`); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); it("combines globbing from path and glob parameters", async () => { const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "ast-grep-glob-")); try {