diff --git a/src/review/review-grounding.ts b/src/review/review-grounding.ts index d5ce8e53dd..378efaafec 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|svg|ico|pdf|lock|min\.js|min\.css|map|woff2?|ttf|eot|mp4|webm|zip|gz|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)$/i; /** The grounding feature flags (subset of reviewbot's FeatureToggles). */ export interface GroundingFlags { diff --git a/test/unit/review-grounding.test.ts b/test/unit/review-grounding.test.ts index bbd6f31ec8..14b7ec8d8d 100644 --- a/test/unit/review-grounding.test.ts +++ b/test/unit/review-grounding.test.ts @@ -152,16 +152,36 @@ describe("review-grounding: fetchFullFileContents (injected FileFetcher, fail-sa }); it("inlines readable files, skips removed/binary, orders source first", async () => { - const fetcher = fetcherFrom({ "src/a.ts": "export const a = 1;", "README.md": "# docs" }); + const reads: string[] = []; + const fetcher: FileFetcher = { + getFileContent: async (path) => { + reads.push(path); + if (path === "src/a.ts") return "export const a = 1;"; + if (path === "README.md") return "# docs"; + return "SHOULD_NOT_FETCH"; + }, + }; + const binary = ["logo.png", "assets/photo.avif", "assets/poster.bmp", "assets/icon.heic", "dist/pkg.tgz"]; const out = await fetchFullFileContents( { ciGrounding: false, fullFileContext: true }, "sha", - files(["README.md"], ["src/a.ts"], ["logo.png"], ["old.ts", "removed"]), + files( + ["README.md"], + ["src/a.ts"], + ["logo.png"], + ["assets/photo.avif"], + ["assets/poster.bmp"], + ["assets/icon.heic"], + ["dist/pkg.tgz"], + ["old.ts", "removed"], + ), fetcher, ); expect(out).toBeDefined(); - // source (priority 0) before docs (priority 2); png + removed excluded + // source (priority 0) before docs (priority 2); binary + removed excluded before fetch expect(out?.map((f) => f.path)).toEqual(["src/a.ts", "README.md"]); + expect(reads).toEqual(["src/a.ts", "README.md"]); + for (const path of binary) expect(reads).not.toContain(path); }); it("degrades to skipping a file when the fetcher throws (never throws itself)", async () => {