diff --git a/src/review/rag-index.ts b/src/review/rag-index.ts index be702b39db..a1bfb98f82 100644 --- a/src/review/rag-index.ts +++ b/src/review/rag-index.ts @@ -15,8 +15,11 @@ // Vectorize/AI binding, a GitHub error, an oversized repo, or a partial batch degrades to "indexed less / // nothing" rather than failing the job. `upsertChunks` itself already no-ops to 0 when infra is absent. // 2. FREE-TIER — `isIndexablePath` filters the tree to CODE (not the content/data corpus), source is -// prioritized, and a hard MAX_CHUNKS_PER_REPO cap bounds stored vectors per repo (the same cap retrieval -// assumes). We stop fetching once the cap is reached. +// prioritized ahead of docs, manifest/config files (package.json, tsconfig*.json, wrangler.*, +// pnpm-workspace.yaml, go.mod, Cargo.toml, pyproject.toml, ...) are prioritized ahead of THAT +// (manifestPriority — on a repo over the cap they'd otherwise tie every other source file and lose +// the alphabetical tiebreaker), and a hard MAX_CHUNKS_PER_REPO cap bounds stored vectors per repo +// (the same cap retrieval assumes). We stop fetching once the cap is reached. // // GATING — the caller (processors.ts) only DISPATCHES indexing when `isRagEnabled(env)` is true, and the cron // only ENQUEUES the fan-out under the same flag; flag-OFF (the default) this module is never invoked, makes no @@ -24,6 +27,7 @@ import { createInstallationToken } from "../github/app"; import { githubRateLimitAdmissionKeyForInstallation, timeoutFetch, type GitHubRateLimitAdmissionKey } from "../github/client"; +import { isConfigFile, isDependencyManifestFile } from "../signals/path-matchers"; import { repoParts } from "../utils/json"; import { createReviewAdapters } from "./adapters"; import { @@ -42,6 +46,23 @@ import { /** A single indexable entry from the repo git tree (path + size, used by isIndexablePath's size guard). */ type TreeEntry = { path: string; size?: number | undefined }; +/** + * Sort key that puts small, high-value manifest/config files (package.json, tsconfig*.json, + * wrangler.jsonc, pnpm-workspace.yaml, go.mod, Cargo.toml, pyproject.toml, requirements*.txt, ...) + * AHEAD of filePriority's code/doc split. On a repo whose file count exceeds MAX_CHUNKS_PER_REPO, + * `indexRepo`'s per-file loop stops once the cap is hit — with only `filePriority` (code=0, doc=1) + * as the sort key, a manifest file ties every other source file at priority 0 and then loses on the + * alphabetical tiebreaker, so it can be starved out entirely by volume (verified in prod: gittensory's + * own package.json never got indexed). These files are already indexable code (JSON/TOML/YAML all + * match CODE_EXT_RE in `./rag`) — this only reorders them, it does not change what's included. + * Reuses the same "manifest-like filename" classifiers signals/path-matchers.ts already exports for + * slop classification (isDependencyManifestFile / isConfigFile) rather than inventing a second + * filename vocabulary. + */ +function manifestPriority(path: string): number { + return isDependencyManifestFile(path) || isConfigFile(path) ? -1 : filePriority(path); +} + /** Cap on how many chunks we upsert per Vectorize/D1 write batch (bounds the bound-param + neuron cost per call; * embedTexts itself batches the AI calls at EMBED_BATCH internally). */ const UPSERT_BATCH = 50; @@ -230,8 +251,9 @@ export type IndexRepoResult = { indexed: number; files: number; capped: boolean /** * FULL (re)index of a repo's CODE into the RAG index. Fetches the git tree at the default branch, filters to - * indexable code/docs (isIndexablePath), prioritizes source over docs, fetches each file's content, chunks it - * (chunkFile), and upserts (embed + Vectorize + repo_chunks via upsertChunks) up to MAX_CHUNKS_PER_REPO. + * indexable code/docs (isIndexablePath), prioritizes manifest/config files first, then source over docs + * (manifestPriority), fetches each file's content, chunks it (chunkFile), and upserts (embed + Vectorize + + * repo_chunks via upsertChunks) up to MAX_CHUNKS_PER_REPO. * * Idempotent: chunk ids are stable (namespace|path::idx) so re-running upserts (ON CONFLICT updates) the same * rows rather than duplicating. Fully FAIL-SAFE — any error (no infra, GitHub down, bad file) degrades to @@ -263,7 +285,7 @@ export async function indexRepo( if (rawTree === null) return empty; const tree = rawTree .filter((entry) => isIndexablePath(entry.path, entry.size)) - .sort((a, b) => filePriority(a.path) - filePriority(b.path) || a.path.localeCompare(b.path)); + .sort((a, b) => manifestPriority(a.path) - manifestPriority(b.path) || a.path.localeCompare(b.path)); await pruneMissingPaths(infra, project, repoName, new Set(tree.map((entry) => entry.path))); if (tree.length === 0) return empty; diff --git a/src/review/rag.ts b/src/review/rag.ts index 1db1e9b8c7..e65653a6d6 100644 --- a/src/review/rag.ts +++ b/src/review/rag.ts @@ -133,7 +133,10 @@ const CODE_EXT_RE = // spellings were missing here, so e.g. NOTES.markdown / guide.asciidoc were // misclassified as skip instead of doc. const DOC_EXT_RE = /\.(md|mdx|markdown|rst|adoc|asciidoc|txt)$/i; -const ALLOW_EXTLESS_RE = /(^|\/)(Dockerfile|Makefile|Justfile|Procfile)$/i; +// `go.mod`/`go.work` (Go's extensionless dependency manifests) belong here for the same reason as +// Dockerfile/Makefile: no recognized extension, but a real, high-value source file that must not +// fall through to "skip" (go.sum/go.work.sum are resolved-tree lockfiles, already excluded above). +const ALLOW_EXTLESS_RE = /(^|\/)(Dockerfile|Makefile|Justfile|Procfile|go\.mod|go\.work)$/i; /** code | doc | skip. Skips dependency/build/content/data/binary paths — RAG indexes code for code * review, not the (potentially huge) submission/content corpus. */ diff --git a/test/unit/rag-index.test.ts b/test/unit/rag-index.test.ts index 1cf0ddd03f..763ebe185d 100644 --- a/test/unit/rag-index.test.ts +++ b/test/unit/rag-index.test.ts @@ -302,6 +302,85 @@ describe("indexRepo: MAX_CHUNKS_PER_REPO cap holds", () => { expect(await countChunks(env, PROJECT, "gittensory")).toBeLessThanOrEqual(MAX_CHUNKS_PER_REPO); expect(result.indexed).toBe(MAX_CHUNKS_PER_REPO); }); + + it("still indexes package.json (and other root manifest/config files) on a repo whose file count exceeds the cap (regression: manifestPriority)", async () => { + const { env } = indexEnv(); + // Alphabetically, "package.json" sorts AFTER most of "src/f0.ts".."src/f.ts" — so without + // manifest-first prioritization it would be starved out once the cap is reached, exactly as it + // was in prod for gittensory's own repo (#confirmed via repo_chunks query). + const overCap = MAX_CHUNKS_PER_REPO + 25; + const tree: Array<{ path: string; size: number }> = [ + { path: "package.json", size: 20 }, + { path: "tsconfig.json", size: 20 }, + { path: "pnpm-workspace.yaml", size: 20 }, + { path: "go.mod", size: 20 }, + { path: "Cargo.toml", size: 20 }, + { path: "pyproject.toml", size: 20 }, + { path: "requirements.txt", size: 20 }, + { path: "wrangler.jsonc", size: 20 }, + ...Array.from({ length: overCap }, (_, i) => ({ path: `src/f${i}.ts`, size: 20 })), + ]; + const files: Record = { + "package.json": '{"name":"gittensory"}\n', + "tsconfig.json": "{}\n", + "pnpm-workspace.yaml": "packages:\n", + "go.mod": "module example.com/foo\n", + "Cargo.toml": "[package]\n", + "pyproject.toml": "[project]\n", + "requirements.txt": "flask\n", + "wrangler.jsonc": "{}\n", + }; + for (let i = 0; i < overCap; i++) files[`src/f${i}.ts`] = `export const f${i} = ${i};\n`; + stubGithub({ tree, files }); + + const result = await indexRepo(env, PROJECT, REPO); + + expect(result.capped).toBe(true); + expect(result.indexed).toBe(MAX_CHUNKS_PER_REPO); + const indexedPaths = await pathsFor(env, PROJECT, "gittensory"); + for (const manifest of [ + "package.json", + "tsconfig.json", + "pnpm-workspace.yaml", + "go.mod", + "Cargo.toml", + "pyproject.toml", + "requirements.txt", + "wrangler.jsonc", + ]) { + expect(indexedPaths).toContain(manifest); + } + }); +}); + +describe("manifestPriority ordering (via indexRepo, well under the cap)", () => { + afterEach(() => vi.unstubAllGlobals()); + + it("orders manifest/config files first, then other code, then docs — byte-identical file SET to before when the cap never matters", async () => { + const { env } = indexEnv(); + stubGithub({ + tree: [ + { path: "README.md", size: 10 }, + { path: "src/z.ts", size: 10 }, + { path: "package.json", size: 10 }, + { path: "src/a.ts", size: 10 }, + ], + files: { + "README.md": "# Title\n", + "src/z.ts": "export const z = 1;\n", + "package.json": '{"name":"x"}\n', + "src/a.ts": "export const a = 1;\n", + }, + }); + + const result = await indexRepo(env, PROJECT, REPO); + + // Nowhere near the cap: every indexable file is still indexed (same SET as filePriority alone + // would have produced) — only the ORDER of indexing/upsert changed, not the outcome. + expect(result.capped).toBe(false); + expect(result.files).toBe(4); + expect(await pathsFor(env, PROJECT, "gittensory")).toEqual(["README.md", "package.json", "src/a.ts", "src/z.ts"]); + }); }); describe("reindexChangedPaths: delete + re-upsert only the changed paths", () => { diff --git a/test/unit/rag.test.ts b/test/unit/rag.test.ts index 6bbe3434d9..8dee900309 100644 --- a/test/unit/rag.test.ts +++ b/test/unit/rag.test.ts @@ -66,6 +66,12 @@ describe("rag: code-not-content filtering (free-tier cost guard)", () => { ]) { expect(classifyRepoFile(p)).toBe("code"); } + // Go's extensionless dependency manifests (go.mod/go.work) are real, high-value source — same + // extensionless-allowlist treatment as Dockerfile/Makefile. Their resolved-tree lockfile + // siblings (go.sum/go.work.sum) stay excluded via SKIP_FILE_RE below. + expect(classifyRepoFile("go.mod")).toBe("code"); + expect(classifyRepoFile("go.work")).toBe("code"); + expect(classifyRepoFile("nested/module/go.mod")).toBe("code"); expect(classifyRepoFile("README.md")).toBe("doc"); expect(classifyRepoFile("docs/architecture.mdx")).toBe("doc"); // long-form doc spellings (parity with signals/path-matchers DOCS_EXTENSIONS) @@ -78,6 +84,9 @@ describe("rag: code-not-content filtering (free-tier cost guard)", () => { expect(classifyRepoFile("dist/bundle.js")).toBe("skip"); expect(classifyRepoFile("package-lock.json")).toBe("skip"); expect(classifyRepoFile("pnpm-lock.yaml")).toBe("skip"); + // 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"); 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