feat(enrichment): skip all binary + lockfile files in history analyzers

review-enrichment/src/analyzers/blame-link.ts

@@ -12,15 +12,13 @@ import type {
 } from "../types.js";
 import type { AnalysisContext } from "../analysis-context.js";
 import { boundedFetchJson } from "../external-fetch.js";
+import { isHistoryUninformativePath } from "./history-path.js";
 
 const GITHUB_API = "https://github.com/ghapi";
 const SLUG_RE = /^[A-Za-z0-9._-]+$/;
 const MAX_FILES_PROBED = 6; // bound the files we probe, matching the other history-class analyzers
 const MAX_LOOKUPS = 12; // hard cap on total GitHub round-trips (each file costs up to 2: commits + pulls)
 const SHA_PREFIX_LEN = 12;
-// Files whose commit history is not a useful "who introduced this" signal — lockfiles, generated output, binaries.
-const SKIP_RE =
-  /(?:^|\/)(?:package-lock\.json|yarn\.lock|pnpm-lock\.yaml|poetry\.lock|go\.sum)$|\.(?:lock|min\.js|map|snap|png|jpe?g|gif|svg|ico|pdf|zip|gz|woff2?)$|(?:^|\/)(?:dist|build|vendor)\//i;
 
 interface ScanOptions {
   signal?: AbortSignal;
@@ -159,7 +157,7 @@ export async function scanBlameLink(
   // additions (a patch with no deletion line has no prior author to attribute).
   const candidates: Array<{ lookupPath: string; displayPath: string; line: number }> = [];
   for (const file of files) {
-    if (file.status === "added" || SKIP_RE.test(file.path)) continue;
+    if (file.status === "added" || isHistoryUninformativePath(file.path)) continue;
     let line = file.patch ? firstTouchedOldLine(file.patch) : null;
     // A removed OR renamed file resolves against the base tree even without a usable patch (binary/truncated, or a
     // pure rename with no content change). Anchor to line 1 as the representative point.

review-enrichment/src/analyzers/churn-hotspot.ts

@@ -11,6 +11,7 @@ import type {
 } from "../types.js";
 import type { AnalysisContext } from "../analysis-context.js";
 import { boundedFetchJson } from "../external-fetch.js";
+import { isHistoryUninformativePath } from "./history-path.js";
 
 const GITHUB_API = "https://github.com/ghapi";
 const SLUG_RE = /^[A-Za-z0-9._-]+$/;
@@ -19,9 +20,6 @@ const PER_PAGE = 100; // one page; a file with a full page of commits in the win
 const MAX_FILES_PROBED = 8; // bound the GitHub round-trips, matching the other history-class analyzers
 const MIN_COMMITS = 8; // a hotspot must change frequently within the window
 const MIN_FIX_FRACTION = 0.3; // and a meaningful share of those changes must be fixes/reverts
-// Files whose commit churn is not a useful code-fragility signal — lockfiles, generated output, and binaries.
-const SKIP_RE =
-  /(?:^|\/)(?:package-lock\.json|yarn\.lock|pnpm-lock\.yaml|poetry\.lock|go\.sum)$|\.(?:lock|min\.js|map|snap|png|jpe?g|gif|svg|ico|pdf|zip|gz|woff2?)$|(?:^|\/)(?:dist|build|vendor)\//i;
 // Defect-correcting commit subjects: fix/bugfix/hotfix/revert/regression (conventional-commit `fix:` included).
 const FIX_RE = /\b(?:fix(?:e[ds]|ing)?|bug ?fix|hotfix|revert(?:ed|s)?|regression)\b/i;
 
@@ -109,7 +107,7 @@ export async function scanChurnHotspot(
   const since = new Date(Date.now() - WINDOW_DAYS * 86_400_000).toISOString();
   // A newly-added file has no prior history; skip it (and non-code/generated files) before spending a round-trip.
   const paths = files
-    .filter((file) => file.status !== "added" && !SKIP_RE.test(file.path))
+    .filter((file) => file.status !== "added" && !isHistoryUninformativePath(file.path))
     .map((file) => file.path)
     .slice(0, MAX_FILES_PROBED);
 

review-enrichment/src/analyzers/history-path.ts

@@ -0,0 +1,20 @@
+// Shared skip predicate for the history-class analyzers (blame-link #2034, churn-hotspot #1513). A file whose
+// commit history carries no useful "who introduced this" / code-fragility signal — a lockfile, generated
+// output, or a binary blob — should be skipped by both, so the rule lives in one place instead of a duplicated
+// per-analyzer regex. Binary and lockfile recognition delegate to the unified inventories (binary-extensions,
+// lockfile-path) so this stays in sync as those grow, rather than a hand-maintained list.
+import { BINARY_EXT_RE } from "./binary-extensions.js";
+import { isSupportedLockfile } from "../lockfile-path.js";
+
+// Non-binary generated output (and the original narrow binary/lockfile set) — kept verbatim from the previous
+// per-analyzer SKIP_RE. Broader binary and lockfile coverage is added by the two shared inventories below.
+const SKIP_RE =
+  /(?:^|\/)(?:package-lock\.json|yarn\.lock|pnpm-lock\.yaml|poetry\.lock|go\.sum)$|\.(?:lock|min\.js|map|snap|png|jpe?g|gif|svg|ico|pdf|zip|gz|woff2?)$|(?:^|\/)(?:dist|build|vendor)\//i;
+
+/** True when a file's commit history is not a useful fragility/attribution signal — a lockfile, generated
+ *  output, or a binary blob. Additive over the original rule: it now also skips the full shared binary
+ *  inventory (e.g. webp/avif/heic/mp4/wasm/safetensors) and the full lockfile set (e.g. Cargo.lock,
+ *  composer.lock, bun.lockb), not just the original narrow list. Pure. */
+export function isHistoryUninformativePath(path: string): boolean {
+  return SKIP_RE.test(path) || BINARY_EXT_RE.test(path) || isSupportedLockfile(path);
+}

review-enrichment/test/churn-hotspot.test.ts

@@ -61,7 +61,14 @@ test("scanChurnHotspot: marks the count capped when the page is full", async ()
 test("scanChurnHotspot: skips lockfiles, binaries, and newly-added files without fetching", async () => {
   let called = false;
   const out = await scanChurnHotspot(
-    req([{ path: "package-lock.json" }, { path: "assets/logo.png" }, { path: "src/new.ts", status: "added" }]),
+    req([
+      { path: "package-lock.json" },
+      { path: "assets/logo.png" },
+      // Broader shared-inventory skips: a heavy binary and a Cargo lockfile the original narrow regex missed.
+      { path: "models/llama.safetensors" },
+      { path: "crates/api/Cargo.lock" },
+      { path: "src/new.ts", status: "added" },
+    ]),
     async () => {
       called = true;
       return jsonResponse(commits(50, 2));

review-enrichment/test/history-path.test.ts

@@ -0,0 +1,67 @@
+// Units for the shared history-class skip predicate used by blame-link and churn-hotspot.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+import { isHistoryUninformativePath } from "../dist/analyzers/history-path.js";
+
+test("isHistoryUninformativePath skips the original lockfile / generated / dir set", () => {
+  // Lockfiles from the original rule.
+  for (const p of [
+    "package-lock.json",
+    "app/yarn.lock",
+    "pkg/pnpm-lock.yaml",
+    "poetry.lock",
+    "svc/go.sum",
+  ]) {
+    assert.equal(isHistoryUninformativePath(p), true, p);
+  }
+  // Non-binary generated output and vendored/build directories.
+  for (const p of [
+    "flake.lock",
+    "app.min.js",
+    "bundle.js.map",
+    "__snapshots__/x.snap",
+    "icons/logo.svg",
+    "dist/app.js",
+    "build/out.js",
+    "vendor/pkg/mod.go",
+  ]) {
+    assert.equal(isHistoryUninformativePath(p), true, p);
+  }
+});
+
+test("isHistoryUninformativePath skips the full binary inventory, not just the original narrow list", () => {
+  // Original narrow binaries still skip.
+  for (const p of ["ui/logo.png", "fonts/Inter.woff2", "docs/spec.pdf"]) {
+    assert.equal(isHistoryUninformativePath(p), true, p);
+  }
+  // Newly-covered shared-inventory binaries (were NOT in the original per-analyzer regex).
+  for (const p of [
+    "media/demo.mp4",
+    "assets/hero.webp",
+    "assets/photo.heic",
+    "vendor/lib.wasm",
+    "models/llama.safetensors",
+    "models/model.gguf",
+  ]) {
+    assert.equal(isHistoryUninformativePath(p), true, p);
+  }
+});
+
+test("isHistoryUninformativePath skips the full lockfile inventory (Cargo/Composer/Bun)", () => {
+  for (const p of ["crates/api/Cargo.lock", "composer.lock", "bun.lockb"]) {
+    assert.equal(isHistoryUninformativePath(p), true, p);
+  }
+});
+
+test("isHistoryUninformativePath does not skip real source/doc files", () => {
+  for (const p of [
+    "src/index.ts",
+    "packages/app/main.rs",
+    "README.md",
+    "Cargo.toml",
+    "notes/cargo.lock.md",
+  ]) {
+    assert.equal(isHistoryUninformativePath(p), false, p);
+  }
+});