diff --git a/src/review/review-diff.ts b/src/review/review-diff.ts index e3ebd8d211..21b5e908a9 100644 --- a/src/review/review-diff.ts +++ b/src/review/review-diff.ts @@ -20,7 +20,7 @@ export const DEFAULT_DIFF_BUDGET = 80_000; * Cypress/Playwright `.cy`/`.e2e`, a bare `spec/` dir), so those tests were ranked as SOURCE(0) and * could displace real source under a tight budget — the exact opposite of this function's job. */ export function diffFilePriority(path: string): number { - if (/(^|\/)(package-lock\.json|npm-shrinkwrap\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lock|bun\.lockb|cargo\.lock|poetry\.lock|pipfile\.lock|composer\.lock|gemfile\.lock|go\.sum|go\.work\.sum|uv\.lock|packages\.lock\.json|flake\.lock|deno\.lock|pubspec\.lock|podfile\.lock|mix\.lock|package\.resolved|gradle\.lockfile|pdm\.lock|conan\.lock|pixi\.lock|cartfile\.resolved|gopkg\.lock|shard\.lock|rebar\.lock|renv\.lock|chart\.lock)$|\.(min\.(js|css)|map|snap)$/i.test(path)) return 4; + if (/(^|\/)(package-lock\.json|npm-shrinkwrap\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lock|bun\.lockb|cargo\.lock|poetry\.lock|pipfile\.lock|composer\.lock|gemfile\.lock|go\.sum|go\.work\.sum|uv\.lock|packages\.lock\.json|flake\.lock|deno\.lock|pubspec\.lock|podfile\.lock|mix\.lock|package\.resolved|gradle\.lockfile|pdm\.lock|conan\.lock|pixi\.lock|cartfile\.resolved|gopkg\.lock|shard\.lock|rebar\.lock|renv\.lock|chart\.lock)$|\.(min\.(js|css)|map|snap|class|jar|pyc|pyo)$/i.test(path)) return 4; if (/(^|\/)(dist|build|out|coverage|vendor|node_modules)\//i.test(path)) return 4; if (/\.(md|mdx|markdown|rst|adoc|asciidoc|txt)$/i.test(path)) return 2; if (isTestPath(path)) return 1; diff --git a/src/review/review-grounding.ts b/src/review/review-grounding.ts index 378efaafec..bfe1abd0fd 100644 --- a/src/review/review-grounding.ts +++ b/src/review/review-grounding.ts @@ -51,7 +51,7 @@ export interface ReviewGrounding { const FILE_CONTENT_BUDGET = 60_000; // total chars inlined across all changed files const MAX_SINGLE_FILE = 24_000; // a file larger than this is marked truncated (review it from the diff) // Binary / generated / lockfile paths carry no review signal as full text — skip inlining them. -const SKIP_EXT = /\.(png|jpe?g|gif|webp|avif|bmp|heic|svg|ico|pdf|lock|min\.js|min\.css|map|woff2?|ttf|eot|mp4|webm|zip|gz|tgz|wasm)$/i; +const SKIP_EXT = /\.(png|jpe?g|gif|webp|avif|bmp|heic|svg|ico|pdf|lock|min\.js|min\.css|map|woff2?|ttf|eot|mp4|webm|zip|gz|tgz|wasm|class|jar|pyc|pyo)$/i; /** The grounding feature flags (subset of reviewbot's FeatureToggles). */ export interface GroundingFlags { @@ -110,7 +110,7 @@ export function buildGrounding(f: GroundingFlags, checks?: CheckAggregate, fileC * copy missed pytest `test_*.py`, Go `*_test.go`, Ruby `*_spec.rb`, Cypress/Playwright `.cy`/`.e2e`, and a * bare `spec/` dir — so those tests ranked as SOURCE(0) and were inlined ahead of real source). */ export function diffFilePriority(path: string): number { - if (/(^|\/)(package-lock\.json|npm-shrinkwrap\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lock|bun\.lockb|cargo\.lock|poetry\.lock|pipfile\.lock|composer\.lock|gemfile\.lock|go\.sum|go\.work\.sum|uv\.lock|packages\.lock\.json|flake\.lock|deno\.lock|pubspec\.lock|podfile\.lock|mix\.lock|package\.resolved|gradle\.lockfile|pdm\.lock|conan\.lock|pixi\.lock|cartfile\.resolved|gopkg\.lock|shard\.lock|rebar\.lock|renv\.lock|chart\.lock)$|\.(min\.(js|css)|map|snap)$/i.test(path)) return 4; + if (/(^|\/)(package-lock\.json|npm-shrinkwrap\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lock|bun\.lockb|cargo\.lock|poetry\.lock|pipfile\.lock|composer\.lock|gemfile\.lock|go\.sum|go\.work\.sum|uv\.lock|packages\.lock\.json|flake\.lock|deno\.lock|pubspec\.lock|podfile\.lock|mix\.lock|package\.resolved|gradle\.lockfile|pdm\.lock|conan\.lock|pixi\.lock|cartfile\.resolved|gopkg\.lock|shard\.lock|rebar\.lock|renv\.lock|chart\.lock)$|\.(min\.(js|css)|map|snap|class|jar|pyc|pyo)$/i.test(path)) return 4; if (/(^|\/)(dist|build|out|coverage|vendor|node_modules)\//i.test(path)) return 4; if (/\.(md|mdx|markdown|rst|adoc|asciidoc|txt)$/i.test(path)) return 2; if (isTestPath(path)) return 1; diff --git a/test/unit/review-diff.test.ts b/test/unit/review-diff.test.ts index 31b35b8746..29e61b6090 100644 --- a/test/unit/review-diff.test.ts +++ b/test/unit/review-diff.test.ts @@ -18,6 +18,13 @@ describe("diffFilePriority — source survives, noise drops first", () => { } }); + it("ranks bytecode and archive artifacts as noise(4), not source(0)", () => { + for (const path of ["build/App.class", "lib/service.jar", "__pycache__/mod.pyc", "gen/stub.pyo"]) { + expect(diffFilePriority(path)).toBe(4); + expect(diffFilePriority(path)).toBeGreaterThan(diffFilePriority("src/a.ts")); + } + }); + it("ranks every path-matchers lockfile as noise(4), not source(0)", () => { for (const path of ["bun.lock", "uv.lock", "deno.lock", "flake.lock", "mix.lock", "chart.lock"]) { expect(diffFilePriority(path)).toBe(4); diff --git a/test/unit/review-grounding.test.ts b/test/unit/review-grounding.test.ts index 14b7ec8d8d..9438944e03 100644 --- a/test/unit/review-grounding.test.ts +++ b/test/unit/review-grounding.test.ts @@ -102,6 +102,13 @@ describe("review-grounding: diffFilePriority (source survives the budget first)" expect(diffFilePriority("src/a.ts")).toBeLessThan(diffFilePriority("README.md")); }); + it("ranks bytecode and archive artifacts as noise(4), not source(0)", () => { + for (const path of ["build/App.class", "lib/service.jar", "__pycache__/mod.pyc", "gen/stub.pyo"]) { + expect(diffFilePriority(path)).toBe(4); + expect(diffFilePriority(path)).toBeGreaterThan(diffFilePriority("src/a.ts")); + } + }); + it("ranks every path-matchers lockfile as noise(4), not source(0)", () => { for (const path of ["bun.lock", "uv.lock", "deno.lock", "flake.lock", "mix.lock", "chart.lock"]) { expect(diffFilePriority(path)).toBe(4); @@ -161,7 +168,17 @@ describe("review-grounding: fetchFullFileContents (injected FileFetcher, fail-sa return "SHOULD_NOT_FETCH"; }, }; - const binary = ["logo.png", "assets/photo.avif", "assets/poster.bmp", "assets/icon.heic", "dist/pkg.tgz"]; + const binary = [ + "logo.png", + "assets/photo.avif", + "assets/poster.bmp", + "assets/icon.heic", + "dist/pkg.tgz", + "build/App.class", + "lib/service.jar", + "__pycache__/mod.pyc", + "gen/stub.pyo", + ]; const out = await fetchFullFileContents( { ciGrounding: false, fullFileContext: true }, "sha", @@ -173,6 +190,10 @@ describe("review-grounding: fetchFullFileContents (injected FileFetcher, fail-sa ["assets/poster.bmp"], ["assets/icon.heic"], ["dist/pkg.tgz"], + ["build/App.class"], + ["lib/service.jar"], + ["__pycache__/mod.pyc"], + ["gen/stub.pyo"], ["old.ts", "removed"], ), fetcher,