diff --git a/migrations/0119_ai_slop_cache.sql b/migrations/0119_ai_slop_cache.sql index 622b6405e9..70269a815c 100644 --- a/migrations/0119_ai_slop_cache.sql +++ b/migrations/0119_ai_slop_cache.sql @@ -11,9 +11,9 @@ CREATE TABLE IF NOT EXISTS ai_slop_cache ( repo_full_name TEXT NOT NULL, pull_number INTEGER NOT NULL, head_sha TEXT NOT NULL, - -- Fingerprints the one input that can change independently of the head SHA: which provider produced the - -- opinion (free/default reviewer vs. a maintainer's BYOK key/model). Title/body/diff/deterministicBand are - -- all already pinned to the head SHA (see getReviewFiles/buildAiReviewDiff), so they need no fingerprinting. + -- Fingerprints mutable prompt inputs (title/body/current built diff/deterministicBand) plus which provider + -- produced the opinion (free/default reviewer vs. a maintainer's BYOK key/model). The row key still scopes + -- cache lifetime to a PR head SHA, while the fingerprint forces misses for same-head prompt drift. input_fingerprint TEXT NOT NULL, status TEXT NOT NULL, band TEXT, diff --git a/src/queue/processors.ts b/src/queue/processors.ts index d75231c13f..711703cb3e 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -7507,13 +7507,16 @@ export async function runAiSlopForAdvisory( model: args.settings.aiReviewModel ?? storedKey.model, } : null; - // #ai-slop-cache: the slop advisory's LLM call is fully deterministic given the same head SHA (no RAG/ - // grounding/enrichment feeds into it, unlike ai review — see ai_slop_cache's migration doc comment), so a - // repeated scheduled sweep pass at an unchanged head reuses the stored result instead of re-spending up to - // 6 free-tier attempts (or a BYOK call) on every tick — confirmed in production: 110 ai_slop_pr calls on a - // single PR in 24h at an unchanged head. The fingerprint only needs to cover which provider would answer - // (free vs. this repo's BYOK key/model); everything else the model sees is already pinned to the head SHA. + // #ai-slop-cache: repeated scheduled sweeps at an unchanged prompt reuse the stored result instead of + // re-spending up to 6 free-tier attempts (or a BYOK call) on every tick. The fingerprint includes the + // provider identity plus the prompt-shaping inputs that can drift for the same head SHA (PR edits, + // retarget/base-diff changes, or deterministic-band setting changes). + const aiSlopDiff = buildAiReviewDiff(args.files); const inputFingerprint = await aiSlopCacheInputFingerprint({ + title: args.pr.title, + body: args.pr.body ?? null, + diff: aiSlopDiff, + deterministicBand: args.deterministicBand, byok: Boolean(providerKey), provider: providerKey?.provider, model: providerKey?.model, @@ -7548,7 +7551,7 @@ export async function runAiSlopForAdvisory( prNumber: args.pr.number, title: args.pr.title, body: args.pr.body ?? undefined, - diff: buildAiReviewDiff(args.files), + diff: aiSlopDiff, actor: args.author, deterministicBand: args.deterministicBand, providerKey, diff --git a/src/review/ai-slop-cache-input.ts b/src/review/ai-slop-cache-input.ts index 7f8368f4bb..9358824611 100644 --- a/src/review/ai-slop-cache-input.ts +++ b/src/review/ai-slop-cache-input.ts @@ -1,27 +1,31 @@ import { sha256Hex } from "../utils/crypto"; -// #ai-slop-cache: unlike ai-review-cache-input.ts (whose fingerprint spans a large, independently-mutable -// prompt-shaping surface -- reviewer plan, model overrides, path instructions, feature toggles, ...), the slop -// advisory's ONLY input that can change independently of the PR's head SHA is which provider writes the -// opinion: the free/default reviewer vs. a maintainer's BYOK key/model (see AiSlopInput in ../services/ai-slop). -// Title/body/diff/deterministicBand are all already pinned to the head SHA -- the same commit always produces -// the same diff and the same deterministic band, so none of them need fingerprinting. A repo flipping BYOK on -// or changing its BYOK provider/model must miss the cache rather than replay an opinion written under a -// different reviewer. -export const AI_SLOP_CACHE_INPUT_VERSION = "ai-slop-input:v1"; +// #ai-slop-cache: the cache key anchors on repo/PR/head SHA, but the prompt still includes mutable PR metadata +// (title/body) plus the currently-built diff and deterministic band. Those can drift for the same head when a PR +// is edited, retargeted, or re-evaluated under changed settings, so they are hashed alongside the provider +// identity to avoid replaying an advisory written for a different prompt. +export const AI_SLOP_CACHE_INPUT_VERSION = "ai-slop-input:v2"; export type AiSlopCacheInput = { + title?: string | null | undefined; + body?: string | null | undefined; + diff?: string | null | undefined; + deterministicBand?: string | null | undefined; byok: boolean; provider: string | null | undefined; model: string | null | undefined; }; export async function aiSlopCacheInputFingerprint(input: AiSlopCacheInput): Promise { - const payload = [ - AI_SLOP_CACHE_INPUT_VERSION, - input.byok ? "1" : "0", - input.provider ?? "", - input.model ?? "", - ].join("|"); - return `${AI_SLOP_CACHE_INPUT_VERSION}:${await sha256Hex(payload)}`; + const payload = { + version: AI_SLOP_CACHE_INPUT_VERSION, + title: input.title ?? "", + body: input.body ?? null, + diff: input.diff ?? "", + deterministicBand: input.deterministicBand ?? null, + byok: input.byok, + provider: input.provider ?? null, + model: input.model ?? null, + }; + return `${AI_SLOP_CACHE_INPUT_VERSION}:${await sha256Hex(JSON.stringify(payload))}`; } diff --git a/test/unit/ai-slop-cache.test.ts b/test/unit/ai-slop-cache.test.ts index f1153dd649..fdb029bd2f 100644 --- a/test/unit/ai-slop-cache.test.ts +++ b/test/unit/ai-slop-cache.test.ts @@ -87,12 +87,30 @@ describe("AI slop advisory cache (#ai-slop-cache)", () => { }); describe("aiSlopCacheInputFingerprint", () => { + const promptInput = { + title: "Tidy", + body: "cleanup", + diff: "### src/a.ts\n@@\n+const x = 1;", + deterministicBand: "elevated", + byok: false, + provider: null, + model: null, + }; + it("is stable for the same input", async () => { - const a = await aiSlopCacheInputFingerprint({ byok: false, provider: null, model: null }); - const b = await aiSlopCacheInputFingerprint({ byok: false, provider: null, model: null }); + const a = await aiSlopCacheInputFingerprint(promptInput); + const b = await aiSlopCacheInputFingerprint(promptInput); expect(a).toBe(b); }); + it("differs when mutable prompt inputs change at the same head SHA", async () => { + const original = await aiSlopCacheInputFingerprint(promptInput); + await expect(aiSlopCacheInputFingerprint({ ...promptInput, title: "Add generated SDK dump" })).resolves.not.toBe(original); + await expect(aiSlopCacheInputFingerprint({ ...promptInput, body: "high-risk generated churn" })).resolves.not.toBe(original); + await expect(aiSlopCacheInputFingerprint({ ...promptInput, diff: "### src/a.ts\n@@\n+// generated wall of text" })).resolves.not.toBe(original); + await expect(aiSlopCacheInputFingerprint({ ...promptInput, deterministicBand: "high" })).resolves.not.toBe(original); + }); + it("differs when byok flips", async () => { const free = await aiSlopCacheInputFingerprint({ byok: false, provider: null, model: null }); const byok = await aiSlopCacheInputFingerprint({ byok: true, provider: null, model: null }); diff --git a/test/unit/ai-slop.test.ts b/test/unit/ai-slop.test.ts index 3a5ebdf75e..323b8202c0 100644 --- a/test/unit/ai-slop.test.ts +++ b/test/unit/ai-slop.test.ts @@ -6,7 +6,7 @@ import { type AiSlopInput, } from "../../src/services/ai-slop"; import { evaluateGateCheck } from "../../src/rules/advisory"; -import { runAiSlopForAdvisory } from "../../src/queue/processors"; +import { buildAiReviewDiff, runAiSlopForAdvisory } from "../../src/queue/processors"; import { getCachedAiSlopAdvisory, putCachedAiSlopAdvisory, recordAiUsageEvent, upsertRepositoryAiKey } from "../../src/db/repositories"; import { aiSlopCacheInputFingerprint } from "../../src/review/ai-slop-cache-input"; import type { Advisory, PullRequestFileRecord, RepositorySettings } from "../../src/types"; @@ -370,6 +370,17 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { ]; const pr = { number: 3, title: "Tidy", body: "cleanup" }; const noByok = { aiReviewByok: false } as RepositorySettings; + const slopFingerprint = (over: Partial[0]> = {}) => + aiSlopCacheInputFingerprint({ + title: pr.title, + body: pr.body, + diff: buildAiReviewDiff(files), + deterministicBand: "high", + byok: false, + provider: null, + model: null, + ...over, + }); it("appends a single ai_slop_advisory finding when the model flags slop", async () => { const adv = advisory(); @@ -480,7 +491,7 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { it("reuses a stored advisory for an unchanged head SHA instead of calling the model again", async () => { const run = vi.fn(async () => ({ response: slopJson({ band: "high" }) })); const env = enabledEnv(run); - const fingerprint = await aiSlopCacheInputFingerprint({ byok: false, provider: null, model: null }); + const fingerprint = await slopFingerprint(); await putCachedAiSlopAdvisory(env, "acme/widgets", 3, "sha3", fingerprint, { status: "ok", band: "high", @@ -497,7 +508,7 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { it("swallows a throwing audit-event write on a cache hit (fail-safe, the finding still reaches the advisory)", async () => { const run = vi.fn(); const env = enabledEnv(run); - const fingerprint = await aiSlopCacheInputFingerprint({ byok: false, provider: null, model: null }); + const fingerprint = await slopFingerprint(); await putCachedAiSlopAdvisory(env, "acme/widgets", 3, "sha3", fingerprint, { status: "ok", band: "high", @@ -542,7 +553,7 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { await runAiSlopForAdvisory(env, { settings: noByok, advisory: adv, repoFullName: "acme/widgets", pr, author: "alice", files, deterministicBand: "elevated", confirmedContributor: true }); expect(run).toHaveBeenCalledTimes(1); // fresh call on the miss - const fingerprint = await aiSlopCacheInputFingerprint({ byok: false, provider: null, model: null }); + const fingerprint = await slopFingerprint({ deterministicBand: "elevated" }); const cached = await getCachedAiSlopAdvisory(env, "acme/widgets", 3, "sha3", fingerprint); expect(cached).toMatchObject({ status: "ok", band: "elevated" }); @@ -561,7 +572,7 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { expect(run).not.toHaveBeenCalled(); // quota_exceeded short-circuits before any model call expect(adv.findings).toEqual([]); - const fingerprint = await aiSlopCacheInputFingerprint({ byok: false, provider: null, model: null }); + const fingerprint = await slopFingerprint({ deterministicBand: "elevated" }); expect(await getCachedAiSlopAdvisory(budgetedEnv, "acme/widgets", 3, "sha3", fingerprint)).toBeNull(); // nothing was persisted // Same head, budget now available — must still attempt the model instead of replaying a quota miss. @@ -580,7 +591,7 @@ describe("runAiSlopForAdvisory (processor wiring)", () => { AI_DAILY_NEURON_BUDGET: "100000", TOKEN_ENCRYPTION_SECRET: "ai-slop-byok-cache-test-encryption-secret-32", }); - const freeFingerprint = await aiSlopCacheInputFingerprint({ byok: false, provider: null, model: null }); + const freeFingerprint = await slopFingerprint({ deterministicBand: "elevated" }); await putCachedAiSlopAdvisory(env, "acme/widgets", 3, "sha3", freeFingerprint, { status: "ok", band: "high",