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
16 changes: 16 additions & 0 deletions migrations/0128_repo_chunks_blob_sha.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
-- Convergence (RAG / codebase index, #4365): per-file change detection so a FULL re-index (the cron fan-out,
-- rag-index.ts's indexRepo) stops unconditionally re-fetching + re-embedding every indexable file on every
-- cycle. Git's tree API already returns a content-addressed blob SHA per file for free (indexRepo already
-- fetches the tree) -- persisting the SHA we last indexed a path AT lets the next full reindex skip any file
-- whose blob SHA is unchanged, entirely (no GitHub content fetch, no chunk, no embed call), while still
-- re-processing anything genuinely new/changed/deleted exactly as today.
--
-- WHO WRITES IT -- upsertChunks (src/review/rag.ts): the new optional blobSha param is stamped onto every
-- chunk row belonging to that upsert call (all chunks of one file share one SHA). WHO READS IT -- the new
-- getStoredChunkMeta (src/review/rag.ts), one grouped query returning {path -> {blobSha, count}} that
-- indexRepo (src/review/rag-index.ts) consults before deciding to fetch a file.
--
-- Nullable + no backfill: existing rows get blob_sha=NULL, which never matches a fresh tree SHA, so the
-- FIRST post-migration full reindex re-embeds every file once (as it already does today) and then converges
-- to the cheap steady state from then on. Additive + idempotent, matching the 0051 repo_chunks convention.
ALTER TABLE repo_chunks ADD COLUMN blob_sha TEXT;
50 changes: 39 additions & 11 deletions src/review/rag-index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
countRepoChunks,
deleteChunksForPaths,
filePriority,
getStoredChunkMeta,
isIndexablePath,
MAX_CHUNKS_PER_REPO,
MAX_FILE_BYTES,
Expand All @@ -44,8 +45,10 @@ import {
upsertChunks,
} from "./rag";

/** A single indexable entry from the repo git tree (path + size, used by isIndexablePath's size guard). */
type TreeEntry = { path: string; size?: number | undefined };
/** A single indexable entry from the repo git tree (path + size, used by isIndexablePath's size guard, + the
* blob SHA (#4365) — git's own content hash, free on the tree response — used to skip re-embedding a file
* whose content hasn't changed since the last full index). */
type TreeEntry = { path: string; size?: number | undefined; sha?: string | undefined };

