From e87c70df4285086a46668fc08c2cc5228c684bc9 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:46:51 -0700 Subject: [PATCH] =?UTF-8?q?feat(replay):=20re-query=20action-match=20mode?= =?UTF-8?q?=20=E2=80=94=20the=20honest=20metric=20for=20the=20one=20nondet?= =?UTF-8?q?erministic=20stage=20(#9028)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes #9028. Requirement 2 (wall-clock capture) was verified already landed: staleness-clock.ts cites this issue in its header, decision_replay_inputs.replay_json carries the captured instant, and decision-replay.test.ts pins both arms per rule plus the CLI's clock-divergence exit. This PR is requirement 1, previously deferred on the prompt-persistence decision -- which is made here, not deferred again. WHAT IT IS. Everything downstream of the model is a pure function the replay harness pins bit-exactly, including time. The model call is the one stage that cannot be: hosted inference is not bit-deterministic even at temperature 0 (batching, kernel scheduling, silent model revisions). So `replay-decision.ts --requery ` re-runs the model n times against the EXACT persisted prompt and reports an ACTION-MATCH RATE -- across fresh runs, how often the verdict lands in the same CLASS the recorded decision acted on (defect vs clean, through the same parseModelReview the live pipeline uses, with the same ai_consensus_defect/ai_review_split boundary the gate acts on). The word "reproducibility" appears only inside the metric's own label as a negation -- "action-match-rate (NOT reproducibility)" -- so a pasted report cannot shed the caveat. THE PERSISTENCE DECISION. decision_replay_prompts (migration 0200): a private sibling of decision_replay_inputs with the identical posture and a deliberately SHORTER 30-day retention -- the prompt embeds the full diff plus contributor content (the largest, most sensitive artifact in the replay family), requery is a debugging tool for recent decisions, and the public promptDigest commitment outlives the text forever. Keyed by the BASE record id so a supersession's :rev row still finds its head's prompt. Oversize prompts are SKIPPED, never truncated: a truncated prompt re-queried would report a rate for a prompt that was never sent. BOTH TURNS, because the first honest test caught the design flaw: the system prompt carries the rubric + config suffixes (what promptDigest commits to), but the DIFF travels in the USER turn -- re-querying with the system prompt alone would ask the model to review nothing. Captured at the orchestration site, never through findings (findings reach public render surfaces). The CLI replays the live call's own shape: a system turn plus a user turn at temperature 0, so the rate is a statement about the decision, not about a different way of asking. Denominator honesty: a transport failure is an UNUSABLE run counted in the denominator, never a silent skip -- shrinking it would inflate the rate exactly when the provider is flakiest. Unusable matches neither class, mirroring the live pipeline's fail-closed inconclusive routing. Provider config is explicit env only (REPLAY_AI_BASE_URL/REPLAY_AI_MODEL or ANTHROPIC_API_KEY/REPLAY_AI_MODEL) -- a tool that spends tokens never guesses which model to spend against. Exit 0 on a produced report (a low rate is a FINDING); 2 on unusable input or missing config. Pure replay mode remains network-free by construction, untouched. Tests: 21 across four suites -- the class boundaries through the live parser, both recorded-class codes, denominator invariants, zero-run degenerate arm, verbatim two-turn passthrough, bundle/env parsing with every named error (including the 30-day retention hint), base-id upsert semantics, oversize skip, swallowed persist failure, the retention rule at its own window, and an end-to-end pass proving a real AI review persists both turns with the diff in the user one. New module: 100% statements, branches, and functions. Full suite: 24,045 green. --- migrations/0200_decision_replay_prompts.sql | 22 +++ scripts/check-schema-drift.ts | 4 + scripts/replay-decision.ts | 107 +++++++++++++ src/db/retention.ts | 6 + src/queue/ai-review-orchestration.ts | 19 +++ src/review/decision-replay.ts | 44 ++++++ src/review/decision-requery.ts | 89 +++++++++++ src/services/ai-review.ts | 11 ++ test/unit/decision-requery.test.ts | 145 ++++++++++++++++++ ...nel-retrigger-pending-force-review.test.ts | 52 +++++++ test/unit/retention.test.ts | 18 +++ 11 files changed, 517 insertions(+) create mode 100644 migrations/0200_decision_replay_prompts.sql create mode 100644 src/review/decision-requery.ts create mode 100644 test/unit/decision-requery.test.ts diff --git a/migrations/0200_decision_replay_prompts.sql b/migrations/0200_decision_replay_prompts.sql new file mode 100644 index 0000000000..d7a6877998 --- /dev/null +++ b/migrations/0200_decision_replay_prompts.sql @@ -0,0 +1,22 @@ +-- #9028 (epic #8828, Replay v2): the exact system prompt sent to the model, for re-query action matching. +-- +-- The public decision record commits to the prompt via `promptDigest` (sha256 of the ACTUAL +-- buildSystemPrompt output for that call, #9124) -- a contributor can verify the commitment, but a digest +-- cannot be re-queried. `scripts/replay-decision.ts --requery` needs the text itself to re-run the model for +-- the same target and report an ACTION-MATCH RATE (never "reproducibility": hosted inference is not +-- bit-deterministic even at temperature 0, and the docs say so). +-- +-- DELIBERATELY A PRIVATE SIBLING TABLE, mirroring decision_replay_inputs' posture exactly (#8838): the +-- prompt embeds the full diff plus contributor content, so it must never live in the public record. Row-size +-- is why it is ALSO not a decision_replay_inputs column: prompts run to hundreds of KB, and pinning them to +-- the replay-input row would drag that table's every read through the blob. +-- +-- Retention is 30 days (src/db/retention.ts), deliberately SHORTER than decision_replay_inputs' 180: the +-- re-query mode is an operator debugging tool for RECENT decisions, the blob is the largest and most +-- sensitive artifact in the replay family, and the public promptDigest commitment outlives the text forever. +CREATE TABLE IF NOT EXISTS decision_replay_prompts ( + record_id TEXT PRIMARY KEY, -- decision_records.id (record:#@) + prompt_json TEXT NOT NULL, -- { systemPrompt } -- exactly what was sent, nothing derived + created_at TEXT NOT NULL +); +CREATE INDEX IF NOT EXISTS idx_decision_replay_prompts_created_at ON decision_replay_prompts(created_at); diff --git a/scripts/check-schema-drift.ts b/scripts/check-schema-drift.ts index 3976ebfe5c..9d256094cf 100644 --- a/scripts/check-schema-drift.ts +++ b/scripts/check-schema-drift.ts @@ -46,6 +46,10 @@ export const RAW_SQL_ONLY_TABLES: Set = new Set([ "decision_ledger_anchors", "decision_records", "decision_replay_inputs", + // #9028: same raw-SQL-only posture as its sibling above — an operator-private blob table the drizzle + // schema never queries (writes go through persistDecisionReplayPrompt's raw statement; reads happen only + // in the operator's own extract SQL for the replay CLI). + "decision_replay_prompts", "ai_review_verdict_flips", "global_agent_controls", "global_contributor_blacklist", diff --git a/scripts/replay-decision.ts b/scripts/replay-decision.ts index fbe4b76715..d7ec476e86 100644 --- a/scripts/replay-decision.ts +++ b/scripts/replay-decision.ts @@ -27,8 +27,38 @@ // Exit codes: 0 = replayed, same verdict ("here is the clause"); 1 = DIVERGENCE — a bug by definition, // file it with the printed stage diff; 2 = unusable input. Replay mode cannot re-query the model or touch // any network/DB by construction: replayDecision is a pure function of the two JSON values. +// +// #9028: `--requery ` is the ONE deliberately-networked mode, for the one stage pure replay cannot cover. +// It re-runs the model n times against the EXACT persisted prompt and reports an ACTION-MATCH RATE — never +// "reproducibility": hosted inference is not bit-deterministic even at temperature 0 (batching, kernel +// scheduling, silent model revisions), so bit-comparing outputs would measure the provider's scheduler, not +// the decision. What CAN be honestly measured is whether fresh runs land in the same verdict CLASS the +// recorded decision acted on (defect vs clean, through the same parseModelReview the live pipeline uses). +// +// The bundle gains an optional `prompt` for this mode. EXTRACT (joins the private prompts sibling by the +// BASE record id, so a supersession's `:rev` row still finds its head's prompt): +// SELECT json_build_object( +// 'record', to_jsonb(dr) || dr.record_json::jsonb, +// 'replayInput', dri.replay_json::jsonb, +// 'prompt', drp.prompt_json::jsonb +// ) +// FROM decision_records dr +// JOIN decision_replay_inputs dri ON dri.record_id = dr.id +// LEFT JOIN decision_replay_prompts drp +// ON drp.record_id = 'record:' || dr.repo_full_name || '#' || dr.pull_number || '@' || dr.head_sha +// WHERE dr.id = 'record:#@'; +// +// Provider config (explicit env, no defaults — this is a debugging tool, not a service): +// REPLAY_AI_BASE_URL + REPLAY_AI_MODEL [+ REPLAY_AI_API_KEY] -> any OpenAI-compatible endpoint (Ollama etc.) +// ANTHROPIC_API_KEY + REPLAY_AI_MODEL -> Anthropic +// +// Requery exit codes: 0 = report produced (a low rate is a FINDING, not a failure); 2 = unusable input or +// missing provider config. Prompts above the persistence cap were skipped at capture time, never truncated — +// a truncated prompt re-queried would report a rate for a prompt that was never sent. import { readFileSync } from "node:fs"; import { replayDecision, type DecisionReplayInput, type ReplayableRecord } from "../src/review/decision-replay"; +import { recordedJudgmentClass, runRequery } from "../src/review/decision-requery"; +import { createAnthropicAi, createOpenAiCompatibleAi, type SelfHostAi } from "../src/selfhost/ai"; /** Parse + normalize a bundle (snake_case SQL rows accepted) and replay it. Exported for tests. * @@ -61,9 +91,86 @@ export function runReplayBundle(raw: string, atMs?: number): { outcome: ReturnTy return { outcome: replayDecision(record, replayInput, atMs === undefined ? {} : { nowMs: atMs }) }; } +/** Parse the bundle's optional prompt + replay input for requery. Exported for tests. */ +export function parseRequeryBundle(raw: string): { systemPrompt: string; userPrompt: string; recordedClass: "defect" | "clean" } | { error: string } { + let bundle: { replayInput?: DecisionReplayInput; prompt?: { systemPrompt?: unknown; userPrompt?: unknown } }; + try { + bundle = JSON.parse(raw) as never; + } catch (error) { + return { error: `unparseable bundle JSON: ${error instanceof Error ? error.message : String(error)}` }; + } + const systemPrompt = bundle.prompt?.systemPrompt; + const userPrompt = bundle.prompt?.userPrompt; + if (typeof systemPrompt !== "string" || systemPrompt.length === 0 || typeof userPrompt !== "string" || userPrompt.length === 0) { + return { error: "requery needs bundle.prompt.{systemPrompt,userPrompt} — extract with the prompts LEFT JOIN in this file's header (rows age out after 30 days; older decisions cannot be re-queried, only replayed)" }; + } + if (!bundle.replayInput || !Array.isArray(bundle.replayInput.findings)) { + return { error: "requery needs bundle.replayInput.findings to derive the recorded verdict class" }; + } + return { systemPrompt, userPrompt, recordedClass: recordedJudgmentClass(bundle.replayInput) }; +} + +/** Build the provider client from explicit env. Exported for tests; returns an error string when unconfigured. */ +export function requeryClientFromEnv(env: Record): { ai: SelfHostAi; model: string } | { error: string } { + const model = env.REPLAY_AI_MODEL; + if (!model) return { error: "requery needs REPLAY_AI_MODEL (explicit — this tool never guesses which model to spend against)" }; + if (env.REPLAY_AI_BASE_URL) { + return { ai: createOpenAiCompatibleAi({ baseUrl: env.REPLAY_AI_BASE_URL, apiKey: env.REPLAY_AI_API_KEY, model }), model }; + } + if (env.ANTHROPIC_API_KEY) { + return { ai: createAnthropicAi({ apiKey: env.ANTHROPIC_API_KEY, model }), model }; + } + return { error: "requery needs REPLAY_AI_BASE_URL (OpenAI-compatible) or ANTHROPIC_API_KEY" }; +} + const invokedDirectly = process.argv[1]?.endsWith("replay-decision.ts") === true; if (invokedDirectly) { const argv = process.argv.slice(2); + const requeryIndex = argv.indexOf("--requery"); + if (requeryIndex !== -1) { + const runsRaw = argv[requeryIndex + 1]; + const runs = Number(runsRaw); + if (!Number.isInteger(runs) || runs < 1 || runs > 25) { + console.error("replay-decision: --requery requires a run count between 1 and 25"); + process.exit(2); + } + const requerySource = argv.filter((_arg, index) => index !== requeryIndex && index !== requeryIndex + 1)[0]; + if (!requerySource) { + console.error("usage: replay-decision.ts --requery "); + process.exit(2); + } + const rawBundle = requerySource === "-" ? readFileSync(0, "utf8") : readFileSync(requerySource, "utf8"); + const parsed = parseRequeryBundle(rawBundle); + if ("error" in parsed) { + console.error(`replay-decision: ${parsed.error}`); + process.exit(2); + } + const client = requeryClientFromEnv(process.env); + if ("error" in client) { + console.error(`replay-decision: ${client.error}`); + process.exit(2); + } + const report = await runRequery({ + systemPrompt: parsed.systemPrompt, + userPrompt: parsed.userPrompt, + runs, + recordedClass: parsed.recordedClass, + // The live call's own shape (ai-review.ts): a system turn plus a user turn, temperature 0. Matching it + // is what makes the rate a statement about the DECISION and not about a different way of asking. + callModel: async (systemPrompt, userPrompt) => { + const result = await client.ai.run(client.model, { + temperature: 0, + messages: [ + { role: "system", content: systemPrompt }, + { role: "user", content: userPrompt }, + ], + }); + return result.response ?? ""; + }, + }); + console.log(JSON.stringify(report, null, 2)); + process.exit(0); + } const atIndex = argv.indexOf("--at"); const atRaw = atIndex === -1 ? undefined : argv[atIndex + 1]; if (atIndex !== -1 && (atRaw === undefined || !Number.isFinite(Number(atRaw)))) { diff --git a/src/db/retention.ts b/src/db/retention.ts index a922bb25c2..ec19f68033 100644 --- a/src/db/retention.ts +++ b/src/db/retention.ts @@ -101,6 +101,10 @@ export const RETENTION_POLICY: readonly RetentionRule[] = [ { table: "predicted_gate_calibration_ledger", column: "created_at", days: 90 }, { table: "contributor_gate_history", column: "created_at", days: 90 }, { table: "decision_replay_inputs", column: "created_at", days: 180 }, + // #9028: deliberately SHORTER than decision_replay_inputs' 180d -- the prompt blob embeds the full diff + // plus contributor content (the largest, most sensitive artifact in the replay family), the re-query mode + // is a debugging tool for RECENT decisions, and the public promptDigest commitment outlives the text. + { table: "decision_replay_prompts", column: "created_at", days: 30 }, ]; // #9083: a real, single-column, indexable primary key for the ordered-range delete below, keyed by table @@ -142,6 +146,8 @@ export const RETENTION_PK_COLUMN: Readonly> = { contributor_gate_history: "id", // decision_replay_inputs keys on record_id (decision_records.id), not an `id` column. decision_replay_inputs: "record_id", + // Same key shape as decision_replay_inputs above, for the same reason. + decision_replay_prompts: "record_id", }; /** diff --git a/src/queue/ai-review-orchestration.ts b/src/queue/ai-review-orchestration.ts index 4f5378513d..346df25ea5 100644 --- a/src/queue/ai-review-orchestration.ts +++ b/src/queue/ai-review-orchestration.ts @@ -26,6 +26,7 @@ import { recordAuditEvent, getDecryptedRepositoryAiKey, getRepository, listCheck import { registerHeldLock, unregisterHeldLock } from "./held-lock-registry"; import { recordRoutingShadow } from "../services/reviewer-routing"; import { scoreJudgmentAgreement } from "../review/judgment-agreement"; +import { persistDecisionReplayPrompt } from "../review/decision-replay"; import { createInstallationToken } from "../github/app"; import type { AgentActionMode } from "../settings/agent-execution"; import { buildAiReviewDiff } from "../review/review-diff"; @@ -927,6 +928,24 @@ export async function runAiReviewForAdvisory( // the REAL reviewer identities and the REAL system prompt into DecisionRecord instead of hardcoding null. const aiJudgmentModelIds = parsedReviewModelIds(result.reviewDiagnostics ?? []); const aiJudgmentPromptDigest = result.systemPromptDigest; + // #9028: capture the ACTUAL prompt text the digest above commits to, so the replay harness's re-query + // mode can re-run the model for this exact target. Written here (not at the decision-record persist) + // because the text must never travel through findings -- findings reach public render surfaces, and the + // prompt embeds the full diff. Keyed by the derivable base record id; an orphan row from a pass that + // never finalizes a decision ages out with the table's 30-day retention. Every verdict class is captured + // -- a CLEAN decision's action-match rate matters exactly as much as a defect's. + /* v8 ignore next -- the no-head arm is a plain skip, not a protection: without a SHA there is nothing to + * key the prompt row by, and a ghost PR's decision record is head-keyed too, so requery would have no row + * to join even if one were written. */ + if (args.advisory.headSha) { + await persistDecisionReplayPrompt(env, { + repoFullName: args.repoFullName, + pullNumber: args.pr.number, + headSha: args.advisory.headSha, + systemPrompt: result.systemPrompt, + userPrompt: result.userPrompt, + }); + } // #8834: inter-run agreement over the stances this review ALREADY produced (#8229's reviewerVotes) — // zero additional AI spend. Computed once and attached to whichever AI-judgment finding is built below, // so the decision record carries a per-decision confidence signal for the calibration set (#8835). diff --git a/src/review/decision-replay.ts b/src/review/decision-replay.ts index 65f91d0a0a..7936d9edfa 100644 --- a/src/review/decision-replay.ts +++ b/src/review/decision-replay.ts @@ -214,6 +214,50 @@ export async function persistDecisionReplayInputForGate( }); } +/** #9028: prompts above this size are SKIPPED, never truncated -- a truncated prompt re-queried against the + * model would report an action-match rate for a prompt that was never sent, which is worse than reporting + * nothing. The public promptDigest commitment is unaffected either way. Sized well under D1's row ceiling. */ +export const DECISION_REPLAY_PROMPT_MAX_CHARS = 900_000; + +/** + * #9028: persist the EXACT system prompt sent to the model, keyed by the BASE record id + * (`record:#@`) rather than a supersession's `:rev` id -- the prompt is a property of + * the (target, head, resolved-config) triple, not of which revision row happened to land, and upsert-last-wins + * means a superseding pass that rebuilt the prompt (config changed between passes) leaves the one that + * matches the LATEST decision. Private sibling of decision_replay_inputs with the same posture and a shorter + * (30-day) retention; the text must never reach the public record or any rendered surface. + * + * Best-effort like every persist in this family: prompt capture must never break the review pass that + * produced it. + */ +export async function persistDecisionReplayPrompt( + env: Env, + args: { repoFullName: string; pullNumber: number; headSha: string; systemPrompt: string; userPrompt: string }, +): Promise { + if (args.systemPrompt.length + args.userPrompt.length > DECISION_REPLAY_PROMPT_MAX_CHARS) { + console.warn( + JSON.stringify({ + event: "decision_replay_prompt_skipped_oversize", + repoFullName: args.repoFullName, + pullNumber: args.pullNumber, + chars: args.systemPrompt.length + args.userPrompt.length, + }), + ); + return; + } + const recordId = `record:${args.repoFullName}#${args.pullNumber}@${args.headSha}`.slice(0, 250); + try { + await env.DB.prepare( + `INSERT INTO decision_replay_prompts (record_id, prompt_json, created_at) VALUES (?, ?, ?) + ON CONFLICT(record_id) DO UPDATE SET prompt_json = excluded.prompt_json, created_at = excluded.created_at`, + ) + .bind(recordId, JSON.stringify({ systemPrompt: args.systemPrompt, userPrompt: args.userPrompt }), nowIso()) + .run(); + } catch (error) { + console.warn(JSON.stringify({ event: "decision_replay_prompt_persist_error", recordId: recordId.slice(0, 120), message: errorMessage(error).slice(0, 160) })); + } +} + export async function persistDecisionReplayInput(env: Env, recordId: string, input: DecisionReplayInput): Promise { try { await env.DB.prepare( diff --git a/src/review/decision-requery.ts b/src/review/decision-requery.ts new file mode 100644 index 0000000000..cde7c9bc29 --- /dev/null +++ b/src/review/decision-requery.ts @@ -0,0 +1,89 @@ +// #9028 (Replay v2): re-query action matching — the honest metric for the one irreducibly nondeterministic +// stage of a gate decision. +// +// Everything downstream of the model is a pure function pinned bit-exactly by the replay harness (#8838, +// including time via the #9028 clock capture). The model call itself is NOT: hosted inference is not +// bit-deterministic even at temperature 0 (batching, kernel scheduling, and silent model revisions all move +// tokens). So this mode NEVER reports "reproducibility" — it re-runs the model against the exact persisted +// prompt and reports an ACTION-MATCH RATE: across N fresh runs, how often did the model's verdict land in the +// same class that drove the recorded decision? +// +// Class, not text: two runs that phrase the same blocker differently are the same ACTION; a run that flips +// defect ⇄ clean is a different action regardless of phrasing. The class boundary is the exact one the live +// pipeline acts on (`ai_consensus_defect` / `ai_review_split` block; their absence does not). +import { parseModelReview } from "../services/ai-review"; +import type { DecisionReplayInput } from "./decision-replay"; + +/** The verdict classes the gate can ACT on. `unusable` is a run whose output the live parser would reject — + * it can never match an action, because the live pipeline routes it to fail-closed inconclusive handling + * rather than either verdict. */ +export type RequeryVerdictClass = "defect" | "clean" | "unusable"; + +/** The finding codes that carry an AI judgment into the gate — the same set the decision-record path keys on + * (`AI_JUDGMENT_BLOCKER_CODES` in processors.ts). Duplicated as a literal here so this module stays + * importable by the offline CLI without dragging the queue module graph along. */ +const AI_JUDGMENT_FINDING_CODES = new Set(["ai_consensus_defect", "ai_review_split"]); + +/** Classify one fresh model response through the SAME parser the live pipeline uses. */ +export function classifyModelResponse(text: string): RequeryVerdictClass { + const parsed = parseModelReview(text); + if (parsed === null) return "unusable"; + return parsed.blockers.length > 0 ? "defect" : "clean"; +} + +/** The class the RECORDED decision acted on, read from the persisted replay input's findings. */ +export function recordedJudgmentClass(replayInput: Pick): "defect" | "clean" { + return replayInput.findings.some((finding) => AI_JUDGMENT_FINDING_CODES.has(finding.code)) ? "defect" : "clean"; +} + +export interface RequeryReport { + mode: "requery"; + /** What this number is and is not, restated in the artifact itself so a pasted report cannot shed the + * caveat: hosted inference is not bit-deterministic even at temperature 0. */ + metric: "action-match-rate (NOT reproducibility)"; + recordedClass: "defect" | "clean"; + runs: number; + matches: number; + actionMatchRate: number; + perRun: RequeryVerdictClass[]; +} + +/** + * Re-run the model `runs` times against the exact persisted prompt and report the action-match rate. + * + * The model call is injected (`callModel`), so this stays a pure orchestration over an IO seam — the CLI + * binds it to a real provider client; tests bind it to a script. + */ +export async function runRequery(args: { + systemPrompt: string; + /** The user turn -- the diff/title/body. The decision inputs SPLIT across the two turns (the system prompt + * is rubric + config), so re-querying with the system prompt alone would ask the model to review nothing. */ + userPrompt: string; + runs: number; + recordedClass: "defect" | "clean"; + callModel: (systemPrompt: string, userPrompt: string) => Promise; +}): Promise { + const perRun: RequeryVerdictClass[] = []; + for (let i = 0; i < args.runs; i += 1) { + let text = ""; + try { + text = await args.callModel(args.systemPrompt, args.userPrompt); + } catch { + // A transport failure is an UNUSABLE run, not a skipped one: the operator asked for N runs, and + // silently shrinking the denominator would inflate the rate exactly when the provider is flakiest. + perRun.push("unusable"); + continue; + } + perRun.push(classifyModelResponse(text)); + } + const matches = perRun.filter((cls) => cls === args.recordedClass).length; + return { + mode: "requery", + metric: "action-match-rate (NOT reproducibility)", + recordedClass: args.recordedClass, + runs: args.runs, + matches, + actionMatchRate: args.runs === 0 ? 0 : Number((matches / args.runs).toFixed(3)), + perRun, + }; +} diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index 23ed78894a..2555b28865 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -462,6 +462,15 @@ export type LoopOverAiReviewResult = * caller commit to the real prompt sent instead of the base constant alone (a changed * `review.instructions` moves this digest). Always present on an "ok" result. */ systemPromptDigest: string; + /** #9028: the ACTUAL texts sent to the model, so the replay harness's re-query mode can re-run it for + * the same target. BOTH turns, because they split the decision inputs between them: the system prompt + * carries the rubric plus resolved config suffixes (what promptDigest commits to), while the USER turn + * carries the diff/title/body -- re-querying with the system prompt alone would ask the model to + * review nothing. PRIVATE-TABLE-BOUND: travels only as far as decision_replay_prompts (the + * operator-private sibling, 30-day retention) and must never reach the public record or any rendered + * surface. */ + systemPrompt: string; + userPrompt: string; }; /** A line-anchored review finding the model can emit for quiet inline PR comments (#inline-comments). `line` is @@ -3140,6 +3149,8 @@ export async function runLoopOverAiReview( valueAssessment, reviewDiagnostics, systemPromptDigest, + systemPrompt: system, + userPrompt: user, }; } diff --git a/test/unit/decision-requery.test.ts b/test/unit/decision-requery.test.ts new file mode 100644 index 0000000000..9eba0ab5f4 --- /dev/null +++ b/test/unit/decision-requery.test.ts @@ -0,0 +1,145 @@ +import { describe, expect, it } from "vitest"; +import { classifyModelResponse, recordedJudgmentClass, runRequery } from "../../src/review/decision-requery"; +import { parseRequeryBundle, requeryClientFromEnv } from "../../scripts/replay-decision"; +import { persistDecisionReplayPrompt, DECISION_REPLAY_PROMPT_MAX_CHARS } from "../../src/review/decision-replay"; +import { createTestEnv } from "../helpers/d1"; + +/** A model response in the exact JSON shape parseModelReview accepts — the live pipeline's own contract + * (blockers are plain strings, per its `typeof x === "string"` filter). */ +function modelResponse(blockers: string[]): string { + return JSON.stringify({ assessment: "Looked at the change.", blockers, nits: [], suggestions: [] }); +} + +// #9028 (Replay v2): everything downstream of the model is pinned bit-exactly by the replay harness — the +// model call is the ONE stage that cannot be, because hosted inference is not bit-deterministic even at +// temperature 0. The honest metric is the ACTION-MATCH RATE: across fresh runs, how often the model's verdict +// lands in the same CLASS the recorded decision acted on. These tests pin that the classes are the live +// pipeline's own boundaries, and that the denominator can never be quietly shrunk. +describe("decision re-query action matching (#9028)", () => { + it("classifies through the LIVE parser: blockers ⇒ defect, none ⇒ clean, unparseable ⇒ unusable", () => { + expect(classifyModelResponse(modelResponse(["Null deref: src/a.ts crashes on empty input"]))).toBe("defect"); + expect(classifyModelResponse(modelResponse([]))).toBe("clean"); + expect(classifyModelResponse("I am not JSON at all")).toBe("unusable"); + }); + + it("derives the recorded class from the SAME finding codes the gate acts on — both codes, and the clean arm", () => { + expect(recordedJudgmentClass({ findings: [{ code: "ai_consensus_defect" }] as never })).toBe("defect"); + expect(recordedJudgmentClass({ findings: [{ code: "ai_review_split" }] as never })).toBe("defect"); + expect(recordedJudgmentClass({ findings: [{ code: "missing_linked_issue" }] as never })).toBe("clean"); + expect(recordedJudgmentClass({ findings: [] as never })).toBe("clean"); + }); + + it("reports the match rate across runs, labeled as action-match and never reproducibility", async () => { + const responses = [ + modelResponse(["Bug: same class, different phrasing"]), + modelResponse(["Other bug: still a defect — phrasing must not matter"]), + modelResponse([]), + ]; + let call = 0; + const report = await runRequery({ + systemPrompt: "the exact persisted prompt", + userPrompt: "the exact persisted diff", + runs: 3, + recordedClass: "defect", + callModel: async () => responses[call++] ?? "", + }); + expect(report).toMatchObject({ mode: "requery", recordedClass: "defect", runs: 3, matches: 2, actionMatchRate: 0.667 }); + expect(report.perRun).toEqual(["defect", "defect", "clean"]); + // The caveat travels IN the artifact, so a pasted report cannot shed it. + expect(report.metric).toBe("action-match-rate (NOT reproducibility)"); + expect(JSON.stringify(report)).not.toMatch(/reproducib(?!ility\)")/); + }); + + it("INVARIANT: a transport failure is an UNUSABLE run in the denominator — never a silently skipped one", async () => { + // Shrinking the denominator would inflate the rate exactly when the provider is flakiest. + const report = await runRequery({ + systemPrompt: "p", + userPrompt: "u", + runs: 2, + recordedClass: "clean", + callModel: async (systemPrompt, userPrompt) => { + if (systemPrompt !== "p" || userPrompt !== "u") throw new Error("both turns must pass through verbatim"); + throw new Error("provider down"); + }, + }); + expect(report).toMatchObject({ runs: 2, matches: 0, actionMatchRate: 0, perRun: ["unusable", "unusable"] }); + }); + + it("INVARIANT: `unusable` matches NEITHER recorded class — the live pipeline routes it fail-closed, not to a verdict", async () => { + const report = await runRequery({ systemPrompt: "p", userPrompt: "u", runs: 1, recordedClass: "clean", callModel: async () => "garbage" }); + expect(report.matches).toBe(0); + }); + + it("zero runs yields a 0 rate rather than NaN (the degenerate-denominator arm)", async () => { + const report = await runRequery({ systemPrompt: "p", userPrompt: "u", runs: 0, recordedClass: "clean", callModel: async () => "" }); + expect(report.actionMatchRate).toBe(0); + }); +}); + +describe("requery bundle + provider parsing (CLI seams)", () => { + const goodBundle = JSON.stringify({ + replayInput: { findings: [{ code: "ai_consensus_defect" }] }, + prompt: { systemPrompt: "the exact prompt sent", userPrompt: "the exact diff sent" }, + }); + + it("accepts a bundle carrying the prompt and derives the recorded class", () => { + const parsed = parseRequeryBundle(goodBundle); + expect(parsed).toEqual({ systemPrompt: "the exact prompt sent", userPrompt: "the exact diff sent", recordedClass: "defect" }); + }); + + it("names each missing piece: bad JSON, absent prompt (with the 30-day retention hint), absent findings", () => { + expect(parseRequeryBundle("{nope")).toMatchObject({ error: expect.stringContaining("unparseable") }); + const noPrompt = parseRequeryBundle(JSON.stringify({ replayInput: { findings: [] } })); + expect(noPrompt).toMatchObject({ error: expect.stringContaining("30 days") }); + // BOTH turns are required: the system prompt alone would ask the model to review nothing. + const systemOnly = parseRequeryBundle(JSON.stringify({ replayInput: { findings: [] }, prompt: { systemPrompt: "p" } })); + expect(systemOnly).toMatchObject({ error: expect.stringContaining("userPrompt") }); + const noFindings = parseRequeryBundle(JSON.stringify({ prompt: { systemPrompt: "p", userPrompt: "u" } })); + expect(noFindings).toMatchObject({ error: expect.stringContaining("findings") }); + }); + + it("builds the provider from explicit env only — model always required, then base-url, then anthropic, else a named error", () => { + expect(requeryClientFromEnv({})).toMatchObject({ error: expect.stringContaining("REPLAY_AI_MODEL") }); + expect(requeryClientFromEnv({ REPLAY_AI_MODEL: "m" })).toMatchObject({ error: expect.stringContaining("REPLAY_AI_BASE_URL") }); + const openai = requeryClientFromEnv({ REPLAY_AI_MODEL: "m", REPLAY_AI_BASE_URL: "http://localhost:11434", REPLAY_AI_API_KEY: "k" }); + expect("ai" in openai && typeof openai.ai.run).toBe("function"); + const anthropic = requeryClientFromEnv({ REPLAY_AI_MODEL: "m", ANTHROPIC_API_KEY: "k" }); + expect("ai" in anthropic && typeof anthropic.ai.run).toBe("function"); + }); +}); + +describe("decision replay prompt capture (#9028)", () => { + it("persists the exact prompt keyed by the BASE record id, and upserts (last writer wins)", async () => { + const env = createTestEnv(); + await persistDecisionReplayPrompt(env, { repoFullName: "o/r", pullNumber: 7, headSha: "sha7", systemPrompt: "first", userPrompt: "diff-a" }); + await persistDecisionReplayPrompt(env, { repoFullName: "o/r", pullNumber: 7, headSha: "sha7", systemPrompt: "second", userPrompt: "diff-b" }); + const row = await env.DB.prepare("select record_id, prompt_json from decision_replay_prompts") + .first<{ record_id: string; prompt_json: string }>(); + expect(row?.record_id).toBe("record:o/r#7@sha7"); + expect(JSON.parse(row?.prompt_json ?? "{}")).toEqual({ systemPrompt: "second", userPrompt: "diff-b" }); + }); + + it("INVARIANT: an oversize prompt is SKIPPED, never truncated — a truncated prompt re-queried would report a rate for a prompt that was never sent", async () => { + const env = createTestEnv(); + await persistDecisionReplayPrompt(env, { + repoFullName: "o/r", + pullNumber: 8, + headSha: "sha8", + // COMBINED size is the cap -- the two turns land in one row. + systemPrompt: "x".repeat(DECISION_REPLAY_PROMPT_MAX_CHARS - 5), + userPrompt: "y".repeat(10), + }); + const row = await env.DB.prepare("select count(*) as n from decision_replay_prompts").first<{ n: number }>(); + expect(row?.n).toBe(0); + }); + + it("INVARIANT: a persist failure is swallowed — prompt capture must never break the review pass", async () => { + const env = createTestEnv(); + (env.DB as unknown as { prepare: () => never }).prepare = () => { + throw new Error("d1 down"); + }; + await expect( + persistDecisionReplayPrompt(env, { repoFullName: "o/r", pullNumber: 9, headSha: "sha9", systemPrompt: "p", userPrompt: "u" }), + ).resolves.toBeUndefined(); + }); +}); diff --git a/test/unit/pr-panel-retrigger-pending-force-review.test.ts b/test/unit/pr-panel-retrigger-pending-force-review.test.ts index 8b473bc57f..413d44400a 100644 --- a/test/unit/pr-panel-retrigger-pending-force-review.test.ts +++ b/test/unit/pr-panel-retrigger-pending-force-review.test.ts @@ -597,4 +597,56 @@ describe("PR-panel retrigger pending force-review marker (#7626)", () => { // non-matching "consumed" sentinel rather than leaving the original marker value behind. expect(cache.values.get(pendingKey as string)).toBe("consumed"); }); + + it("#9028: a pass that runs the AI persists the EXACT prompt, keyed by the base record id", async () => { + // The replay harness's --requery mode re-runs the model against decision_replay_prompts.prompt_json. + // Captured at the orchestration site (never through findings, which reach public render surfaces), keyed + // record:#@ so a supersession's :rev row still finds its head's prompt. + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: JSON.stringify({ assessment: "Fine.", blockers: [], nits: [], suggestions: [] }) }) } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRetriggerRepo(env); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 704, + title: "Prompt capture PR", + state: "open", + user: { login: "contributor" }, + author_association: "CONTRIBUTOR", + head: { sha: "shaPrompt704" }, + base: { ref: "main" }, + labels: [], + body: "Closes #1", + }); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); + if (url.includes("/pulls/704/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const marker9028 = true;" }]); + if (url.endsWith("/pulls/704")) return Response.json({ number: 704, title: "Prompt capture PR", state: "open", user: { login: "contributor" }, head: { sha: "shaPrompt704" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/shaPrompt704/check-runs")) return Response.json({ total_count: 1, check_runs: [{ name: "test", status: "completed", conclusion: "success", app: { slug: "github-actions" } }] }); + if (url.includes("/commits/shaPrompt704/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/issues/704/comments") && (method === "POST" || method === "PATCH")) return Response.json({ id: 1 }, { status: 201 }); + if (url.includes("/issues/704/comments")) return Response.json([]); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + + await reReviewStoredPullRequest(env, "prompt-capture-704", 123, "JSONbored/gittensory", 704); + + const row = await env.DB.prepare("select record_id, prompt_json from decision_replay_prompts where record_id = ?") + .bind("record:JSONbored/gittensory#704@shaPrompt704") + .first<{ record_id: string; prompt_json: string }>(); + expect(row).toBeTruthy(); + const prompt = JSON.parse(row?.prompt_json ?? "{}") as { systemPrompt: string; userPrompt: string }; + // BOTH turns, because the decision inputs split across them: the system prompt carries the rubric, and + // the USER turn carries this PR's actual diff -- which is exactly why the table is operator-private with + // 30-day retention, and why capturing the system prompt alone would make requery review nothing. + expect(prompt.systemPrompt).toContain("senior open-source maintainer"); + expect(prompt.userPrompt).toContain("marker9028"); + }); }); diff --git a/test/unit/retention.test.ts b/test/unit/retention.test.ts index fda8607286..941d6cccf1 100644 --- a/test/unit/retention.test.ts +++ b/test/unit/retention.test.ts @@ -106,6 +106,24 @@ describe("pruneExpiredRecords", () => { expect(remaining?.n).toBe(1); // one old row left for the next run }); + it("prunes decision_replay_prompts at its own SHORTER 30-day window, keyed by record_id (#9028)", async () => { + // Deliberately shorter than decision_replay_inputs' 180d: the prompt blob embeds the full diff (the + // largest, most sensitive artifact in the replay family), and the public promptDigest commitment + // outlives the text. Past the window, --requery honestly refuses; replay itself still works forever. + const env = createTestEnv(); + await env.DB.prepare( + `INSERT INTO decision_replay_prompts (record_id, prompt_json, created_at) VALUES + ('record:o/r#1@old', '{"systemPrompt":"aged out"}', ?), + ('record:o/r#2@new', '{"systemPrompt":"kept"}', ?)`, + ) + .bind(new Date(NOW - 31 * 86_400_000).toISOString(), new Date(NOW - 29 * 86_400_000).toISOString()) + .run(); + const results = await pruneExpiredRecords(env, { nowMs: NOW }); + expect(results.find((r) => r.table === "decision_replay_prompts")?.deleted).toBe(1); + const rows = await env.DB.prepare("SELECT record_id FROM decision_replay_prompts").all<{ record_id: string }>(); + expect((rows.results ?? []).map((r) => r.record_id)).toEqual(["record:o/r#2@new"]); + }); + it("rejects an unsafe table/column identifier (defensive guard)", async () => { const env = createTestEnv(); await expect(pruneExpiredRecords(env, { policy: [{ table: "webhook_events; DROP TABLE x", column: "received_at", days: 1 }] })).rejects.toThrow("Unsafe retention identifier");