Skip to content

Commit d9cf5e1

Browse files
authored
feat(review): capture per-reviewer AI votes with swap-proof attribution (#8229) (#8265)
Block-mode dual review now attaches one vote per reviewer leg at production time (a.review <-> primary.model, b.review <-> secondary.model), immune to the tie-break judge's order swap. Ok results expose reviewerVotes; orchestration persists each as a best-effort reviewer_vote audit event (actor = model id, stance in metadata) for the later per-provider track-record stages. Advisory runs and unparseable legs cast no votes.
1 parent e46031a commit d9cf5e1

4 files changed

Lines changed: 171 additions & 1 deletion

File tree

src/queue/ai-review-orchestration.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
type TransientLockClaim,
2121
} from "./transient-locks";
2222
import { buildPullRequestAdvisory } from "../rules/advisory";
23-
import { getDecryptedRepositoryAiKey, getRepository, listCheckSummaries, listPullRequestFiles } from "../db/repositories";
23+
import { recordAuditEvent, getDecryptedRepositoryAiKey, getRepository, listCheckSummaries, listPullRequestFiles } from "../db/repositories";
2424
import { createInstallationToken } from "../github/app";
2525
import type { AgentActionMode } from "../settings/agent-execution";
2626
import { buildAiReviewDiff } from "../review/review-diff";
@@ -721,6 +721,19 @@ export async function runAiReviewForAdvisory(
721721
improvementSignal: args.improvementSignal === true,
722722
});
723723
if (result.status !== "ok") return undefined;
724+
// #8229 stage 0: persist each reviewer's stance for the provider track records — best-effort like every
725+
// calibration write (a vote-store failure must never affect the review), one audit event per reviewer,
726+
// attribution already swap-proof from the runner (votes attach at leg production time).
727+
for (const vote of result.reviewerVotes) {
728+
await recordAuditEvent(env, {
729+
eventType: "reviewer_vote",
730+
actor: vote.reviewer,
731+
targetKey: `${args.repoFullName}#${args.pr.number}`,
732+
outcome: "completed",
733+
detail: vote.votedFail ? "flagged a blocking defect" : "did not flag a blocking defect",
734+
metadata: { repoFullName: args.repoFullName, vote: vote.votedFail ? "fail" : "non_fail" },
735+
}).catch(() => undefined);
736+
}
724737
const findings: AdvisoryFinding[] = [];
725738
if (result.consensusDefect) {
726739
findings.push({

src/services/ai-review.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,11 @@ export type LoopOverAiReviewResult =
392392
inconclusive: boolean;
393393
estimatedNeurons: number;
394394
reviewerCount: number;
395+
/** Per-reviewer stances for the provider track records (#8229 stage 0). Attribution attaches at leg
396+
* PRODUCTION time (a.review ↔ primary.model, b.review ↔ secondary.model), so the tie-break judge's
397+
* order-swap — which operates downstream on copies — can never misattribute a vote. Block-mode only
398+
* (the gate corpus is what the track records score against); empty in advisory-only runs. */
399+
reviewerVotes: { reviewer: string; votedFail: boolean }[];
395400
inlineFindings: InlineFinding[];
396401
/** Combined improvement/value judgment (#4743), public-safe and ready to render. ALWAYS present (`null`
397402
* when `input.improvementSignal` is falsy, when neither reviewer emitted a usable judgment, or when the
@@ -2338,6 +2343,7 @@ export async function runLoopOverAiReview(
23382343
}
23392344

23402345
let consensusDefect: AiConsensusDefect | null = null;
2346+
const reviewerVotes: { reviewer: string; votedFail: boolean }[] = [];
23412347
let secondReview: ModelReview | null = null;
23422348
let aiReviewSplit = false;
23432349
let splitConfidence: number | undefined;
@@ -2375,6 +2381,9 @@ export async function runLoopOverAiReview(
23752381
]);
23762382
if (a.fallbackNote) fallbackNotes.push(a.fallbackNote);
23772383
if (b.fallbackNote) fallbackNotes.push(b.fallbackNote);
2384+
// #8229 stage 0: attach votes HERE, where slot↔model is unambiguous by construction.
2385+
if (a.review) reviewerVotes.push({ reviewer: primary.model, votedFail: a.review.blockers.length > 0 });
2386+
if (b.review) reviewerVotes.push({ reviewer: secondary.model, votedFail: b.review.blockers.length > 0 });
23782387
secondReview = b.review;
23792388
// Combine per the configured strategy (#dual-ai-combiner). Default `consensus` is byte-identical to the
23802389
// historical logic: block only on agreement, lone blocker → split, a missing opinion → inconclusive
@@ -2435,6 +2444,8 @@ export async function runLoopOverAiReview(
24352444
)
24362445
: ({ review: advisoryReview } as ReviewerOpinionOutcome);
24372446
if (a.fallbackNote) fallbackNotes.push(a.fallbackNote);
2447+
// #8229 stage 0: single-reviewer stance, attributed to the model that actually produced it.
2448+
if (a.review) reviewerVotes.push({ reviewer: primary.model, votedFail: a.review.blockers.length > 0 });
24382449
const combined = combineReviews([a.review], { strategy: "single" });
24392450
consensusDefect = combined.defect;
24402451
inconclusive = combined.inconclusive;
@@ -2498,6 +2509,7 @@ export async function runLoopOverAiReview(
24982509
return {
24992510
status: "ok",
25002511
advisoryNotes,
2512+
reviewerVotes,
25012513
consensusDefect,
25022514
split: aiReviewSplit,
25032515
// Carry the split's calibrated confidence (#8) so the caller can apply the same `aiReviewCloseConfidence`

test/unit/ai-review.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1680,6 +1680,49 @@ describe("runLoopOverAiReview self-host dual-AI plan (#dual-ai-combiner)", () =>
16801680
expect([...seen].sort()).toEqual(["claude-code", "codex"]);
16811681
});
16821682

1683+
it("#8229 stage 0: reviewerVotes attribute each stance to the model that produced it — split case, both stances distinct", async () => {
1684+
const env = planEnv(
1685+
{ reviewers: [{ model: "claude-code" }, { model: "codex" }], combine: "consensus" },
1686+
async (model) =>
1687+
model === "codex"
1688+
? { response: reviewJson({ present: true, title: "Race condition in src/x.ts" }) }
1689+
: { response: reviewJson({ present: false }) },
1690+
);
1691+
const result = await runLoopOverAiReview(env, { ...baseInput, mode: "block" });
1692+
if (result.status !== "ok") throw new Error("expected ok");
1693+
// The tie-break judge re-runs order-swapped internally on disagreement — attribution must be immune
1694+
// to it because votes attach at leg production time, not slot interpretation.
1695+
const votes = [...result.reviewerVotes].sort((a, b) => a.reviewer.localeCompare(b.reviewer));
1696+
expect(votes).toEqual([
1697+
{ reviewer: "claude-code", votedFail: false },
1698+
{ reviewer: "codex", votedFail: true },
1699+
]);
1700+
});
1701+
1702+
it("#8229 stage 0: an unparseable leg casts NO vote — never a fabricated stance for its model", async () => {
1703+
const env = planEnv(
1704+
{ reviewers: [{ model: "claude-code" }, { model: "codex" }], combine: "consensus" },
1705+
async (model) =>
1706+
model === "claude-code" ? { response: "not json at all" } : { response: reviewJson({ present: false }) },
1707+
);
1708+
const result = await runLoopOverAiReview(env, { ...baseInput, mode: "block" });
1709+
if (result.status !== "ok") throw new Error("expected ok");
1710+
expect(result.reviewerVotes).toEqual([{ reviewer: "codex", votedFail: false }]);
1711+
});
1712+
1713+
it("#8229 stage 0: advisory-only runs carry no votes (block-mode corpus only)", async () => {
1714+
const run = vi.fn(async () => ({ response: reviewJson() }));
1715+
const env = createTestEnv({
1716+
AI: { run } as unknown as Ai,
1717+
AI_SUMMARIES_ENABLED: "true",
1718+
AI_PUBLIC_COMMENTS_ENABLED: "true",
1719+
AI_DAILY_NEURON_BUDGET: "100000",
1720+
});
1721+
const result = await runLoopOverAiReview(env, baseInput);
1722+
if (result.status !== "ok") throw new Error("expected ok");
1723+
expect(result.reviewerVotes).toEqual([]);
1724+
});
1725+
16831726
it("single + BYOK: the provider writes the advisory; the one decision reviewer runs via the router", async () => {
16841727
vi.stubGlobal(
16851728
"fetch",
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
import { runAiReviewForAdvisory } from "../../src/queue/processors";
3+
import * as repositories from "../../src/db/repositories";
4+
import { createTestEnv } from "../helpers/d1";
5+
import type { Advisory, RepositorySettings } from "../../src/types";
6+
7+
// #8229 stage 0: the persistence half of reviewer-vote capture. The runner-side attribution invariants
8+
// (leg-production-time attachment, swap immunity, advisory-only emptiness) live in ai-review.test.ts;
9+
// this file pins that an ok block-mode review writes ONE reviewer_vote audit row per reviewer with the
10+
// provider identity as actor and the stance in metadata — and that a vote-store failure never touches
11+
// the review result (best-effort, like every calibration write).
12+
13+
const REPO = "acme/widgets";
14+
15+
function reviewJson(present: boolean): string {
16+
return JSON.stringify({
17+
assessment: present ? "Likely defect." : "Looks fine.",
18+
blockers: present ? ["Race condition in src/x.ts: unguarded shared write."] : [],
19+
nits: [],
20+
suggestions: [],
21+
confidence: present ? 0.96 : 0.7,
22+
});
23+
}
24+
25+
function advisory(number: number): Advisory {
26+
return {
27+
id: `adv-${number}`,
28+
targetType: "pull_request",
29+
targetKey: `${REPO}#${number}`,
30+
repoFullName: REPO,
31+
pullNumber: number,
32+
headSha: `sha${number}`,
33+
conclusion: "neutral",
34+
severity: "info",
35+
title: "LoopOver advisory available",
36+
summary: "ok",
37+
findings: [],
38+
generatedAt: "2026-07-23T00:00:00.000Z",
39+
};
40+
}
41+
42+
function voteEnv(seen: string[]): Env {
43+
return createTestEnv({
44+
AI_SUMMARIES_ENABLED: "true",
45+
AI_PUBLIC_COMMENTS_ENABLED: "true",
46+
AI_DAILY_NEURON_BUDGET: "1000000",
47+
AI: {
48+
run: vi.fn(async (model: string) => {
49+
if (!seen.includes(model)) seen.push(model);
50+
// Second DISTINCT model flags; the first stays clean — a split with distinct stances.
51+
return { response: reviewJson(seen.indexOf(model) === 1) };
52+
}),
53+
} as unknown as Ai,
54+
});
55+
}
56+
57+
describe("reviewer-vote capture persistence (#8229 stage 0)", () => {
58+
it("persists one reviewer_vote audit row per reviewer: provider as actor, stance in metadata", async () => {
59+
const seen: string[] = [];
60+
const env = voteEnv(seen);
61+
await runAiReviewForAdvisory(env, {
62+
mode: "live",
63+
settings: { aiReviewMode: "block" } as RepositorySettings,
64+
repoFullName: REPO,
65+
pr: { number: 7, title: "Add helper", body: "Adds a helper." },
66+
author: "alice",
67+
confirmedContributor: true,
68+
advisory: advisory(7),
69+
});
70+
71+
const rows = await env.DB.prepare("SELECT actor, metadata_json FROM audit_events WHERE event_type = 'reviewer_vote' AND target_key = ?")
72+
.bind(`${REPO}#7`)
73+
.all<{ actor: string; metadata_json: string }>();
74+
const votes = (rows.results ?? [])
75+
.map((row) => ({ actor: row.actor, vote: (JSON.parse(row.metadata_json) as { vote: string }).vote }))
76+
.sort((a, b) => a.actor.localeCompare(b.actor));
77+
expect(votes).toHaveLength(2);
78+
// Exactly the two REVIEWER models (the disagreement tie-break judge may add later calls with other
79+
// model ids — judges never vote), each with ITS OWN stance.
80+
expect(new Set(votes.map((v) => v.actor))).toEqual(new Set(seen.slice(0, 2)));
81+
const byActor = Object.fromEntries(votes.map((v) => [v.actor, v.vote]));
82+
expect(byActor[seen[0]!]).toBe("non_fail");
83+
expect(byActor[seen[1]!]).toBe("fail");
84+
});
85+
86+
it("a rejecting vote write is swallowed: the review outcome is untouched (best-effort discipline)", async () => {
87+
const seen: string[] = [];
88+
const env = voteEnv(seen);
89+
vi.spyOn(repositories, "recordAuditEvent").mockRejectedValue(new Error("vote store down"));
90+
const result = await runAiReviewForAdvisory(env, {
91+
mode: "live",
92+
settings: { aiReviewMode: "block" } as RepositorySettings,
93+
repoFullName: REPO,
94+
pr: { number: 8, title: "Add helper", body: "Adds a helper." },
95+
author: "alice",
96+
confirmedContributor: true,
97+
advisory: advisory(8),
98+
});
99+
expect(result).toBeDefined(); // the review completed despite every vote write rejecting
100+
vi.restoreAllMocks();
101+
});
102+
});

0 commit comments

Comments
 (0)