/**
* Sort key that puts small, high-value manifest/config files (package.json, tsconfig*.json,
Expand Down Expand Up @@ -109,11 +112,15 @@ async function fetchRepoTree(env: Env, repoFullName: string, ref: string, token:
...(admissionKey ? { githubRateLimitAdmissionKey: admissionKey } : {}),
});
if (!response.ok) return null;
const body = (await response.json()) as { tree?: Array<{ path?: string; type?: string; size?: number }> } | null;
const body = (await response.json()) as { tree?: Array<{ path?: string; type?: string; size?: number; sha?: string }> } | null;
const entries: TreeEntry[] = [];
for (const node of body?.tree ?? []) {
if (node.type !== "blob" || typeof node.path !== "string" || node.path.length === 0) continue;
entries.push(typeof node.size === "number" ? { path: node.path, size: node.size } : { path: node.path });
entries.push({
path: node.path,
...(typeof node.size === "number" ? { size: node.size } : {}),
...(typeof node.sha === "string" && node.sha.length > 0 ? { sha: node.sha } : {}),
});
}
return entries;
} catch (error) {
Expand Down Expand Up @@ -192,16 +199,17 @@ function indexRef(defaultBranch: string | null | undefined): string {
}

/** Upsert a set of chunks to the index in bounded batches, honoring the per-repo cap. Returns the number
* actually upserted. Each batch is independent: a failed batch (upsertChunks returns 0) doesn't abort the rest. */
async function upsertChunksCapped(env: Env, project: string, repo: string, chunks: RagChunk[], alreadyStored: number): Promise<number> {
* actually upserted. Each batch is independent: a failed batch (upsertChunks returns 0) doesn't abort the rest.
* `blobSha` (#4365) is threaded straight through to upsertChunks — see its doc comment. */
async function upsertChunksCapped(env: Env, project: string, repo: string, chunks: RagChunk[], alreadyStored: number, blobSha?: string): Promise<number> {
const infra = createReviewAdapters(env);
let stored = alreadyStored;
let upserted = 0;
for (let i = 0; i < chunks.length && stored < MAX_CHUNKS_PER_REPO; i += UPSERT_BATCH) {
const remaining = MAX_CHUNKS_PER_REPO - stored;
const batch = chunks.slice(i, i + Math.min(UPSERT_BATCH, remaining));
if (batch.length === 0) break;
const n = await upsertChunks(infra, project, repo, batch);
const n = await upsertChunks(infra, project, repo, batch, blobSha);
upserted += n;
stored += n;
}
Expand Down Expand Up @@ -256,6 +264,13 @@ export type IndexRepoResult = { indexed: number; files: number; capped: boolean
* (manifestPriority), fetches each file's content, chunks it (chunkFile), and upserts (embed + Vectorize +
* repo_chunks via upsertChunks) up to MAX_CHUNKS_PER_REPO.
*
* Embedding cache (#4365): a file whose git blob SHA (free on the tree response) matches what we stored the
* last time we indexed that path is skipped ENTIRELY — no content fetch, no chunk, no embed call — since its
* content provably hasn't changed. This is what keeps the cron fan-out cheap on repeat runs: only genuinely
* new/changed files ever reach the embedding model. A changed file's old chunks are deleted before its new
* ones are upserted (mirrors reindexChangedPaths), which also prevents stale trailing chunks when a file
* shrinks to fewer chunks than it had before.
*
* 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
* "indexed fewer / nothing"; this NEVER throws.
Expand Down Expand Up @@ -290,29 +305,42 @@ export async function indexRepo(
await pruneMissingPaths(infra, project, repoName, new Set(tree.map((entry) => entry.path)));
if (tree.length === 0) return empty;

// 2. Fetch + chunk + upsert, stopping once the per-repo vector cap is reached.
let stored = 0;
// 2. Fetch + chunk + upsert, stopping once the per-repo vector cap is reached. `stored` seeds from the
// real post-prune total (not 0) so a run that skips most files still caps correctly against everything
// already retained, not just what THIS run touches.
const knownChunks = await getStoredChunkMeta(infra.storage, project, repoName);
let stored = await countRepoChunks(infra.storage, project, repoName);
let upserted = 0;
let filesIndexed = 0;
let skipped = 0;
let capped = false;
for (const entry of tree) {
if (stored >= MAX_CHUNKS_PER_REPO) {
capped = true;
break;
}
const known = knownChunks.get(entry.path);
if (entry.sha && known?.blobSha && known.blobSha === entry.sha) {
skipped += 1;
continue; // unchanged since the last full index — skip the fetch/chunk/embed entirely
}
const text = await fetchFileText(env, repoFullName, entry.path, ref, token, admissionKey);
if (text === null) continue;
const chunks = chunkFile(entry.path, text, namespace);
if (chunks.length === 0) continue;
const n = await upsertChunksCapped(env, project, repoName, chunks, stored);
if (known && known.count > 0) {
await deleteChunksForPaths(infra, project, repoName, [entry.path]);
stored -= known.count;
}
const n = await upsertChunksCapped(env, project, repoName, chunks, stored, entry.sha);
if (n > 0) {
upserted += n;
stored += n;
filesIndexed += 1;
}
}
console.log(
JSON.stringify({ event: "rag_index_repo", project, repo: repoFullName, files: filesIndexed, indexed: upserted, capped }),
JSON.stringify({ event: "rag_index_repo", project, repo: repoFullName, files: filesIndexed, indexed: upserted, skipped, capped }),
);
return { indexed: upserted, files: filesIndexed, capped };
} catch (error) {
Expand Down
34 changes: 29 additions & 5 deletions src/review/rag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,26 @@ export async function countRepoChunks(storage: StorageAdapter, project: string,
}
}

/** Per-path {blobSha, count} for every path currently stored for a repo (#4365 embedding-cache). One grouped
* query so a full reindex can decide, per file, "unchanged since last index → skip the fetch/chunk/embed"
* without an N+1 lookup. A path's chunks always share one blob_sha (upsertChunks stamps it uniformly across
* a file's chunks in the same call), so MAX(blob_sha) is just "that file's one value", not a real aggregate
* choice. Fail-safe: an empty map on any storage error degrades the caller to "treat everything as changed"
* (indexRepo's existing behavior today), never a crash or a wrongly-skipped file. */
export async function getStoredChunkMeta(storage: StorageAdapter, project: string, repo: string): Promise<Map<string, { blobSha: string | null; count: number }>> {
const out = new Map<string, { blobSha: string | null; count: number }>();
try {
const rows = await storage
.prepare("SELECT path, MAX(blob_sha) AS blob_sha, COUNT(*) AS cnt FROM repo_chunks WHERE project = ? AND repo = ? GROUP BY path")
.bind(project, repo)
.all<{ path: string; blob_sha: string | null; cnt: number }>();
for (const row of rows.results ?? []) out.set(row.path, { blobSha: row.blob_sha, count: row.cnt });
return out;
} catch {
return out;
}
}

// ── Embedding (fail-safe: null on any failure) ────────────────────────────────────────────────────
export async function embedTexts(
inference: InferenceAdapter | undefined,
Expand Down Expand Up @@ -323,8 +343,12 @@ export async function embedTexts(

// ── Index write (used by ingestion): embed + vector upsert + chunk-text store ─────────────────────
/** Upsert chunks: write text to the storage table (source of truth) + vectors+light metadata to the vector
* index. Returns the number upserted (0 on any failure — ingestion treats that as "try again later"). */
export async function upsertChunks(infra: RagInfra, project: string, repo: string, chunks: RagChunk[]): Promise<number> {
* index. Returns the number upserted (0 on any failure — ingestion treats that as "try again later").
* `blobSha` (#4365) is the source file's git blob SHA at index time, stamped onto every chunk row for
* getStoredChunkMeta to compare against on the next full reindex — omit it (e.g. the incremental
* reindexChangedPaths caller, which isn't handed tree SHAs) and the column just stays NULL, which simply
* never matches a future SHA and self-heals on that path's next full-reindex pass. */
export async function upsertChunks(infra: RagInfra, project: string, repo: string, chunks: RagChunk[], blobSha?: string): Promise<number> {
const { storage: db, vector: vec, inference } = infra;
if (!vec || !inference || chunks.length === 0) return 0;
const namespace = ragNamespace(project, repo);
Expand All @@ -341,9 +365,9 @@ export async function upsertChunks(infra: RagInfra, project: string, repo: strin
);
const stmts = chunks.map((c) =>
db.prepare(
"INSERT INTO repo_chunks (id, project, repo, path, chunk_index, kind, text) VALUES (?,?,?,?,?,?,?) " +
"ON CONFLICT(id) DO UPDATE SET text=excluded.text, kind=excluded.kind, chunk_index=excluded.chunk_index, updated_at=CURRENT_TIMESTAMP",
).bind(c.id, project, repo, c.path, c.chunkIndex, c.kind, c.text),
"INSERT INTO repo_chunks (id, project, repo, path, chunk_index, kind, text, blob_sha) VALUES (?,?,?,?,?,?,?,?) " +
"ON CONFLICT(id) DO UPDATE SET text=excluded.text, kind=excluded.kind, chunk_index=excluded.chunk_index, blob_sha=excluded.blob_sha, updated_at=CURRENT_TIMESTAMP",
).bind(c.id, project, repo, c.path, c.chunkIndex, c.kind, c.text, blobSha ?? null),
);
await db.batch(stmts);
return chunks.length;
Expand Down
128 changes: 127 additions & 1 deletion test/unit/rag-index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const QUEUE_PROJECT = "JSONbored";

/** Stub global fetch for the git-tree + raw-contents calls the populator makes. */
function stubGithub(opts: {
tree?: Array<{ path: string; type?: string; size?: number }>;
tree?: Array<{ path: string; type?: string; size?: number; sha?: string }>;
files?: Record<string, string>;
treeStatus?: number;
}) {
Expand Down Expand Up @@ -302,6 +302,132 @@ describe("indexRepo: full repo index (tree → chunk → embed → upsert)", ()
});
});

describe("indexRepo: embedding cache (#4365) — skip unchanged files by git blob SHA", () => {
afterEach(() => vi.unstubAllGlobals());

it("skips a file whose blob SHA is unchanged across two full-index runs (no re-fetch, no re-embed)", async () => {
const { env, ai } = indexEnv();
stubGithub({
tree: [{ path: "src/a.ts", size: 30, sha: "sha-a-v1" }],
files: { "src/a.ts": "export const a = 1;\n" },
});
const first = await indexRepo(env, PROJECT, REPO);
expect(first).toMatchObject({ files: 1, indexed: 1 });
const embedCallsAfterFirst = ai.run.mock.calls.length;

// Same tree/sha, but the content fetch now 404s: if the skip logic failed to trigger, this run would
// either drop the file (fetch fails) or spend a redundant embed call. A correctly-skipped file leaves
// both the result and the embed-call count untouched.
stubGithub({ tree: [{ path: "src/a.ts", size: 30, sha: "sha-a-v1" }], files: {} });
const second = await indexRepo(env, PROJECT, REPO);

expect(second).toEqual({ indexed: 0, files: 0, capped: false });
expect(ai.run.mock.calls.length).toBe(embedCallsAfterFirst);
expect(await countChunks(env, PROJECT, "gittensory")).toBe(1);
});

it("re-embeds a file whose blob SHA changed since the last index, replacing its old chunk row", async () => {
const { env, ai } = indexEnv();
stubGithub({
tree: [{ path: "src/a.ts", size: 30, sha: "sha-a-v1" }],
files: { "src/a.ts": "export const a = 1;\n" },
});
await indexRepo(env, PROJECT, REPO);
expect(await countChunks(env, PROJECT, "gittensory")).toBe(1);

stubGithub({
tree: [{ path: "src/a.ts", size: 30, sha: "sha-a-v2" }],
files: { "src/a.ts": "export const a = 2; // changed\n" },
});
const second = await indexRepo(env, PROJECT, REPO);

expect(second).toMatchObject({ files: 1, indexed: 1 });
expect(ai.run).toHaveBeenCalledTimes(2); // one embed call per run — the second run's SHA changed, so it re-embedded
const row = await env.DB.prepare("SELECT text, blob_sha FROM repo_chunks WHERE project=? AND repo=? AND path=?")
.bind(PROJECT, "gittensory", "src/a.ts")
.first<{ text: string; blob_sha: string }>();
expect(row?.text).toBe("export const a = 2; // changed\n");
expect(row?.blob_sha).toBe("sha-a-v2");
});

it("removes stale trailing chunks when a changed file now chunks into FEWER pieces than it had before", async () => {
const { env } = indexEnv();
const ns = ragNamespace(PROJECT, "gittensory");
// Simulate a prior index where src/a.ts had 2 chunks under an old blob SHA.
for (const idx of [0, 1]) {
await env.DB.prepare("INSERT INTO repo_chunks (id, project, repo, path, chunk_index, kind, text, blob_sha) VALUES (?,?,?,?,?,?,?,?)")
.bind(`${ns}|src/a.ts::${idx}`, PROJECT, "gittensory", "src/a.ts", idx, "code", `old chunk ${idx}`, "sha-old")
.run();
}
expect(await countChunks(env, PROJECT, "gittensory")).toBe(2);

// New tree: same path, a DIFFERENT sha, and small content that chunkFile packs into exactly one chunk.
stubGithub({
tree: [{ path: "src/a.ts", size: 20, sha: "sha-new" }],
files: { "src/a.ts": "export const a = 1;\n" },
});
const result = await indexRepo(env, PROJECT, REPO);

expect(result).toMatchObject({ files: 1, indexed: 1 });
// Only the new single chunk remains — the old chunk_index:1 row was deleted, not left orphaned.
expect(await countChunks(env, PROJECT, "gittensory")).toBe(1);
const row = await env.DB.prepare("SELECT id FROM repo_chunks WHERE project=? AND repo=? AND path=?")
.bind(PROJECT, "gittensory", "src/a.ts")
.first<{ id: string }>();
expect(row?.id).toBe(`${ns}|src/a.ts::0`);
});

it("a file with no blob SHA in the tree response always reprocesses (defensive: never skips without proof of a match)", async () => {
const { env, ai } = indexEnv();
const ns = ragNamespace(PROJECT, "gittensory");
await env.DB.prepare("INSERT INTO repo_chunks (id, project, repo, path, chunk_index, kind, text, blob_sha) VALUES (?,?,?,?,?,?,?,?)")
.bind(`${ns}|src/a.ts::0`, PROJECT, "gittensory", "src/a.ts", 0, "code", "old", "sha-old")
.run();
// The tree entry omits `sha` entirely (a defensive/older-API-shape case) — must not be treated as a match.
stubGithub({ tree: [{ path: "src/a.ts", size: 20 }], files: { "src/a.ts": "export const a = 1;\n" } });

const result = await indexRepo(env, PROJECT, REPO);

expect(result).toMatchObject({ files: 1, indexed: 1 });
expect(ai.run).toHaveBeenCalledTimes(1);
});

it("counts pre-existing (skipped, unchanged) chunks toward the cap so a mixed skip+new run never exceeds MAX_CHUNKS_PER_REPO", async () => {
const { env } = indexEnv();
const ns = ragNamespace(PROJECT, "gittensory");
const existingCount = MAX_CHUNKS_PER_REPO - 1;
await env.DB.batch(
Array.from({ length: existingCount }, (_, i) =>
env.DB.prepare("INSERT INTO repo_chunks (id, project, repo, path, chunk_index, kind, text, blob_sha) VALUES (?,?,?,?,?,?,?,?)").bind(
`${ns}|src/existing${i}.ts::0`,
PROJECT,
"gittensory",
`src/existing${i}.ts`,
0,
"code",
"old",
`sha-existing-${i}`,
),
),
);
const tree = [
// Unchanged (matching sha) → every one of these is skipped, not re-embedded.
...Array.from({ length: existingCount }, (_, i) => ({ path: `src/existing${i}.ts`, size: 10, sha: `sha-existing-${i}` })),
// Two brand-new files — alphabetically after "existing", so reached only once the skips are exhausted.
{ path: "src/new-a.ts", size: 10 },
{ path: "src/new-b.ts", size: 10 },
];
const files: Record<string, string> = { "src/new-a.ts": "export const a = 1;\n", "src/new-b.ts": "export const b = 1;\n" };
stubGithub({ tree, files });

const result = await indexRepo(env, PROJECT, REPO);

expect(result.capped).toBe(true);
expect(result.files).toBe(1); // only one of the two new files fit under the cap
expect(await countChunks(env, PROJECT, "gittensory")).toBe(MAX_CHUNKS_PER_REPO); // never exceeds the cap
});
});

describe("indexRepo: MAX_CHUNKS_PER_REPO cap holds", () => {
afterEach(() => vi.unstubAllGlobals());

Expand Down
Loading
Loading