Skip to content

fix(review): skip lockfiles in RAG via the canonical isLockfile set #8678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JSONbored merged 1 commit into from
Jul 25, 2026
Merged

fix(review): skip lockfiles in RAG via the canonical isLockfile set #8678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 8 additions & 3 deletions src/review/rag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import { errorStack } from "../utils/json";
import { neutralizePromptInjection } from "./prompt-injection";
import { isLockfile } from "../signals/path-matchers";

// ── Injected infra interfaces (inlined from reviewbot src/platform/types.ts) ──────────────────────
// These mirror the platform-adapter shapes so the host can pass its Vectorize/self-host-AI/D1-backed
Expand Down Expand Up @@ -152,8 +153,12 @@ export function ragEmbedBatchFromEnv(value: string | undefined): number {
// ── Filtering: index CODE, not content/data corpora (the primary free-tier cost guard) ───────────
const SKIP_DIR_RE =
/(^|\/)(node_modules|dist|build|out|coverage|vendor|\.git|\.next|\.nuxt|\.svelte-kit|\.turbo|\.cache|target|\.gradle|_build|\.venv|venv|__pycache__|\.mypy_cache|\.pytest_cache|\.ruff_cache|\.tox|\.terraform|content|data|fixtures|__snapshots__|__fixtures__|testdata|generated|public)\//i;
const SKIP_FILE_RE =
/(^|\/)(package-lock\.json|pnpm-lock\.yaml|yarn\.lock|bun\.lockb|cargo\.lock|poetry\.lock|composer\.lock|go\.sum)$|\.(min\.(js|css)|map|lock|snap)$/i;
// Lockfile NAMES are matched by the canonical isLockfile()/LOCKFILE_NAMES set (path-matchers.ts) in
// classifyRepoFile below, mirroring how DOC_EXT_RE already delegates to the canonical DOCS set -- a
// hand-rolled name list here could (and did, #8649) miss non-.lock-suffixed lockfiles like
// npm-shrinkwrap.json / packages.lock.json. This regex now only covers the suffix-shaped skips
// (minified bundles, sourcemaps, the generic `.lock` suffix, snapshots).
const SKIP_FILE_RE = /\.(min\.(js|css)|map|lock|snap)$/i;
const BINARY_EXT_RE =
/\.(png|jpe?g|gif|webp|avif|bmp|tiff?|heic|psd|svg|ico|pdf|zip|gz|tgz|tar|bz2|xz|zst|7z|rar|wasm|woff2?|otf|ttf|eot|mp4|mov|webm|mkv|mp3|wav|flac|ogg|opus|bin|exe|dll|so|dylib|node|class|jar|pyc|sqlite|db|parquet|onnx|gguf|safetensors|pt|pth|ckpt|npy|npz)$/i;
const CODE_EXT_RE =
Expand All @@ -171,7 +176,7 @@ const ALLOW_EXTLESS_RE = /(^|\/)(Dockerfile|Makefile|Justfile|Procfile|go\.mod|g
/** code | doc | skip. Skips dependency/build/content/data/binary paths — RAG indexes code for code
* review, not the (potentially huge) submission/content corpus. */
export function classifyRepoFile(path: string): RagKind | "skip" {
if (SKIP_DIR_RE.test(path) || SKIP_FILE_RE.test(path) || BINARY_EXT_RE.test(path)) return "skip";
if (SKIP_DIR_RE.test(path) || SKIP_FILE_RE.test(path) || isLockfile(path) || BINARY_EXT_RE.test(path)) return "skip";
if (DOC_EXT_RE.test(path)) return "doc";
if (CODE_EXT_RE.test(path) || ALLOW_EXTLESS_RE.test(path)) return "code";
return "skip";
Expand Down
9 changes: 9 additions & 0 deletions test/unit/rag.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,15 @@ describe("rag: code-not-content filtering (free-tier cost guard)", () => {
// go.sum stays skipped despite go.mod/go.work now being recognized — SKIP_FILE_RE's lockfile
// check runs before ALLOW_EXTLESS_RE, so the resolved-tree lockfile never becomes indexable.
expect(classifyRepoFile("go.sum")).toBe("skip");
// #8649: lockfile names that don't end in `.lock` are now skipped via the canonical isLockfile() set,
// not just the generic `.lock` suffix -- npm-shrinkwrap.json / packages.lock.json (.NET) were being
// misclassified as "code" and chunked into the RAG index. The rest of the canonical set still skips too.
expect(classifyRepoFile("npm-shrinkwrap.json")).toBe("skip");
expect(classifyRepoFile("packages.lock.json")).toBe("skip");
expect(classifyRepoFile("nested/dir/npm-shrinkwrap.json")).toBe("skip");
for (const lock of ["yarn.lock", "bun.lockb", "cargo.lock", "poetry.lock", "composer.lock"]) {
expect(classifyRepoFile(lock), lock).toBe("skip");
}
expect(classifyRepoFile("public/logo.png")).toBe("skip");
expect(classifyRepoFile("app.min.js")).toBe("skip");
// more binary blobs: media/archives/fonts/compiled artifacts and ML model weights
Expand Down