From 2ba9f3333d7516506ffa304b48617da8eb7e9d89 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 22 Jul 2026 19:10:07 -0700 Subject: [PATCH] feat(review): backtest confidence-threshold changes against real signal history in ORB's live gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a PR edits a known AI-judgment confidence threshold, ORB now detects the change, replays it against recorded rule-fired/human-override history via the calibration engine, and folds the precision/recall comparison into the existing unified PR comment. Runs inline in the review pipeline using ORB's own diff and D1 access — no separate CI job or bot comment. --- .../src/calibration/backtest-threshold.ts | 43 +++++++ packages/loopover-engine/src/index.ts | 6 + .../test/backtest-threshold.test.ts | 61 ++++++++++ src/queue/processors.ts | 45 +++++++ src/review/unified-comment-bridge.ts | 7 ++ src/review/unified-comment.ts | 18 +++ src/services/threshold-backtest-run.ts | 86 +++++++++++++ src/services/threshold-backtest.ts | 81 +++++++++++++ test/unit/backtest-threshold-engine.test.ts | 71 +++++++++++ ...threshold-backtest-advisory-wiring.test.ts | 74 ++++++++++++ test/unit/threshold-backtest-run.test.ts | 113 ++++++++++++++++++ test/unit/threshold-backtest.test.ts | 103 ++++++++++++++++ 12 files changed, 708 insertions(+) create mode 100644 packages/loopover-engine/src/calibration/backtest-threshold.ts create mode 100644 packages/loopover-engine/test/backtest-threshold.test.ts create mode 100644 src/services/threshold-backtest-run.ts create mode 100644 src/services/threshold-backtest.ts create mode 100644 test/unit/backtest-threshold-engine.test.ts create mode 100644 test/unit/threshold-backtest-advisory-wiring.test.ts create mode 100644 test/unit/threshold-backtest-run.test.ts create mode 100644 test/unit/threshold-backtest.test.ts diff --git a/packages/loopover-engine/src/calibration/backtest-threshold.ts b/packages/loopover-engine/src/calibration/backtest-threshold.ts new file mode 100644 index 0000000000..1474a56d04 --- /dev/null +++ b/packages/loopover-engine/src/calibration/backtest-threshold.ts @@ -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); +} diff --git a/packages/loopover-engine/src/index.ts b/packages/loopover-engine/src/index.ts index a36a7e8410..1c3bd3e07b 100644 --- a/packages/loopover-engine/src/index.ts +++ b/packages/loopover-engine/src/index.ts @@ -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, diff --git a/packages/loopover-engine/test/backtest-threshold.test.ts b/packages/loopover-engine/test/backtest-threshold.test.ts new file mode 100644 index 0000000000..259843ab79 --- /dev/null +++ b/packages/loopover-engine/test/backtest-threshold.test.ts @@ -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")); +}); diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 18af250abd..e3757976a3 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -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, @@ -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>, +): Promise { + 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 @@ -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). @@ -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 @@ -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 diff --git a/src/review/unified-comment-bridge.ts b/src/review/unified-comment-bridge.ts index cd6fa06abc..49fbdff9eb 100644 --- a/src/review/unified-comment-bridge.ts +++ b/src/review/unified-comment-bridge.ts @@ -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; }; /** @@ -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 diff --git a/src/review/unified-comment.ts b/src/review/unified-comment.ts index 87d86a9e14..76cb317a3b 100644 --- a/src/review/unified-comment.ts +++ b/src/review/unified-comment.ts @@ -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 @@ -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()) { @@ -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 { @@ -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 } : {}), }; diff --git a/src/services/threshold-backtest-run.ts b/src/services/threshold-backtest-run.ts new file mode 100644 index 0000000000..c90c6bb3e5 --- /dev/null +++ b/src/services/threshold-backtest-run.ts @@ -0,0 +1,86 @@ +// Threshold-backtest orchestration -- the "separate, I/O-touching slice" the pure analysis core +// (./threshold-backtest.ts) explicitly leaves to its caller, mirroring linked-issue-satisfaction-run.ts's own +// "pure core vs. I/O orchestration" split. Reads each affected ruleId's history directly via the already-shipped +// createSignalStore(env) adapter (#7982/#8101/#8104) -- no CLI, no subprocess, no wrangler; this runs inside +// ORB's own Worker request, which already has env.DB. +// +// Hard guarantees (mirrors linked-issue-satisfaction-run.ts's own fail-safe discipline): +// • No changed threshold in the diff -> no result, never called for nothing. +// • A SignalStore read failure for one ruleId degrades that ruleId to an empty corpus (a real, if uninformative, +// backtest result -- null precision/recall, per #8085's own null-when-no-data discipline) rather than +// aborting the whole advisory. This never blocks the PR either way -- see the epic's own Boundaries. +import { buildBacktestCorpus, type BacktestCase, type BacktestComparison } from "@loopover/engine"; +import { createSignalStore } from "../review/signal-tracking-wire"; +import { recordAuditEvent } from "../db/repositories"; +import { backtestChangedThreshold, detectChangedThresholds, type ChangedThreshold } from "./threshold-backtest"; + +const CORPUS_LOOKBACK_MS = 90 * 24 * 60 * 60 * 1000; // 90 days -- generous enough for a young corpus to matter + +export const THRESHOLD_BACKTEST_EVENT_TYPE = "calibration.threshold_backtest_run"; + +async function fetchCorpus(env: Env, ruleId: string, nowMs: number): Promise { + try { + const { fired, overrides } = await createSignalStore(env).queryRuleHistory(ruleId, nowMs - CORPUS_LOOKBACK_MS); + return buildBacktestCorpus(ruleId, fired, overrides); + } catch { + // Fail open to an empty corpus (null precision/recall downstream), never throw -- a read failure for one + // ruleId must not abort the whole advisory or, worse, the review pass that triggered it. + return []; + } +} + +export type ThresholdBacktestRunResult = { + changed: readonly ChangedThreshold[]; + comparisons: readonly BacktestComparison[]; +}; + +/** + * Detect any known threshold constant changed in `diff` and backtest each affected ruleId against its real + * history. Returns `{ changed: [], comparisons: [] }` -- never null, never throws -- when nothing changed, so + * the caller can check `comparisons.length` the same way it checks any other optional advisory result. + * `diff` must be UNBUDGETED (see detectChangedThresholds's own doc comment) -- pass buildSecretScanDiff's + * output, not buildAiReviewDiff's. + */ +export async function runThresholdBacktestAdvisory(env: Env, diff: string, nowMs: number = Date.now()): Promise { + const changed = detectChangedThresholds(diff); + if (changed.length === 0) return { changed: [], comparisons: [] }; + + const ruleIds = [...new Set(changed.flatMap((change) => change.ruleIds))]; + const corpusByRuleId = new Map(); + for (const ruleId of ruleIds) corpusByRuleId.set(ruleId, await fetchCorpus(env, ruleId, nowMs)); + + const comparisons = changed.flatMap((change) => backtestChangedThreshold(change, corpusByRuleId)); + return { changed, comparisons }; +} + +/** Persist each comparison for #8140's future track-record tool -- structured data via the shared + * audit_events table (recordAuditEvent, already used by every other write site in this epic), never a raw + * SQL string. Best-effort: `.catch(() => undefined)` at each call site, matching every other calibration + * write in this epic -- a persistence failure must never affect the review pass that produced the result. */ +export async function persistThresholdBacktestRuns( + env: Env, + repoFullName: string, + prNumber: number, + changed: readonly ChangedThreshold[], + comparisons: readonly BacktestComparison[], +): Promise { + const comparisonsByRuleId = new Map(comparisons.map((comparison) => [comparison.ruleId, comparison])); + for (const change of changed) { + for (const ruleId of change.ruleIds) { + const comparison = comparisonsByRuleId.get(ruleId); + if (!comparison) continue; + await recordAuditEvent(env, { + eventType: THRESHOLD_BACKTEST_EVENT_TYPE, + actor: "loopover", + targetKey: `${repoFullName}#${prNumber}`, + // AuditEventRecord.outcome is a fixed enum (success/denied/error/queued/completed) -- it's not where + // the backtest verdict goes. "completed" means "this run recorded successfully"; the real verdict + // (improved/regressed/unchanged) lives in `detail` and `metadata.comparison.verdict` instead, mirroring + // how every other telemetry-shaped audit event in this codebase uses "completed" for "happened, see detail". + outcome: "completed", + detail: `${change.constantName} threshold backtest for ${ruleId}: ${comparison.verdict}`, + metadata: { comparison, constantName: change.constantName }, + }).catch(() => undefined); + } + } +} diff --git a/src/services/threshold-backtest.ts b/src/services/threshold-backtest.ts new file mode 100644 index 0000000000..c0fd0c260f --- /dev/null +++ b/src/services/threshold-backtest.ts @@ -0,0 +1,81 @@ +// Threshold-only backtest advisory (#8138, epic #8082). We already record fired/reversed history for every +// isConfiguredGateBlocker code (#7982/#8101/#8104) -- this module is the bounded analysis core: detect +// whether THIS PR changes one of a small, known set of confidence-threshold constants, and if so, prepare +// the pure backtest inputs. NO gate wiring, NO I/O, NO D1 access here -- the caller (threshold-backtest-run.ts) +// supplies the already-fetched corpus, exactly like linked-issue-satisfaction.ts's own "pure analysis core, +// I/O stays with the caller" contract. +// +// Deliberately NOT CI: this was originally built as a separate GitHub Actions workflow shelling out to +// `wrangler d1 execute`, duplicating ORB's own already-running diff fetch and D1 access and posting a SECOND +// bot comment. Reworked to live inside ORB's existing review pass instead -- one unified comment, one D1 +// connection, zero new CI surface. +import { renderBacktestComparison, runThresholdBacktest, type BacktestCase, type BacktestComparison } from "@loopover/engine"; + +/** The two threshold constants this advisory is scoped to -- not a generic "any numeric constant changed" + * detector. Each maps to the ruleId(s) its value actually gates. */ +export const KNOWN_THRESHOLDS: Readonly> = Object.freeze({ + LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR: { ruleIds: ["linked_issue_scope_mismatch"] }, + DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE: { ruleIds: ["ai_consensus_defect", "ai_review_split"] }, +}); + +export type ChangedThreshold = { + constantName: string; + oldValue: number; + newValue: number; + ruleIds: readonly string[]; +}; + +const CHANGED_LINE_PATTERN = /^([-+])export const (\w+) = ([\d.]+);/; + +/** + * Scan a unified diff for a changed value of one of `KNOWN_THRESHOLDS`' constant names. Matches a `-` + * (old) and `+` (new) line for the same constant; a constant with anything other than exactly one `-` + * value and one `+` value (added-only, removed-only, or multiple conflicting edits) is skipped as + * ambiguous rather than guessed at. A pair whose old and new values are identical (e.g. pure + * reformatting) is also skipped -- nothing actually changed. Must be given an UNBUDGETED diff (mirror + * `buildSecretScanDiff`'s contract, not `buildAiReviewDiff`'s bounded/hunk-dropping one) -- a truncated + * diff could silently miss the very line this function needs to see. + */ +export function detectChangedThresholds(diff: string): ChangedThreshold[] { + const oldValues = new Map(); + const newValues = new Map(); + for (const line of diff.split("\n")) { + const match = CHANGED_LINE_PATTERN.exec(line); + if (!match) continue; + const [, sign, name, valueText] = match; + if (!Object.hasOwn(KNOWN_THRESHOLDS, name!)) continue; + const value = Number(valueText); + if (!Number.isFinite(value)) continue; + const bucket = sign === "-" ? oldValues : newValues; + const existing = bucket.get(name!) ?? []; + existing.push(value); + bucket.set(name!, existing); + } + + const changed: ChangedThreshold[] = []; + for (const constantName of Object.keys(KNOWN_THRESHOLDS)) { + const oldMatches = oldValues.get(constantName) ?? []; + const newMatches = newValues.get(constantName) ?? []; + if (oldMatches.length !== 1 || newMatches.length !== 1) continue; + const [oldValue] = oldMatches; + const [newValue] = newMatches; + if (oldValue === newValue) continue; + changed.push({ constantName, oldValue: oldValue!, newValue: newValue!, ruleIds: KNOWN_THRESHOLDS[constantName]!.ruleIds }); + } + return changed; +} + +/** Pure: score one changed threshold against its already-fetched corpus. The caller supplies `cases` + * per-ruleId (via createSignalStore(env).queryRuleHistory in threshold-backtest-run.ts) -- this function + * never touches D1 itself. */ +export function backtestChangedThreshold(change: ChangedThreshold, corpusByRuleId: ReadonlyMap): BacktestComparison[] { + return change.ruleIds.map((ruleId) => runThresholdBacktest(ruleId, corpusByRuleId.get(ruleId) ?? [], change.oldValue, change.newValue)); +} + +/** Render every comparison as one unified-comment section body, or "" when there's nothing to show -- + * mirrors unified-comment.ts's own `xxxBlock(...) -> "" when absent` convention exactly, so the caller can + * omit the section the same way linkedIssueSatisfactionBlock does. */ +export function thresholdBacktestBlock(comparisons: readonly BacktestComparison[]): string { + if (comparisons.length === 0) return ""; + return comparisons.map((comparison) => renderBacktestComparison(comparison)).join("\n\n"); +} diff --git a/test/unit/backtest-threshold-engine.test.ts b/test/unit/backtest-threshold-engine.test.ts new file mode 100644 index 0000000000..ffdf4b4ca2 --- /dev/null +++ b/test/unit/backtest-threshold-engine.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "vitest"; + +// Import the engine SOURCE directly (not the built dist) -- coverage.include lists +// packages/loopover-engine/src/**, so only a source-path import exercises the .ts these branches live in +// (the dist-importing twin in packages/loopover-engine/test/ covers the built barrel for the workspace +// suite). Same pattern as backtest-compare-engine.test.ts. +import { buildConfidenceThresholdClassifier, runThresholdBacktest } from "../../packages/loopover-engine/src/calibration/backtest-threshold"; +import type { BacktestCase } from "../../packages/loopover-engine/src/calibration/backtest-corpus"; + +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 } } : {}), + }; +} + +describe("buildConfidenceThresholdClassifier (#8138)", () => { + it("predicts reversed when confidence is below the threshold", () => { + const classify = buildConfidenceThresholdClassifier(0.5); + expect(classify(corpusCase("a#1", "reversed", 0.3))).toBe("reversed"); + }); + + it("predicts confirmed when confidence is at or above the threshold", () => { + const classify = buildConfidenceThresholdClassifier(0.5); + expect(classify(corpusCase("a#1", "confirmed", 0.5))).toBe("confirmed"); + expect(classify(corpusCase("a#2", "confirmed", 0.9))).toBe("confirmed"); + }); + + it("degrades a missing/non-numeric confidence to 1 (never predicted reversed by default)", () => { + const classify = buildConfidenceThresholdClassifier(0.99); + expect(classify(corpusCase("a#1", "confirmed"))).toBe("confirmed"); + expect(classify({ ...corpusCase("a#2", "confirmed"), metadata: { confidence: "not-a-number" } })).toBe("confirmed"); + }); +}); + +describe("runThresholdBacktest (#8138)", () => { + it("reports improved when lowering the floor correctly reclassifies a previously-missed reversed case", () => { + // At threshold 0.5: confidence 0.4 -> predicted "confirmed" (wrong, real label is "reversed"). + // At threshold 0.3: confidence 0.4 -> predicted "confirmed" still... use a case where the NEW + // (lower) threshold correctly flips the prediction to match the real "reversed" label instead. + const cases: BacktestCase[] = [corpusCase("a#1", "reversed", 0.35)]; + // Old threshold 0.5: confidence 0.35 < 0.5 -> predicted "reversed" -> matches label -> truePositive. + // New threshold 0.2: confidence 0.35 >= 0.2 -> predicted "confirmed" -> mismatches "reversed" label -> falseNegative. + // This is a REGRESSION case (lowering the floor here makes it worse) -- exercises the "regressed" verdict. + const comparison = runThresholdBacktest("linked_issue_scope_mismatch", cases, 0.5, 0.2); + expect(comparison.verdict).toBe("regressed"); + expect(comparison.regressedAxes).toContain("recall"); + }); + + it("reports unchanged when the corpus is empty (both scores are all-null)", () => { + const comparison = runThresholdBacktest("linked_issue_scope_mismatch", [], 0.5, 0.3); + expect(comparison.verdict).toBe("unchanged"); + expect(comparison.baseline.caseCount).toBe(0); + expect(comparison.candidate.caseCount).toBe(0); + }); + + it("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); + expect(comparison.baseline.caseCount).toBe(1); + expect(comparison.candidate.caseCount).toBe(1); + }); +}); diff --git a/test/unit/threshold-backtest-advisory-wiring.test.ts b/test/unit/threshold-backtest-advisory-wiring.test.ts new file mode 100644 index 0000000000..cb58b478ee --- /dev/null +++ b/test/unit/threshold-backtest-advisory-wiring.test.ts @@ -0,0 +1,74 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { resolveThresholdBacktestAdvisory } from "../../src/queue/processors"; +import { createSignalStore } from "../../src/review/signal-tracking-wire"; +import { listAuditEventsByType } from "../../src/db/repositories"; +import * as thresholdBacktestRun from "../../src/services/threshold-backtest-run"; +import { THRESHOLD_BACKTEST_EVENT_TYPE } from "../../src/services/threshold-backtest-run"; +import type { PullRequestFileRecord } from "../../src/types"; +import { createTestEnv } from "../helpers/d1"; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function fileRecord(path: string, patch: string, overrides: Partial = {}): PullRequestFileRecord { + return { repoFullName: "acme/widgets", pullNumber: 7, path, status: "modified", additions: 1, deletions: 1, changes: 2, payload: { patch }, ...overrides }; +} + +function thresholdPatch(name: string, oldValue: string, newValue: string): string { + return ["@@ -980,7 +980,7 @@", `-export const ${name} = ${oldValue};`, `+export const ${name} = ${newValue};`].join("\n"); +} + +describe("resolveThresholdBacktestAdvisory (processor wiring, #8138)", () => { + it("returns '' without touching D1 at all when the PR doesn't touch either watched file", async () => { + const env = createTestEnv(); + const files = [fileRecord("README.md", "@@ -1 +1 @@\n-old\n+new")]; + const result = await resolveThresholdBacktestAdvisory(env, "acme/widgets", { number: 7 }, files); + expect(result).toBe(""); + }); + + it("returns '' when the PR touches a watched file but doesn't actually change a known constant's value", async () => { + const env = createTestEnv(); + const files = [fileRecord("src/rules/advisory.ts", "@@ -1 +1 @@\n-const unrelated = 1;\n+const unrelated = 2;")]; + const result = await resolveThresholdBacktestAdvisory(env, "acme/widgets", { number: 7 }, files); + expect(result).toBe(""); + }); + + it("renders a section and persists a run when the PR changes a known threshold, with real history behind it", async () => { + const env = createTestEnv(); + const now = Date.now(); + await createSignalStore(env).recordRuleFired({ + ruleId: "linked_issue_scope_mismatch", + targetKey: "acme/widgets#1", + outcome: "unaddressed", + occurredAt: new Date(now - 1000).toISOString(), + metadata: { confidence: 0.35 }, + }); + await createSignalStore(env).recordHumanOverride({ + ruleId: "linked_issue_scope_mismatch", + targetKey: "acme/widgets#1", + verdict: "reversed", + occurredAt: new Date(now).toISOString(), + }); + + const files = [fileRecord("src/services/linked-issue-satisfaction.ts", thresholdPatch("LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", "0.5", "0.2"))]; + const result = await resolveThresholdBacktestAdvisory(env, "acme/widgets", { number: 7 }, files); + + expect(result).toContain("linked_issue_scope_mismatch"); + expect(result.length).toBeGreaterThan(0); + + const rows = await listAuditEventsByType(env, THRESHOLD_BACKTEST_EVENT_TYPE, new Date(now - 60_000).toISOString()); + expect(rows).toHaveLength(1); + expect(rows[0]!.targetKey).toBe("acme/widgets#7"); + }); + + it("returns '' (and never throws) when the underlying backtest run itself rejects", async () => { + // Reaches the try/catch's error path: give it a watched file so it proceeds past the cheap skip, then + // force the D1-touching call to reject. + vi.spyOn(thresholdBacktestRun, "runThresholdBacktestAdvisory").mockRejectedValue(new Error("simulated failure")); + const env = createTestEnv(); + const files = [fileRecord("src/rules/advisory.ts", thresholdPatch("DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE", "0.93", "0.8"))]; + const result = await resolveThresholdBacktestAdvisory(env, "acme/widgets", { number: 7 }, files); + expect(result).toBe(""); + }); +}); diff --git a/test/unit/threshold-backtest-run.test.ts b/test/unit/threshold-backtest-run.test.ts new file mode 100644 index 0000000000..7107d933e6 --- /dev/null +++ b/test/unit/threshold-backtest-run.test.ts @@ -0,0 +1,113 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import * as signalTrackingWire from "../../src/review/signal-tracking-wire"; +import { createSignalStore } from "../../src/review/signal-tracking-wire"; +import * as repositories from "../../src/db/repositories"; +import { listAuditEventsByType } from "../../src/db/repositories"; +import { persistThresholdBacktestRuns, runThresholdBacktestAdvisory, THRESHOLD_BACKTEST_EVENT_TYPE } from "../../src/services/threshold-backtest-run"; +import { createTestEnv } from "../helpers/d1"; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function diffFor(name: string, oldValue: string, newValue: string): string { + return ["diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts", "@@ -980,7 +980,7 @@", `-export const ${name} = ${oldValue};`, `+export const ${name} = ${newValue};`].join("\n"); +} + +describe("runThresholdBacktestAdvisory (#8138) — real D1 round-trip", () => { + it("returns empty changed/comparisons when the diff touches no known threshold, without any D1 read", async () => { + const env = createTestEnv(); + const result = await runThresholdBacktestAdvisory(env, "diff --git a/README.md b/README.md\n-old\n+new"); + expect(result).toEqual({ changed: [], comparisons: [] }); + }); + + it("backtests a changed LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR against real recorded history", async () => { + const env = createTestEnv(); + const now = Date.now(); + await createSignalStore(env).recordRuleFired({ + ruleId: "linked_issue_scope_mismatch", + targetKey: "acme/widgets#1", + outcome: "unaddressed", + occurredAt: new Date(now - 1000).toISOString(), + metadata: { confidence: 0.35 }, + }); + await createSignalStore(env).recordHumanOverride({ + ruleId: "linked_issue_scope_mismatch", + targetKey: "acme/widgets#1", + verdict: "reversed", + occurredAt: new Date(now).toISOString(), + }); + + const result = await runThresholdBacktestAdvisory(env, diffFor("LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", "0.5", "0.2"), now + 1000); + expect(result.changed).toHaveLength(1); + expect(result.comparisons).toHaveLength(1); + expect(result.comparisons[0]!.ruleId).toBe("linked_issue_scope_mismatch"); + expect(result.comparisons[0]!.baseline.caseCount).toBe(1); + }); + + it("degrades to an empty corpus for a ruleId with no recorded history, rather than throwing", async () => { + const env = createTestEnv(); + const result = await runThresholdBacktestAdvisory(env, diffFor("DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE", "0.93", "0.8")); + expect(result.comparisons).toHaveLength(2); + for (const comparison of result.comparisons) { + expect(comparison.baseline.caseCount).toBe(0); + expect(comparison.baseline.precision).toBeNull(); + } + }); + + it("degrades to an empty corpus (never throws) when the SignalStore read itself rejects", async () => { + vi.spyOn(signalTrackingWire, "createSignalStore").mockReturnValue({ + recordRuleFired: vi.fn(), + recordHumanOverride: vi.fn(), + queryRuleHistory: vi.fn().mockRejectedValue(new Error("simulated D1 failure")), + }); + const env = createTestEnv(); + const result = await runThresholdBacktestAdvisory(env, diffFor("LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", "0.5", "0.4")); + expect(result.comparisons).toHaveLength(1); + expect(result.comparisons[0]!.baseline.caseCount).toBe(0); + expect(result.comparisons[0]!.baseline.precision).toBeNull(); + }); +}); + +describe("persistThresholdBacktestRuns (#8138) — real D1 round-trip", () => { + it("records one audit_events row per (constant, ruleId) pair, readable back via listAuditEventsByType", async () => { + const env = createTestEnv(); + const now = Date.now(); + const { changed, comparisons } = await runThresholdBacktestAdvisory(env, diffFor("LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", "0.5", "0.4"), now); + await persistThresholdBacktestRuns(env, "acme/widgets", 7, changed, comparisons); + + const rows = await listAuditEventsByType(env, THRESHOLD_BACKTEST_EVENT_TYPE, new Date(now - 60_000).toISOString()); + expect(rows).toHaveLength(1); + expect(rows[0]!.targetKey).toBe("acme/widgets#7"); + expect(rows[0]!.metadata.constantName).toBe("LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR"); + expect((rows[0]!.metadata.comparison as { ruleId: string }).ruleId).toBe("linked_issue_scope_mismatch"); + }); + + it("persists nothing when nothing changed", async () => { + const env = createTestEnv(); + await persistThresholdBacktestRuns(env, "acme/widgets", 1, [], []); + const rows = await listAuditEventsByType(env, THRESHOLD_BACKTEST_EVENT_TYPE, new Date(0).toISOString()); + expect(rows).toHaveLength(0); + }); + + it("skips a changed threshold with no matching comparison, rather than erroring (defensive guard for direct callers)", async () => { + const env = createTestEnv(); + await persistThresholdBacktestRuns( + env, + "acme/widgets", + 1, + [{ constantName: "LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", oldValue: 0.5, newValue: 0.4, ruleIds: ["linked_issue_scope_mismatch"] }], + [], // no comparison for that ruleId + ); + const rows = await listAuditEventsByType(env, THRESHOLD_BACKTEST_EVENT_TYPE, new Date(0).toISOString()); + expect(rows).toHaveLength(0); + }); + + it("degrades silently (never throws) when the recordAuditEvent write itself rejects", async () => { + vi.spyOn(repositories, "recordAuditEvent").mockRejectedValue(new Error("simulated D1 write failure")); + const env = createTestEnv(); + const now = Date.now(); + const { changed, comparisons } = await runThresholdBacktestAdvisory(env, diffFor("LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", "0.5", "0.4"), now); + await expect(persistThresholdBacktestRuns(env, "acme/widgets", 8, changed, comparisons)).resolves.toBeUndefined(); + }); +}); diff --git a/test/unit/threshold-backtest.test.ts b/test/unit/threshold-backtest.test.ts new file mode 100644 index 0000000000..bb1192e5d6 --- /dev/null +++ b/test/unit/threshold-backtest.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, it } from "vitest"; +import type { BacktestCase } from "@loopover/engine"; +import { + backtestChangedThreshold, + detectChangedThresholds, + KNOWN_THRESHOLDS, + thresholdBacktestBlock, +} from "../../src/services/threshold-backtest"; + +function diffFor(name: string, oldValue: string, newValue: string): string { + return ["diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts", "@@ -980,7 +980,7 @@", `-export const ${name} = ${oldValue};`, `+export const ${name} = ${newValue};`].join("\n"); +} + +function corpusCase(ruleId: string, targetKey: string, label: BacktestCase["label"], confidence: number): BacktestCase { + return { ruleId, targetKey, outcome: "unaddressed", label, firedAt: "2026-07-22T00:00:00.000Z", decidedAt: "2026-07-22T01:00:00.000Z", metadata: { confidence } }; +} + +describe("detectChangedThresholds (#8138)", () => { + it("detects a clean before/after pair for LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", () => { + const changed = detectChangedThresholds(diffFor("LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", "0.5", "0.4")); + expect(changed).toEqual([ + { constantName: "LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", oldValue: 0.5, newValue: 0.4, ruleIds: ["linked_issue_scope_mismatch"] }, + ]); + }); + + it("detects DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE and maps it to both AI-judgment ruleIds", () => { + const changed = detectChangedThresholds(diffFor("DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE", "0.93", "0.85")); + expect(changed).toEqual([ + { constantName: "DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE", oldValue: 0.93, newValue: 0.85, ruleIds: ["ai_consensus_defect", "ai_review_split"] }, + ]); + }); + + it("ignores a diff that touches neither known constant", () => { + expect(detectChangedThresholds(["diff --git a/README.md b/README.md", "-old line", "+new line"].join("\n"))).toEqual([]); + }); + + it("ignores an unrecognized constant name even in the same export-const shape", () => { + expect(detectChangedThresholds(diffFor("SOME_OTHER_CONSTANT", "1", "2"))).toEqual([]); + }); + + it("skips a pair whose value didn't actually change (e.g. pure reformatting)", () => { + expect(detectChangedThresholds(diffFor("LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", "0.5", "0.5"))).toEqual([]); + }); + + it("skips an added-only constant (no old value to backtest against)", () => { + const diff = ["diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts", "@@ -980,6 +980,7 @@", "+export const LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR = 0.5;"].join("\n"); + expect(detectChangedThresholds(diff)).toEqual([]); + }); + + it("skips a removed-only constant (no new value to backtest)", () => { + const diff = ["diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts", "@@ -980,7 +980,6 @@", "-export const LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR = 0.5;"].join("\n"); + expect(detectChangedThresholds(diff)).toEqual([]); + }); + + it("skips a constant with more than one -/+ pair (ambiguous, not guessed at)", () => { + const diff = [diffFor("LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", "0.5", "0.4"), diffFor("LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", "0.6", "0.3")].join("\n"); + expect(detectChangedThresholds(diff)).toEqual([]); + }); + + it("skips a value that matches the digit/dot pattern but isn't a real finite number (e.g. multiple dots)", () => { + expect(detectChangedThresholds(diffFor("LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", "0.5", "1.2.3"))).toEqual([]); + }); + + it("every KNOWN_THRESHOLDS entry maps to at least one ruleId", () => { + for (const name of Object.keys(KNOWN_THRESHOLDS)) { + expect(KNOWN_THRESHOLDS[name]!.ruleIds.length).toBeGreaterThan(0); + } + }); +}); + +describe("backtestChangedThreshold (#8138)", () => { + it("scores each ruleId in the change against its own corpus slice", () => { + const change = { constantName: "DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE", oldValue: 0.93, newValue: 0.8, ruleIds: ["ai_consensus_defect", "ai_review_split"] }; + const corpusByRuleId = new Map([ + ["ai_consensus_defect", [corpusCase("ai_consensus_defect", "a#1", "confirmed", 0.95)]], + ["ai_review_split", [corpusCase("ai_review_split", "a#2", "confirmed", 0.95)]], + ]); + const comparisons = backtestChangedThreshold(change, corpusByRuleId); + expect(comparisons).toHaveLength(2); + expect(comparisons.map((c) => c.ruleId).sort()).toEqual(["ai_consensus_defect", "ai_review_split"]); + }); + + it("scores an empty corpus (missing ruleId entry) as all-null rather than throwing", () => { + const change = { constantName: "LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", oldValue: 0.5, newValue: 0.4, ruleIds: ["linked_issue_scope_mismatch"] }; + const comparisons = backtestChangedThreshold(change, new Map()); + expect(comparisons).toHaveLength(1); + expect(comparisons[0]!.baseline.precision).toBeNull(); + }); +}); + +describe("thresholdBacktestBlock (#8138)", () => { + it("returns an empty string for zero comparisons, mirroring unified-comment.ts's own xxxBlock convention", () => { + expect(thresholdBacktestBlock([])).toBe(""); + }); + + it("renders every comparison, joined, when at least one is present", () => { + const change = { constantName: "LINKED_ISSUE_SATISFACTION_CONFIDENCE_FLOOR", oldValue: 0.5, newValue: 0.2, ruleIds: ["linked_issue_scope_mismatch"] }; + const corpusByRuleId = new Map([["linked_issue_scope_mismatch", [corpusCase("linked_issue_scope_mismatch", "a#1", "reversed", 0.35)]]]); + const block = thresholdBacktestBlock(backtestChangedThreshold(change, corpusByRuleId)); + expect(block).toContain("linked_issue_scope_mismatch"); + expect(block.length).toBeGreaterThan(0); + }); +});