From 01bceb458be841469a51adaebbe4306e2cbd9de5 Mon Sep 17 00:00:00 2001 From: serenakeyitan Date: Thu, 23 Apr 2026 20:58:01 -0700 Subject: [PATCH] fix(gardener/classifiers): raise DIFF_CAP and filter noise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refs #338. Both classifiers were capping the diff at 20KB — roughly 6% of Haiku 4.5's 200K context window — which truncated typical feature PRs at ~300-400 lines and let lockfile regeneration eat the entire budget. - DIFF_CAP: 20_000 → 200_000 (both anthropic.ts and claude-cli.ts) - DIGEST_BUDGET_BYTES: 30_000 → 100_000 (tree-digest.ts) - New shared diff-filter.ts extracting DIFF_NOISE_PATTERNS from sync.ts. Both classifiers now strip lockfiles, dist/build/out/ coverage/node_modules/__pycache__ hunks, and minified/map/snap artifacts BEFORE applying the byte cap, so the cap bounds real code instead of noise. - Skip the whole Diff section when filtering leaves nothing behind. Total input rises from ~55KB to ~310KB (~38% of Haiku's window), leaving headroom for prompt-caching stability, tokenizer variance, and Anthropic's soft ~180-190K prompt-length threshold. Tests: - New tests/gardener/gardener-diff-filter.test.ts (11 cases) - pnpm typecheck clean - pnpm test: 1203 passed, 0 failed --- .../gardener/engine/classifiers/anthropic.ts | 20 ++- .../gardener/engine/classifiers/claude-cli.ts | 20 ++- .../engine/classifiers/diff-filter.ts | 58 ++++++++ .../engine/classifiers/tree-digest.ts | 2 +- tests/gardener/gardener-diff-filter.test.ts | 138 ++++++++++++++++++ 5 files changed, 221 insertions(+), 17 deletions(-) create mode 100644 src/products/gardener/engine/classifiers/diff-filter.ts create mode 100644 tests/gardener/gardener-diff-filter.test.ts diff --git a/src/products/gardener/engine/classifiers/anthropic.ts b/src/products/gardener/engine/classifiers/anthropic.ts index 21e6da2..89f24b5 100644 --- a/src/products/gardener/engine/classifiers/anthropic.ts +++ b/src/products/gardener/engine/classifiers/anthropic.ts @@ -24,6 +24,7 @@ import type { ClassifyOutput, } from "../comment.js"; import { collectTreeDigest, formatDigest } from "./tree-digest.js"; +import { filterDiffNoise } from "./diff-filter.js"; import { parseVerdictJson, validateAndGroundNodes, @@ -37,7 +38,7 @@ const DEFAULT_MODEL = "claude-haiku-4-5"; const ANTHROPIC_URL = "https://api.anthropic.com/v1/messages"; const ANTHROPIC_VERSION = "2023-06-01"; const MAX_TOKENS = 1024; -const DIFF_CAP = 20_000; +const DIFF_CAP = 200_000; const FETCH_TIMEOUT_MS = 60_000; export interface AnthropicClassifierOptions { @@ -160,14 +161,17 @@ function buildUserPrompt(input: ClassifyInput, digest: string): string { parts.push(input.prView.body); } if (input.diff) { - parts.push(""); - parts.push("## Diff"); - parts.push("```diff"); - parts.push(input.diff.slice(0, DIFF_CAP)); - if (input.diff.length > DIFF_CAP) { - parts.push(`... (truncated, ${input.diff.length - DIFF_CAP} bytes omitted)`); + const filtered = filterDiffNoise(input.diff); + if (filtered.length > 0) { + parts.push(""); + parts.push("## Diff"); + parts.push("```diff"); + parts.push(filtered.slice(0, DIFF_CAP)); + if (filtered.length > DIFF_CAP) { + parts.push(`... (truncated, ${filtered.length - DIFF_CAP} bytes omitted)`); + } + parts.push("```"); } - parts.push("```"); } } else if (input.type === "issue" && input.issueView) { parts.push(`## Issue #${input.issueView.number ?? "?"}: ${input.issueView.title ?? ""}`); diff --git a/src/products/gardener/engine/classifiers/claude-cli.ts b/src/products/gardener/engine/classifiers/claude-cli.ts index 52805b3..3b3aaff 100644 --- a/src/products/gardener/engine/classifiers/claude-cli.ts +++ b/src/products/gardener/engine/classifiers/claude-cli.ts @@ -27,13 +27,14 @@ import type { ClassifyOutput, } from "../comment.js"; import { collectTreeDigest, formatDigest } from "./tree-digest.js"; +import { filterDiffNoise } from "./diff-filter.js"; import { parseVerdictJson, validateAndGroundNodes, } from "./verdict-parse.js"; const DEFAULT_MODEL = "claude-haiku-4-5"; -const DIFF_CAP = 20_000; +const DIFF_CAP = 200_000; const SPAWN_TIMEOUT_MS = 90_000; export type ClaudeCliFailureKind = @@ -272,14 +273,17 @@ function buildPrompt(input: ClassifyInput, digest: string): string { parts.push(input.prView.body); } if (input.diff) { - parts.push(""); - parts.push("## Diff"); - parts.push("```diff"); - parts.push(input.diff.slice(0, DIFF_CAP)); - if (input.diff.length > DIFF_CAP) { - parts.push(`... (truncated, ${input.diff.length - DIFF_CAP} bytes omitted)`); + const filtered = filterDiffNoise(input.diff); + if (filtered.length > 0) { + parts.push(""); + parts.push("## Diff"); + parts.push("```diff"); + parts.push(filtered.slice(0, DIFF_CAP)); + if (filtered.length > DIFF_CAP) { + parts.push(`... (truncated, ${filtered.length - DIFF_CAP} bytes omitted)`); + } + parts.push("```"); } - parts.push("```"); } } else if (input.type === "issue" && input.issueView) { parts.push(`## Issue #${input.issueView.number ?? "?"}: ${input.issueView.title ?? ""}`); diff --git a/src/products/gardener/engine/classifiers/diff-filter.ts b/src/products/gardener/engine/classifiers/diff-filter.ts new file mode 100644 index 0000000..98df827 --- /dev/null +++ b/src/products/gardener/engine/classifiers/diff-filter.ts @@ -0,0 +1,58 @@ +/** + * Shared noise filter for PR diffs fed to classifiers. + * + * A PR that regenerates `pnpm-lock.yaml` or ships minified build output + * can blow past the classifier's byte cap before any real-code hunk + * reaches the model. Filter those files out of the diff BEFORE applying + * the cap, so the cap bounds real code instead of noise. + * + * Regex list originated in `src/products/tree/engine/sync.ts` (the + * `DIFF_NOISE_PATTERNS` constant used by `formatPrDiffForPrompt`) — + * extracted here so sync and both classifiers share one source of truth. + */ + +export const DIFF_NOISE_PATTERNS: readonly RegExp[] = [ + /(^|\/)(?:package-lock\.json|pnpm-lock\.yaml|yarn\.lock|Cargo\.lock|poetry\.lock|Gemfile\.lock)$/, + /(^|\/)(?:dist|build|out|coverage|node_modules|__pycache__)\//, + /\.(?:lock|min\.js|min\.css|map|snap)$/, +]; + +/** True when `filename` matches any noise pattern. */ +export function isDiffNoise(filename: string): boolean { + return DIFF_NOISE_PATTERNS.some((re) => re.test(filename)); +} + +/** + * Strip noise-file hunks from a unified-diff text. The input is the raw + * output of `gh pr diff` (or equivalent) — a concatenation of per-file + * hunks each starting with `diff --git a/ b/`. + * + * The parser is deliberately simple: it splits on the `diff --git` + * marker, extracts the `b/` target filename from each hunk header, + * and drops the whole hunk if that path is noise. Anything before the + * first `diff --git` marker (rare — usually empty) is preserved as-is. + * + * Malformed hunks that don't yield a parseable filename are kept by + * default: we'd rather show too much than silently drop a real change. + */ +export function filterDiffNoise(diff: string): string { + if (diff === "") return diff; + + // Split keeping the delimiter at the start of each segment. The first + // segment is anything before the first `diff --git` (usually empty). + const parts = diff.split(/(?=^diff --git )/m); + const kept: string[] = []; + for (const part of parts) { + if (!part.startsWith("diff --git ")) { + if (part.length > 0) kept.push(part); + continue; + } + const header = part.slice(0, part.indexOf("\n")); + // `diff --git a/ b/` — pull the b-side path. + const match = header.match(/^diff --git a\/.+ b\/(.+)$/); + const filename = match?.[1]?.trim(); + if (filename && isDiffNoise(filename)) continue; + kept.push(part); + } + return kept.join(""); +} diff --git a/src/products/gardener/engine/classifiers/tree-digest.ts b/src/products/gardener/engine/classifiers/tree-digest.ts index ee04a6d..29da0c1 100644 --- a/src/products/gardener/engine/classifiers/tree-digest.ts +++ b/src/products/gardener/engine/classifiers/tree-digest.ts @@ -22,7 +22,7 @@ export interface TreeNodeEntry { summary: string; } -const DIGEST_BUDGET_BYTES = 30_000; +const DIGEST_BUDGET_BYTES = 100_000; const PER_NODE_SUMMARY_CAP = 400; const SKIP_DIRS = new Set([ ".git", diff --git a/tests/gardener/gardener-diff-filter.test.ts b/tests/gardener/gardener-diff-filter.test.ts new file mode 100644 index 0000000..ce673b5 --- /dev/null +++ b/tests/gardener/gardener-diff-filter.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, it } from "vitest"; +import { + DIFF_NOISE_PATTERNS, + filterDiffNoise, + isDiffNoise, +} from "#products/gardener/engine/classifiers/diff-filter.js"; + +describe("isDiffNoise", () => { + it("flags lockfiles at root and in subdirs", () => { + expect(isDiffNoise("pnpm-lock.yaml")).toBe(true); + expect(isDiffNoise("apps/web/package-lock.json")).toBe(true); + expect(isDiffNoise("rust/Cargo.lock")).toBe(true); + expect(isDiffNoise("py/poetry.lock")).toBe(true); + }); + + it("flags build output dirs", () => { + expect(isDiffNoise("dist/index.js")).toBe(true); + expect(isDiffNoise("build/output.js")).toBe(true); + expect(isDiffNoise("coverage/lcov.info")).toBe(true); + expect(isDiffNoise("node_modules/foo/index.js")).toBe(true); + expect(isDiffNoise("__pycache__/mod.cpython-311.pyc")).toBe(true); + }); + + it("flags minified and map artifacts", () => { + expect(isDiffNoise("vendor/jquery.min.js")).toBe(true); + expect(isDiffNoise("styles.min.css")).toBe(true); + expect(isDiffNoise("bundle.js.map")).toBe(true); + expect(isDiffNoise("snapshots/foo.snap")).toBe(true); + }); + + it("does not flag real source files", () => { + expect(isDiffNoise("src/index.ts")).toBe(false); + expect(isDiffNoise("README.md")).toBe(false); + expect(isDiffNoise("apps/web/src/App.tsx")).toBe(false); + // A file literally named "lock.ts" should NOT match — the .lock + // pattern requires the extension, not a substring. + expect(isDiffNoise("lock.ts")).toBe(false); + }); + + it("exports a non-empty pattern list", () => { + expect(DIFF_NOISE_PATTERNS.length).toBeGreaterThan(0); + }); +}); + +describe("filterDiffNoise", () => { + it("returns empty diff untouched", () => { + expect(filterDiffNoise("")).toBe(""); + }); + + it("drops a lockfile hunk while keeping real-code hunks", () => { + const diff = [ + "diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml", + "index 111..222 100644", + "--- a/pnpm-lock.yaml", + "+++ b/pnpm-lock.yaml", + "@@ -1,1 +1,1 @@", + "-old", + "+new", + "diff --git a/src/index.ts b/src/index.ts", + "index 333..444 100644", + "--- a/src/index.ts", + "+++ b/src/index.ts", + "@@ -1,1 +1,1 @@", + "-export const x = 1;", + "+export const x = 2;", + "", + ].join("\n"); + const out = filterDiffNoise(diff); + expect(out).not.toContain("pnpm-lock.yaml"); + expect(out).toContain("src/index.ts"); + expect(out).toContain("export const x = 2;"); + }); + + it("drops dist/ hunks", () => { + const diff = [ + "diff --git a/dist/bundle.js b/dist/bundle.js", + "@@ -1 +1 @@", + "-a", + "+b", + "diff --git a/src/app.ts b/src/app.ts", + "@@ -1 +1 @@", + "-c", + "+d", + "", + ].join("\n"); + const out = filterDiffNoise(diff); + expect(out).not.toContain("dist/bundle.js"); + expect(out).toContain("src/app.ts"); + }); + + it("keeps entire diff when nothing is noise", () => { + const diff = [ + "diff --git a/src/a.ts b/src/a.ts", + "@@ -1 +1 @@", + "-x", + "+y", + "diff --git a/src/b.ts b/src/b.ts", + "@@ -1 +1 @@", + "-m", + "+n", + "", + ].join("\n"); + const out = filterDiffNoise(diff); + expect(out).toBe(diff); + }); + + it("returns empty when every hunk is noise", () => { + const diff = [ + "diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml", + "@@ -1 +1 @@", + "-a", + "+b", + "diff --git a/dist/out.js b/dist/out.js", + "@@ -1 +1 @@", + "-c", + "+d", + "", + ].join("\n"); + const out = filterDiffNoise(diff); + expect(out.trim()).toBe(""); + }); + + it("preserves hunks with unparseable headers (fail-open)", () => { + const diff = [ + "diff --git malformed-header-no-paths", + "some body", + "diff --git a/src/ok.ts b/src/ok.ts", + "@@ -1 +1 @@", + "-x", + "+y", + "", + ].join("\n"); + const out = filterDiffNoise(diff); + // malformed hunk should NOT be silently dropped + expect(out).toContain("malformed-header-no-paths"); + expect(out).toContain("src/ok.ts"); + }); +});