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
7 changes: 6 additions & 1 deletion src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
13 changes: 9 additions & 4 deletions src/review/ai-review-cache-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
};
};

Expand Down Expand Up @@ -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))}`;
Expand Down
1 change: 1 addition & 0 deletions test/unit/ai-review-cache-input.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const baseInput = (): AiReviewCacheInput => ({
enrichment: false,
reputation: false,
cultureProfile: false,
impactMap: false,
},
});

Expand Down
1 change: 1 addition & 0 deletions test/unit/ai-review-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const baseFingerprintInput = (): AiReviewCacheInput => ({
enrichment: false,
reputation: false,
cultureProfile: false,
impactMap: false,
},
});

Expand Down
99 changes: 89 additions & 10 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
}),
},
});
Expand Down Expand Up @@ -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 },
}),
},
});
Expand Down Expand Up @@ -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 },
}),
},
});
Expand Down Expand Up @@ -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 },
}),
},
});
Expand Down Expand Up @@ -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 },
}),
},
});
Expand Down Expand Up @@ -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 },
}),
},
});
Expand Down Expand Up @@ -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 },
}),
},
});
Expand Down Expand Up @@ -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 },
}),
},
});
Expand Down Expand Up @@ -5316,6 +5316,7 @@ describe("queue processors", () => {
enrichment: false,
reputation: false,
cultureProfile: false,
impactMap: false,
},
});
await putCachedAiReview(env, "owner/agent-repo", 7, "a7", "block", {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down