diff --git a/migrations/0074_ai_review_cache.sql b/migrations/0074_ai_review_cache.sql new file mode 100644 index 0000000000..365f3cda7f --- /dev/null +++ b/migrations/0074_ai_review_cache.sql @@ -0,0 +1,14 @@ +-- #1 (self-host perf): cache the expensive AI review by (repo, pull, head_sha) so a re-delivered webhook and the +-- block-mode ~2-min re-gate sweep (which re-runs the AI for every open PR) don't re-spend the LLM call for the same +-- commit. The deterministic gate still re-evaluates every time; only the AI leg is reused. A new head SHA (new code) +-- or a changed review mode invalidates the entry. On self-host there is no AI gateway, so this is the only AI cache. +CREATE TABLE IF NOT EXISTS ai_review_cache ( + repo_full_name TEXT NOT NULL, + pull_number INTEGER NOT NULL, + head_sha TEXT NOT NULL, + ai_review_mode TEXT NOT NULL, + notes TEXT NOT NULL, + reviewer_count INTEGER NOT NULL, + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (repo_full_name, pull_number, head_sha) +); diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 1d8e7a55d4..85d9bd3365 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -3198,6 +3198,46 @@ export async function persistAdvisory(env: Env, advisory: Advisory): Promise { + if (!headSha) return null; + const row = await env.DB + .prepare("SELECT notes, reviewer_count AS reviewerCount, ai_review_mode AS mode FROM ai_review_cache WHERE repo_full_name = ? AND pull_number = ? AND head_sha = ?") + .bind(repoFullName, pullNumber, headSha) + .first<{ notes: string; reviewerCount: number; mode: string }>(); + if (!row || row.mode !== mode) return null; + return { notes: row.notes, reviewerCount: row.reviewerCount }; +} + +/** Upsert the AI review for (repo, pull, head SHA). A nullish head SHA is a no-op. */ +export async function putCachedAiReview( + env: Env, + repoFullName: string, + pullNumber: number, + headSha: string | null | undefined, + mode: string, + review: { notes: string; reviewerCount: number }, +): Promise { + if (!headSha) return; + await env.DB + .prepare( + `INSERT INTO ai_review_cache (repo_full_name, pull_number, head_sha, ai_review_mode, notes, reviewer_count) + VALUES (?, ?, ?, ?, ?, ?) + ON CONFLICT(repo_full_name, pull_number, head_sha) DO UPDATE SET + ai_review_mode = excluded.ai_review_mode, notes = excluded.notes, reviewer_count = excluded.reviewer_count, created_at = CURRENT_TIMESTAMP`, + ) + .bind(repoFullName, pullNumber, headSha, mode, review.notes, review.reviewerCount) + .run(); +} + export async function replaceCollisionEdges(env: Env, repoFullName: string, edges: CollisionEdgeRecord[]): Promise { const db = getDb(env.DB); await env.DB.prepare("DELETE FROM collision_edges WHERE repo_full_name = ?").bind(repoFullName).run(); diff --git a/src/queue/processors.ts b/src/queue/processors.ts index e9b1138ccf..9d8c28377d 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -38,6 +38,8 @@ import { markInstallationDeleted, markRepositoriesRemovedFromInstallation, persistAdvisory, + getCachedAiReview, + putCachedAiReview, markPullRequestsRegated, getLatestRegatedAt, claimRegateFanoutSlot, @@ -2695,24 +2697,34 @@ async function maybePublishPrPublicSurface( await createOrUpdatePrIntelligenceComment(env, installationId, repoFullName, pr.number, placeholderBody, { mode }).catch(() => undefined); } if (aiReviewWillRun) { - // `.gittensory.yml` review.profile + review.path_instructions + review.exclude_paths (#review-profile / - // #review-path-instructions / #review-exclude-paths): resolve from the manifest (cached from settings - // resolution, so a cheap cache hit — no extra fetch) and thread them into the AI review. Profile shapes - // nitpickiness; path-instructions add per-path guidance; exclude-paths drop files from review. Absent ⇒ - // byte-identical prompt. Fail-safe to defaults on any read error (resolveReviewPromptOverrides). - const { profile: reviewProfile, pathInstructions: reviewPathInstructions, excludePaths: reviewExcludePaths } = resolveReviewPromptOverrides(await loadRepoFocusManifest(env, repoFullName).catch(() => null)); - aiReview = await runAiReviewForAdvisory(env, { - settings, - advisory, - repoFullName, - pr, - author, - confirmedContributor, - files: await getReviewFiles(), - reviewProfile, - reviewPathInstructions, - reviewExcludePaths, - }); + // #1 self-host AI-review cache: the LLM output for a PR changes only when the code (head SHA) or the review + // mode changes, so reuse a prior review for this exact (repo, pr, head SHA, mode) — a re-delivered webhook or + // the block-mode ~2-min re-gate sweep (which re-runs the AI for every open PR) need not re-spend the call. On + // self-host there is no AI gateway, so this is the only AI cache. The deterministic gate below still runs. + const cachedReview = await getCachedAiReview(env, repoFullName, pr.number, advisory.headSha, settings.aiReviewMode).catch(() => null); + if (cachedReview) { + aiReview = cachedReview; + } else { + // `.gittensory.yml` review.profile + review.path_instructions + review.exclude_paths (#review-profile / + // #review-path-instructions / #review-exclude-paths): resolve from the manifest (cached from settings + // resolution, so a cheap cache hit — no extra fetch) and thread them into the AI review. Profile shapes + // nitpickiness; path-instructions add per-path guidance; exclude-paths drop files from review. Absent ⇒ + // byte-identical prompt. Fail-safe to defaults on any read error (resolveReviewPromptOverrides). + const { profile: reviewProfile, pathInstructions: reviewPathInstructions, excludePaths: reviewExcludePaths } = resolveReviewPromptOverrides(await loadRepoFocusManifest(env, repoFullName).catch(() => null)); + aiReview = await runAiReviewForAdvisory(env, { + settings, + advisory, + repoFullName, + pr, + author, + confirmedContributor, + files: await getReviewFiles(), + reviewProfile, + reviewPathInstructions, + reviewExcludePaths, + }); + if (aiReview) await putCachedAiReview(env, repoFullName, pr.number, advisory.headSha, settings.aiReviewMode, aiReview).catch(() => undefined); + } } // Secrets-scan (#audit-3.4): always scans the REAL resolved diff and, on a CONCRETE credential hit, appends a diff --git a/test/unit/ai-review-cache.test.ts b/test/unit/ai-review-cache.test.ts new file mode 100644 index 0000000000..c8b7246731 --- /dev/null +++ b/test/unit/ai-review-cache.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { getCachedAiReview, putCachedAiReview } from "../../src/db/repositories"; +import { createTestEnv } from "../helpers/d1"; + +describe("AI review cache (#1)", () => { + it("misses on a nullish head SHA (read returns null; write is a no-op)", async () => { + const env = createTestEnv(); + expect(await getCachedAiReview(env, "o/r", 1, null, "advisory")).toBeNull(); + expect(await getCachedAiReview(env, "o/r", 1, undefined, "advisory")).toBeNull(); + await putCachedAiReview(env, "o/r", 1, null, "advisory", { notes: "x", reviewerCount: 1 }); // no-op, no throw + expect(await getCachedAiReview(env, "o/r", 1, "sha", "advisory")).toBeNull(); // nothing was stored + }); + + it("reuses a stored review ONLY on the same (repo, pull, head SHA, mode)", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 7, "sha1", "block", { notes: "the review", reviewerCount: 2 }); + expect(await getCachedAiReview(env, "o/r", 7, "sha1", "block")).toEqual({ notes: "the review", reviewerCount: 2 }); + expect(await getCachedAiReview(env, "o/r", 7, "sha1", "advisory")).toBeNull(); // mode changed → miss + expect(await getCachedAiReview(env, "o/r", 7, "sha2", "block")).toBeNull(); // new head SHA → miss + expect(await getCachedAiReview(env, "o/r", 8, "sha1", "block")).toBeNull(); // different PR → miss + }); + + it("upserts — a re-run at the same key replaces the stored review (+ mode)", async () => { + const env = createTestEnv(); + await putCachedAiReview(env, "o/r", 7, "sha1", "advisory", { notes: "first", reviewerCount: 1 }); + await putCachedAiReview(env, "o/r", 7, "sha1", "block", { notes: "second", reviewerCount: 2 }); + expect(await getCachedAiReview(env, "o/r", 7, "sha1", "block")).toEqual({ notes: "second", reviewerCount: 2 }); + }); +}); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index f34c6a63c4..0447fea8e3 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -37,6 +37,7 @@ import { upsertIssueWatchSubscription, upsertRepositorySettings, upsertRepositoryFromGitHub, + putCachedAiReview, } from "../../src/db/repositories"; import { changedPathsForGuardrail, processJob } from "../../src/queue/processors"; import { upsertRepoFocusManifest } from "../../src/signals/focus-manifest-loader"; @@ -716,6 +717,41 @@ describe("queue processors", () => { expect(mergeAudit?.n).toBe(0); }); + it("#1: the block-mode re-gate sweep reuses a cached AI review for the same head SHA — no AI call re-spent", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Critical defect found.", blockers: ["x"], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_DAILY_NEURON_BUDGET: "100000", + }); + await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9001); + await upsertRepositorySettings(env, { repoFullName: "owner/agent-repo", autonomy: { merge: "auto" }, aiReviewMode: "block", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Stale PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + await upsertPullRequestFile(env, { repoFullName: "owner/agent-repo", pullNumber: 7, path: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, payload: { patch: "@@\n+export const ok = value.length;" } }); + // Pre-seed the AI review for this exact head SHA + mode → the sweep's block-mode review must reuse it, not re-run. + await putCachedAiReview(env, "owner/agent-repo", 7, "a7", "block", { notes: "cached review", reviewerCount: 2 }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/7/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = value.length;" }]); + if (url.endsWith("/pulls/7")) return Response.json({ number: 7, title: "Stale PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }); + if (url.includes("/commits/a7/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a7/status")) return Response.json({ state: "success", statuses: [] }); + if (url.endsWith("/pulls/7/merge")) return new Response(null, { status: 204 }); + if (url.endsWith("/pulls/7/reviews") && init?.method === "POST") return Response.json({ id: 1 }); + if (url.endsWith("/pulls/7/reviews")) return Response.json([]); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + + await sweepAndDrainPerPr(env, "owner/agent-repo"); + + expect(aiCalls).toBe(0); // the cached AI review was reused — the LLM was never called for this head SHA + }); + it("posts the 🟪 reviewing placeholder before the AI review runs, then overwrites it with the verdict (#reviewing-placeholder)", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(),