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
15 changes: 14 additions & 1 deletion src/queue/ai-review-orchestration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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({
Expand Down
12 changes: 12 additions & 0 deletions src/services/ai-review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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`
Expand Down
43 changes: 43 additions & 0 deletions test/unit/ai-review.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
102 changes: 102 additions & 0 deletions test/unit/reviewer-vote-capture.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});