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
50 changes: 33 additions & 17 deletions review-enrichment/src/analyzers/asset-weight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ const BINARY_EXTS = new Set([
"ico",
"webp",
"avif",
"heic",
"heif",
"woff",
"woff2",
"ttf",
Expand Down Expand Up @@ -134,8 +136,10 @@ async function fetchGithubJson<T>(
diagnostics: options.diagnostics,
phase: "asset-weight",
subcall: endpointCategory,
maxBytes: endpointCategory === "github-trees" ? 4 * 1024 * 1024 : 256 * 1024,
maxCallsPerCategory: endpointCategory === "github-contents" ? MAX_PATH_SIZE_LOOKUPS : 2,
maxBytes:
endpointCategory === "github-trees" ? 4 * 1024 * 1024 : 256 * 1024,
maxCallsPerCategory:
endpointCategory === "github-contents" ? MAX_PATH_SIZE_LOOKUPS : 2,
};
const response = options.analysis
? await options.analysis.fetchJson<T>(url, fetchOptions)
Expand Down Expand Up @@ -204,14 +208,9 @@ async function fetchPathSizes(
const encodedPath = encodeRepoPath(path);
if (!encodedPath) continue;
const url = `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${encodedPath}?ref=${encodeURIComponent(sha)}`;
const json = await fetchGithubJson<{ type?: string; size?: number } | unknown[]>(
url,
token,
fetchImpl,
signal,
options,
"github-contents",
);
const json = await fetchGithubJson<
{ type?: string; size?: number } | unknown[]
>(url, token, fetchImpl, signal, options, "github-contents");
if (!json) continue;
if (!Array.isArray(json) && typeof json.size === "number") {
sizes.set(path, json.size);
Expand All @@ -230,9 +229,26 @@ async function fetchRelevantSizes(
signal: AbortSignal | undefined,
options: ScanOptions,
): Promise<Map<string, number>> {
const tree = await fetchTreeSizes(owner, repo, sha, token, fetchImpl, signal, options);
const tree = await fetchTreeSizes(
owner,
repo,
sha,
token,
fetchImpl,
signal,
options,
);
if (!tree.truncated) return tree.sizes;
return fetchPathSizes(owner, repo, sha, token, paths, fetchImpl, signal, options);
return fetchPathSizes(
owner,
repo,
sha,
token,
paths,
fetchImpl,
signal,
options,
);
}

/** Analyzer entrypoint: flag heavy binary assets the PR adds or grows past the threshold. Pure size arithmetic over
Expand Down Expand Up @@ -271,11 +287,11 @@ export async function scanAssetWeight(
repo.repo,
req.baseSha,
token,
basePaths,
fetchImpl,
options.signal,
options,
)
basePaths,
fetchImpl,
options.signal,
options,
)
: new Map<string, number>();

const findings: AssetWeightFinding[] = [];
Expand Down
38 changes: 33 additions & 5 deletions review-enrichment/test/asset-weight.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,28 @@ import {

test("isBinaryAsset flags genuine binary extensions and ignores text/case", () => {
// Genuine binary assets (image / font / media / archive / native binary).
for (const p of ["ui/logo.png", "fonts/Inter.woff2", "media/demo.mp4", "vendor/lib.wasm", "dist/app.zip"]) {
for (const p of [
"ui/logo.png",
"fonts/Inter.woff2",
"media/demo.mp4",
"vendor/lib.wasm",
"dist/app.zip",
]) {
assert.equal(isBinaryAsset(p), true, p);
}
// Apple HEIC/HEIF photos are binary image assets (siblings of webp/avif) — their bytes are not in the textual diff.
assert.equal(isBinaryAsset("photos/IMG_0001.heic"), true);
assert.equal(isBinaryAsset("photos/scan.heif"), true);
assert.equal(isBinaryAsset("photos/IMG_0001.HEIC"), true);
// Extension match is case-insensitive.
assert.equal(isBinaryAsset("assets/HERO.PNG"), true);
// Text formats whose bytes are already in the diff are NOT binary assets.
for (const p of ["icons/logo.svg", "data/config.json", "src/index.ts", "README.md"]) {
for (const p of [
"icons/logo.svg",
"data/config.json",
"src/index.ts",
"README.md",
]) {
assert.equal(isBinaryAsset(p), false, p);
}
// A path with no extension, or a dotfile with no real extension after the dot, is not a binary asset.
Expand All @@ -25,9 +40,22 @@ test("isBinaryAsset flags genuine binary extensions and ignores text/case", () =
});

test("basePathForGrowth resolves the base-side path per file status", () => {
assert.equal(basePathForGrowth({ path: "a.png", status: "modified" }), "a.png");
assert.equal(basePathForGrowth({ path: "a.png", status: "changed" }), "a.png");
assert.equal(basePathForGrowth({ path: "new.png", previousPath: "old.png", status: "renamed" }), "old.png");
assert.equal(
basePathForGrowth({ path: "a.png", status: "modified" }),
"a.png",
);
assert.equal(
basePathForGrowth({ path: "a.png", status: "changed" }),
"a.png",
);
assert.equal(
basePathForGrowth({
path: "new.png",
previousPath: "old.png",
status: "renamed",
}),
"old.png",
);
// A rename with no previousPath has no comparable base.
assert.equal(basePathForGrowth({ path: "new.png", status: "renamed" }), null);
// Added/removed files have no base size to grow from.
Expand Down
Loading