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
16 changes: 14 additions & 2 deletions src/queue/ai-review-orchestration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
import { buildPullRequestAdvisory } from "../rules/advisory";
import { recordAuditEvent, getDecryptedRepositoryAiKey, getRepository, listCheckSummaries, listPullRequestFiles } from "../db/repositories";
import { registerHeldLock, unregisterHeldLock } from "./held-lock-registry";
import { recordRoutingShadow } from "../services/reviewer-routing";
import { recordRoutingShadow, routingShadowMetrics } from "../services/reviewer-routing";
import { judgmentAgreementMetrics, scoreJudgmentAgreement } from "../review/judgment-agreement";
import { persistDecisionReplayPrompt } from "../review/decision-replay";
import { createInstallationToken } from "../github/app";
Expand Down Expand Up @@ -992,11 +992,23 @@ export async function runAiReviewForAdvisory(
// preferred for this repo (audit metadata only; the recap aggregates it). Same best-effort discipline
// as the votes above: internally fail-safe, zero AI spend, and a no-signal review records nothing.
if (result.reviewerVotes.length >= 2) {
await recordRoutingShadow(env, {
const shadow = await recordRoutingShadow(env, {
repoFullName: args.repoFullName,
prNumber: args.pr.number,
actualProviders: result.reviewerVotes.map((vote) => vote.reviewer),
});
// #10265: the same shadow decision onto the review's own AI trace, so how decisively the evidence
// favours a reviewer can be read against what that review cost. Reuses the decision `recordRoutingShadow`
// already returned rather than re-reading the track records. A null decision (no density / tie / read
// error) emits nothing, and the capture is a no-op with PostHog off or no ambient trace.
if (shadow) {
for (const metric of routingShadowMetrics(shadow)) {
capturePostHogAiMetric({
...metric,
context: { repo: args.repoFullName, pullNumber: args.pr.number, agent: shadow.preferredProvider },
});
}
}
}
const findings: AdvisoryFinding[] = [];
// #9124: the model/prompt commitments an AI-judgment finding carries — computed once, shared by both the
Expand Down
29 changes: 29 additions & 0 deletions src/services/reviewer-routing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,35 @@ export type RoutingShadowDecision = {
basis: Array<{ provider: string; decided: number; precision: number }>;
};

/**
* PURE: the shadow decision as PostHog `$ai_metric` measurements (#10265), joined by the caller to the
* review's own AI trace so the routing evidence reads against the cost and model of the run that produced it.
*
* Deliberately NOT "did the shadow agree with what ran": `computeWouldHaveRouted` picks its leader OUT of
* `actualProviders`, so a preferred provider is by construction always one that ran, and an agreement metric
* would be a constant 1. What varies — and what the shadow actually measures — is how DECISIVELY the evidence
* favours the leader, so that is what is reported:
*
* - `routing_shadow_precision_margin` — leader precision minus runner-up. Strictly > 0 (a tie yields no
* decision at all), and a margin near zero says the preference is real but weak.
* - `routing_shadow_preferred_precision` — the leader's own precision, without which the margin cannot be
* read (0.02 between 0.97 and 0.95 is a different claim than between 0.05 and 0.03).
*
* `basis` arrives in `actualProviders` order rather than ranked, so the top two are taken by sorting here.
* A decision always carries at least two entries — the null guards upstream (lone reviewer, missing/floor-shy
* row, tie) mean this is only ever reached with a strict leader over a runner-up.
*/
export function routingShadowMetrics(decision: RoutingShadowDecision): Array<{ name: string; value: number }> {
const ranked = [...decision.basis].sort((a, b) => b.precision - a.precision);
const leader = ranked[0];
const runnerUp = ranked[1];
if (!leader || !runnerUp) return [];
return [
{ name: "routing_shadow_precision_margin", value: leader.precision - runnerUp.precision },
{ name: "routing_shadow_preferred_precision", value: leader.precision },
];
}

/**
* PURE: what would evidence-weighted routing have preferred for this repo, given the current track
* records and the providers the review ACTUALLY used? Null — record nothing — unless EVERY actual
Expand Down
32 changes: 32 additions & 0 deletions test/unit/reviewer-routing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
REVIEWER_VOTE_EVENT_TYPE,
REVIEWER_VOTE_SCAN_LIMIT,
ROUTING_MIN_DECIDED,
routingShadowMetrics,
} from "../../src/services/reviewer-routing";
import { buildRoutingRecapSection } from "../../src/services/maintainer-recap-routing";
import { formatMaintainerRecap, runMaintainerRecap } from "../../src/services/maintainer-recap";
Expand Down Expand Up @@ -68,6 +69,37 @@ describe("computeWouldHaveRouted (#8229 stage 1)", () => {
});
});

describe("routingShadowMetrics (#10265)", () => {
const PAIR = ["claude-code", "codex"];

it("reports the leader's margin over the runner-up AND the leader's own precision", () => {
const decision = computeWouldHaveRouted([record("claude-code", { precision: 0.9 }), record("codex", { precision: 0.6 })], REPO, PAIR)!;
expect(routingShadowMetrics(decision)).toEqual([
{ name: "routing_shadow_precision_margin", value: 0.9 - 0.6 },
{ name: "routing_shadow_preferred_precision", value: 0.9 },
]);
});

it("ranks the basis itself rather than trusting its order -- basis arrives in actualProviders order, not by precision", () => {
// codex leads, but claude-code is first in `basis` because that is the order the review ran them in.
const decision = computeWouldHaveRouted([record("claude-code", { precision: 0.4 }), record("codex", { precision: 0.95 })], REPO, PAIR)!;
expect(decision.basis[0]?.provider).toBe("claude-code"); // unranked, as the decision contract states
expect(decision.preferredProvider).toBe("codex");
expect(routingShadowMetrics(decision)).toEqual([
{ name: "routing_shadow_precision_margin", value: 0.95 - 0.4 },
{ name: "routing_shadow_preferred_precision", value: 0.95 },
]);
});

it("reports nothing without a leader AND a runner-up to compare -- a margin needs two", () => {
// Unreachable through computeWouldHaveRouted (its lone-reviewer guard fires first), so constructed
// directly: the helper must not invent a margin from a one-sided basis if a caller ever hands it one.
const base = { repoFullName: REPO, preferredProvider: "claude-code", actualProviders: PAIR };
expect(routingShadowMetrics({ ...base, basis: [] })).toEqual([]);
expect(routingShadowMetrics({ ...base, basis: [{ provider: "claude-code", decided: 12, precision: 0.9 }] })).toEqual([]);
});
});

describe("loadLiveProviderTrackRecords (#8229 stage 1 read path)", () => {
it("joins live reviewer_vote rows to the labeled corpus; corrupt/malformed vote rows are never evidence", async () => {
const env = createTestEnv();
Expand Down
40 changes: 40 additions & 0 deletions test/unit/reviewer-vote-capture.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { describe, expect, it, vi } from "vitest";
import { runAiReviewForAdvisory } from "../../src/queue/processors";
import * as repositories from "../../src/db/repositories";
import * as posthogModule from "../../src/selfhost/posthog";
import * as routingModule from "../../src/services/reviewer-routing";
import { createTestEnv } from "../helpers/d1";
import type { Advisory, RepositorySettings } from "../../src/types";

Expand Down Expand Up @@ -105,6 +107,7 @@ describe("routing shadow orchestration hook (#8229 stage 1)", () => {
it("a dual ok review invokes the shadow (recording nothing on a sparse corpus); the review outcome is untouched", async () => {
const seen: string[] = [];
const env = voteEnv(seen);
const metricSpy = vi.spyOn(posthogModule, "capturePostHogAiMetric").mockImplementation(() => undefined);
const result = await runAiReviewForAdvisory(env, {
mode: "live",
settings: { aiReviewMode: "block" } as RepositorySettings,
Expand All @@ -121,5 +124,42 @@ describe("routing shadow orchestration hook (#8229 stage 1)", () => {
// The votes themselves persisted — the hook runs strictly after and independently of them.
const votes = await env.DB.prepare("SELECT COUNT(*) AS n FROM audit_events WHERE event_type = 'reviewer_vote' AND target_key = ?").bind(`${REPO}#21`).first<{ n: number }>();
expect(votes?.n).toBe(2);
// #10265: no decision ⇒ no metric. An orphan margin would join to a review the shadow never ranked.
expect(metricSpy.mock.calls.filter((call) => call[0].name.startsWith("routing_shadow_"))).toEqual([]);
vi.restoreAllMocks();
});

it("#10265: a shadow decision emits the margin and the leader's precision onto the review's trace", async () => {
const seen: string[] = [];
const env = voteEnv(seen);
const metricSpy = vi.spyOn(posthogModule, "capturePostHogAiMetric").mockImplementation(() => undefined);
// The corpus math that PRODUCES a decision is pinned in reviewer-routing.test.ts; this test owns the
// wiring only, so the hook is stubbed with a decision whose basis is deliberately NOT precision-ordered.
vi.spyOn(routingModule, "recordRoutingShadow").mockResolvedValue({
repoFullName: REPO,
preferredProvider: "codex",
actualProviders: ["claude-code", "codex"],
basis: [
{ provider: "claude-code", decided: 12, precision: 0.5 },
{ provider: "codex", decided: 14, precision: 0.8 },
],
});
const result = await runAiReviewForAdvisory(env, {
mode: "live",
settings: { aiReviewMode: "block" } as RepositorySettings,
repoFullName: REPO,
pr: { number: 22, title: "Add helper", body: "Adds a helper." },
author: "alice",
confirmedContributor: true,
advisory: advisory(22),
});
expect(result).toBeDefined();
const shadowMetrics = metricSpy.mock.calls.map((call) => call[0]).filter((event) => event.name.startsWith("routing_shadow_"));
expect(shadowMetrics).toEqual([
// The margin is ranked from the basis, not read off its (unordered) first entry.
{ name: "routing_shadow_precision_margin", value: 0.8 - 0.5, context: { repo: REPO, pullNumber: 22, agent: "codex" } },
{ name: "routing_shadow_preferred_precision", value: 0.8, context: { repo: REPO, pullNumber: 22, agent: "codex" } },
]);
vi.restoreAllMocks();
});
});