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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<typeof astEditSchema, AstEditToolD
|
||||
signal,
|
||||
});
|
||||
|
||||
const dedupedParseErrors = dedupeParseErrors(result.parseErrors);
|
||||
const { errors: cappedParseErrors, total: parseErrorsTotal } = capParseErrors(result.parseErrors);
|
||||
const formatPath = (filePath: string): string =>
|
||||
formatResultPath(filePath, isDirectory, resolvedSearchPath, this.session.cwd);
|
||||
|
||||
@@ -237,15 +239,15 @@ export class AstEditTool implements AgentTool<typeof astEditSchema, AstEditToolD
|
||||
filesSearched: result.filesSearched,
|
||||
applied: result.applied,
|
||||
limitReached: result.limitReached,
|
||||
...(dedupedParseErrors.length > 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<typeof astEditSchema, AstEditToolD
|
||||
if (result.limitReached) {
|
||||
outputLines.push("", "Limit reached; narrow paths.");
|
||||
}
|
||||
if (dedupedParseErrors.length) {
|
||||
outputLines.push("", ...formatParseErrors(dedupedParseErrors));
|
||||
if (cappedParseErrors.length) {
|
||||
outputLines.push("", ...formatParseErrors(cappedParseErrors, parseErrorsTotal));
|
||||
}
|
||||
|
||||
// Register pending action so `resolve` can apply or discard these previewed changes
|
||||
@@ -326,7 +328,9 @@ export class AstEditTool implements AgentTool<typeof astEditSchema, AstEditToolD
|
||||
maxFiles,
|
||||
failOnParseError: false,
|
||||
});
|
||||
const dedupedApplyParseErrors = dedupeParseErrors(applyResult.parseErrors);
|
||||
const { errors: cappedApplyParseErrors, total: applyParseErrorsTotal } = capParseErrors(
|
||||
applyResult.parseErrors,
|
||||
);
|
||||
const { record: recordAppliedFile, list: appliedFileList } = createFileRecorder();
|
||||
const appliedFileReplacementCounts = new Map<string, number>();
|
||||
for (const fileChange of applyResult.fileChanges) {
|
||||
@@ -350,7 +354,9 @@ export class AstEditTool implements AgentTool<typeof astEditSchema, AstEditToolD
|
||||
filesSearched: applyResult.filesSearched,
|
||||
applied: applyResult.applied,
|
||||
limitReached: applyResult.limitReached,
|
||||
...(dedupedApplyParseErrors.length > 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,
|
||||
|
||||
@@ -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<typeof astGrepSchema, AstGrepToolD
|
||||
const parseError = error.match(/^.+: (.+: parse error \(syntax tree contains error nodes\))$/);
|
||||
return parseError?.[1] ?? error;
|
||||
});
|
||||
const dedupedParseErrors = dedupeParseErrors(normalizedParseErrors);
|
||||
const { errors: cappedParseErrors, total: parseErrorsTotal } = capParseErrors(normalizedParseErrors);
|
||||
const formatPath = (filePath: string): string =>
|
||||
formatResultPath(filePath, isDirectory, resolvedSearchPath, this.session.cwd);
|
||||
|
||||
@@ -193,18 +195,18 @@ export class AstGrepTool implements AgentTool<typeof astGrepSchema, AstGrepToolD
|
||||
fileCount: result.filesWithMatches,
|
||||
filesSearched: result.filesSearched,
|
||||
limitReached: result.limitReached,
|
||||
...(dedupedParseErrors.length > 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<typeof astGrepSchema, AstGrepToolD
|
||||
if (result.limitReached) {
|
||||
outputLines.push("", "Result limit reached; narrow paths or increase limit.");
|
||||
}
|
||||
if (dedupedParseErrors.length) {
|
||||
outputLines.push("", ...formatParseErrors(dedupedParseErrors));
|
||||
if (cappedParseErrors.length) {
|
||||
outputLines.push("", ...formatParseErrors(cappedParseErrors, parseErrorsTotal));
|
||||
}
|
||||
|
||||
return toolResult(details).text(outputLines.join("\n")).done();
|
||||
@@ -329,7 +331,7 @@ export const astGrepToolRenderer = {
|
||||
const lines = [header, formatEmptyMessage("No matches found", uiTheme)];
|
||||
if (details?.parseErrors?.length) {
|
||||
lines.push(uiTheme.fg("warning", "Query may be mis-scoped; narrow `paths` before concluding absence"));
|
||||
appendParseErrorsBulletList(lines, details.parseErrors, uiTheme);
|
||||
appendParseErrorsBulletList(lines, details.parseErrors, uiTheme, details.parseErrorsTotal);
|
||||
}
|
||||
return new Text(lines.join("\n"), 0, 0);
|
||||
}
|
||||
@@ -356,7 +358,9 @@ export const astGrepToolRenderer = {
|
||||
extraLines.push(uiTheme.fg("warning", "limit reached; narrow paths or increase limit"));
|
||||
}
|
||||
if (details?.parseErrors?.length) {
|
||||
extraLines.push(uiTheme.fg("warning", formatParseErrorsCountLabel(details.parseErrors)));
|
||||
extraLines.push(
|
||||
uiTheme.fg("warning", formatParseErrorsCountLabel(details.parseErrors, details.parseErrorsTotal)),
|
||||
);
|
||||
}
|
||||
|
||||
return createCachedComponent(
|
||||
|
||||
@@ -633,17 +633,29 @@ export function dedupeParseErrors(errors: string[] | undefined): string[] {
|
||||
return deduped;
|
||||
}
|
||||
|
||||
export function formatParseErrors(errors: string[]): string[] {
|
||||
export function formatParseErrors(errors: string[], total?: number): string[] {
|
||||
const deduped = dedupeParseErrors(errors);
|
||||
if (deduped.length === 0) return [];
|
||||
const fullCount = total ?? deduped.length;
|
||||
const capped = deduped.slice(0, PARSE_ERRORS_LIMIT);
|
||||
const header =
|
||||
deduped.length > 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" : ""}`;
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user