Skip to content

Commit 4575968

Browse files
JSONboredJSONbored
andauthored
feat(orb): gate determinism — deterministic test-evidence facts, decision confidence, and replayable time (#8833, #8834, #9028) (#9256)
* fix(review): demote whole-PR test-absence blockers the path classifier contradicts (#8833) Whether a PR carries test-path evidence is a deterministic fact — signals/test-evidence.ts's isTestPath owns it, and slop.ts's missing_test_evidence finding already decides its severity. The reviewer prompt nonetheless fed the same fact as free text and let the model re-decide blocker-vs-nit, so a model could close a PR on a claim the engine had already answered the other way. Extends the existing parse-time fact-authority pattern (demoteCiClaimBlockers #8845, demoteEvidenceAbsenceBlockers #8961) to this criterion: a whole-PR "no tests were added" blocker is demoted to a nit when the classifier confirms the PR does change test paths. Fires only in the arm where the claim is provably a fact error — a genuine test-absence blocker on a genuinely test-free PR is untouched — and demotes rather than drops, so the observation still reaches the human. Coverage-DEPTH claims that narrow to a specific target ("no tests for the nullish branch") are deliberately excluded: the classifier cannot check those, so they remain the model's judgment to make and keep blocking. * feat(orb): capture the decision-time wall clock so staleness rules are replayable (#9028) gate.requireFreshRebaseWindow compared the base branch's tip against an inline Date.now() read inside maybeForceFreshRebase. Time is a decision INPUT, and nothing recorded which instant the comparison used — so re-deriving such a decision later could silently reach the opposite answer purely because the wall clock had moved, and report it as a match. The decision pass now takes ONE Date.now() reading, records it into decision_replay_inputs.replay_json as `clock`, and passes it to every clock-dependent rule instead of each calling the clock itself. Both staleness rules move to a pure, clock-injected module: isWithinFreshRebaseWindow takes the instant explicitly, and isBaseStaleByAheadBy is stated as the commit-count comparison it is — provably instant-independent, not merely assumed so. replayDecision gains a stage-0 `clock` check: replaying at the recorded instant (the CLI default) is bit-exact, while naming a different instant reports a `clock` divergence rather than silently certifying a re-derivation that never reproduced the original evaluation. Records written before this change carry no instant, so the stage is skipped rather than guessed. The CLI exposes it as `--at <epoch ms>`. Also corrects the decision_replay_inputs migration reference in two doc comments (0181 is alert_dedup_claims; the table is created in 0182). * feat(orb): record a per-decision confidence signal from inter-run agreement (#8834) Verbalized confidence alone is poorly calibrated — #8845 already had to stop reading an absent confidence as certainty. Sampling-based consistency is the better-behaved signal, and the risk-control literature this epic builds on finds two samples capture most of the benefit. Scores inter-run agreement across the reviewer stances the engine ALREADY produces (#8229's reviewerVotes) and folds it into the verbalized confidence, at zero additional AI spend. The combined score multiplies the two so it is monotonically below either input — a judgment is only as trustworthy as both how sure the judge said it was and how reproducibly the judges reached it, which is the property an abstention threshold depends on. A lone run is recorded as UNCORROBORATED at a 0.5 agreement floor rather than fabricated unanimity, so a single-reviewer or budget-degraded review records a strictly lower confidence than a genuinely corroborated one. Zero samples refuses to invent a score at all. The signal rides to DecisionRecord.aiAgreement (schema v5) so every decision joins the risk-control calibration set with its reproducibility attached. Deliberately ADDITIVE: it does not re-route the gate. Disagreement already routes to a hold today — differing stances ARE the ai_review_split finding, which blocks or holds via the existing confidence floor — so a second parallel route would double-count the same evidence instead of measuring it. * test(orb): update decision-record schema assertions for the v5 agreement field (#8834) The DECISION_RECORD_SCHEMA_VERSION bump to v5 moves the pinned version assertions in the backfill bundle and queue decision-record suites. The #9124 record test additionally carries the agreement on its cached finding and asserts it threads through to the record, mirroring exactly how that test already proves modelIds/promptDigest pass through rather than being re-derived. --------- Co-authored-by: JSONbored <airdroptopian@gmail.com>
1 parent b09cd9c commit 4575968

16 files changed

Lines changed: 544 additions & 22 deletions

scripts/replay-decision.ts

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44
//
55
// node --experimental-strip-types scripts/replay-decision.ts <bundle.json>
66
// ... | node --experimental-strip-types scripts/replay-decision.ts -
7+
// node --experimental-strip-types scripts/replay-decision.ts <bundle.json> --at <epoch ms>
8+
//
9+
// #9028: `--at` names the instant to replay AT. Omit it to replay at the instant the decision itself recorded
10+
// (`replayInput.clock.nowMs`) — the bit-exact case. Passing a DIFFERENT instant exits 1 with a `clock`
11+
// divergence rather than reporting a match: time is a decision INPUT, and a clock-dependent rule
12+
// (`gate.requireFreshRebaseWindow`) can flip purely because the wall clock moved.
713
//
814
// The bundle is one JSON object: { record: {...decision_records row}, replayInput: {...replay_json} }.
915
// EXTRACT (operator, against the instance DB):
@@ -24,8 +30,13 @@
2430
import { readFileSync } from "node:fs";
2531
import { replayDecision, type DecisionReplayInput, type ReplayableRecord } from "../src/review/decision-replay";
2632

27-
/** Parse + normalize a bundle (snake_case SQL rows accepted) and replay it. Exported for tests. */
28-
export function runReplayBundle(raw: string): { outcome: ReturnType<typeof replayDecision> | null; error?: string } {
33+
/** Parse + normalize a bundle (snake_case SQL rows accepted) and replay it. Exported for tests.
34+
*
35+
* #9028: `atMs` names the instant to replay AT. Omitted (the default) replays at the instant the decision
36+
* recorded, which is the bit-exact case. Supplying a DIFFERENT instant is reported as a `clock` divergence,
37+
* never silently accepted — a clock-dependent rule can legitimately flip its answer as the wall clock moves,
38+
* so "it still matches at a different instant" is not a re-derivation of the original decision. */
39+
export function runReplayBundle(raw: string, atMs?: number): { outcome: ReturnType<typeof replayDecision> | null; error?: string } {
2940
let bundle: { record?: Record<string, unknown>; replayInput?: unknown };
3041
try {
3142
bundle = JSON.parse(raw) as never;
@@ -47,18 +58,25 @@ export function runReplayBundle(raw: string): { outcome: ReturnType<typeof repla
4758
if (!record || !replayInput || !Array.isArray(replayInput.findings) || typeof replayInput.evaluated !== "object") {
4859
return { outcome: null, error: "bundle must carry {record: {id, reason_code|reasonCode, action}, replayInput: {findings, policy, evaluated}}" };
4960
}
50-
return { outcome: replayDecision(record, replayInput) };
61+
return { outcome: replayDecision(record, replayInput, atMs === undefined ? {} : { nowMs: atMs }) };
5162
}
5263

5364
const invokedDirectly = process.argv[1]?.endsWith("replay-decision.ts") === true;
5465
if (invokedDirectly) {
55-
const source = process.argv[2];
66+
const argv = process.argv.slice(2);
67+
const atIndex = argv.indexOf("--at");
68+
const atRaw = atIndex === -1 ? undefined : argv[atIndex + 1];
69+
if (atIndex !== -1 && (atRaw === undefined || !Number.isFinite(Number(atRaw)))) {
70+
console.error("replay-decision: --at requires a Unix-epoch-milliseconds value");
71+
process.exit(2);
72+
}
73+
const source = argv.filter((arg, index) => index !== atIndex && index !== atIndex + 1)[0];
5674
if (!source) {
57-
console.error("usage: replay-decision.ts <bundle.json | ->");
75+
console.error("usage: replay-decision.ts <bundle.json | -> [--at <epoch ms>]");
5876
process.exit(2);
5977
}
6078
const raw = source === "-" ? readFileSync(0, "utf8") : readFileSync(source, "utf8");
61-
const { outcome, error } = runReplayBundle(raw);
79+
const { outcome, error } = runReplayBundle(raw, atRaw === undefined ? undefined : Number(atRaw));
6280
if (!outcome) {
6381
console.error(`replay-decision: ${error}`);
6482
process.exit(2);

src/queue/ai-review-orchestration.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { buildPullRequestAdvisory } from "../rules/advisory";
2323
import { recordAuditEvent, getDecryptedRepositoryAiKey, getRepository, listCheckSummaries, listPullRequestFiles } from "../db/repositories";
2424
import { registerHeldLock, unregisterHeldLock } from "./held-lock-registry";
2525
import { recordRoutingShadow } from "../services/reviewer-routing";
26+
import { scoreJudgmentAgreement } from "../review/judgment-agreement";
2627
import { createInstallationToken } from "../github/app";
2728
import type { AgentActionMode } from "../settings/agent-execution";
2829
import { buildAiReviewDiff } from "../review/review-diff";
@@ -889,6 +890,10 @@ export async function runAiReviewForAdvisory(
889890
// the REAL reviewer identities and the REAL system prompt into DecisionRecord instead of hardcoding null.
890891
const aiJudgmentModelIds = parsedReviewModelIds(result.reviewDiagnostics ?? []);
891892
const aiJudgmentPromptDigest = result.systemPromptDigest;
893+
// #8834: inter-run agreement over the stances this review ALREADY produced (#8229's reviewerVotes) —
894+
// zero additional AI spend. Computed once and attached to whichever AI-judgment finding is built below,
895+
// so the decision record carries a per-decision confidence signal for the calibration set (#8835).
896+
const aiJudgmentAgreement = (verbalizedConfidence: number) => scoreJudgmentAgreement(result.reviewerVotes, verbalizedConfidence);
892897
if (result.consensusDefect) {
893898
findings.push({
894899
code: "ai_consensus_defect",
@@ -913,6 +918,7 @@ export async function runAiReviewForAdvisory(
913918
confidence: result.consensusDefect.confidence,
914919
modelIds: aiJudgmentModelIds,
915920
promptDigest: aiJudgmentPromptDigest,
921+
agreement: aiJudgmentAgreement(result.consensusDefect.confidence),
916922
});
917923
} else if (result.split) {
918924
// The reviewers DISAGREED — exactly one flagged a blocking defect. reviewbot's quorum treats any reviewer
@@ -940,6 +946,10 @@ export async function runAiReviewForAdvisory(
940946
: {}),
941947
modelIds: aiJudgmentModelIds,
942948
promptDigest: aiJudgmentPromptDigest,
949+
// A split IS the disagreement case: the stances differ, so agreement scores strictly below unanimity
950+
// and the recorded confidence falls with it. #8834's "disagreement routes to hold" is already this
951+
// finding's existing behavior via the confidence floor; this measures it rather than re-routing it.
952+
agreement: aiJudgmentAgreement(result.splitConfidence ?? 1),
943953
});
944954
} else if (result.inconclusive) {
945955
// Fail-CLOSED (#ai-fail-closed): block-mode AI could not return a usable verdict. Hold the PR for a human

src/queue/processors.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,7 @@ import {
663663
import { AI_JUDGMENT_BLOCKER_CODES } from "../rules/advisory";
664664
import { computeSalvageabilityForTarget } from "../review/salvageability-wire";
665665
import { deriveDecisionReasonCode, persistDecisionReplayInputForGate } from "../review/decision-replay";
666+
import { isWithinFreshRebaseWindow, type DecisionClockCapture } from "../review/staleness-clock";
666667
import { recordVerdictFlip } from "../review/verdict-flip-store";
667668
import { resolveAutomaticCloseConfidence } from "../review/risk-control-wire";
668669
import { maybeApplyCloseAuditHoldout } from "../review/close-audit-holdout";
@@ -3175,6 +3176,12 @@ async function runAgentMaintenancePlanAndExecute(
31753176
},
31763177
): Promise<void> {
31773178
const { installationId, repoFullName, pr, settings, otherOpenPullRequests, deliveryId, gate } = args;
3179+
// #9028: ONE wall-clock read for this whole decision pass, recorded into the replay input below and passed
3180+
// to every clock-dependent gate rule (today `gate.requireFreshRebaseWindow` via maybeForceFreshRebase)
3181+
// instead of each of them calling Date.now() independently. Two rules reading the clock at two different
3182+
// moments cannot both be replayed from one recorded instant — and an unrecorded instant is an unrecorded
3183+
// decision INPUT, which is exactly what makes a time-dependent decision unreplayable.
3184+
const decisionClock: DecisionClockCapture = { nowMs: Date.now() };
31783185

31793186
// Convergence safety: feed the planner the PR's changed paths + the repo's hard-guardrail globs so guarded
31803187
// paths force manual review, and flag owner-authored PRs so they are never auto-closed (standing rule).
@@ -3756,18 +3763,22 @@ async function runAgentMaintenancePlanAndExecute(
37563763
modelIds: aiJudgment?.modelIds ?? null,
37573764
promptDigest: aiJudgment?.promptDigest ?? null,
37583765
aiConfidence: aiJudgment?.confidence ?? null,
3766+
// #8834: the inter-run agreement signal computed at review time (ai-review-orchestration.ts) and
3767+
// carried on the finding, so the record captures HOW REPRODUCIBLY the judgment was reached, not just
3768+
// what the model claimed. null for a rule-only decision, exactly like the fields above.
3769+
aiAgreement: aiJudgment?.agreement ?? null,
37593770
salvageability,
37603771
// #9135: legible on the record's own face — see maybeApplyCloseAuditHoldout's doc comment.
37613772
divertedByHoldout: closeAuditHoldout?.diverted ?? false,
37623773
});
37633774
const recordId = await persistDecisionRecord(env, record, recordDigest);
3764-
// #8838: persist the evaluation's own exact inputs beside the record (PRIVATE sibling, migration 0181)
3775+
// #8838: persist the evaluation's own exact inputs beside the record (PRIVATE sibling, migration 0182)
37653776
// so the replay harness can re-derive this decision bit-exactly. Best-effort, like the record itself;
37663777
// the no-replay no-op (synthetic content-lane/bridge evaluations) lives inside the helper. Keyed to the
37673778
// id persistDecisionRecord actually wrote (#9123: a supersession at the same head gets a revisioned id,
37683779
// not the base one this used to always recompute independently). #9135: the holdout outcome rides along
37693780
// so `holdout_consistency` has something to check the public record against.
3770-
if (recordId !== null) await persistDecisionReplayInputForGate(env, recordId, gate, policyCloseKind ?? null, closeAuditHoldout ?? null);
3781+
if (recordId !== null) await persistDecisionReplayInputForGate(env, recordId, gate, policyCloseKind ?? null, closeAuditHoldout ?? null, decisionClock);
37713782
}
37723783
// #2349 (PR 1): additive per-contributor calibration data, gated identically to recordNativeGateDecision
37733784
// above -- see src/review/contributor-calibration.ts's doc comment. Currently write-only; nothing reads
@@ -3897,6 +3908,7 @@ async function runAgentMaintenancePlanAndExecute(
38973908
token,
38983909
admissionKey,
38993910
deliveryId,
3911+
nowMs: decisionClock.nowMs,
39003912
}))
39013913
) {
39023914
return;
@@ -4806,17 +4818,20 @@ async function maybeForceFreshRebase(
48064818
token: string | undefined;
48074819
admissionKey: GitHubRateLimitAdmissionKey | undefined;
48084820
deliveryId: string;
4821+
// #9028: the decision pass's single captured instant — this rule reads it instead of the clock, so the
4822+
// window comparison is replayable from `decision_replay_inputs.replay_json`.
4823+
nowMs: number;
48094824
},
48104825
): Promise<boolean> {
4811-
const { installationId, repoFullName, pr, settings, windowMinutes, baseRef, token, admissionKey, deliveryId } = args;
4826+
const { installationId, repoFullName, pr, settings, windowMinutes, baseRef, token, admissionKey, deliveryId, nowMs } = args;
48124827
/* v8 ignore next -- structurally unreachable: the caller only invokes this after confirming
48134828
* (liveMergeState ?? pr.mergeableState) === "clean", which GitHub can never compute for a PR with no
48144829
* head commit; the null check is belt-and-suspenders against the field's optional TS type. */
48154830
if (!pr.headSha) return false;
48164831
const advancedAt = await fetchLiveBaseBranchAdvancedAt(env, repoFullName, baseRef, token, admissionKey);
48174832
if (!advancedAt) return false; // fail-open: unreadable base commit -> no forced rebase
48184833
const advancedAtMs = Date.parse(advancedAt);
4819-
if (!Number.isFinite(advancedAtMs) || Date.now() - advancedAtMs >= windowMinutes * 60_000) return false;
4834+
if (!isWithinFreshRebaseWindow({ baseAdvancedAtMs: advancedAtMs, windowMinutes, nowMs })) return false;
48204835

48214836
const countKey = freshRebaseForceCountKey(repoFullName, pr.number);
48224837
const storedCount = Number(await getTransientKey(env, countKey));

src/review/decision-record.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
import { errorMessage, nowIso } from "../utils/json";
4040

4141
/** Bump when the record's FIELD SET changes meaning — consumers compare records only within a version. */
42-
export const DECISION_RECORD_SCHEMA_VERSION = "4"; // v4 (#9124/#9135): configDigest digests the resolved policy (+ settingsDigest split out), promptDigest digests the actual sent prompt, modelId -> modelIds (real identities), ciState populated, + divertedByHoldout; v3 (#8962): + salvageability {score, factors}; v2 (#8834): + aiConfidence, model/prompt commitments
42+
export const DECISION_RECORD_SCHEMA_VERSION = "5"; // v5 (#8834): + aiAgreement (inter-run agreement folded with the verbalized confidence); v4 (#9124/#9135): configDigest digests the resolved policy (+ settingsDigest split out), promptDigest digests the actual sent prompt, modelId -> modelIds (real identities), ciState populated, + divertedByHoldout; v3 (#8962): + salvageability {score, factors}; v2 (#8834): + aiConfidence, model/prompt commitments
4343

4444
/**
4545
* Canonical JSON: recursively key-sorted, no insignificant whitespace — the ONE serialization every digest
@@ -118,6 +118,12 @@ export type DecisionRecord = {
118118
* defect / split), null when no AI judgment contributed. Persisted so every decision joins the
119119
* risk-control calibration set (#8835) with its confidence attached. */
120120
aiConfidence: number | null;
121+
/** #8834: the per-decision confidence signal — inter-run agreement across the reviewer stances that
122+
* produced the AI judgment, folded together with that judgment's verbalized confidence (see
123+
* src/review/judgment-agreement.ts). `aiConfidence` above records what the model SAID; this records how
124+
* reproducibly the reviewers reached it, which is the input a calibrated abstention threshold (#8835)
125+
* needs. null when no AI judgment contributed, and for every record predating v5. */
126+
aiAgreement: { agreement: number; confidence: number; sampleCount: number; uncorroborated: boolean } | null;
121127
/** #8962: the deterministic salvageability score + its named factors when an AI judgment shaped the
122128
* decision — the second-axis evidence for auditing the close/hold boundary. null for rule-only decisions
123129
* (and for reconstructed/backfilled records predating v3). */
@@ -135,12 +141,13 @@ export type DecisionRecord = {
135141
/** Assemble the record and its own content digest. PURE given pre-computed digests. Normalizes the
136142
* optional-shaped caller fields (undefined -> null) HERE so call sites carry no fallback arms of their own. */
137143
export async function buildDecisionRecord(
138-
input: Omit<DecisionRecord, "schemaVersion" | "decidedAt" | "gatePack" | "ciState" | "baseSha" | "aiConfidence" | "salvageability" | "settingsDigest" | "divertedByHoldout"> & {
144+
input: Omit<DecisionRecord, "schemaVersion" | "decidedAt" | "gatePack" | "ciState" | "baseSha" | "aiConfidence" | "aiAgreement" | "salvageability" | "settingsDigest" | "divertedByHoldout"> & {
139145
decidedAt?: string;
140146
gatePack?: string | null | undefined;
141147
ciState?: string | null | undefined;
142148
baseSha?: string | null | undefined;
143149
aiConfidence?: number | null | undefined;
150+
aiAgreement?: { agreement: number; confidence: number; sampleCount: number; uncorroborated: boolean } | null | undefined;
144151
salvageability?: { score: number; factors: string[] } | null | undefined;
145152
settingsDigest?: string | null | undefined;
146153
divertedByHoldout?: boolean | undefined;
@@ -154,6 +161,7 @@ export async function buildDecisionRecord(
154161
ciState: input.ciState ?? null,
155162
baseSha: input.baseSha ?? null,
156163
aiConfidence: input.aiConfidence ?? null,
164+
aiAgreement: input.aiAgreement ?? null,
157165
salvageability: input.salvageability ?? null,
158166
settingsDigest: input.settingsDigest ?? null,
159167
divertedByHoldout: input.divertedByHoldout ?? false,

0 commit comments

Comments
 (0)