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
43 changes: 43 additions & 0 deletions packages/loopover-engine/src/calibration/backtest-threshold.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Threshold-only backtest (#8138) -- the honestly-backtestable slice of a confidence-floor change today:
// BacktestCase (#8083) stores outcome + metadata (e.g. { confidence }) but never the raw diff/issue content
// a rule evaluated, so a classifier can only re-simulate a decision using what's actually IN the corpus.
// Comparing a stored metadata.confidence against a NEW threshold value needs no raw content -- this is the
// one case the corpus already fully supports. Logic/regex-change backtesting needs raw context (#8129/#8130)
// and lives in a separate module once that lands.

import { compareBacktestScores, type BacktestComparison } from "./backtest-compare.js";
import type { BacktestCase } from "./backtest-corpus.js";
import { scoreBacktest } from "./backtest-score.js";

/**
* Build a classify function that predicts `"reversed"` (the rule's original firing was wrong) when a
* case's stored `metadata.confidence` is below `threshold`, `"confirmed"` otherwise. A case with no
* numeric `metadata.confidence` degrades to confidence `1` -- mirrors this codebase's own established
* "absent/unparseable confidence degrades to 1.0" fallback (see `LinkedIssueSatisfactionResult`'s own
* confidence handling), so an unscored case is never predicted `"reversed"` by default.
*/
export function buildConfidenceThresholdClassifier(threshold: number): (backtestCase: BacktestCase) => "reversed" | "confirmed" {
return (backtestCase) => {
const confidence = typeof backtestCase.metadata?.confidence === "number" ? backtestCase.metadata.confidence : 1;
return confidence < threshold ? "reversed" : "confirmed";
};
}

/**
* Backtest a proposed change to a single confidence threshold: score the OLD and NEW threshold values as
* two classifiers over the same corpus (`scoreBacktest`, #8085), then compare them with the Pareto-floor
* discipline (`compareBacktestScores`, #8086) -- a regression on either precision or recall wins, even if
* the other axis improved. Pure; the corpus is the caller's responsibility to fetch/filter to one `ruleId`
* first (the `cases` array may safely contain other ruleIds too -- both `scoreBacktest` calls filter on
* `ruleId` internally).
*/
export function runThresholdBacktest(
ruleId: string,
cases: readonly BacktestCase[],
oldThreshold: number,
newThreshold: number,
): BacktestComparison {
const baseline = scoreBacktest(ruleId, cases, buildConfidenceThresholdClassifier(oldThreshold));
const candidate = scoreBacktest(ruleId, cases, buildConfidenceThresholdClassifier(newThreshold));
return compareBacktestScores(baseline, candidate);
}
6 changes: 6 additions & 0 deletions packages/loopover-engine/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,12 @@ export * from "./calibration/backtest-corpus.js";
export * from "./calibration/backtest-score.js";
export * from "./calibration/backtest-compare.js";
export * from "./calibration/backtest-report.js";
// #8087 shipped this file but never added its barrel export -- every existing consumer happened to import
// it via the direct relative source path instead, so this was latent rather than broken. Fixing it here
// since #8138 is the first consumer that actually needs the package-name import (@loopover/engine), the
// same way scripts/backtest-corpus-export.ts already imports BacktestCase.
export * from "./calibration/backtest-split.js";
export * from "./calibration/backtest-threshold.js";
export {
GOVERNOR_LEDGER_EVENT_TYPES,
normalizeGovernorLedgerEvent,
Expand Down
61 changes: 61 additions & 0 deletions packages/loopover-engine/test/backtest-threshold.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import assert from "node:assert/strict";
import { test } from "node:test";

import { buildConfidenceThresholdClassifier, runThresholdBacktest, type BacktestCase } from "../dist/index.js";

function corpusCase(targetKey: string, label: BacktestCase["label"], confidence?: number): BacktestCase {
return {
ruleId: "linked_issue_scope_mismatch",
targetKey,
outcome: "unaddressed",
label,
firedAt: "2026-07-22T00:00:00.000Z",
decidedAt: "2026-07-22T01:00:00.000Z",
...(confidence !== undefined ? { metadata: { confidence } } : {}),
};
}

test("barrel: the public entrypoint re-exports the threshold-backtest primitives (#8138)", () => {
assert.equal(typeof buildConfidenceThresholdClassifier, "function");
assert.equal(typeof runThresholdBacktest, "function");
});

test("buildConfidenceThresholdClassifier: below-threshold confidence predicts reversed", () => {
const classify = buildConfidenceThresholdClassifier(0.5);
assert.equal(classify(corpusCase("a#1", "reversed", 0.3)), "reversed");
});

test("buildConfidenceThresholdClassifier: at-or-above-threshold confidence predicts confirmed", () => {
const classify = buildConfidenceThresholdClassifier(0.5);
assert.equal(classify(corpusCase("a#1", "confirmed", 0.5)), "confirmed");
assert.equal(classify(corpusCase("a#2", "confirmed", 1)), "confirmed");
});

test("buildConfidenceThresholdClassifier: missing confidence degrades to 1, never predicted reversed by default", () => {
const classify = buildConfidenceThresholdClassifier(0.99);
assert.equal(classify(corpusCase("a#1", "confirmed")), "confirmed");
});

test("runThresholdBacktest: an empty corpus reports unchanged with null precision/recall on both sides", () => {
const comparison = runThresholdBacktest("linked_issue_scope_mismatch", [], 0.5, 0.3);
assert.equal(comparison.verdict, "unchanged");
assert.equal(comparison.baseline.precision, null);
assert.equal(comparison.candidate.precision, null);
});

test("runThresholdBacktest: only scores cases matching the given ruleId", () => {
const cases: BacktestCase[] = [
{ ...corpusCase("a#1", "reversed", 0.1), ruleId: "other_rule" },
corpusCase("a#2", "confirmed", 0.9),
];
const comparison = runThresholdBacktest("linked_issue_scope_mismatch", cases, 0.5, 0.3);
assert.equal(comparison.baseline.caseCount, 1);
assert.equal(comparison.candidate.caseCount, 1);
});

test("runThresholdBacktest: a lower floor that flips a correct reversed prediction to a missed one regresses recall", () => {
const cases: BacktestCase[] = [corpusCase("a#1", "reversed", 0.35)];
const comparison = runThresholdBacktest("linked_issue_scope_mismatch", cases, 0.5, 0.2);
assert.equal(comparison.verdict, "regressed");
assert.ok(comparison.regressedAxes.includes("recall"));
});
45 changes: 45 additions & 0 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,8 @@ import { copycatWouldActOnPersistedScore } from "../signals/copycat";
import { buildStructuralImprovementAssessment } from "../signals/improvement";
import { runLoopOverLinkedIssueSatisfaction } from "../services/linked-issue-satisfaction-run";
import { MAX_BODY_CHARS, MAX_DIFF_CHARS, MAX_ISSUE_TEXT_CHARS } from "../services/linked-issue-satisfaction";
import { persistThresholdBacktestRuns, runThresholdBacktestAdvisory } from "../services/threshold-backtest-run";
import { thresholdBacktestBlock } from "../services/threshold-backtest";
import { decidePublicSurface } from "../signals/settings-preview";
import {
buildFocusManifestGuidance,
Expand Down Expand Up @@ -7377,6 +7379,41 @@ export async function maybeAddScreenshotTableAdvisoryFinding(
}
}

const THRESHOLD_BACKTEST_WATCHED_PATHS = new Set(["src/services/linked-issue-satisfaction.ts", "src/rules/advisory.ts"]);

/**
* Threshold-only backtest advisory (#8138, epic #8082) — independent of any gate mode setting; this checks
* whether the PR's OWN diff changes a known confidence-threshold constant, not anything about a linked
* issue or AI review. Returns the rendered Markdown block for the comment's "Threshold backtest" section
* (via `thresholdBacktestBlock`), or `""` when nothing applies — matching every other advisory resolver in
* this file's "empty/absent means no section" contract.
*
* Cheap to skip: a path check against `files` before paying for `buildSecretScanDiff`'s full, UNBUDGETED
* string build (`detectChangedThresholds` needs every hunk, unlike `buildAiReviewDiff`'s bounded view — see
* that function's own doc comment for why the two diff builders aren't interchangeable here). Fail-safe,
* like `runLinkedIssueSatisfactionForAdvisory` below: any error is swallowed (logged, not thrown) so the
* gate/review pass this is called from always finalizes regardless. `runThresholdBacktestAdvisory`'s own
* SignalStore read and `persistThresholdBacktestRuns`'s own write already fail open internally; this is
* defense-in-depth against anything else in the chain (e.g. a malformed diff) throwing.
*/
export async function resolveThresholdBacktestAdvisory(
env: Env,
repoFullName: string,
pr: { number: number },
files: Awaited<ReturnType<typeof listPullRequestFiles>>,
): Promise<string> {
if (!files.some((file) => THRESHOLD_BACKTEST_WATCHED_PATHS.has(file.path))) return "";
try {
const { changed, comparisons } = await runThresholdBacktestAdvisory(env, buildSecretScanDiff(files));
if (comparisons.length === 0) return "";
await persistThresholdBacktestRuns(env, repoFullName, pr.number, changed, comparisons);
return thresholdBacktestBlock(comparisons);
} catch (error) {
console.error(JSON.stringify({ level: "error", event: "threshold_backtest_failed", repoFullName, pullNumber: pr.number, error: errorMessage(error) }));
return "";
}
}

/**
* Run the linked-issue satisfaction assessment for advisory purposes (#1961/#3906) — opt-in via
* `linkedIssueSatisfactionGateMode != "off"`. Assesses only the PR's PRIMARY (first) linked issue: v1 chooses
Expand Down Expand Up @@ -8896,6 +8933,10 @@ async function maybePublishPrPublicSurface(
// unified comment. Declared undefined/null-equivalent by default so an unopted-in repo (linkedIssueSatisfactionGateMode:
// "off", the default) or a caught error never threads a section into the comment.
let linkedIssueSatisfaction: { status: "addressed" | "partial" | "unaddressed"; rationale: string } | null = null;
// Threshold-only backtest advisory (#8138, epic #8082), same hoisting reason as linkedIssueSatisfaction
// above. Empty string (not computed, or nothing to report) never threads a section into the comment --
// see UnifiedReviewInput.thresholdBacktest's own doc comment for the "no section when empty" contract.
let thresholdBacktest = "";
// inlineFindings is present ONLY on a FRESH review (cache miss) with inline comments enabled; the AI cache
// round-trips notes + reviewerCount + the gate findings (so a cache hit replays consensus/split/inconclusive
// blockers — see below), but NOT inlineFindings, so a cache hit never re-posts inline comments (#inline-comments).
Expand Down Expand Up @@ -9439,6 +9480,9 @@ async function maybePublishPrPublicSurface(
});
}
}
// Threshold-only backtest advisory (#8138, epic #8082): independent of linkedIssueSatisfactionGateMode
// above -- see resolveThresholdBacktestAdvisory's own doc comment for why.
thresholdBacktest = await resolveThresholdBacktestAdvisory(env, repoFullName, pr, await getReviewFiles());
// Content-lane linked-issue deliverable check (#content-lane-deliverable, opt-in via
// contentLaneDeliverableGateMode). Independent gate mode from the AI-based satisfaction assessment above --
// `off` (default) short-circuits before any fetch, so this is byte-identical to before this feature existed
Expand Down Expand Up @@ -11173,6 +11217,7 @@ async function maybePublishPrPublicSurface(
...(aiReview !== undefined ? { aiReview } : {}),
advisoryFindings: advisory.findings,
...(linkedIssueSatisfaction !== null ? { linkedIssueSatisfaction } : {}),
...(thresholdBacktest ? { thresholdBacktest } : {}),
// review.auto_merge_summary (#2051/#4147): deterministic, no-AI — reuses the SAME ciState/
// mergeStateLabel/gate/linkedIssues facts this pass already resolved for mergeReadiness and the gate
// verdict above, no extra fetch. gatePassing mirrors the gate's own "no hard blocker" definition
Expand Down
7 changes: 7 additions & 0 deletions src/review/unified-comment-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,12 @@ export type UnifiedCommentBridgeArgs = {
* applies. Absent (default; the processor only resolves this when linkedIssueSatisfactionGateMode !=
* "off") ⇒ no section is rendered, byte-identical to today. */
linkedIssueSatisfaction?: { status: "addressed" | "partial" | "unaddressed"; rationale: string } | undefined;
/** Threshold-only backtest advisory (#8138), already rendered to Markdown by the processor
* (thresholdBacktestBlock, src/services/threshold-backtest.ts) — passed straight through to
* buildUnifiedReviewInput's field of the same name. Presentation only. Absent/empty (default; the
* processor only resolves this when a PR's diff changes a known confidence threshold constant) ⇒ no
* section is rendered, byte-identical to today. */
thresholdBacktest?: string | undefined;
};

/**
Expand Down Expand Up @@ -886,6 +892,7 @@ export function buildUnifiedCommentBody(args: UnifiedCommentBridgeArgs): string
...(args.maxFindingsCaps !== undefined ? { maxFindingsCaps: args.maxFindingsCaps } : {}),
...(args.findingCategories !== undefined ? { inlineFindings: args.findingCategories } : {}),
...(args.linkedIssueSatisfaction !== undefined ? { linkedIssueSatisfaction: args.linkedIssueSatisfaction } : {}),
...(args.thresholdBacktest !== undefined ? { thresholdBacktest: args.thresholdBacktest } : {}),
...(blockerFixHandoffBlocks.length > 0 ? { blockerFixContext: blockerFixHandoffBlocks } : {}),
});
// The gate already produced 0/1 reviewer notes from a synthesis of the model pair; reflect the caller's
Expand Down
18 changes: 18 additions & 0 deletions src/review/unified-comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,14 @@ export interface UnifiedReviewInput {
* (default; the host only resolves this when `review.linkedIssueSatisfaction` is on) ⇒ no section is
* rendered, byte-identical to today. */
linkedIssueSatisfaction?: { status: "addressed" | "partial" | "unaddressed"; rationale: string };
/** Threshold-only backtest advisory (#8138, epic #8082): already-rendered Markdown (via
* `thresholdBacktestBlock`/`renderBacktestComparison`, `src/services/threshold-backtest.ts` /
* `@loopover/engine`) reporting whether a PR's changed confidence threshold regresses precision/recall
* against real historical outcomes. A STRING, not structured data — this module is a self-contained port
* with no imports (see the file header), so rendering happens in the caller, not here. PRESENTATION
* ONLY — never changes `status`/the gate verdict. Absent/empty (default; only set when the host detected
* a changed known threshold constant) ⇒ no section is rendered, byte-identical to today. */
thresholdBacktest?: string;
/** The review's line-anchored inline findings (only their `category` is read), used ONLY to render a compact
* category-tally line (#2150). Structural shape so an `InlineFinding[]` is assignable without importing it —
* this renderer stays self-contained. Absent/empty (default; the host passes them only when it produced
Expand Down Expand Up @@ -819,6 +827,14 @@ export function renderUnifiedReviewComment(input: UnifiedReviewInput, ctx: Unifi
blocks.push(details("Linked issue satisfaction", satisfactionBody, undefined, collapsiblesOpen));
}

// Threshold-only backtest advisory (#8138): same additive, collapsed-by-default, never-verdict-affecting
// shape as the linked-issue satisfaction section immediately above. Already-rendered Markdown -- see
// UnifiedReviewInput.thresholdBacktest's own doc comment for why this module doesn't render it itself.
const thresholdBacktestBody = input.thresholdBacktest?.trim() ?? "";
if (thresholdBacktestBody && verbosity !== "quiet") {
blocks.push(details("Threshold backtest", thresholdBacktestBody, "never blocks the verdict", collapsiblesOpen));
}

if (verbosity !== "quiet") {
for (const c of ctx.extraCollapsibles ?? []) {
if (c.body.trim()) {
Expand Down Expand Up @@ -865,6 +881,7 @@ export function buildUnifiedReviewInput(opts: {
reviewEffort?: { band: 1 | 2 | 3 | 4 | 5; minutes: number };
maxFindingsCaps?: { blockers: number | null; nits: number | null };
linkedIssueSatisfaction?: { status: "addressed" | "partial" | "unaddressed"; rationale: string };
thresholdBacktest?: string;
inlineFindings?: ReadonlyArray<{ category?: UnifiedFindingCategory | undefined }>;
blockerFixContext?: ReadonlyArray<{ path: string; line?: number; body: string }>;
}): UnifiedReviewInput {
Expand All @@ -886,6 +903,7 @@ export function buildUnifiedReviewInput(opts: {
...(opts.reviewEffort !== undefined ? { reviewEffort: opts.reviewEffort } : {}),
...(opts.maxFindingsCaps !== undefined ? { maxFindingsCaps: opts.maxFindingsCaps } : {}),
...(opts.linkedIssueSatisfaction !== undefined ? { linkedIssueSatisfaction: opts.linkedIssueSatisfaction } : {}),
...(opts.thresholdBacktest !== undefined ? { thresholdBacktest: opts.thresholdBacktest } : {}),
...(opts.inlineFindings !== undefined ? { inlineFindings: opts.inlineFindings } : {}),
...(opts.blockerFixContext !== undefined ? { blockerFixContext: opts.blockerFixContext } : {}),
};
Expand Down
Loading