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
20 changes: 17 additions & 3 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ import {
} from "../review/rag-wire";
import { createReviewAdapters } from "../review/adapters";
import { extractChangedSymbols } from "../review/impact-symbols";
import { computeImpactMap } from "../review/impact-map";
import { computeImpactMap, type ImpactMapEntry } from "../review/impact-map";
import { formatImpactMapPromptSection, shouldComputeImpactMap } from "../review/impact-map-wire";
import { shouldEmitFixHandoff } from "../review/fix-handoff";
import { buildFixHandoffBlocks } from "../review/fix-handoff-render";
Expand Down Expand Up @@ -6951,6 +6951,10 @@ export async function runAiReviewForAdvisory(
notes: string;
reviewerCount: number;
inlineFindings: InlineFinding[];
// Deterministic impact-map entries this pass computed for the AI prompt (#1971), threaded out so the
// publish site can ALSO render them as the unified comment's "Impact map" collapsible. Empty when the
// feature is off (flag/manifest) — the render arm keys on `.length`, so off ⇒ no section.
impactMap?: ImpactMapEntry[] | undefined;
findings: AdvisoryFinding[];
metadata?: Record<string, unknown> | undefined;
cacheable?: boolean | undefined;
Expand Down Expand Up @@ -7119,6 +7123,9 @@ export async function runAiReviewForAdvisory(
// undefined so the prompt is byte-identical to today. Fully fail-safe (computeImpactMap never throws; a
// missing/cold RAG index degrades to an empty impact map, which formats to "" and appends nothing).
let impactMapContext: string | undefined;
// The computed entries are ALSO threaded out of this function (#1971) so the publish site can render the
// "Impact map" collapsible from the exact same array — no second RAG query. Empty when the feature is off.
let impactMapEntries: ImpactMapEntry[] = [];
if (shouldComputeImpactMap(env, args.reviewImpactMap === true)) {
const [impactMapProject, impactMapRepo] = splitRepoForRag(args.repoFullName);
const changedSymbols = extractChangedSymbols(
Expand All @@ -7127,12 +7134,12 @@ export async function runAiReviewForAdvisory(
patch: typeof file.payload?.patch === "string" ? file.payload.patch : undefined,
})),
);
const impactMap = await computeImpactMap(changedSymbols, {
impactMapEntries = await computeImpactMap(changedSymbols, {
infra: createReviewAdapters(env),
project: impactMapProject,
repo: impactMapRepo,
});
impactMapContext = formatImpactMapPromptSection(impactMap);
impactMapContext = formatImpactMapPromptSection(impactMapEntries);
}
// Repo quality-culture profile (#2995, flag-gated by GITTENSORY_REVIEW_CULTURE_PROFILE AND the per-repo
// `review.culture_profile` opt-in). Derives a compact reference block from the repo's OWN merge history
Expand Down Expand Up @@ -7329,6 +7336,7 @@ export async function runAiReviewForAdvisory(
notes: result.advisoryNotes!,
reviewerCount: result.reviewerCount,
inlineFindings: result.inlineFindings,
impactMap: impactMapEntries,
findings,
metadata: metadataFor(result.advisoryNotes, result.inlineFindings),
};
Expand Down Expand Up @@ -8272,6 +8280,7 @@ async function maybePublishPrPublicSurface(
notes: string;
reviewerCount: number;
inlineFindings?: InlineFinding[];
impactMap?: ImpactMapEntry[] | undefined;
findings?: AdvisoryFinding[];
metadata?: Record<string, unknown> | undefined;
cacheable?: boolean | undefined;
Expand Down Expand Up @@ -10175,6 +10184,11 @@ async function maybePublishPrPublicSurface(
...(findingCategoriesEnabledForReview && aiReview?.inlineFindings?.length
? { findingCategories: aiReview.inlineFindings }
: {}),
// review.impact_map render (#1971): the deterministic impact-map entries this fresh pass already computed
// for the AI prompt ALSO render here as the "Impact map" collapsible — no second RAG query. A cache hit /
// frozen reuse / skipped review carries none (undefined ⇒ []); buildImpactMapCollapsible returns null for
// an empty list, so off/empty ⇒ no section ⇒ byte-identical. `ImpactMapEntry` IS `ImpactMapSummaryInput`.
impactMap: aiReview?.impactMap ?? [],
// review.fixHandoff emission (#1962): the SAME fresh inline findings feed the fix-handoff blocks —
// present ONLY on a cache-miss review with inline comments enabled — so a cache hit never re-emits them,
// exactly like findingCategories above. Flag-OFF ⇒ omitted ⇒ the rendered comment is byte-identical.
Expand Down
5 changes: 5 additions & 0 deletions test/unit/impact-map-processor-wiring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ describe("impact map wired into runAiReviewForAdvisory (#2186)", () => {
expect(user).toContain("IMPACT MAP");
expect(user).toContain("src/review/impact-map.ts");
expect(user).toContain("src/review/caller.ts");
// #1971: the SAME computed entries are threaded out of the review result so the publish site can render the
// "Impact map" collapsible from them — no second RAG query.
expect(result?.impactMap?.length).toBeGreaterThan(0);
expect(result?.impactMap?.[0]?.changedModule).toBe("src/review/impact-map.ts");
expect(result?.impactMap?.[0]?.affectedModules).toContain("src/review/caller.ts");
});

it("FLAG-OFF (operator env unset): no impact-map computation, prompt has no IMPACT MAP section", async () => {
Expand Down
46 changes: 46 additions & 0 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18256,6 +18256,52 @@ describe("queue processors", () => {
expect(unifiedCommentBody).toContain("| Security | 1 |");
});

// #1971: a FROZEN (manual-review) PR reuses its last published AI review, which carries no impact-map entries —
// the unified comment still renders, and the impact-map render arm degrades to no section (aiReview present but
// aiReview.impactMap undefined ⇒ `aiReview?.impactMap ?? []` ⇒ [] ⇒ buildImpactMapCollapsible null).
it("renders the unified comment WITHOUT an Impact map section when a frozen review is reused (no threaded entries)", async () => {
let aiCalls = 0;
const env = createTestEnv({
GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(),
GITTENSORY_REVIEW_UNIFIED_COMMENT: "1",
AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Fresh.", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai,
AI_SUMMARIES_ENABLED: "true",
AI_PUBLIC_COMMENTS_ENABLED: "true",
AI_DAILY_NEURON_BUDGET: "100000",
});
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" });
await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 77, title: "Held PR", state: "open", user: { login: "contributor" }, head: { sha: "a77" }, labels: [{ name: "manual-review" }], body: "Closes #1" });
await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 77, status: "complete", reviewsSyncedAt: new Date().toISOString() });
// A prior PUBLISHED review for this exact head — the freeze path reuses it (aiReview = frozenReview) instead of
// spending a fresh AI call. Its cached shape has notes+reviewerCount but NO impactMap, so the render arm's
// nullish arm fires.
await putCachedAiReview(env, "JSONbored/gittensory", 77, "a77", "block", { notes: "Prior published review.", reviewerCount: 1 });
await markAiReviewPublished(env, "JSONbored/gittensory", 77, "a77");
let unifiedCommentBody = "";
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: "fake-installation-token" });
if (url.includes("/pulls/77/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/77")) return Response.json({ number: 77, title: "Held PR", state: "open", user: { login: "contributor" }, head: { sha: "a77" }, labels: [{ name: "manual-review" }], body: "Closes #1", mergeable_state: "clean" });
if (url.includes("/commits/a77/check-runs")) return Response.json({ total_count: 0, check_runs: [] });
if (url.includes("/commits/a77/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/77/comments") && method === "GET") return Response.json([]);
if (url.includes("/issues/77/comments")) { unifiedCommentBody = String((JSON.parse(String(init?.body ?? "{}")) as { body?: string }).body ?? unifiedCommentBody); return Response.json({ id: 1 }, { status: 201 }); }
if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } });
return Response.json({});
});

await processJob(env, { type: "agent-regate-pr", deliveryId: "impact-map-frozen-reuse", repoFullName: "JSONbored/gittensory", prNumber: 77, installationId: 123 });

expect(aiCalls).toBe(0); // frozen ⇒ reused, no fresh AI
expect(unifiedCommentBody).toContain("gittensory-pr-panel"); // the unified panel rendered from the frozen review
expect(unifiedCommentBody).not.toContain("Impact map"); // ...with no impact-map section (reused review has none)
});

// #1962: with BOTH the operator flag and the manifest opt-in on, the review emits a "Fix handoff" collapsible —
// one machine-readable block per inline finding a contributor's own local agent can consume — in the unified
// comment. Flag-OFF (every other review test) ⇒ no such section.
Expand Down