From 95536e494c956cf320fc7af064fef906a61c23c6 Mon Sep 17 00:00:00 2001 From: oktofeesh1 <287075021+oktofeesh1@users.noreply.github.com> Date: Sat, 27 Jun 2026 23:31:28 -0700 Subject: [PATCH 1/6] feat(enrichment): image/binary asset weight-delta analyzer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a REES analyzer that flags a PR committing or growing a heavy image/font/ binary blob — repo + CDN/cold-start bloat the textual diff hides behind 'Binary files differ'. Binary sizes are not in the patch, so it fetches the repo's git tree at headSha (and baseSha for modified files) with the request's short-lived GitHub token — one recursive call returns every blob size and sidesteps the Contents API 1 MB cap — then it is pure size arithmetic. Flags a newly-added blob >= 100 KB or growth >= 100 KB; text formats (.svg/.json) are excluded. repoFullName is validated to exactly two safe owner/repo segments (no extra slash, no '.'/'..' traversal) and sha to a commit-SHA pattern, and every path segment is URL-encoded, so a hostile repoFullName cannot redirect the token-bearing request to another repo. Fail-safe: returns [] without a token or on a failed/unsafe fetch. Bounded by MAX_FILES; injects fetch for tests, mirroring the dependency-scan analyzer. Closes #1506. --- .../src/analyzers/asset-weight.ts | 187 ++++++++++++++++ review-enrichment/src/brief.ts | 2 + review-enrichment/src/render.ts | 20 ++ review-enrichment/src/types.ts | 10 + review-enrichment/test/enrichment.test.ts | 205 ++++++++++++++++-- 5 files changed, 405 insertions(+), 19 deletions(-) create mode 100644 review-enrichment/src/analyzers/asset-weight.ts diff --git a/review-enrichment/src/analyzers/asset-weight.ts b/review-enrichment/src/analyzers/asset-weight.ts new file mode 100644 index 0000000000..a35ae3f44c --- /dev/null +++ b/review-enrichment/src/analyzers/asset-weight.ts @@ -0,0 +1,187 @@ +// Image/binary asset weight-delta analyzer (#1506). Flags a PR that commits or grows a heavy image/font/binary +// blob — repo + CDN/cold-start bloat the textual diff hides behind "Binary files differ". Binary sizes are not in +// the patch, so this is the one analyzer that needs the GitHub API: the git tree at headSha (and baseSha, for +// modified files) is fetched with the request's short-lived token — one recursive call returns every blob's size, +// which also sidesteps the Contents API's 1 MB cap. Pure size arithmetic after that; no external service. +// Fail-safe: returns [] without a token/headSha or when the tree fetch is not OK. +import type { EnrichRequest, AssetWeightFinding } from "../types.js"; + +const MAX_FILES = 50; // cap binary files inspected per PR +const THRESHOLD_BYTES = 100 * 1024; // flag a newly-added blob >= 100 KB, or growth >= 100 KB +const GITHUB_API = "https://api.github.com"; + +// Extensions that are genuinely binary (text formats like .svg/.json are excluded — their bytes are in the diff). +const BINARY_EXTS = new Set([ + "png", + "jpg", + "jpeg", + "gif", + "bmp", + "tiff", + "tif", + "ico", + "webp", + "avif", + "woff", + "woff2", + "ttf", + "otf", + "eot", + "mp4", + "mov", + "avi", + "webm", + "mkv", + "mp3", + "wav", + "flac", + "ogg", + "zip", + "tar", + "gz", + "tgz", + "bz2", + "7z", + "rar", + "xz", + "pdf", + "psd", + "ai", + "sketch", + "fig", + "xcf", + "exe", + "dll", + "so", + "dylib", + "bin", + "dat", + "wasm", + "node", + "jar", + "class", +]); + +interface ScanOptions { + signal?: AbortSignal; +} + +// A single repo path segment (owner or name): word chars, dot, dash only. Whole-segment `.`/`..` are rejected +// separately so they can't traverse. A commit SHA: hex only — we only ever fetch a real object, never an arbitrary ref. +const REPO_SEGMENT = /^[A-Za-z0-9._-]+$/; +const SHA_RE = /^[0-9a-fA-F]{7,64}$/; + +function isBinaryAsset(path: string): boolean { + const dot = path.lastIndexOf("."); + return dot >= 0 && BINARY_EXTS.has(path.slice(dot + 1).toLowerCase()); +} + +/** Parse `owner/repo`, rejecting anything that isn't exactly two safe segments — no extra `/`, no `.`/`..` + * traversal, no query/fragment characters. This stops a hostile `repoFullName` from redirecting the + * token-bearing request to another repository. Returns null when unsafe. */ +function parseRepo( + repoFullName: string, +): { owner: string; repo: string } | null { + const parts = repoFullName.split("/"); + if (parts.length !== 2) return null; + const [owner, repo] = parts; + for (const seg of [owner, repo]) { + if (!seg || seg === "." || seg === ".." || !REPO_SEGMENT.test(seg)) { + return null; + } + } + return { owner: owner!, repo: repo! }; +} + +/** Fetch every blob's byte size in the repo's git tree at `sha`. One recursive call. Empty map on an invalid SHA + * or a non-OK reply. `owner`/`repo` are validated by the caller; every segment is URL-encoded here (defense in + * depth) so nothing user-derived can break out of the intended API path. */ +async function fetchTreeSizes( + owner: string, + repo: string, + sha: string, + token: string, + fetchImpl: typeof fetch, + signal?: AbortSignal, +): Promise> { + const sizes = new Map(); + if (!SHA_RE.test(sha)) return sizes; + const url = `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/git/trees/${encodeURIComponent(sha)}?recursive=1`; + const res = await fetchImpl(url, { + headers: { + Authorization: `Bearer ${token}`, + Accept: "application/vnd.github+json", + "User-Agent": "gittensory-review-enrichment", + }, + signal, + }); + if (!res.ok) return sizes; + const json = (await res.json()) as { + tree?: Array<{ path?: string; type?: string; size?: number }>; + }; + for (const entry of json.tree ?? []) { + if (entry.type === "blob" && typeof entry.size === "number" && entry.path) { + sizes.set(entry.path, entry.size); + } + } + return sizes; +} + +/** Analyzer entrypoint: flag heavy binary assets the PR adds or grows past the threshold. Pure size arithmetic over + * the GitHub git tree; fail-safe (returns [] without a token or on a failed tree fetch). */ +export async function scanAssetWeight( + req: EnrichRequest, + fetchImpl: typeof fetch = fetch, + options: ScanOptions = {}, +): Promise { + const token = req.githubToken; + if (!token || !req.headSha) return []; + const repo = parseRepo(req.repoFullName); + if (!repo) return []; + + const binaries = (req.files ?? []) + .filter((f) => f.status !== "removed" && isBinaryAsset(f.path)) + .slice(0, MAX_FILES); + if (!binaries.length) return []; + + const headSizes = await fetchTreeSizes( + repo.owner, + repo.repo, + req.headSha, + token, + fetchImpl, + options.signal, + ); + const needBase = binaries.some( + (f) => f.status === "modified" || f.status === "changed", + ); + const baseSizes = + needBase && req.baseSha + ? await fetchTreeSizes( + repo.owner, + repo.repo, + req.baseSha, + token, + fetchImpl, + options.signal, + ) + : new Map(); + + const findings: AssetWeightFinding[] = []; + for (const file of binaries) { + const bytes = headSizes.get(file.path); + if (typeof bytes !== "number") continue; + const baseBytes = baseSizes.get(file.path) ?? 0; + const isNew = baseBytes === 0; + const deltaBytes = bytes - baseBytes; + if (isNew ? bytes >= THRESHOLD_BYTES : deltaBytes >= THRESHOLD_BYTES) { + findings.push({ + path: file.path, + bytes, + deltaBytes, + status: isNew ? "added" : "grown", + }); + } + } + return findings; +} diff --git a/review-enrichment/src/brief.ts b/review-enrichment/src/brief.ts index a2ca974bd4..6cbf74b70c 100644 --- a/review-enrichment/src/brief.ts +++ b/review-enrichment/src/brief.ts @@ -17,6 +17,7 @@ import { scanRedos } from "./analyzers/redos.js"; import { scanProvenance } from "./analyzers/provenance.js"; import { scanCodeowners } from "./analyzers/codeowners.js"; import { scanSecretLog } from "./analyzers/secret-log.js"; +import { scanAssetWeight } from "./analyzers/asset-weight.js"; import { renderBrief } from "./render.js"; type AnalyzerFn = (req: EnrichRequest, signal: AbortSignal) => Promise; @@ -33,6 +34,7 @@ const ANALYZERS: Record = { provenance: (req, signal) => scanProvenance(req, fetch, { signal }), codeowners: (req, signal) => scanCodeowners(req, fetch, { signal }), secretLog: (req, signal) => scanSecretLog(req, signal), + assetWeight: (req, signal) => scanAssetWeight(req, fetch, { signal }), }; function runWithTimeout( diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 2533c42122..9aa4863c13 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -34,6 +34,12 @@ function promptText(value: string): string { .replace(/([*_{}[\]()#+.!|-])/g, "\\$1"); } +function formatBytes(n: number): string { + if (n >= 1048576) return `${(n / 1048576).toFixed(1)} MB`; + if (n >= 1024) return `${(n / 1024).toFixed(0)} KB`; + return `${n} B`; +} + /** Build the `promptSection` (verbatim splice) + a one-line `systemSuffix` from the findings. Empty when nothing found. */ export function renderBrief( findings: BriefFindings, @@ -195,6 +201,20 @@ export function renderBrief( } } + const assets = findings.assetWeight ?? []; + if (assets.length) { + lines.push( + "### Heavy binary assets (optimize, or move to a CDN / Git LFS)", + ); + for (const item of assets) { + const detail = + item.status === "added" + ? `adds ${formatBytes(item.bytes)}` + : `grows +${formatBytes(item.deltaBytes)} to ${formatBytes(item.bytes)}`; + lines.push(`- ${safeCodeSpan(item.path)} ${detail}`); + } + } + if (!lines.length) return { promptSection: "", systemSuffix: "" }; const header = diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index f533c9f743..cf1c4d7d25 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -123,6 +123,15 @@ export interface SecretLogFinding { category: "secret" | "pii" | "request-object"; } +/** A heavy binary asset the PR adds or grows. `bytes` is the size at headSha; `deltaBytes` is the growth vs base + * (equal to `bytes` for a newly-added file). */ +export interface AssetWeightFinding { + path: string; + bytes: number; + deltaBytes: number; + status: "added" | "grown"; +} + /** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */ export interface BriefFindings { dependency?: DependencyFinding[]; @@ -135,6 +144,7 @@ export interface BriefFindings { provenance?: ProvenanceFinding[]; codeowners?: CodeownersFinding[]; secretLog?: SecretLogFinding[]; + assetWeight?: AssetWeightFinding[]; } export type AnalyzerStatus = "ok" | "degraded" | "skipped"; diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index 743983a95d..efe02a5c90 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -21,6 +21,7 @@ import { scanPatchForRedos, scanRedos, } from "../dist/analyzers/redos.js"; +import { scanAssetWeight } from "../dist/analyzers/asset-weight.js"; import { classifyAddedFile, isSafeToCheck, @@ -29,6 +30,7 @@ import { matchesPypiVersion, scanProvenance, } from "../dist/analyzers/provenance.js"; +import { findOwners, parseCodeowners, patternToRegex, @@ -1504,6 +1506,78 @@ test("scanProvenance: handles undefined files gracefully", async () => { assert.deepEqual(findings, []); }); +const treeReply = (tree) => ({ ok: true, json: async () => ({ tree }) }); +const HEAD_SHA = "1111111111111111111111111111111111111111"; +const BASE_SHA = "2222222222222222222222222222222222222222"; + +test("scanAssetWeight: flags a large newly-added binary, ignores small + non-binary files", async () => { + const findings = await scanAssetWeight( + { + repoFullName: "o/r", + prNumber: 1, + headSha: HEAD_SHA, + githubToken: "t", + files: [ + { path: "img/logo.png", status: "added" }, + { path: "icon.svg", status: "added" }, + { path: "src/x.ts", status: "added" }, + { path: "tiny.gif", status: "added" }, + ], + }, + async () => + treeReply([ + { path: "img/logo.png", type: "blob", size: 250000 }, + { path: "tiny.gif", type: "blob", size: 2000 }, + ]), + ); + assert.equal(findings.length, 1); + assert.equal(findings[0].path, "img/logo.png"); + assert.equal(findings[0].status, "added"); + assert.equal(findings[0].bytes, 250000); + assert.equal(findings[0].deltaBytes, 250000); +}); + +test("scanAssetWeight: flags a binary that GREW past the threshold (base vs head)", async () => { + const fetchImpl = async (url) => + String(url).includes(BASE_SHA) + ? treeReply([{ path: "video.mp4", type: "blob", size: 50000 }]) + : treeReply([{ path: "video.mp4", type: "blob", size: 250000 }]); + const findings = await scanAssetWeight( + { + repoFullName: "o/r", + prNumber: 1, + headSha: HEAD_SHA, + baseSha: BASE_SHA, + githubToken: "t", + files: [{ path: "video.mp4", status: "modified" }], + }, + fetchImpl, + ); + assert.equal(findings.length, 1); + assert.equal(findings[0].status, "grown"); + assert.equal(findings[0].deltaBytes, 200000); + assert.equal(findings[0].bytes, 250000); +}); + +test("scanAssetWeight: small growth is not flagged", async () => { + const fetchImpl = async (url) => + String(url).includes(BASE_SHA) + ? treeReply([{ path: "a.png", type: "blob", size: 300000 }]) + : treeReply([{ path: "a.png", type: "blob", size: 310000 }]); + const findings = await scanAssetWeight( + { + repoFullName: "o/r", + prNumber: 1, + headSha: HEAD_SHA, + baseSha: BASE_SHA, + githubToken: "t", + files: [{ path: "a.png", status: "modified" }], + }, + fetchImpl, + ); + assert.deepEqual(findings, []); +}); + // --------------------------------------------------------------------------- // renderBrief: provenance block // --------------------------------------------------------------------------- @@ -1562,6 +1636,109 @@ test("buildBrief: provenance analyzer runs, flags binary file and missing npm at return { ok: false, status: 404, json: async () => ({}) }; return { ok: true, json: async () => ({}) }; }; + try { + const brief = await buildBrief({ + repoFullName: "o/r", + prNumber: 1, + files: [ + { path: "native/tool.exe", status: "added" }, + { path: "package.json", patch: '+ "no-attest": "1.0.0",' }, + ], + }); + assert.equal(brief.analyzerStatus.provenance, "ok"); + assert.ok(brief.findings.provenance.length >= 2); + assert.match(brief.promptSection, /provenance/); + } finally { + globalThis.fetch = realFetch; + } +}); + +test("scanAssetWeight: fail-safe — no token, no binaries, or failed fetch returns []", async () => { + const tree = async () => + treeReply([{ path: "a.png", type: "blob", size: 999999 }]); + assert.deepEqual( + await scanAssetWeight( + { repoFullName: "o/r", prNumber: 1, headSha: HEAD_SHA, files: [{ path: "a.png", status: "added" }] }, + tree, + ), + [], + ); // no token + assert.deepEqual( + await scanAssetWeight( + { repoFullName: "o/r", prNumber: 1, headSha: HEAD_SHA, githubToken: "t", files: [{ path: "readme.md", status: "added" }] }, + tree, + ), + [], + ); // no binary files + assert.deepEqual( + await scanAssetWeight( + { repoFullName: "o/r", prNumber: 1, headSha: HEAD_SHA, githubToken: "t", files: [{ path: "a.png", status: "added" }] }, + async () => ({ ok: false, json: async () => ({}) }), + ), + [], + ); // tree fetch not OK +}); + +test("scanAssetWeight: rejects path-traversal repoFullName + non-SHA refs (no token-bearing fetch)", async () => { + let fetched = false; + const spy = async () => { + fetched = true; + return treeReply([{ path: "a.png", type: "blob", size: 999999 }]); + }; + const file = { path: "a.png", status: "added" }; + for (const repoFullName of ["a/b/../../x/y", "../evil", "owner/repo/extra", "o/.."]) { + assert.deepEqual( + await scanAssetWeight( + { repoFullName, prNumber: 1, headSha: HEAD_SHA, githubToken: "t", files: [file] }, + spy, + ), + [], + ); + } + assert.deepEqual( + await scanAssetWeight( + { repoFullName: "o/r", prNumber: 1, headSha: "main", githubToken: "t", files: [file] }, + spy, + ), + [], + ); + assert.equal(fetched, false, "the token-bearing fetch never runs for unsafe input"); +}); + +test("renderBrief: renders the asset-weight block with human-readable sizes", () => { + const r = renderBrief({ + assetWeight: [ + { path: "img/logo.png", bytes: 2500000, deltaBytes: 2500000, status: "added" }, + { path: "v.mp4", bytes: 300000, deltaBytes: 200000, status: "grown" }, + ], + }); + assert.match(r.promptSection, /Heavy binary assets/); + assert.match(r.promptSection, /`img\/logo\.png` adds 2\.4 MB/); + assert.match(r.promptSection, /`v\.mp4` grows \+195 KB to 293 KB/); +}); + +test("buildBrief: asset-weight analyzer runs", async () => { + const realFetch = globalThis.fetch; + globalThis.fetch = async (url) => + String(url).includes("git/trees") + ? treeReply([{ path: "big.png", type: "blob", size: 300000 }]) + : { ok: true, json: async () => ({}) }; + try { + const brief = await buildBrief({ + repoFullName: "o/r", + prNumber: 1, + headSha: HEAD_SHA, + githubToken: "t", + files: [{ path: "big.png", status: "added" }], + }); + assert.equal(brief.analyzerStatus.assetWeight, "ok"); + assert.equal(brief.findings.assetWeight.length, 1); + assert.match(brief.promptSection, /Heavy binary assets/); + } finally { + globalThis.fetch = realFetch; + } +}); + test("codeOnly: blanks string messages, keeps ${...} interpolation bodies", () => { assert.equal(codeOnly('"a secret here"'), " "); assert.equal(codeOnly("'plain'"), " "); @@ -1717,21 +1894,22 @@ test("buildBrief: secret-log analyzer runs (pure, no network)", async () => { repoFullName: "o/r", prNumber: 1, files: [ - { path: "native/tool.exe", status: "added" }, - { path: "package.json", patch: '+ "no-attest": "1.0.0",' }, + { + path: "src/a.ts", + patch: "@@ -1,0 +1,1 @@\n+console.log(req.headers.authorization);", + }, ], }); - assert.equal(brief.analyzerStatus.provenance, "ok"); - assert.ok(brief.findings.provenance.length >= 2); - assert.match(brief.promptSection, /provenance/); + assert.equal(brief.analyzerStatus.secretLog, "ok"); + assert.equal(brief.findings.secretLog.length, 1); + assert.match(brief.promptSection, /Secrets \/ PII reaching a log/); } finally { globalThis.fetch = realFetch; } }); -test("buildBrief: provenance analyzer throw → degraded + partial", async () => { +test("buildBrief: provenance analyzer fetch failure fails safe", async () => { const realFetch = globalThis.fetch; - // Cause all fetches to fail (provenance uses fetch for attestation checks) globalThis.fetch = async () => { throw new Error("network down"); }; try { const brief = await buildBrief({ @@ -1740,20 +1918,9 @@ test("buildBrief: provenance analyzer throw → degraded + partial", async () => analyzers: ["provenance"], files: [{ path: "package.json", patch: '+ "pkg": "1.0.0",' }], }); - // provenance fetch throws → degraded; binary scan still ran but that's pure - // The analyzer as a whole may succeed (binary scan is pure) or degrade on fetch. - // Because hasNpmAttestation catches fetch errors (fail-safe), the analyzer succeeds. assert.equal(brief.analyzerStatus.provenance, "ok"); assert.equal(brief.partial, false); - { - path: "src/a.ts", - patch: "@@ -1,0 +1,1 @@\n+console.log(req.headers.authorization);", - }, - ], - }); - assert.equal(brief.analyzerStatus.secretLog, "ok"); - assert.equal(brief.findings.secretLog.length, 1); - assert.match(brief.promptSection, /Secrets \/ PII reaching a log/); + assert.deepEqual(brief.findings.provenance, []); } finally { globalThis.fetch = realFetch; } From 2fe0d2c0b4638f89bf6bb86d5495a752f2c010d5 Mon Sep 17 00:00:00 2001 From: oktofeesh1 <287075021+oktofeesh1@users.noreply.github.com> Date: Sun, 28 Jun 2026 05:35:27 -0700 Subject: [PATCH 2/6] fix(enrichment): forward file status to asset analyzer --- review-enrichment/test/enrichment.test.ts | 12 +++++++++--- src/review/enrichment-wire.ts | 1 + test/unit/enrichment-wire.test.ts | 6 +++--- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index efe02a5c90..626dc52816 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -1717,23 +1717,29 @@ test("renderBrief: renders the asset-weight block with human-readable sizes", () assert.match(r.promptSection, /`v\.mp4` grows \+195 KB to 293 KB/); }); -test("buildBrief: asset-weight analyzer runs", async () => { +test("buildBrief: asset-weight analyzer reports grown binaries from request file status", async () => { const realFetch = globalThis.fetch; globalThis.fetch = async (url) => String(url).includes("git/trees") - ? treeReply([{ path: "big.png", type: "blob", size: 300000 }]) + ? String(url).includes(BASE_SHA) + ? treeReply([{ path: "big.png", type: "blob", size: 50000 }]) + : treeReply([{ path: "big.png", type: "blob", size: 300000 }]) : { ok: true, json: async () => ({}) }; try { const brief = await buildBrief({ repoFullName: "o/r", prNumber: 1, headSha: HEAD_SHA, + baseSha: BASE_SHA, githubToken: "t", - files: [{ path: "big.png", status: "added" }], + files: [{ path: "big.png", status: "modified" }], }); assert.equal(brief.analyzerStatus.assetWeight, "ok"); assert.equal(brief.findings.assetWeight.length, 1); + assert.equal(brief.findings.assetWeight[0].status, "grown"); + assert.equal(brief.findings.assetWeight[0].deltaBytes, 250000); assert.match(brief.promptSection, /Heavy binary assets/); + assert.match(brief.promptSection, /grows/); } finally { globalThis.fetch = realFetch; } diff --git a/src/review/enrichment-wire.ts b/src/review/enrichment-wire.ts index 4ec00e5624..1da182d875 100644 --- a/src/review/enrichment-wire.ts +++ b/src/review/enrichment-wire.ts @@ -83,6 +83,7 @@ export async function buildReviewEnrichment( title: input.title, files: input.files.map((file) => ({ path: file.path, + status: file.status ?? undefined, patch: typeof file.payload?.patch === "string" ? file.payload.patch diff --git a/test/unit/enrichment-wire.test.ts b/test/unit/enrichment-wire.test.ts index 65e7680428..f991112290 100644 --- a/test/unit/enrichment-wire.test.ts +++ b/test/unit/enrichment-wire.test.ts @@ -11,7 +11,7 @@ const input = { headSha: "abc", title: "t", files: [ - { path: "a.ts", payload: { patch: "@@ +1 @@" } }, + { path: "a.ts", status: "modified", payload: { patch: "@@ +1 @@" } }, { path: "b.ts" }, ] as never, diff: "the diff", @@ -81,8 +81,8 @@ describe("buildReviewEnrichment", () => { const body = JSON.parse(calls[0]!.init.body as string); expect(body.repoFullName).toBe("o/r"); expect(body.files).toEqual([ - { path: "a.ts", patch: "@@ +1 @@" }, - { path: "b.ts", patch: undefined }, + { path: "a.ts", status: "modified", patch: "@@ +1 @@" }, + { path: "b.ts", status: undefined, patch: undefined }, ]); }); From d9ee46c749db5e891ba604810f5db4816e842680 Mon Sep 17 00:00:00 2001 From: oktofeesh1 <287075021+oktofeesh1@users.noreply.github.com> Date: Sun, 28 Jun 2026 15:06:38 -0700 Subject: [PATCH 3/6] fix(enrichment): require base size for grown assets --- .../src/analyzers/asset-weight.ts | 29 +++++++++--- review-enrichment/test/enrichment.test.ts | 47 +++++++++++++++++++ 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/review-enrichment/src/analyzers/asset-weight.ts b/review-enrichment/src/analyzers/asset-weight.ts index a35ae3f44c..9c1e16d753 100644 --- a/review-enrichment/src/analyzers/asset-weight.ts +++ b/review-enrichment/src/analyzers/asset-weight.ts @@ -3,7 +3,8 @@ // the patch, so this is the one analyzer that needs the GitHub API: the git tree at headSha (and baseSha, for // modified files) is fetched with the request's short-lived token — one recursive call returns every blob's size, // which also sidesteps the Contents API's 1 MB cap. Pure size arithmetic after that; no external service. -// Fail-safe: returns [] without a token/headSha or when the tree fetch is not OK. +// Fail-safe: returns [] without a token/headSha or when the head tree fetch is not OK; growth findings require a +// matching base size. import type { EnrichRequest, AssetWeightFinding } from "../types.js"; const MAX_FILES = 50; // cap binary files inspected per PR @@ -128,7 +129,7 @@ async function fetchTreeSizes( } /** Analyzer entrypoint: flag heavy binary assets the PR adds or grows past the threshold. Pure size arithmetic over - * the GitHub git tree; fail-safe (returns [] without a token or on a failed tree fetch). */ + * the GitHub git tree; fail-safe (returns [] without a token or on a failed head tree fetch). */ export async function scanAssetWeight( req: EnrichRequest, fetchImpl: typeof fetch = fetch, @@ -171,15 +172,29 @@ export async function scanAssetWeight( for (const file of binaries) { const bytes = headSizes.get(file.path); if (typeof bytes !== "number") continue; - const baseBytes = baseSizes.get(file.path) ?? 0; - const isNew = baseBytes === 0; - const deltaBytes = bytes - baseBytes; - if (isNew ? bytes >= THRESHOLD_BYTES : deltaBytes >= THRESHOLD_BYTES) { + + if (file.status === "added") { + if (bytes >= THRESHOLD_BYTES) { + findings.push({ + path: file.path, + bytes, + deltaBytes: bytes, + status: "added", + }); + } + continue; + } + + if (file.status === "modified" || file.status === "changed") { + const baseBytes = baseSizes.get(file.path); + if (typeof baseBytes !== "number") continue; + const deltaBytes = bytes - baseBytes; + if (deltaBytes < THRESHOLD_BYTES) continue; findings.push({ path: file.path, bytes, deltaBytes, - status: isNew ? "added" : "grown", + status: "grown", }); } } diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index 626dc52816..5f0788807f 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -1653,6 +1653,53 @@ test("buildBrief: provenance analyzer runs, flags binary file and missing npm at } }); +test("scanAssetWeight: failed base fetch does not reclassify modified binaries as added", async () => { + const findings = await scanAssetWeight( + { + repoFullName: "o/r", + prNumber: 1, + headSha: HEAD_SHA, + baseSha: BASE_SHA, + githubToken: "t", + files: [ + { path: "video.mp4", status: "modified" }, + { path: "clip.mov", status: "changed" }, + { path: "poster.png", status: "added" }, + ], + }, + async (url) => + String(url).includes(BASE_SHA) + ? { ok: false, json: async () => ({}) } + : treeReply([ + { path: "video.mp4", type: "blob", size: 250000 }, + { path: "clip.mov", type: "blob", size: 260000 }, + { path: "poster.png", type: "blob", size: 270000 }, + ]), + ); + assert.deepEqual(findings, [ + { + path: "poster.png", + bytes: 270000, + deltaBytes: 270000, + status: "added", + }, + ]); +}); + +test("scanAssetWeight: missing baseSha does not reclassify modified binaries as added", async () => { + const findings = await scanAssetWeight( + { + repoFullName: "o/r", + prNumber: 1, + headSha: HEAD_SHA, + githubToken: "t", + files: [{ path: "video.mp4", status: "modified" }], + }, + async () => treeReply([{ path: "video.mp4", type: "blob", size: 250000 }]), + ); + assert.deepEqual(findings, []); +}); + test("scanAssetWeight: fail-safe — no token, no binaries, or failed fetch returns []", async () => { const tree = async () => treeReply([{ path: "a.png", type: "blob", size: 999999 }]); From d70231b9c13e32f3f726369e2959a22904eed8de Mon Sep 17 00:00:00 2001 From: oktofeesh1 <287075021+oktofeesh1@users.noreply.github.com> Date: Sun, 28 Jun 2026 15:21:30 -0700 Subject: [PATCH 4/6] fix(enrichment): avoid partial asset weight scans --- .../src/analyzers/asset-weight.ts | 19 ++-- review-enrichment/test/enrichment.test.ts | 94 +++++++++++++++++++ 2 files changed, 106 insertions(+), 7 deletions(-) diff --git a/review-enrichment/src/analyzers/asset-weight.ts b/review-enrichment/src/analyzers/asset-weight.ts index 9c1e16d753..40032f75d6 100644 --- a/review-enrichment/src/analyzers/asset-weight.ts +++ b/review-enrichment/src/analyzers/asset-weight.ts @@ -7,7 +7,7 @@ // matching base size. import type { EnrichRequest, AssetWeightFinding } from "../types.js"; -const MAX_FILES = 50; // cap binary files inspected per PR +const MAX_FINDINGS = 50; // keep the brief bounded after evaluating every changed binary candidate const THRESHOLD_BYTES = 100 * 1024; // flag a newly-added blob >= 100 KB, or growth >= 100 KB const GITHUB_API = "https://api.github.com"; @@ -95,8 +95,9 @@ function parseRepo( } /** Fetch every blob's byte size in the repo's git tree at `sha`. One recursive call. Empty map on an invalid SHA - * or a non-OK reply. `owner`/`repo` are validated by the caller; every segment is URL-encoded here (defense in - * depth) so nothing user-derived can break out of the intended API path. */ + * or a non-OK reply; throws on truncated recursive replies so the orchestrator degrades instead of trusting + * partial data. `owner`/`repo` are validated by the caller; every segment is URL-encoded here (defense in depth) + * so nothing user-derived can break out of the intended API path. */ async function fetchTreeSizes( owner: string, repo: string, @@ -119,7 +120,9 @@ async function fetchTreeSizes( if (!res.ok) return sizes; const json = (await res.json()) as { tree?: Array<{ path?: string; type?: string; size?: number }>; + truncated?: boolean; }; + if (json.truncated) throw new Error("github_tree_truncated"); for (const entry of json.tree ?? []) { if (entry.type === "blob" && typeof entry.size === "number" && entry.path) { sizes.set(entry.path, entry.size); @@ -140,9 +143,9 @@ export async function scanAssetWeight( const repo = parseRepo(req.repoFullName); if (!repo) return []; - const binaries = (req.files ?? []) - .filter((f) => f.status !== "removed" && isBinaryAsset(f.path)) - .slice(0, MAX_FILES); + const binaries = (req.files ?? []).filter( + (f) => f.status !== "removed" && isBinaryAsset(f.path), + ); if (!binaries.length) return []; const headSizes = await fetchTreeSizes( @@ -198,5 +201,7 @@ export async function scanAssetWeight( }); } } - return findings; + return findings + .sort((a, b) => b.deltaBytes - a.deltaBytes) + .slice(0, MAX_FINDINGS); } diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index 5f0788807f..6f8e557668 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -1537,6 +1537,67 @@ test("scanAssetWeight: flags a large newly-added binary, ignores small + non-bin assert.equal(findings[0].deltaBytes, 250000); }); +test("scanAssetWeight: evaluates large binaries after the first 50 candidate paths", async () => { + const smallFiles = Array.from({ length: 50 }, (_, i) => ({ + path: `small-${i}.png`, + status: "added", + })); + const files = [...smallFiles, { path: "late-large.png", status: "added" }]; + const findings = await scanAssetWeight( + { + repoFullName: "o/r", + prNumber: 1, + headSha: HEAD_SHA, + githubToken: "t", + files, + }, + async () => + treeReply([ + ...smallFiles.map((file) => ({ + path: file.path, + type: "blob", + size: 2000, + })), + { path: "late-large.png", type: "blob", size: 10_000_000 }, + ]), + ); + assert.deepEqual(findings, [ + { + path: "late-large.png", + bytes: 10_000_000, + deltaBytes: 10_000_000, + status: "added", + }, + ]); +}); + +test("scanAssetWeight: caps findings after ranking by size, not by PR file order", async () => { + const files = Array.from({ length: 51 }, (_, i) => ({ + path: `asset-${i}.png`, + status: "added", + })); + const findings = await scanAssetWeight( + { + repoFullName: "o/r", + prNumber: 1, + headSha: HEAD_SHA, + githubToken: "t", + files, + }, + async () => + treeReply( + files.map((file, i) => ({ + path: file.path, + type: "blob", + size: i === 50 ? 10_000_000 : 150000, + })), + ), + ); + assert.equal(findings.length, 50); + assert.equal(findings[0].path, "asset-50.png"); + assert.equal(findings[0].bytes, 10_000_000); +}); + test("scanAssetWeight: flags a binary that GREW past the threshold (base vs head)", async () => { const fetchImpl = async (url) => String(url).includes(BASE_SHA) @@ -1700,6 +1761,39 @@ test("scanAssetWeight: missing baseSha does not reclassify modified binaries as assert.deepEqual(findings, []); }); +test("buildBrief: asset-weight degrades instead of trusting truncated tree responses", async () => { + const realFetch = globalThis.fetch; + globalThis.fetch = async (url) => + String(url).includes("git/trees") + ? String(url).includes(BASE_SHA) + ? { + ok: true, + json: async () => ({ + truncated: true, + tree: [{ path: "big.png", type: "blob", size: 50000 }], + }), + } + : treeReply([{ path: "big.png", type: "blob", size: 300000 }]) + : { ok: true, json: async () => ({}) }; + try { + const brief = await buildBrief({ + repoFullName: "o/r", + prNumber: 1, + headSha: HEAD_SHA, + baseSha: BASE_SHA, + githubToken: "t", + files: [{ path: "big.png", status: "modified" }], + analyzers: ["assetWeight"], + }); + assert.equal(brief.partial, true); + assert.equal(brief.analyzerStatus.assetWeight, "degraded"); + assert.equal(brief.findings.assetWeight, undefined); + assert.doesNotMatch(brief.promptSection, /Heavy binary assets/); + } finally { + globalThis.fetch = realFetch; + } +}); + test("scanAssetWeight: fail-safe — no token, no binaries, or failed fetch returns []", async () => { const tree = async () => treeReply([{ path: "a.png", type: "blob", size: 999999 }]); From ec216c9b5600f9f01adeb25a82d82e564bce4c09 Mon Sep 17 00:00:00 2001 From: oktofeesh1 <287075021+oktofeesh1@users.noreply.github.com> Date: Sun, 28 Jun 2026 15:39:20 -0700 Subject: [PATCH 5/6] fix(enrichment): harden asset weight growth scans Compare renamed and copied binary assets against their previous paths, and keep truncated Git tree responses useful by falling back to candidate-path size lookups. Pin the GitHub API version for asset size fetches and render binary-scaled sizes with KiB/MiB labels. Validation: npm run test:ci; npm audit --audit-level=moderate. --- .../src/analyzers/asset-weight.ts | 100 +++++++++++--- review-enrichment/src/render.ts | 4 +- review-enrichment/src/types.ts | 1 + review-enrichment/test/enrichment.test.ts | 129 +++++++++++++++--- src/review/enrichment-wire.ts | 1 + test/unit/enrichment-wire.test.ts | 14 +- 6 files changed, 208 insertions(+), 41 deletions(-) diff --git a/review-enrichment/src/analyzers/asset-weight.ts b/review-enrichment/src/analyzers/asset-weight.ts index 40032f75d6..ba47ac4b7d 100644 --- a/review-enrichment/src/analyzers/asset-weight.ts +++ b/review-enrichment/src/analyzers/asset-weight.ts @@ -10,6 +10,7 @@ import type { EnrichRequest, AssetWeightFinding } from "../types.js"; const MAX_FINDINGS = 50; // keep the brief bounded after evaluating every changed binary candidate const THRESHOLD_BYTES = 100 * 1024; // flag a newly-added blob >= 100 KB, or growth >= 100 KB const GITHUB_API = "https://api.github.com"; +const GITHUB_API_VERSION = "2022-11-28"; // Extensions that are genuinely binary (text formats like .svg/.json are excluded — their bytes are in the diff). const BINARY_EXTS = new Set([ @@ -77,6 +78,32 @@ function isBinaryAsset(path: string): boolean { return dot >= 0 && BINARY_EXTS.has(path.slice(dot + 1).toLowerCase()); } +type EnrichFile = NonNullable[number]; + +function basePathForGrowth(file: EnrichFile): string | null { + if (file.status === "modified" || file.status === "changed") return file.path; + if (file.status === "renamed" || file.status === "copied") + return file.previousPath || null; + return null; +} + +function githubHeaders(token: string): Record { + return { + Authorization: `Bearer ${token}`, + Accept: "application/vnd.github+json", + "X-GitHub-Api-Version": GITHUB_API_VERSION, + "User-Agent": "gittensory-review-enrichment", + }; +} + +function encodeRepoPath(path: string): string | null { + const segments = path.split("/"); + if (!path || segments.some((seg) => !seg || seg === "." || seg === "..")) { + return null; + } + return segments.map(encodeURIComponent).join("/"); +} + /** Parse `owner/repo`, rejecting anything that isn't exactly two safe segments — no extra `/`, no `.`/`..` * traversal, no query/fragment characters. This stops a hostile `repoFullName` from redirecting the * token-bearing request to another repository. Returns null when unsafe. */ @@ -95,9 +122,8 @@ function parseRepo( } /** Fetch every blob's byte size in the repo's git tree at `sha`. One recursive call. Empty map on an invalid SHA - * or a non-OK reply; throws on truncated recursive replies so the orchestrator degrades instead of trusting - * partial data. `owner`/`repo` are validated by the caller; every segment is URL-encoded here (defense in depth) - * so nothing user-derived can break out of the intended API path. */ + * or a non-OK reply. `owner`/`repo` are validated by the caller; every segment is URL-encoded here (defense in + * depth) so nothing user-derived can break out of the intended API path. */ async function fetchTreeSizes( owner: string, repo: string, @@ -105,32 +131,66 @@ async function fetchTreeSizes( token: string, fetchImpl: typeof fetch, signal?: AbortSignal, -): Promise> { +): Promise<{ sizes: Map; truncated: boolean }> { const sizes = new Map(); - if (!SHA_RE.test(sha)) return sizes; + if (!SHA_RE.test(sha)) return { sizes, truncated: false }; const url = `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/git/trees/${encodeURIComponent(sha)}?recursive=1`; const res = await fetchImpl(url, { - headers: { - Authorization: `Bearer ${token}`, - Accept: "application/vnd.github+json", - "User-Agent": "gittensory-review-enrichment", - }, + headers: githubHeaders(token), signal, }); - if (!res.ok) return sizes; + if (!res.ok) return { sizes, truncated: false }; const json = (await res.json()) as { tree?: Array<{ path?: string; type?: string; size?: number }>; truncated?: boolean; }; - if (json.truncated) throw new Error("github_tree_truncated"); for (const entry of json.tree ?? []) { if (entry.type === "blob" && typeof entry.size === "number" && entry.path) { sizes.set(entry.path, entry.size); } } + return { sizes, truncated: json.truncated === true }; +} + +async function fetchPathSizes( + owner: string, + repo: string, + sha: string, + token: string, + paths: Iterable, + fetchImpl: typeof fetch, + signal?: AbortSignal, +): Promise> { + const sizes = new Map(); + if (!SHA_RE.test(sha)) return sizes; + for (const path of new Set(paths)) { + const encodedPath = encodeRepoPath(path); + if (!encodedPath) continue; + const url = `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${encodedPath}?ref=${encodeURIComponent(sha)}`; + const res = await fetchImpl(url, { headers: githubHeaders(token), signal }); + if (!res.ok) continue; + const json = (await res.json()) as { type?: string; size?: number } | unknown[]; + if (!Array.isArray(json) && typeof json.size === "number") { + sizes.set(path, json.size); + } + } return sizes; } +async function fetchRelevantSizes( + owner: string, + repo: string, + sha: string, + token: string, + paths: Iterable, + fetchImpl: typeof fetch, + signal?: AbortSignal, +): Promise> { + const tree = await fetchTreeSizes(owner, repo, sha, token, fetchImpl, signal); + if (!tree.truncated) return tree.sizes; + return fetchPathSizes(owner, repo, sha, token, paths, fetchImpl, signal); +} + /** Analyzer entrypoint: flag heavy binary assets the PR adds or grows past the threshold. Pure size arithmetic over * the GitHub git tree; fail-safe (returns [] without a token or on a failed head tree fetch). */ export async function scanAssetWeight( @@ -148,24 +208,25 @@ export async function scanAssetWeight( ); if (!binaries.length) return []; - const headSizes = await fetchTreeSizes( + const headSizes = await fetchRelevantSizes( repo.owner, repo.repo, req.headSha, token, + binaries.map((file) => file.path), fetchImpl, options.signal, ); - const needBase = binaries.some( - (f) => f.status === "modified" || f.status === "changed", - ); + const basePaths = binaries.flatMap((file) => basePathForGrowth(file) ?? []); + const needBase = binaries.some((f) => basePathForGrowth(f) !== null); const baseSizes = needBase && req.baseSha - ? await fetchTreeSizes( + ? await fetchRelevantSizes( repo.owner, repo.repo, req.baseSha, token, + basePaths, fetchImpl, options.signal, ) @@ -188,8 +249,9 @@ export async function scanAssetWeight( continue; } - if (file.status === "modified" || file.status === "changed") { - const baseBytes = baseSizes.get(file.path); + const basePath = basePathForGrowth(file); + if (basePath) { + const baseBytes = baseSizes.get(basePath); if (typeof baseBytes !== "number") continue; const deltaBytes = bytes - baseBytes; if (deltaBytes < THRESHOLD_BYTES) continue; diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 9aa4863c13..3008b706f0 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -35,8 +35,8 @@ function promptText(value: string): string { } function formatBytes(n: number): string { - if (n >= 1048576) return `${(n / 1048576).toFixed(1)} MB`; - if (n >= 1024) return `${(n / 1024).toFixed(0)} KB`; + if (n >= 1048576) return `${(n / 1048576).toFixed(1)} MiB`; + if (n >= 1024) return `${(n / 1024).toFixed(0)} KiB`; return `${n} B`; } diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index cf1c4d7d25..b7de952841 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -13,6 +13,7 @@ export interface EnrichRequest { files?: Array<{ path: string; status?: string; + previousPath?: string; patch?: string; additions?: number; deletions?: number; diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index 6f8e557668..8a3a076b79 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -1620,6 +1620,88 @@ test("scanAssetWeight: flags a binary that GREW past the threshold (base vs head assert.equal(findings[0].bytes, 250000); }); +test("scanAssetWeight: flags a renamed binary that grew using its previous path", async () => { + const fetchImpl = async (url) => + String(url).includes(BASE_SHA) + ? treeReply([{ path: "old/video.mp4", type: "blob", size: 50000 }]) + : treeReply([{ path: "new/video.mp4", type: "blob", size: 250000 }]); + const findings = await scanAssetWeight( + { + repoFullName: "o/r", + prNumber: 1, + headSha: HEAD_SHA, + baseSha: BASE_SHA, + githubToken: "t", + files: [ + { + path: "new/video.mp4", + status: "renamed", + previousPath: "old/video.mp4", + }, + ], + }, + fetchImpl, + ); + assert.deepEqual(findings, [ + { + path: "new/video.mp4", + bytes: 250000, + deltaBytes: 200000, + status: "grown", + }, + ]); +}); + +test("scanAssetWeight: flags a copied binary that grew using its previous path", async () => { + const fetchImpl = async (url) => + String(url).includes(BASE_SHA) + ? treeReply([{ path: "old/data.bin", type: "blob", size: 50000 }]) + : treeReply([{ path: "copy/data.bin", type: "blob", size: 260000 }]); + const findings = await scanAssetWeight( + { + repoFullName: "o/r", + prNumber: 1, + headSha: HEAD_SHA, + baseSha: BASE_SHA, + githubToken: "t", + files: [ + { + path: "copy/data.bin", + status: "copied", + previousPath: "old/data.bin", + }, + ], + }, + fetchImpl, + ); + assert.deepEqual(findings, [ + { + path: "copy/data.bin", + bytes: 260000, + deltaBytes: 210000, + status: "grown", + }, + ]); +}); + +test("scanAssetWeight: renamed binaries need a previous path before reporting growth", async () => { + const findings = await scanAssetWeight( + { + repoFullName: "o/r", + prNumber: 1, + headSha: HEAD_SHA, + baseSha: BASE_SHA, + githubToken: "t", + files: [{ path: "video.mp4", status: "renamed" }], + }, + async (url) => + String(url).includes(BASE_SHA) + ? treeReply([{ path: "video.mp4", type: "blob", size: 50000 }]) + : treeReply([{ path: "video.mp4", type: "blob", size: 250000 }]), + ); + assert.deepEqual(findings, []); +}); + test("scanAssetWeight: small growth is not flagged", async () => { const fetchImpl = async (url) => String(url).includes(BASE_SHA) @@ -1761,20 +1843,27 @@ test("scanAssetWeight: missing baseSha does not reclassify modified binaries as assert.deepEqual(findings, []); }); -test("buildBrief: asset-weight degrades instead of trusting truncated tree responses", async () => { +test("buildBrief: asset-weight falls back to candidate paths for truncated tree responses", async () => { const realFetch = globalThis.fetch; - globalThis.fetch = async (url) => - String(url).includes("git/trees") - ? String(url).includes(BASE_SHA) - ? { - ok: true, - json: async () => ({ - truncated: true, - tree: [{ path: "big.png", type: "blob", size: 50000 }], - }), - } - : treeReply([{ path: "big.png", type: "blob", size: 300000 }]) - : { ok: true, json: async () => ({}) }; + const apiVersions: Array = []; + globalThis.fetch = async (url, init) => { + apiVersions.push( + (init?.headers as Record | undefined)?.[ + "X-GitHub-Api-Version" + ], + ); + const href = String(url); + if (href.includes("git/trees")) { + return { ok: true, json: async () => ({ truncated: true, tree: [] }) }; + } + if (href.includes("contents/big.png") && href.includes(HEAD_SHA)) { + return { ok: true, json: async () => ({ type: "file", size: 300000 }) }; + } + if (href.includes("contents/big.png") && href.includes(BASE_SHA)) { + return { ok: true, json: async () => ({ type: "file", size: 50000 }) }; + } + return { ok: true, json: async () => ({}) }; + }; try { const brief = await buildBrief({ repoFullName: "o/r", @@ -1785,10 +1874,12 @@ test("buildBrief: asset-weight degrades instead of trusting truncated tree respo files: [{ path: "big.png", status: "modified" }], analyzers: ["assetWeight"], }); - assert.equal(brief.partial, true); - assert.equal(brief.analyzerStatus.assetWeight, "degraded"); - assert.equal(brief.findings.assetWeight, undefined); - assert.doesNotMatch(brief.promptSection, /Heavy binary assets/); + assert.equal(brief.partial, false); + assert.equal(brief.analyzerStatus.assetWeight, "ok"); + assert.equal(brief.findings.assetWeight?.[0]?.status, "grown"); + assert.equal(brief.findings.assetWeight?.[0]?.deltaBytes, 250000); + assert.match(brief.promptSection, /Heavy binary assets/); + assert.ok(apiVersions.every((version) => version === "2022-11-28")); } finally { globalThis.fetch = realFetch; } @@ -1854,8 +1945,8 @@ test("renderBrief: renders the asset-weight block with human-readable sizes", () ], }); assert.match(r.promptSection, /Heavy binary assets/); - assert.match(r.promptSection, /`img\/logo\.png` adds 2\.4 MB/); - assert.match(r.promptSection, /`v\.mp4` grows \+195 KB to 293 KB/); + assert.match(r.promptSection, /`img\/logo\.png` adds 2\.4 MiB/); + assert.match(r.promptSection, /`v\.mp4` grows \+195 KiB to 293 KiB/); }); test("buildBrief: asset-weight analyzer reports grown binaries from request file status", async () => { diff --git a/src/review/enrichment-wire.ts b/src/review/enrichment-wire.ts index 1da182d875..3a976dab2f 100644 --- a/src/review/enrichment-wire.ts +++ b/src/review/enrichment-wire.ts @@ -84,6 +84,7 @@ export async function buildReviewEnrichment( files: input.files.map((file) => ({ path: file.path, status: file.status ?? undefined, + previousPath: file.previousFilename ?? undefined, patch: typeof file.payload?.patch === "string" ? file.payload.patch diff --git a/test/unit/enrichment-wire.test.ts b/test/unit/enrichment-wire.test.ts index f991112290..db6cd8d511 100644 --- a/test/unit/enrichment-wire.test.ts +++ b/test/unit/enrichment-wire.test.ts @@ -12,6 +12,12 @@ const input = { title: "t", files: [ { path: "a.ts", status: "modified", payload: { patch: "@@ +1 @@" } }, + { + path: "renamed.png", + status: "renamed", + previousFilename: "old.png", + payload: { patch: "@@ +2 @@" }, + }, { path: "b.ts" }, ] as never, diff: "the diff", @@ -81,7 +87,13 @@ describe("buildReviewEnrichment", () => { const body = JSON.parse(calls[0]!.init.body as string); expect(body.repoFullName).toBe("o/r"); expect(body.files).toEqual([ - { path: "a.ts", status: "modified", patch: "@@ +1 @@" }, + { path: "a.ts", status: "modified", previousPath: undefined, patch: "@@ +1 @@" }, + { + path: "renamed.png", + status: "renamed", + previousPath: "old.png", + patch: "@@ +2 @@", + }, { path: "b.ts", status: undefined, patch: undefined }, ]); }); From e11542b0eb6bdeca464ede6dfe14ee7d4e25d03d Mon Sep 17 00:00:00 2001 From: oktofeesh1 <287075021+oktofeesh1@users.noreply.github.com> Date: Sun, 28 Jun 2026 17:47:02 -0700 Subject: [PATCH 6/6] fix(enrichment): classify copied assets as additions Treat copied binary paths as newly added assets so heavy copied files are reported by their full introduced size instead of being discounted against the source path. Add a regression test that covers an equal-size copied binary and verifies the base tree is not fetched for that classification. --- review-enrichment/src/analyzers/asset-weight.ts | 5 ++--- review-enrichment/test/enrichment.test.ts | 16 ++++++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/review-enrichment/src/analyzers/asset-weight.ts b/review-enrichment/src/analyzers/asset-weight.ts index ba47ac4b7d..158e535938 100644 --- a/review-enrichment/src/analyzers/asset-weight.ts +++ b/review-enrichment/src/analyzers/asset-weight.ts @@ -82,8 +82,7 @@ type EnrichFile = NonNullable[number]; function basePathForGrowth(file: EnrichFile): string | null { if (file.status === "modified" || file.status === "changed") return file.path; - if (file.status === "renamed" || file.status === "copied") - return file.previousPath || null; + if (file.status === "renamed") return file.previousPath || null; return null; } @@ -237,7 +236,7 @@ export async function scanAssetWeight( const bytes = headSizes.get(file.path); if (typeof bytes !== "number") continue; - if (file.status === "added") { + if (file.status === "added" || file.status === "copied") { if (bytes >= THRESHOLD_BYTES) { findings.push({ path: file.path, diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index 8a3a076b79..4d9d65db87 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -1652,11 +1652,11 @@ test("scanAssetWeight: flags a renamed binary that grew using its previous path" ]); }); -test("scanAssetWeight: flags a copied binary that grew using its previous path", async () => { - const fetchImpl = async (url) => - String(url).includes(BASE_SHA) - ? treeReply([{ path: "old/data.bin", type: "blob", size: 50000 }]) - : treeReply([{ path: "copy/data.bin", type: "blob", size: 260000 }]); +test("scanAssetWeight: flags a copied binary as an added heavy path", async () => { + const fetchImpl = async (url) => { + assert.doesNotMatch(String(url), new RegExp(BASE_SHA)); + return treeReply([{ path: "copy/data.bin", type: "blob", size: 10485760 }]); + }; const findings = await scanAssetWeight( { repoFullName: "o/r", @@ -1677,9 +1677,9 @@ test("scanAssetWeight: flags a copied binary that grew using its previous path", assert.deepEqual(findings, [ { path: "copy/data.bin", - bytes: 260000, - deltaBytes: 210000, - status: "grown", + bytes: 10485760, + deltaBytes: 10485760, + status: "added", }, ]); });