diff --git a/src/queue/ai-review-orchestration.ts b/src/queue/ai-review-orchestration.ts index e3951f898..165dcc908 100644 --- a/src/queue/ai-review-orchestration.ts +++ b/src/queue/ai-review-orchestration.ts @@ -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"; @@ -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 diff --git a/src/services/reviewer-routing.ts b/src/services/reviewer-routing.ts index 15d224a59..9786bcb2d 100644 --- a/src/services/reviewer-routing.ts +++ b/src/services/reviewer-routing.ts @@ -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 diff --git a/test/unit/reviewer-routing.test.ts b/test/unit/reviewer-routing.test.ts index 6c167671a..bc5b13e0c 100644 --- a/test/unit/reviewer-routing.test.ts +++ b/test/unit/reviewer-routing.test.ts @@ -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"; @@ -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(); diff --git a/test/unit/reviewer-vote-capture.test.ts b/test/unit/reviewer-vote-capture.test.ts index baccd3fbb..5576b99c3 100644 --- a/test/unit/reviewer-vote-capture.test.ts +++ b/test/unit/reviewer-vote-capture.test.ts @@ -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"; @@ -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, @@ -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(); }); });