diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 8fc8791851..495b84dbb6 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -8727,13 +8727,18 @@ async function maybePublishPrPublicSurface( // so a repo with it active also bypasses the AI-review result cache rather than fingerprinting a // value that can't prove freshness. cultureProfile: isRepoCultureProfileEnabled(env) && reviewCultureProfile === true, + // Impact map (#2182-#2186): queries the SAME live vector index RAG does (computeImpactMap issues + // its own retrieveContextWithMetrics calls), so it can go stale for the SAME head SHA exactly like + // RAG — a repo with it active also bypasses the AI-review result cache. + impactMap: shouldComputeImpactMap(env, reviewImpactMap === true), }; const dynamicReviewContextActive = dynamicReviewFeatures.grounding || dynamicReviewFeatures.rag || dynamicReviewFeatures.enrichment || dynamicReviewFeatures.reputation || - dynamicReviewFeatures.cultureProfile; + dynamicReviewFeatures.cultureProfile || + dynamicReviewFeatures.impactMap; const inputFingerprint = await aiReviewCacheInputFingerprint({ title: pr.title, mode: settings.aiReviewMode, diff --git a/src/review/ai-review-cache-input.ts b/src/review/ai-review-cache-input.ts index d794f64efb..026db7f4b2 100644 --- a/src/review/ai-review-cache-input.ts +++ b/src/review/ai-review-cache-input.ts @@ -5,10 +5,11 @@ import type { } from "../signals/focus-manifest"; import { sha256Hex } from "../utils/crypto"; -// Bumped v1→v2 (#2995): `features` gained a `cultureProfile` member. Every prior cached review's fingerprint was -// computed without that key, so bumping the version guarantees a clean cache miss on the first review after -// upgrade rather than silently reusing a hash computed under a different payload shape. -export const AI_REVIEW_CACHE_INPUT_VERSION = "ai-review-input:v2"; +// Bumped v1→v2 (#2995): `features` gained a `cultureProfile` member. Bumped v2→v3 (#2182-#2186): `features` +// gained an `impactMap` member. Every prior cached review's fingerprint was computed without that key, so +// bumping the version guarantees a clean cache miss on the first review after upgrade rather than silently +// reusing a hash computed under a different payload shape. +export const AI_REVIEW_CACHE_INPUT_VERSION = "ai-review-input:v3"; // #regate-churn (root cause, confirmed in production): this fingerprint USED to also hash the PR's live // `baseSha`, on the theory that a rebase/retarget can change the diff GitHub reports for an otherwise-unchanged @@ -111,6 +112,9 @@ export type AiReviewCacheInput = { // #2995: added alongside the repo quality-culture profile. Explicitly enumerated below (not passed through // raw) so a FUTURE new feature key can't silently change every existing cache entry's fingerprint again. cultureProfile: boolean; + // #2182-#2186: impact-map computation queries the same live vector index RAG does (computeImpactMap issues + // its own retrieveContextWithMetrics calls), so it can go stale for an unchanged head SHA exactly like RAG. + impactMap: boolean; }; }; @@ -197,6 +201,7 @@ export async function aiReviewCacheInputFingerprint(input: AiReviewCacheInput): enrichment: input.features.enrichment, reputation: input.features.reputation, cultureProfile: input.features.cultureProfile, + impactMap: input.features.impactMap, }, }; return `${AI_REVIEW_CACHE_INPUT_VERSION}:${await sha256Hex(stableStringify(payload))}`; diff --git a/test/unit/ai-review-cache-input.test.ts b/test/unit/ai-review-cache-input.test.ts index 853a8be991..a8ebaabfb3 100644 --- a/test/unit/ai-review-cache-input.test.ts +++ b/test/unit/ai-review-cache-input.test.ts @@ -35,6 +35,7 @@ const baseInput = (): AiReviewCacheInput => ({ enrichment: false, reputation: false, cultureProfile: false, + impactMap: false, }, }); diff --git a/test/unit/ai-review-cache.test.ts b/test/unit/ai-review-cache.test.ts index 80b8c31c92..4bc19981cf 100644 --- a/test/unit/ai-review-cache.test.ts +++ b/test/unit/ai-review-cache.test.ts @@ -34,6 +34,7 @@ const baseFingerprintInput = (): AiReviewCacheInput => ({ enrichment: false, reputation: false, cultureProfile: false, + impactMap: false, }, }); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 3704ab9759..0808bcb28f 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -3531,7 +3531,7 @@ describe("queue processors", () => { aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], - features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false }, + features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, }), }, }); @@ -4046,7 +4046,7 @@ describe("queue processors", () => { excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], - features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false }, + features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, }), }, }); @@ -4119,7 +4119,7 @@ describe("queue processors", () => { aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], - features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false }, + features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, }), }, }); @@ -4176,7 +4176,7 @@ describe("queue processors", () => { aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], - features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false }, + features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, }), }, }); @@ -4232,7 +4232,7 @@ describe("queue processors", () => { aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], - features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false }, + features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, }), }, }); @@ -4285,7 +4285,7 @@ describe("queue processors", () => { aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], - features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false }, + features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, }), }, }); @@ -4334,7 +4334,7 @@ describe("queue processors", () => { aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], - features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false }, + features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, }), }, }); @@ -4386,7 +4386,7 @@ describe("queue processors", () => { aiReviewCloseConfidence: undefined, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, gatePack: "oss-anti-slop", reviewerPlan: env.AI_REVIEW_PLAN, selfHostProviderConfig: null, selfHostAiModelOverride: { claudeModel: null, claudeEffort: null, codexModel: null, codexEffort: null }, reviewFiles: [{ path: "src/a.ts", status: "modified", patch: "@@\n+export const ok = true;", additions: 1, deletions: 0 }], profile: null, securityFocus: false, inlineComments: false, pathInstructions: [], pathGuidance: "", repoInstructions: null, excludePaths: [], pathFilters: [], changedPaths: ["src/a.ts"], - features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false }, + features: { grounding: false, rag: false, enrichment: false, reputation: false, cultureProfile: false, impactMap: false }, }), }, }); @@ -5316,6 +5316,7 @@ describe("queue processors", () => { enrichment: false, reputation: false, cultureProfile: false, + impactMap: false, }, }); await putCachedAiReview(env, "owner/agent-repo", 7, "a7", "block", { @@ -5725,6 +5726,84 @@ describe("queue processors", () => { expect(aiCalls).toBeGreaterThan(0); }); + it("marks a cached AI review non-durable (cacheable=0) when the impact-map feature is on, even with grounding/rag/enrichment/reputation all off (#2182-#2186)", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { + run: async () => { + aiCalls += 1; + return { response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }; + }, + } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + // Both gates on: the global capability switch, and (like culture-profile above, unlike + // grounding/enrichment/RAG/reputation which are env-only) the per-repo `.gittensory.yml` opt-in mocked + // below, so `dynamicReviewFeatures.impactMap` (src/queue/processors.ts) actually evaluates + // shouldComputeImpactMap's `&&` right-hand side true, not just short-circuits. + GITTENSORY_REVIEW_IMPACT_MAP: "true", + }); + await persistRegistrySnapshot( + env, + normalizeRegistryPayload( + { "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0 } }, + { kind: "raw-github", url: "https://example.test" }, + "2026-05-23T00:00:00.000Z", + ), + ); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "all_prs", + publicSurface: "comment_only", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "enabled", + aiReviewMode: "block", + gatePack: "oss-anti-slop", + }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + 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 = true;" }]); + if (url.endsWith("/pulls/7")) return Response.json({ number: 7, title: "Clean 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.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/issues/7/comments") && method === "GET") return Response.json([]); + if (url.includes("/issues/7/comments") && method === "POST") return Response.json({ id: 1 }, { status: 201 }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + // The repo's own review.impact_map opt-in. + if (url === "https://raw.githubusercontent.com/JSONbored/gittensory/HEAD/.gittensory.yml") { + return new Response("review:\n impact_map: true\n"); + } + return Response.json({}); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "impact-map-non-durable", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 7, title: "Clean PR", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, labels: [], body: "Closes #1" }, + }, + }); + + expect(aiCalls).toBeGreaterThan(0); + const cached = await env.DB.prepare("select cacheable from ai_review_cache where repo_full_name = ? and pull_number = ? and head_sha = ?") + .bind("JSONbored/gittensory", 7, "a7") + .first<{ cacheable: number }>(); + // Never durably cacheable on its own merits, even though grounding/rag/enrichment/reputation are all off in + // this env -- impact-map alone is enough to trip dynamicReviewContextActive. + expect(cached?.cacheable).toBe(0); + }); + it("reuses a dynamic-context (grounding) AI review indefinitely once published, even long past the old cooldown window (#2119, #regate-churn)", async () => { // Grounding/RAG/enrichment/reputation each pull TIME-VARYING external context (live CI checks, the vector // index, REES/CVE data, reputation) that can change for the SAME head SHA without the feature flags @@ -14709,10 +14788,10 @@ describe("queue processors", () => { const usage = await env.DB.prepare("select feature, status from ai_usage_events where feature = ?").bind("ai_review_pr").first<{ feature: string; status: string }>(); expect(usage).toMatchObject({ feature: "ai_review_pr", status: "ok" }); expect(cacheReadSpy).toHaveBeenCalled(); - expect(cacheReadSpy.mock.calls[0]?.[5]).toMatch(/^ai-review-input:v2:/); + expect(cacheReadSpy.mock.calls[0]?.[5]).toMatch(/^ai-review-input:v3:/); expect(cacheWriteSpy).toHaveBeenCalled(); expect(cacheWriteSpy.mock.calls[0]?.[5]).toMatchObject({ - metadata: { inputFingerprint: expect.stringMatching(/^ai-review-input:v2:/) }, + metadata: { inputFingerprint: expect.stringMatching(/^ai-review-input:v3:/) }, }); cacheReadSpy.mockRestore(); cacheWriteSpy.mockRestore();