Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/review/review-grounding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
26 changes: 23 additions & 3 deletions test/unit/review-grounding.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Loading