diff --git a/src/queue/ai-review-orchestration.ts b/src/queue/ai-review-orchestration.ts index 4207911ee..ad0c4ac20 100644 --- a/src/queue/ai-review-orchestration.ts +++ b/src/queue/ai-review-orchestration.ts @@ -20,7 +20,7 @@ import { type TransientLockClaim, } from "./transient-locks"; import { buildPullRequestAdvisory } from "../rules/advisory"; -import { getDecryptedRepositoryAiKey, getRepository, listCheckSummaries, listPullRequestFiles } from "../db/repositories"; +import { recordAuditEvent, getDecryptedRepositoryAiKey, getRepository, listCheckSummaries, listPullRequestFiles } from "../db/repositories"; import { createInstallationToken } from "../github/app"; import type { AgentActionMode } from "../settings/agent-execution"; import { buildAiReviewDiff } from "../review/review-diff"; @@ -721,6 +721,19 @@ export async function runAiReviewForAdvisory( improvementSignal: args.improvementSignal === true, }); if (result.status !== "ok") return undefined; + // #8229 stage 0: persist each reviewer's stance for the provider track records — best-effort like every + // calibration write (a vote-store failure must never affect the review), one audit event per reviewer, + // attribution already swap-proof from the runner (votes attach at leg production time). + for (const vote of result.reviewerVotes) { + await recordAuditEvent(env, { + eventType: "reviewer_vote", + actor: vote.reviewer, + targetKey: `${args.repoFullName}#${args.pr.number}`, + outcome: "completed", + detail: vote.votedFail ? "flagged a blocking defect" : "did not flag a blocking defect", + metadata: { repoFullName: args.repoFullName, vote: vote.votedFail ? "fail" : "non_fail" }, + }).catch(() => undefined); + } const findings: AdvisoryFinding[] = []; if (result.consensusDefect) { findings.push({ diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 3f0c89116..768792b9b 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -392,6 +392,11 @@ export type LoopOverAiReviewResult = inconclusive: boolean; estimatedNeurons: number; reviewerCount: number; + /** Per-reviewer stances for the provider track records (#8229 stage 0). Attribution attaches at leg + * PRODUCTION time (a.review ↔ primary.model, b.review ↔ secondary.model), so the tie-break judge's + * order-swap — which operates downstream on copies — can never misattribute a vote. Block-mode only + * (the gate corpus is what the track records score against); empty in advisory-only runs. */ + reviewerVotes: { reviewer: string; votedFail: boolean }[]; inlineFindings: InlineFinding[]; /** Combined improvement/value judgment (#4743), public-safe and ready to render. ALWAYS present (`null` * when `input.improvementSignal` is falsy, when neither reviewer emitted a usable judgment, or when the @@ -2338,6 +2343,7 @@ export async function runLoopOverAiReview( } let consensusDefect: AiConsensusDefect | null = null; + const reviewerVotes: { reviewer: string; votedFail: boolean }[] = []; let secondReview: ModelReview | null = null; let aiReviewSplit = false; let splitConfidence: number | undefined; @@ -2375,6 +2381,9 @@ export async function runLoopOverAiReview( ]); if (a.fallbackNote) fallbackNotes.push(a.fallbackNote); if (b.fallbackNote) fallbackNotes.push(b.fallbackNote); + // #8229 stage 0: attach votes HERE, where slot↔model is unambiguous by construction. + if (a.review) reviewerVotes.push({ reviewer: primary.model, votedFail: a.review.blockers.length > 0 }); + if (b.review) reviewerVotes.push({ reviewer: secondary.model, votedFail: b.review.blockers.length > 0 }); secondReview = b.review; // Combine per the configured strategy (#dual-ai-combiner). Default `consensus` is byte-identical to the // historical logic: block only on agreement, lone blocker → split, a missing opinion → inconclusive @@ -2435,6 +2444,8 @@ export async function runLoopOverAiReview( ) : ({ review: advisoryReview } as ReviewerOpinionOutcome); if (a.fallbackNote) fallbackNotes.push(a.fallbackNote); + // #8229 stage 0: single-reviewer stance, attributed to the model that actually produced it. + if (a.review) reviewerVotes.push({ reviewer: primary.model, votedFail: a.review.blockers.length > 0 }); const combined = combineReviews([a.review], { strategy: "single" }); consensusDefect = combined.defect; inconclusive = combined.inconclusive; @@ -2498,6 +2509,7 @@ export async function runLoopOverAiReview( return { status: "ok", advisoryNotes, + reviewerVotes, consensusDefect, split: aiReviewSplit, // Carry the split's calibrated confidence (#8) so the caller can apply the same `aiReviewCloseConfidence` diff --git a/test/unit/ai-review.test.ts b/test/unit/ai-review.test.ts index a250560dc..9e582cc85 100644 --- a/test/unit/ai-review.test.ts +++ b/test/unit/ai-review.test.ts @@ -1680,6 +1680,49 @@ describe("runLoopOverAiReview self-host dual-AI plan (#dual-ai-combiner)", () => expect([...seen].sort()).toEqual(["claude-code", "codex"]); }); + it("#8229 stage 0: reviewerVotes attribute each stance to the model that produced it — split case, both stances distinct", async () => { + const env = planEnv( + { reviewers: [{ model: "claude-code" }, { model: "codex" }], combine: "consensus" }, + async (model) => + model === "codex" + ? { response: reviewJson({ present: true, title: "Race condition in src/x.ts" }) } + : { response: reviewJson({ present: false }) }, + ); + const result = await runLoopOverAiReview(env, { ...baseInput, mode: "block" }); + if (result.status !== "ok") throw new Error("expected ok"); + // The tie-break judge re-runs order-swapped internally on disagreement — attribution must be immune + // to it because votes attach at leg production time, not slot interpretation. + const votes = [...result.reviewerVotes].sort((a, b) => a.reviewer.localeCompare(b.reviewer)); + expect(votes).toEqual([ + { reviewer: "claude-code", votedFail: false }, + { reviewer: "codex", votedFail: true }, + ]); + }); + + it("#8229 stage 0: an unparseable leg casts NO vote — never a fabricated stance for its model", async () => { + const env = planEnv( + { reviewers: [{ model: "claude-code" }, { model: "codex" }], combine: "consensus" }, + async (model) => + model === "claude-code" ? { response: "not json at all" } : { response: reviewJson({ present: false }) }, + ); + const result = await runLoopOverAiReview(env, { ...baseInput, mode: "block" }); + if (result.status !== "ok") throw new Error("expected ok"); + expect(result.reviewerVotes).toEqual([{ reviewer: "codex", votedFail: false }]); + }); + + it("#8229 stage 0: advisory-only runs carry no votes (block-mode corpus only)", async () => { + const run = vi.fn(async () => ({ response: reviewJson() })); + const env = createTestEnv({ + AI: { run } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + const result = await runLoopOverAiReview(env, baseInput); + if (result.status !== "ok") throw new Error("expected ok"); + expect(result.reviewerVotes).toEqual([]); + }); + it("single + BYOK: the provider writes the advisory; the one decision reviewer runs via the router", async () => { vi.stubGlobal( "fetch", diff --git a/test/unit/reviewer-vote-capture.test.ts b/test/unit/reviewer-vote-capture.test.ts new file mode 100644 index 000000000..d29c4a73d --- /dev/null +++ b/test/unit/reviewer-vote-capture.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, it, vi } from "vitest"; +import { runAiReviewForAdvisory } from "../../src/queue/processors"; +import * as repositories from "../../src/db/repositories"; +import { createTestEnv } from "../helpers/d1"; +import type { Advisory, RepositorySettings } from "../../src/types"; + +// #8229 stage 0: the persistence half of reviewer-vote capture. The runner-side attribution invariants +// (leg-production-time attachment, swap immunity, advisory-only emptiness) live in ai-review.test.ts; +// this file pins that an ok block-mode review writes ONE reviewer_vote audit row per reviewer with the +// provider identity as actor and the stance in metadata — and that a vote-store failure never touches +// the review result (best-effort, like every calibration write). + +const REPO = "acme/widgets"; + +function reviewJson(present: boolean): string { + return JSON.stringify({ + assessment: present ? "Likely defect." : "Looks fine.", + blockers: present ? ["Race condition in src/x.ts: unguarded shared write."] : [], + nits: [], + suggestions: [], + confidence: present ? 0.96 : 0.7, + }); +} + +function advisory(number: number): Advisory { + return { + id: `adv-${number}`, + targetType: "pull_request", + targetKey: `${REPO}#${number}`, + repoFullName: REPO, + pullNumber: number, + headSha: `sha${number}`, + conclusion: "neutral", + severity: "info", + title: "LoopOver advisory available", + summary: "ok", + findings: [], + generatedAt: "2026-07-23T00:00:00.000Z", + }; +} + +function voteEnv(seen: string[]): Env { + return createTestEnv({ + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "1000000", + AI: { + run: vi.fn(async (model: string) => { + if (!seen.includes(model)) seen.push(model); + // Second DISTINCT model flags; the first stays clean — a split with distinct stances. + return { response: reviewJson(seen.indexOf(model) === 1) }; + }), + } as unknown as Ai, + }); +} + +describe("reviewer-vote capture persistence (#8229 stage 0)", () => { + it("persists one reviewer_vote audit row per reviewer: provider as actor, stance in metadata", async () => { + const seen: string[] = []; + const env = voteEnv(seen); + await runAiReviewForAdvisory(env, { + mode: "live", + settings: { aiReviewMode: "block" } as RepositorySettings, + repoFullName: REPO, + pr: { number: 7, title: "Add helper", body: "Adds a helper." }, + author: "alice", + confirmedContributor: true, + advisory: advisory(7), + }); + + const rows = await env.DB.prepare("SELECT actor, metadata_json FROM audit_events WHERE event_type = 'reviewer_vote' AND target_key = ?") + .bind(`${REPO}#7`) + .all<{ actor: string; metadata_json: string }>(); + const votes = (rows.results ?? []) + .map((row) => ({ actor: row.actor, vote: (JSON.parse(row.metadata_json) as { vote: string }).vote })) + .sort((a, b) => a.actor.localeCompare(b.actor)); + expect(votes).toHaveLength(2); + // Exactly the two REVIEWER models (the disagreement tie-break judge may add later calls with other + // model ids — judges never vote), each with ITS OWN stance. + expect(new Set(votes.map((v) => v.actor))).toEqual(new Set(seen.slice(0, 2))); + const byActor = Object.fromEntries(votes.map((v) => [v.actor, v.vote])); + expect(byActor[seen[0]!]).toBe("non_fail"); + expect(byActor[seen[1]!]).toBe("fail"); + }); + + it("a rejecting vote write is swallowed: the review outcome is untouched (best-effort discipline)", async () => { + const seen: string[] = []; + const env = voteEnv(seen); + vi.spyOn(repositories, "recordAuditEvent").mockRejectedValue(new Error("vote store down")); + const result = await runAiReviewForAdvisory(env, { + mode: "live", + settings: { aiReviewMode: "block" } as RepositorySettings, + repoFullName: REPO, + pr: { number: 8, title: "Add helper", body: "Adds a helper." }, + author: "alice", + confirmedContributor: true, + advisory: advisory(8), + }); + expect(result).toBeDefined(); // the review completed despite every vote write rejecting + vi.restoreAllMocks(); + }); +});