diff --git a/apps/loopover-ui/content/docs/backtest-calibration.mdx b/apps/loopover-ui/content/docs/backtest-calibration.mdx index e90ba2a8f..6b6c0b125 100644 --- a/apps/loopover-ui/content/docs/backtest-calibration.mdx +++ b/apps/loopover-ui/content/docs/backtest-calibration.mdx @@ -108,6 +108,31 @@ loosening loop owns that direction). One alert per drift episode: a standing unc re-alerts, a changed one does, and recovery clears the episode. The sentinel is **alert-only** — it never writes a knob value — and the knobs status endpoint shows each knob's current drift report regardless of the flag, so a standing drift is always visible. +## Tightening under the same evidence standard + +Historically the two directions ran on different evidence regimes: loosening moved only on Pareto-floored +backtests, while tightening lived in a legacy loop (precision circuit-breakers plus a shadow-soaked +advisor). The tightening unification brings threshold *raises* under the same backtest standard — with the +axes orientation made **explicit** instead of reused blind. A deliberate tightening exists to move one axis +at a bounded cost to the other; which axis is which depends on corpus polarity (for the confidence-threshold +classifier, the positive class is "predicted reversed", so raising a bar helps *recall* and risks +*precision* — the inverse of the rule-firing frame). Each knob's optional tightening ladder therefore +declares its own orientation: the axis a raise must strictly improve, and the maximum sacrifice the other +axis may suffer per comparison slice before the trade is a regression. + +Everything else is the loosening discipline transposed verbatim: the same split seed and fraction (held-out +membership is identical in both directions), the same never-on-noise sample floors, smallest step first, a +hard maximum no evidence may cross, and per-direction double gating — the tighten loop and the above-shipped +override read are both gated by the ladder's **own** default-off flag, separate from the loosening flag, so +each direction's autonomy is opted into independently and flipping either flag off instantly restores the +shipped default for that direction. A declared ladder also joins the drift sentinel's candidate pool, so +tighter-alternative findings and the tighten apply path judge the same values under the same floors. + +The legacy tightening triggers that are *not* threshold moves stay bespoke on purpose: the merge/close +precision circuit-breakers are boolean capability breakers judged on realized outcomes (the strongest +evidence regime there is), and re-basing them on fired-signal backtests would weaken, not strengthen, their +grounds. Their behavior is pinned byte-stable by their own suites. + ## Reliability curves Beside the candidate-ladder machinery, each live knob's status now carries its rule's diff --git a/packages/loopover-engine/src/calibration/backtest-compare.ts b/packages/loopover-engine/src/calibration/backtest-compare.ts index b8152b9c0..b777881b7 100644 --- a/packages/loopover-engine/src/calibration/backtest-compare.ts +++ b/packages/loopover-engine/src/calibration/backtest-compare.ts @@ -8,7 +8,7 @@ import type { BacktestScoreReport } from "./backtest-score.js"; /** The two comparable axes of a {@link BacktestScoreReport}. */ -type ComparisonAxis = "precision" | "recall"; +export type ComparisonAxis = "precision" | "recall"; export type BacktestComparison = { ruleId: string; @@ -50,3 +50,65 @@ export function compareBacktestScores(baseline: BacktestScoreReport, candidate: verdict: regressedAxes.length > 0 ? "regressed" : improvedAxes.length > 0 ? "improved" : "unchanged", }; } + +/** The explicit axes orientation of a directional comparison (#8225): which axis the change exists to move + * (and must move strictly up to earn "improved"), and how much the OTHER axis may be sacrificed for it. */ +export type DirectionalOrientation = { + mustImprove: ComparisonAxis; + /** Absolute drop the non-`mustImprove` axis may suffer before the trade is a regression. */ + maxSacrifice: number; +}; + +/** + * Direction-aware comparator for a deliberate axis trade (#8225) -- a TIGHTENING exists to move one axis at + * a bounded cost to the other, so reusing the symmetric {@link compareBacktestScores} blind would brand + * every honest trade "regressed" the moment the sacrificed axis dips. Which axis is which depends on the + * corpus polarity (for the confidence-threshold classifier the positive class is "predicted reversed", so + * RAISING a threshold helps recall and risks precision -- the inverse of the rule-firing frame), hence the + * orientation is the CALLER's explicit declaration, never an assumption baked in here. The re-oriented + * floor: + * • the `mustImprove` axis must move STRICTLY up for an "improved" verdict, and any drop on it is + * "regressed" -- a trade that loses the axis it exists to win is simply wrong; + * • the other axis may drop by at most `maxSacrifice`; a within-bound drop is the accepted trade and + * appears in NEITHER axis list, an over-bound drop is "regressed", and a gain still counts; + * • a null on either side of an axis excludes that axis entirely -- unknown stays unknown, exactly as in + * the symmetric comparator (so a corpus with no judgeable win-axis can never yield "improved"). + * Throws on a rule mismatch or a non-finite/negative bound: both are caller bugs, not valid comparisons. + */ +export function compareDirectionalBacktestScores( + baseline: BacktestScoreReport, + candidate: BacktestScoreReport, + orientation: DirectionalOrientation, +): BacktestComparison { + if (baseline.ruleId !== candidate.ruleId) { + throw new Error(`cannot compare backtest scores for different rules: ${baseline.ruleId} vs ${candidate.ruleId}`); + } + if (!Number.isFinite(orientation.maxSacrifice) || orientation.maxSacrifice < 0) { + throw new Error(`maxSacrifice must be a non-negative finite number, got ${orientation.maxSacrifice}`); + } + const sacrificeAxis: ComparisonAxis = orientation.mustImprove === "precision" ? "recall" : "precision"; + const regressedAxes: ComparisonAxis[] = []; + const improvedAxes: ComparisonAxis[] = []; + const winBaseline = baseline[orientation.mustImprove]; + const winCandidate = candidate[orientation.mustImprove]; + if (winBaseline !== null && winCandidate !== null) { + if (winCandidate < winBaseline) regressedAxes.push(orientation.mustImprove); + else if (winCandidate > winBaseline) improvedAxes.push(orientation.mustImprove); + } + const sacBaseline = baseline[sacrificeAxis]; + const sacCandidate = candidate[sacrificeAxis]; + if (sacBaseline !== null && sacCandidate !== null) { + if (sacBaseline - sacCandidate > orientation.maxSacrifice) regressedAxes.push(sacrificeAxis); + else if (sacCandidate > sacBaseline) improvedAxes.push(sacrificeAxis); + } + return { + ruleId: baseline.ruleId, + baseline, + candidate, + regressedAxes, + improvedAxes, + // "improved" requires the WIN axis specifically -- a lone gain on the sacrifice axis is not what the + // trade is for, so it stays "unchanged" (harmless, but no evidence the step earned its keep). + verdict: regressedAxes.length > 0 ? "regressed" : improvedAxes.includes(orientation.mustImprove) ? "improved" : "unchanged", + }; +} diff --git a/packages/loopover-engine/test/backtest-compare.test.ts b/packages/loopover-engine/test/backtest-compare.test.ts index adfbd7b3a..65da30d12 100644 --- a/packages/loopover-engine/test/backtest-compare.test.ts +++ b/packages/loopover-engine/test/backtest-compare.test.ts @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { compareBacktestScores, type BacktestScoreReport } from "../dist/index.js"; +import { compareBacktestScores, compareDirectionalBacktestScores, type BacktestScoreReport } from "../dist/index.js"; function report(overrides: Partial = {}): BacktestScoreReport { return { @@ -61,3 +61,15 @@ test("compareBacktestScores: mismatched ruleIds throw, naming both rules", () => /cannot compare backtest scores for different rules: missing_linked_issue vs other_rule/, ); }); + +test("barrel: the public entrypoint re-exports the direction-aware comparator (#8225)", () => { + assert.equal(typeof compareDirectionalBacktestScores, "function"); +}); + +test("compareDirectionalBacktestScores: win axis up + within-budget sacrifice is improved; over-budget regresses", () => { + const orientation = { mustImprove: "recall" as const, maxSacrifice: 0.1 }; + const ok = compareDirectionalBacktestScores(report(), report({ recall: 0.7, precision: 0.45 }), orientation); + assert.equal(ok.verdict, "improved"); + const over = compareDirectionalBacktestScores(report(), report({ recall: 0.9, precision: 0.3 }), orientation); + assert.equal(over.verdict, "regressed"); +}); diff --git a/src/queue/job-dispatch.ts b/src/queue/job-dispatch.ts index 03bcc8864..eaffbfaf3 100644 --- a/src/queue/job-dispatch.ts +++ b/src/queue/job-dispatch.ts @@ -31,7 +31,7 @@ import { isPrReconciliationEnabled, resolvePrReconciliationManifestOverride, run import { isActiveReviewReconciliationEnabled, resolveActiveReviewReconciliationManifestOverride, runActiveReviewReconciliation } from "../review/active-review-reconciliation"; import { isSelfTuneEnabled, runSelfTune } from "../review/selftune-wire"; import { isSatisfactionFloorAutotuneEnabled, runScheduledSatisfactionFloorLoosening } from "../services/satisfaction-floor-loosening-run"; -import { GENERIC_LIVE_KNOBS, isConfigDriftSentinelEnabled, isKnobAutotuneEnabled, runConfigDriftSentinel, runPerRepoKnobLoosening, runScheduledKnobLoosening } from "../services/knob-loosening-run"; +import { GENERIC_LIVE_KNOBS, isConfigDriftSentinelEnabled, isKnobAutotuneEnabled, isKnobTightenEnabled, runConfigDriftSentinel, runPerRepoKnobLoosening, runScheduledKnobLoosening, runScheduledKnobTightening } from "../services/knob-loosening-run"; import { runSelfTuneBreaker } from "../review/outcomes-wire"; import { isRagEnabled } from "../review/rag-wire"; import { processSubmitDraft } from "../services/draft"; @@ -358,6 +358,9 @@ export async function processJob(env: Env, message: JobMessage): Promise { // inherit global. Bounded per tick with a rotating cursor; fail-safe internally. await runPerRepoKnobLoosening(env, knob); } + // #8225: the tighten direction rides the same tick under its OWN per-knob default-off var — + // direction autonomy is opted into separately, never inherited from the loosening flag. + if (isKnobTightenEnabled(env, knob)) await runScheduledKnobTightening(env, knob); } // #8213: the drift sentinel rides the same calibration tick, behind its own default-off flag. // Alert-only — it never writes a knob value — and internally fail-safe per knob. diff --git a/src/services/knob-loosening-run.ts b/src/services/knob-loosening-run.ts index 216fa2786..7107476f9 100644 --- a/src/services/knob-loosening-run.ts +++ b/src/services/knob-loosening-run.ts @@ -22,7 +22,16 @@ import { } from "@loopover/engine"; import { createSignalStore } from "../review/signal-tracking-wire"; import { recordAuditEvent } from "../db/repositories"; -import { evaluateKnobDrift, evaluateKnobLoosening, LOOSENABLE_KNOBS, type KnobDriftReport, type KnobLooseningProposal, type LoosenableKnob } from "./loosening-knobs"; +import { + evaluateKnobDrift, + evaluateKnobLoosening, + evaluateKnobTightening, + LOOSENABLE_KNOBS, + type KnobDriftReport, + type KnobLooseningProposal, + type KnobTighteningProposal, + type LoosenableKnob, +} from "./loosening-knobs"; const CORPUS_LOOKBACK_MS = 90 * 24 * 60 * 60 * 1000; // mirrors threshold-backtest-run's 90-day window @@ -41,15 +50,27 @@ export function isKnobAutotuneEnabled(env: Env, knob: LoosenableKnob): boolean { return value === "1" || value === "true" || value === "on" || value === "yes"; } +/** Truthy-string env flag for `knob`'s TIGHTENING autonomy (#8225) — a separate, default-off var per + * direction, so tighten-autonomy is opted into independently of the loosening loop. Always false for a + * knob that declares no ladder. */ +export function isKnobTightenEnabled(env: Env, knob: LoosenableKnob): boolean { + if (!knob.tightening) return false; + const raw = (env as unknown as Record)[knob.tightening.autotuneEnvVar]; + const value = (typeof raw === "string" ? raw : "").trim().toLowerCase(); + return value === "1" || value === "true" || value === "on" || value === "yes"; +} + /** - * Read a knob's live override. Null (caller uses the shipped default) when: the knob's autotune flag is - * off, no override row exists, or the stored value fails validation — an override may only ever sit BELOW - * the shipped value and AT/ABOVE the hard minimum, so a corrupted/hand-edited row can never tighten the - * knob or loosen it past safety. Fail-safe null on any DB error. + * Read a knob's live override. Null (caller uses the shipped default) when no override row exists or the + * stored value fails DIRECTION-AWARE validation (#8225): a value BELOW shipped (a loosening) requires the + * loosening autotune flag and must sit at/above the hard minimum; a value ABOVE shipped (a tightening) + * requires a declared ladder AND its own tighten flag and must sit at/below the ladder's hard maximum. So + * flipping either direction's flag off instantly restores shipped behavior for that direction, and a + * corrupted/hand-edited row can never move the knob past either bound. Fail-safe null on any DB error. */ export async function getKnobOverride(env: Env, knob: LoosenableKnob): Promise { - if (!isKnobAutotuneEnabled(env, knob)) return null; - return readValidatedOverrideRow(env, knob, knob.overrideFlagKey); + if (!isKnobAutotuneEnabled(env, knob) && !isKnobTightenEnabled(env, knob)) return null; + return readValidatedOverrideRow(env, knob, knob.overrideFlagKey, { allowTightened: true }); } /** Per-repo override storage (#8216): one system_flags key per (knob, repo) beside the global key. The @@ -70,21 +91,34 @@ export function repoKnobOverrideFlagKey(knob: LoosenableKnob, repoFullName: stri * autotune flag gates EVERY scope — flipping it off restores shipped behavior everywhere instantly. */ export async function getKnobOverrideForRepo(env: Env, knob: LoosenableKnob, repoFullName: string | null): Promise { - if (!isKnobAutotuneEnabled(env, knob)) return null; + if (!isKnobAutotuneEnabled(env, knob) && !isKnobTightenEnabled(env, knob)) return null; if (repoFullName !== null) { - const repoValue = await readValidatedOverrideRow(env, knob, repoKnobOverrideFlagKey(knob, repoFullName)); + // Repo-scoped rows stay LOOSENING-ONLY (#8225): the tighten loop applies globally, so an above-shipped + // repo row has no legitimate writer and is rejected as corruption rather than honored. + const repoValue = await readValidatedOverrideRow(env, knob, repoKnobOverrideFlagKey(knob, repoFullName), { allowTightened: false }); if (repoValue !== null) return repoValue; } - return readValidatedOverrideRow(env, knob, knob.overrideFlagKey); + return readValidatedOverrideRow(env, knob, knob.overrideFlagKey, { allowTightened: true }); } -async function readValidatedOverrideRow(env: Env, knob: LoosenableKnob, key: string): Promise { +async function readValidatedOverrideRow(env: Env, knob: LoosenableKnob, key: string, opts: { allowTightened: boolean }): Promise { try { const row = await env.DB.prepare("SELECT value FROM system_flags WHERE key = ?").bind(key).first<{ value: string }>(); if (!row) return null; const parsed = Number(row.value); - if (!Number.isFinite(parsed) || parsed >= knob.shippedValue || parsed < knob.hardMinimum) return null; - return parsed; + if (!Number.isFinite(parsed)) return null; + // Loosening side: below shipped, at/above the hard minimum, and the loosening flag must be ON. + if (parsed < knob.shippedValue) { + return parsed >= knob.hardMinimum && isKnobAutotuneEnabled(env, knob) ? parsed : null; + } + // Tightening side (#8225): above shipped, at/below the ladder's hard maximum, ladder declared, its own + // flag ON, and only where the caller allows a tightened row. A value EQUAL to shipped is meaningless + // as an override and is rejected in both directions. + const ladder = knob.tightening; + if (opts.allowTightened && ladder && parsed > knob.shippedValue && parsed <= ladder.hardMaximum && isKnobTightenEnabled(env, knob)) { + return parsed; + } + return null; } catch { return null; } @@ -273,6 +307,82 @@ export async function runPerRepoKnobLoosening(env: Env, knob: LoosenableKnob, no return results; } +// ── Tightening apply path (#8225, epic #8211 track D) ──────────────────────────────────────────────────── + +export type KnobTighteningRunResult = + | { applied: false; reason: "no_ladder" | "report_only" | "flag_off" | "no_proposal" | "already_applied" } + | { applied: true; proposal: KnobTighteningProposal }; + +/** + * Evaluate and (when justified) apply a backtest-gated TIGHTENING of `knob` — the direction mirror of + * {@link runKnobLoosening}, with the same discipline transposed: only knobs declaring a ladder, only live + * knobs, only with the tighten flag ON, and the write path independently refuses anything that isn't a + * strict, bounded raise. Persists the same override row plus the ladder's own audit event type carrying + * both direction-aware split comparisons. Audit write is best-effort; the override write throws. + */ +export async function runKnobTightening(env: Env, knob: LoosenableKnob, nowMs: number = Date.now()): Promise { + const ladder = knob.tightening; + if (!ladder) return { applied: false, reason: "no_ladder" }; + if (knob.applyMode !== "live") return { applied: false, reason: "report_only" }; + if (!isKnobTightenEnabled(env, knob)) return { applied: false, reason: "flag_off" }; + + const currentValue = (await getKnobOverride(env, knob)) ?? knob.shippedValue; + if (currentValue >= ladder.hardMaximum) return { applied: false, reason: "already_applied" }; + + const { fired, overrides } = await createSignalStore(env).queryRuleHistory(knob.ruleId, nowMs - CORPUS_LOOKBACK_MS); + const proposal = evaluateKnobTightening(knob, buildBacktestCorpus(knob.ruleId, fired, overrides), currentValue); + if (!proposal) return { applied: false, reason: "no_proposal" }; + // Defense in depth: the write path independently refuses anything that isn't a strict, bounded raise. + if (proposal.proposedValue <= currentValue || proposal.proposedValue > ladder.hardMaximum) { + return { applied: false, reason: "no_proposal" }; + } + + await env.DB.prepare( + "INSERT INTO system_flags (key, value, updated_at) VALUES (?, ?, CURRENT_TIMESTAMP) ON CONFLICT(key) DO UPDATE SET value = excluded.value, updated_at = excluded.updated_at", + ) + .bind(knob.overrideFlagKey, String(proposal.proposedValue)) + .run(); + + await recordAuditEvent(env, { + eventType: ladder.eventType, + actor: "loopover", + targetKey: knob.ruleId, + outcome: "completed", + detail: `${knob.knobId} tightened ${proposal.currentValue} -> ${proposal.proposedValue} (backtest-gated, direction-aware: win axis up within the sacrifice budget)`, + metadata: { proposal }, + }).catch(() => undefined); + + return { applied: true, proposal }; +} + +/** The cron-tick wrapper for the tighten direction — one evaluation per laddered knob, failing SAFE; an + * applied step emits ONE structured error-level alert on the same notify path as the loosening wrapper. */ +export async function runScheduledKnobTightening(env: Env, knob: LoosenableKnob): Promise { + try { + const result = await runKnobTightening(env, knob); + if (result.applied) { + console.error( + JSON.stringify({ + level: "error", + event: "calibration_knob_tightened", + ev: knob.knobId, + at: new Date().toISOString(), + currentValue: result.proposal.currentValue, + proposedValue: result.proposal.proposedValue, + visibleCases: result.proposal.visibleCases, + heldOutCases: result.proposal.heldOutCases, + }), + ); + } + return result; + } catch (error) { + console.warn( + JSON.stringify({ level: "warn", event: "knob_tightening_tick_failed", ev: knob.knobId, error: error instanceof Error ? error.message : "unknown error" }), + ); + return null; + } +} + // ── Config-drift sentinel (#8213, epic #8211 track A) ──────────────────────────────────────────────────── /** Truthy-string flag for the drift sentinel — default off, so a deploy is byte-identical until opted in. */ @@ -352,6 +462,8 @@ export async function runConfigDriftSentinel(env: Env, knobs: readonly Loosenabl export type KnobAppliedEntry = { at: string; + /** Which direction's apply wrote this entry (#8225) — projected from the audit event type. */ + direction: "loosened" | "tightened"; currentValue: number | null; proposedValue: number | null; visibleCases: number | null; @@ -365,6 +477,8 @@ export type KnobRepoOverride = { repoFullName: string; value: number }; export type KnobStatus = { knobId: string; flagEnabled: boolean; + /** The tighten direction's own flag (#8225) — null for a knob that declares no tightening ladder. */ + tightenFlagEnabled: boolean | null; shippedValue: number; /** The value the live consumption actually uses right now: the validated override when the flag is on, * else the shipped constant. */ @@ -412,13 +526,18 @@ function verdictOrNull(value: unknown): string | null { */ export async function loadKnobStatus(env: Env, knob: LoosenableKnob): Promise { const flagEnabled = isKnobAutotuneEnabled(env, knob); + const tightenFlagEnabled = knob.tightening ? isKnobTightenEnabled(env, knob) : null; let storedOverride: number | null = null; try { const row = await env.DB.prepare("SELECT value FROM system_flags WHERE key = ?").bind(knob.overrideFlagKey).first<{ value: string }>(); if (row) { const parsed = Number(row.value); - if (Number.isFinite(parsed) && parsed < knob.shippedValue && parsed >= knob.hardMinimum) storedOverride = parsed; + // Direction-aware display bounds (#8225): a lingering row in EITHER direction is shown regardless of + // flag state — an operator must see what would take effect the moment the matching flag flips. + const loosened = parsed < knob.shippedValue && parsed >= knob.hardMinimum; + const tightened = knob.tightening !== undefined && parsed > knob.shippedValue && parsed <= knob.tightening.hardMaximum; + if (Number.isFinite(parsed) && (loosened || tightened)) storedOverride = parsed; } } catch { storedOverride = null; @@ -441,9 +560,13 @@ export async function loadKnobStatus(env: Env, knob: LoosenableKnob): Promise(); + // One history, both directions (#8225): a knob with no ladder binds its loosening type twice, which is + // an equality match — no behavior change for ladder-less knobs. + const tighteningEventType = knob.tightening?.eventType ?? knob.looseningEventType; + const rows = await env.DB.prepare("SELECT created_at, event_type, metadata_json FROM audit_events WHERE event_type IN (?, ?) ORDER BY created_at DESC LIMIT ?") + .bind(knob.looseningEventType, tighteningEventType, KNOB_STATUS_HISTORY_LIMIT) + .all<{ created_at: string; event_type: string; metadata_json: string }>(); /* v8 ignore next -- .all() over a live D1/TestD1 always yields a defined results array; the ?? [] guards * a future driver-shape change, mirroring loadSatisfactionFloorStatus's identical note. */ for (const row of rows.results ?? []) { @@ -479,6 +605,7 @@ export async function loadKnobStatus(env: Env, knob: LoosenableKnob): Promise> = Object.freeze({ @@ -88,6 +110,20 @@ export const LOOSENABLE_KNOBS: Readonly> = Object overrideFlagKey: "ai_review_close_confidence_override", looseningEventType: "calibration.ai_review_close_confidence_loosened", autotuneEnvVar: "AI_REVIEW_CLOSE_CONFIDENCE_AUTOTUNE_ENABLED", + // #8225's first tightening ladder: a HIGHER close-confidence bar means FEWER auto-closes — strictly + // caution-ward, the correct direction to trust first. Two steps, hard ceiling 0.97, tight sacrifice + // budget. + tightening: { + candidates: [0.95, 0.97], + hardMaximum: 0.97, + // Corpus polarity: the threshold classifier's positive class is "predicted reversed", so raising the + // bar catches MORE genuinely-reversed closes (recall, the win) at the risk of withholding good ones + // (precision, the bounded sacrifice — at most 5 points per slice). + mustImprove: "recall", + maxSacrifice: 0.05, + autotuneEnvVar: "AI_REVIEW_CLOSE_CONFIDENCE_TIGHTEN_ENABLED", + eventType: "calibration.ai_review_close_confidence_tightened", + }, }, }); @@ -138,6 +174,59 @@ export function evaluateKnobLoosening( return null; } +export type KnobTighteningProposal = { + knobId: string; + ruleId: string; + currentValue: number; + proposedValue: number; + visibleCases: number; + heldOutCases: number; + visible: BacktestComparison; + heldOut: BacktestComparison; +}; + +/** + * Evaluate whether `knob` can be justifiably TIGHTENED from `currentValue` (#8225) — the direction mirror + * of {@link evaluateKnobLoosening} with the orientation made explicit instead of reused blind: the smallest + * declared candidate ABOVE `currentValue` (never above the ladder's hard maximum) whose direction-aware + * verdict is strictly `"improved"` on the visible split AND non-`"regressed"` on the held-out split, where + * improved means the ladder's declared win axis strictly up and any cost on the other axis within its + * declared sacrifice bound. + * Same split seed/fraction and the same never-on-noise sample floors as the loosening side — held-out + * membership is identical for both directions of the same knob. Null when the knob declares no ladder, the + * corpus is too small, no candidate qualifies, or the current value already sits at/above the hard maximum. + * Pure and deterministic — same knob + corpus + value ⇒ same proposal. + */ +export function evaluateKnobTightening( + knob: LoosenableKnob, + cases: readonly BacktestCase[], + currentValue: number = knob.shippedValue, +): KnobTighteningProposal | null { + const ladder = knob.tightening; + if (!ladder) return null; + const { visible, heldOut } = splitBacktestCorpus(cases, knob.heldOutFraction, knob.splitSeed); + if (visible.length < knob.minVisibleCases || heldOut.length < knob.minHeldOutCases) return null; + + for (const candidate of ladder.candidates) { + if (candidate <= currentValue || candidate > ladder.hardMaximum) continue; + const visibleComparison = compareTighteningOnSlice(knob.ruleId, visible, currentValue, candidate, ladder); + if (visibleComparison.verdict !== "improved") continue; + const heldOutComparison = compareTighteningOnSlice(knob.ruleId, heldOut, currentValue, candidate, ladder); + if (heldOutComparison.verdict === "regressed") continue; + return { + knobId: knob.knobId, + ruleId: knob.ruleId, + currentValue, + proposedValue: candidate, + visibleCases: visible.length, + heldOutCases: heldOut.length, + visible: visibleComparison, + heldOut: heldOutComparison, + }; + } + return null; +} + export type KnobDriftDirection = "looser" | "tighter" | "shipped"; export type KnobDriftReport = { @@ -177,7 +266,9 @@ export function evaluateKnobDrift( const { visible, heldOut } = splitBacktestCorpus(cases, knob.heldOutFraction, knob.splitSeed); if (visible.length < knob.minVisibleCases || heldOut.length < knob.minHeldOutCases) return null; - const alternatives = [...new Set([knob.shippedValue, ...knob.candidates])] + // #8225: a declared tightening ladder joins the pool, so the sentinel's tighter findings and the tighten + // apply path judge the SAME candidate values (bounded by the ladder's own hard maximum via declaration). + const alternatives = [...new Set([knob.shippedValue, ...knob.candidates, ...(knob.tightening?.candidates ?? [])])] .filter((value) => value !== liveValue && value >= knob.hardMinimum) .sort((left, right) => Math.abs(left - liveValue) - Math.abs(right - liveValue) || right - left); @@ -206,3 +297,15 @@ function compareOnSlice(ruleId: string, slice: readonly BacktestCase[], currentV const proposed = scoreBacktest(ruleId, slice, buildConfidenceThresholdClassifier(candidate)); return compareBacktestScores(baseline, proposed); } + +function compareTighteningOnSlice( + ruleId: string, + slice: readonly BacktestCase[], + currentValue: number, + candidate: number, + ladder: KnobTighteningLadder, +): BacktestComparison { + const baseline = scoreBacktest(ruleId, slice, buildConfidenceThresholdClassifier(currentValue)); + const proposed = scoreBacktest(ruleId, slice, buildConfidenceThresholdClassifier(candidate)); + return compareDirectionalBacktestScores(baseline, proposed, { mustImprove: ladder.mustImprove, maxSacrifice: ladder.maxSacrifice }); +} diff --git a/test/helpers/d1.ts b/test/helpers/d1.ts index c16b53d8a..74a0364d6 100644 --- a/test/helpers/d1.ts +++ b/test/helpers/d1.ts @@ -89,6 +89,7 @@ export function createTestEnv(overrides: Partial = {}): Env { LOOPOVER_AUTO_FILE_DRIFT_ISSUES: "false", SATISFACTION_FLOOR_AUTOTUNE_ENABLED: "false", AI_REVIEW_CLOSE_CONFIDENCE_AUTOTUNE_ENABLED: "false", + AI_REVIEW_CLOSE_CONFIDENCE_TIGHTEN_ENABLED: "false", CONFIG_DRIFT_SENTINEL_ENABLED: "false", // Deliberately NOT "JSONbored/gittensory" (the old pre-rename repo name most test fixtures use as their // generic placeholder repoFullName) and NOT "JSONbored/loopover" (the real self-repo default) -- either diff --git a/test/unit/auto-tune.test.ts b/test/unit/auto-tune.test.ts index ebf6d6fb9..8df920124 100644 --- a/test/unit/auto-tune.test.ts +++ b/test/unit/auto-tune.test.ts @@ -11,6 +11,7 @@ import { maybeAutoClearHoldOnly, planAutoTune, planCloseAutoTune, + RISK_MERGE_PRECISION, shouldAutoClear, shouldAutoClearClose, } from "../../src/review/auto-tune"; @@ -464,3 +465,34 @@ describe("computeTuningRecommendations (#self-improve)", () => { expect(recs.map((r) => r.project)).toEqual(["alpha", "zeta"]); }); }); + +describe("T3 byte-stability pins (#8225 migration map)", () => { + it("the advisor tighten trigger's constants and payload stay byte-stable until its registry cutover lands", () => { + // #8225 migrated the MECHANISM (direction-aware registry tightening) but deliberately NOT this trigger: + // its target tunable (qualityGateMinScore) has no global shipped default to declare a knob around. Until + // that cutover is its own reviewed diff, the trigger's behavior is pinned here byte-for-byte. + expect(RISK_MERGE_PRECISION).toBe(0.9); + const row: GateEvalRow = { + project: "acme/widgets", + wouldMerge: 12, + mergeConfirmed: 10, + mergeFalse: 2, + wouldClose: 0, + closeConfirmed: 0, + closeFalse: 0, + hold: 0, + decided: 12, + mergePrecision: 10 / 12, + closePrecision: null, + weightedMergeConfirmed: 10, + weightedCloseConfirmed: 0, + weightedMergePrecision: 10 / 12, + weightedClosePrecision: null, + }; + const recs = computeTuningRecommendations({ rows: [row], hasSignal: true }); + const tighten = recs.find((rec) => rec.overridePayload !== undefined); + expect(tighten).toBeDefined(); + expect(tighten!.severity).toBe("warn"); + expect(tighten!.overridePayload).toEqual({ confidenceFloor: 0.95 }); // TIGHTEN_FLOOR_TARGET === READY bar + }); +}); diff --git a/test/unit/backtest-compare-engine.test.ts b/test/unit/backtest-compare-engine.test.ts index 9fe9e5cae..833c6dd77 100644 --- a/test/unit/backtest-compare-engine.test.ts +++ b/test/unit/backtest-compare-engine.test.ts @@ -4,7 +4,7 @@ import { describe, expect, it } from "vitest"; // 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-corpus-engine.test.ts / miner-deny-hook-synthesis.test.ts. -import { compareBacktestScores } from "../../packages/loopover-engine/src/calibration/backtest-compare"; +import { compareBacktestScores, compareDirectionalBacktestScores } from "../../packages/loopover-engine/src/calibration/backtest-compare"; import type { BacktestScoreReport } from "../../packages/loopover-engine/src/calibration/backtest-score"; function report(overrides: Partial = {}): BacktestScoreReport { @@ -71,3 +71,61 @@ describe("compareBacktestScores (#8086)", () => { ); }); }); + +describe("compareDirectionalBacktestScores (#8225)", () => { + const RECALL_WIN = { mustImprove: "recall" as const, maxSacrifice: 0.1 }; + + it("recall up with a WITHIN-BUDGET precision drop is improved — the sacrificed axis appears in NEITHER list", () => { + const comparison = compareDirectionalBacktestScores(report(), report({ recall: 0.7, precision: 0.45 }), RECALL_WIN); + expect(comparison.improvedAxes).toEqual(["recall"]); + expect(comparison.regressedAxes).toEqual([]); + expect(comparison.verdict).toBe("improved"); + }); + + it("an OVER-BUDGET sacrifice-axis drop is regressed, even with the win axis up", () => { + const comparison = compareDirectionalBacktestScores(report(), report({ recall: 0.9, precision: 0.3 }), RECALL_WIN); + expect(comparison.regressedAxes).toEqual(["precision"]); + expect(comparison.verdict).toBe("regressed"); + }); + + it("ANY drop on the win axis is regressed, whatever the other axis does", () => { + const comparison = compareDirectionalBacktestScores(report(), report({ recall: 0.49, precision: 0.9 }), RECALL_WIN); + expect(comparison.regressedAxes).toEqual(["recall"]); + expect(comparison.improvedAxes).toEqual(["precision"]); + expect(comparison.verdict).toBe("regressed"); + }); + + it("a lone sacrifice-axis gain is unchanged — no evidence the step earned its keep on the axis it exists to win", () => { + const comparison = compareDirectionalBacktestScores(report(), report({ precision: 0.9 }), RECALL_WIN); + expect(comparison.improvedAxes).toEqual(["precision"]); + expect(comparison.regressedAxes).toEqual([]); + expect(comparison.verdict).toBe("unchanged"); + }); + + it("a null win axis can never yield improved; a null sacrifice axis is excluded — unknown stays unknown", () => { + const nullWin = compareDirectionalBacktestScores(report({ recall: null }), report({ recall: 0.9, precision: 0.9 }), RECALL_WIN); + expect(nullWin.verdict).toBe("unchanged"); + const nullSacrifice = compareDirectionalBacktestScores(report(), report({ recall: 0.7, precision: null }), RECALL_WIN); + expect(nullSacrifice.improvedAxes).toEqual(["recall"]); + expect(nullSacrifice.verdict).toBe("improved"); + const equal = compareDirectionalBacktestScores(report(), report(), RECALL_WIN); + expect(equal.verdict).toBe("unchanged"); + }); + + it("mustImprove precision flips the sacrifice axis to recall (the rule-firing frame)", () => { + const comparison = compareDirectionalBacktestScores(report(), report({ precision: 0.7, recall: 0.42 }), { mustImprove: "precision", maxSacrifice: 0.1 }); + expect(comparison.improvedAxes).toEqual(["precision"]); + expect(comparison.regressedAxes).toEqual([]); + expect(comparison.verdict).toBe("improved"); + const overBudget = compareDirectionalBacktestScores(report(), report({ precision: 0.7, recall: 0.3 }), { mustImprove: "precision", maxSacrifice: 0.1 }); + expect(overBudget.verdict).toBe("regressed"); + }); + + it("throws on mismatched rules and on a negative or non-finite sacrifice bound — caller bugs, not comparisons", () => { + expect(() => compareDirectionalBacktestScores(report(), report({ ruleId: "other_rule" }), RECALL_WIN)).toThrow( + "cannot compare backtest scores for different rules: missing_linked_issue vs other_rule", + ); + expect(() => compareDirectionalBacktestScores(report(), report(), { mustImprove: "recall", maxSacrifice: -0.1 })).toThrow("maxSacrifice"); + expect(() => compareDirectionalBacktestScores(report(), report(), { mustImprove: "recall", maxSacrifice: Number.NaN })).toThrow("maxSacrifice"); + }); +}); diff --git a/test/unit/knob-loosening-run.test.ts b/test/unit/knob-loosening-run.test.ts index 4ac012ebd..2d0781a99 100644 --- a/test/unit/knob-loosening-run.test.ts +++ b/test/unit/knob-loosening-run.test.ts @@ -16,9 +16,12 @@ import { loadLiveKnobStatuses, runPerRepoKnobLoosening, PER_REPO_LOOSENING_MAX_REPOS_PER_TICK, + isKnobTightenEnabled, runConfigDriftSentinel, runKnobLoosening, + runKnobTightening, runScheduledKnobLoosening, + runScheduledKnobTightening, } from "../../src/services/knob-loosening-run"; import { SATISFACTION_FLOOR_LOOSENING_EVENT_TYPE, @@ -582,3 +585,231 @@ describe("loadKnobStatus / loadLiveKnobStatuses (#8161 generalized)", () => { expect((await loadLiveKnobStatuses(createTestEnv(), [reportOnly])).map((s) => s.knobId)).toEqual([]); }); }); + +// ── #8225: the tighten direction — separate flag, direction-aware storage, transposed apply path ──────────── + +const LADDER = AI_KNOB.tightening!; +const tightenEnv = (overrides: Partial = {}) => createTestEnv({ AI_REVIEW_CLOSE_CONFIDENCE_TIGHTEN_ENABLED: "true" as never, ...overrides }); + +// Band firings at 0.94 a human REVERSED: shipped 0.93 trusts them; the 0.95 raise catches them — recall up +// with precision held (every predicted-reversed case really was reversed). Same membership-probe technique +// as the loosening seeder, plus deep-low reversed anchors so precision has a denominator on both sides. +async function seedAiTighteningFriendlyHistory(env: Env): Promise { + const pool = Array.from({ length: 400 }, (_, i) => `acme/widgets#${i + 1}`); + const probe = pool.map((targetKey) => ({ + ruleId: AI_KNOB.ruleId, + targetKey, + outcome: "unaddressed", + label: "confirmed" as const, + firedAt: "2026-07-01T00:00:00.000Z", + decidedAt: "2026-07-02T00:00:00.000Z", + })); + const { visible, heldOut } = splitBacktestCorpus(probe, AI_KNOB.heldOutFraction, AI_KNOB.splitSeed); + const store = createSignalStore(env); + const now = Date.now(); + const keys = [ + ...visible.slice(0, AI_KNOB.minVisibleCases + 4).map((c) => c.targetKey), + ...heldOut.slice(0, AI_KNOB.minHeldOutCases + 2).map((c) => c.targetKey), + ]; + for (const [i, targetKey] of keys.entries()) { + await store.recordRuleFired({ + ruleId: AI_KNOB.ruleId, + targetKey, + outcome: "unaddressed", + occurredAt: new Date(now - 10_000 - i).toISOString(), + metadata: { confidence: 0.94 }, + }); + await store.recordHumanOverride({ ruleId: AI_KNOB.ruleId, targetKey, verdict: "reversed", occurredAt: new Date(now - i).toISOString() }); + } + for (const targetKey of [visible[AI_KNOB.minVisibleCases + 5]!.targetKey, heldOut[AI_KNOB.minHeldOutCases + 3]!.targetKey]) { + await store.recordRuleFired({ + ruleId: AI_KNOB.ruleId, + targetKey, + outcome: "unaddressed", + occurredAt: new Date(now - 20_000).toISOString(), + metadata: { confidence: 0.2 }, + }); + await store.recordHumanOverride({ ruleId: AI_KNOB.ruleId, targetKey, verdict: "reversed", occurredAt: new Date(now - 5000).toISOString() }); + } +} + +describe("isKnobTightenEnabled / direction-aware override read (#8225)", () => { + it("parses the ladder's own var; a ladder-less knob is ALWAYS off, whatever the env says", () => { + for (const value of ["1", "true", "on", "yes", " TRUE "]) { + expect(isKnobTightenEnabled({ AI_REVIEW_CLOSE_CONFIDENCE_TIGHTEN_ENABLED: value } as unknown as Env, AI_KNOB)).toBe(true); + } + for (const value of ["false", "0", "", undefined, 1 as unknown as string]) { + expect(isKnobTightenEnabled({ AI_REVIEW_CLOSE_CONFIDENCE_TIGHTEN_ENABLED: value } as unknown as Env, AI_KNOB)).toBe(false); + } + expect(isKnobTightenEnabled({ AI_REVIEW_CLOSE_CONFIDENCE_TIGHTEN_ENABLED: "true" } as unknown as Env, SATISFACTION_KNOB)).toBe(false); + }); + + it("a tightened row needs ITS flag: readable with tighten ON, shipped-inert with tighten OFF (loosening flag irrelevant)", async () => { + const env = tightenEnv(); + await setOverrideRow(env, AI_KNOB.overrideFlagKey, "0.95"); + expect(await getKnobOverride(env, AI_KNOB)).toBe(0.95); + + const offEnv = enabledEnv(); // loosening ON, tighten OFF — the tightened row must NOT act + await setOverrideRow(offEnv, AI_KNOB.overrideFlagKey, "0.95"); + expect(await getKnobOverride(offEnv, AI_KNOB)).toBeNull(); + }); + + it("direction validation rejects above-hard-maximum, equal-to-shipped, and a LOOSENED row when only tighten is on", async () => { + const env = tightenEnv(); + await setOverrideRow(env, AI_KNOB.overrideFlagKey, String(LADDER.hardMaximum + 0.01)); + expect(await getKnobOverride(env, AI_KNOB)).toBeNull(); + await setOverrideRow(env, AI_KNOB.overrideFlagKey, String(AI_KNOB.shippedValue)); + expect(await getKnobOverride(env, AI_KNOB)).toBeNull(); + await setOverrideRow(env, AI_KNOB.overrideFlagKey, "0.9"); // a loosening — needs the LOOSENING flag, which is off + expect(await getKnobOverride(env, AI_KNOB)).toBeNull(); + }); + + it("per-repo rows stay loosening-only: a tightened repo row is rejected and the global tightened row wins", async () => { + const env = tightenEnv(); + await setOverrideRow(env, repoKnobOverrideFlagKey(AI_KNOB, "acme/widgets"), "0.97"); + await setOverrideRow(env, AI_KNOB.overrideFlagKey, "0.95"); + expect(await getKnobOverrideForRepo(env, AI_KNOB, "acme/widgets")).toBe(0.95); + }); +}); + +describe("runKnobTightening (#8225)", () => { + it("refuses in order: no ladder, report-only, flag off — before any evaluation work", async () => { + expect(await runKnobTightening(tightenEnv(), SATISFACTION_KNOB)).toEqual({ applied: false, reason: "no_ladder" }); + const reportOnly = { ...AI_KNOB, applyMode: "report_only" as const }; + expect(await runKnobTightening(tightenEnv(), reportOnly)).toEqual({ applied: false, reason: "report_only" }); + expect(await runKnobTightening(createTestEnv(), AI_KNOB)).toEqual({ applied: false, reason: "flag_off" }); + }); + + it("returns no_proposal on an empty corpus and already_applied at the hard maximum", async () => { + expect(await runKnobTightening(tightenEnv(), AI_KNOB)).toEqual({ applied: false, reason: "no_proposal" }); + const env = tightenEnv(); + await setOverrideRow(env, AI_KNOB.overrideFlagKey, String(LADDER.hardMaximum)); + expect(await runKnobTightening(env, AI_KNOB)).toEqual({ applied: false, reason: "already_applied" }); + }); + + it("applies a backtest-cleared raise: writes the override row + the ladder's own audit event", async () => { + const env = tightenEnv(); + await seedAiTighteningFriendlyHistory(env); + const result = await runKnobTightening(env, AI_KNOB); + expect(result.applied).toBe(true); + if (!result.applied) throw new Error("unreachable"); + expect(result.proposal.proposedValue).toBe(0.95); + + expect(await getKnobOverride(env, AI_KNOB)).toBe(0.95); + const events = await env.DB.prepare("SELECT metadata_json FROM audit_events WHERE event_type = ?") + .bind(LADDER.eventType) + .all<{ metadata_json: string }>(); + expect(events.results).toHaveLength(1); + const proposal = (JSON.parse(events.results![0]!.metadata_json) as { proposal: { currentValue: number; proposedValue: number } }).proposal; + expect(proposal).toMatchObject({ currentValue: AI_KNOB.shippedValue, proposedValue: 0.95 }); + }); + + it("defense in depth: the write path independently refuses a non-raise or above-maximum proposal, whatever the evaluator claims", async () => { + const env = tightenEnv(); + const base = { + knobId: AI_KNOB.knobId, + ruleId: AI_KNOB.ruleId, + visibleCases: 60, + heldOutCases: 15, + visible: {} as never, + heldOut: {} as never, + }; + const spy = vi.spyOn(looseningKnobs, "evaluateKnobTightening"); + spy.mockReturnValueOnce({ ...base, currentValue: AI_KNOB.shippedValue, proposedValue: AI_KNOB.shippedValue }); + expect(await runKnobTightening(env, AI_KNOB)).toEqual({ applied: false, reason: "no_proposal" }); + spy.mockReturnValueOnce({ ...base, currentValue: AI_KNOB.shippedValue, proposedValue: LADDER.hardMaximum + 0.01 }); + expect(await runKnobTightening(env, AI_KNOB)).toEqual({ applied: false, reason: "no_proposal" }); + expect(await getKnobOverride(env, AI_KNOB)).toBeNull(); // nothing was written either time + }); + + it("the audit-event write is best-effort: a rejecting recordAuditEvent still applies the override (the catch arm)", async () => { + const env = tightenEnv(); + await seedAiTighteningFriendlyHistory(env); + const repositories = await import("../../src/db/repositories"); + vi.spyOn(repositories, "recordAuditEvent").mockRejectedValue(new Error("audit write down")); + const result = await runKnobTightening(env, AI_KNOB); + expect(result.applied).toBe(true); + expect(await getKnobOverride(env, AI_KNOB)).toBe(0.95); + }); +}); + +describe("runScheduledKnobTightening + tick wiring (#8225)", () => { + it("emits exactly ONE structured error-level alert on an applied step, none on the settled re-run", async () => { + const env = tightenEnv(); + await seedAiTighteningFriendlyHistory(env); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + const applied = await runScheduledKnobTightening(env, AI_KNOB); + expect(applied?.applied).toBe(true); + const alerts = errorSpy.mock.calls.map((call) => String(call[0])).filter((line) => line.includes("calibration_knob_tightened")); + expect(alerts).toHaveLength(1); + expect(alerts[0]).toContain('"ev":"ai_review_close_confidence"'); + + errorSpy.mockClear(); + const second = await runScheduledKnobTightening(env, AI_KNOB); // starts from the tightened value + expect(second?.applied).toBe(false); + expect(errorSpy.mock.calls.filter((call) => String(call[0]).includes("calibration_knob_tightened"))).toHaveLength(0); + }); + + it("fails SAFE on a thrown evaluation (Error and non-Error alike), never rethrowing into the queue", async () => { + const env = tightenEnv(); + env.DB = { prepare: () => { throw new Error("store down"); } } as never; + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); + expect(await runScheduledKnobTightening(env, AI_KNOB)).toBeNull(); + expect(warnSpy.mock.calls.some((call) => String(call[0]).includes("knob_tightening_tick_failed"))).toBe(true); + + const stringThrowEnv = tightenEnv(); + stringThrowEnv.DB = { prepare: () => { throw "string boom"; } } as never; + expect(await runScheduledKnobTightening(stringThrowEnv, AI_KNOB)).toBeNull(); + expect(warnSpy.mock.calls.some((call) => String(call[0]).includes('"error":"unknown error"'))).toBe(true); + }); + + it("the calibration tick runs the tighten loop ONLY under its own flag — loosening-only envs never tighten", async () => { + const looseningOnly = enabledEnv(); + await seedAiTighteningFriendlyHistory(looseningOnly); + await processJob(looseningOnly, { type: "satisfaction-floor-loosening", requestedBy: "schedule" }); + expect(await getKnobOverride(tightenEnv({ DB: looseningOnly.DB }), AI_KNOB)).toBeNull(); + + const tightenOn = tightenEnv(); + await seedAiTighteningFriendlyHistory(tightenOn); + await processJob(tightenOn, { type: "satisfaction-floor-loosening", requestedBy: "schedule" }); + expect(await getKnobOverride(tightenOn, AI_KNOB)).toBe(0.95); + }); +}); + +describe("loadKnobStatus with a tightening ladder (#8225)", () => { + it("reports tightenFlagEnabled (null without a ladder), a lingering tightened row flag-OFF, and the tightened live value flag-ON", async () => { + const env = tightenEnv(); + await setOverrideRow(env, AI_KNOB.overrideFlagKey, "0.95"); + const status = await loadKnobStatus(env, AI_KNOB); + expect(status.tightenFlagEnabled).toBe(true); + expect(status.storedOverride).toBe(0.95); + expect(status.liveValue).toBe(0.95); + + const offEnv = createTestEnv(); + await setOverrideRow(offEnv, AI_KNOB.overrideFlagKey, "0.95"); + const offStatus = await loadKnobStatus(offEnv, AI_KNOB); + expect(offStatus.tightenFlagEnabled).toBe(false); + expect(offStatus.storedOverride).toBe(0.95); // the lingering row stays visible + expect(offStatus.liveValue).toBe(AI_KNOB.shippedValue); // but does not act + + expect((await loadKnobStatus(createTestEnv(), SATISFACTION_KNOB)).tightenFlagEnabled).toBeNull(); + }); + + it("projects BOTH directions into one history with the direction tag", async () => { + const env = tightenEnv(); + await seedAiTighteningFriendlyHistory(env); + expect((await runKnobTightening(env, AI_KNOB)).applied).toBe(true); + await recordAuditEvent(env, { + eventType: AI_KNOB.looseningEventType, + actor: "loopover", + targetKey: AI_KNOB.ruleId, + outcome: "completed", + detail: "fixture loosening", + metadata: { proposal: { currentValue: 0.93, proposedValue: 0.9, visibleCases: 60, heldOutCases: 15 } }, + }); + const status = await loadKnobStatus(env, AI_KNOB); + expect(status.applied.map((entry) => entry.direction).sort()).toEqual(["loosened", "tightened"]); + const tightened = status.applied.find((entry) => entry.direction === "tightened")!; + expect(tightened.proposedValue).toBe(0.95); + }); +}); diff --git a/test/unit/loosening-knobs.test.ts b/test/unit/loosening-knobs.test.ts index 174b2d141..ff33e7e79 100644 --- a/test/unit/loosening-knobs.test.ts +++ b/test/unit/loosening-knobs.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { splitBacktestCorpus, type BacktestCase } from "@loopover/engine"; -import { evaluateKnobDrift, evaluateKnobLoosening, LOOSENABLE_KNOBS, type LoosenableKnob } from "../../src/services/loosening-knobs"; +import { evaluateKnobDrift, evaluateKnobLoosening, evaluateKnobTightening, LOOSENABLE_KNOBS, type LoosenableKnob } from "../../src/services/loosening-knobs"; import { SATISFACTION_FLOOR_HARD_MINIMUM, SATISFACTION_FLOOR_HELD_OUT_FRACTION, @@ -59,6 +59,21 @@ describe("LOOSENABLE_KNOBS registry invariants (#8159)", () => { expect(knob.minHeldOutCases).toBeGreaterThan(0); expect(["live", "report_only"]).toContain(knob.applyMode); } + for (const knob of knobs) { + // #8225: a declared tightening ladder must be structurally safe — ascending candidates strictly + // above shipped, none past the hard maximum, its own distinct env var and event type. + if (!knob.tightening) continue; + expect(knob.tightening.candidates.length).toBeGreaterThan(0); + for (const candidate of knob.tightening.candidates) { + expect(candidate).toBeGreaterThan(knob.shippedValue); + expect(candidate).toBeLessThanOrEqual(knob.tightening.hardMaximum); + } + expect([...knob.tightening.candidates].sort((a, b) => a - b)).toEqual([...knob.tightening.candidates]); // nearest-first + expect(knob.tightening.maxSacrifice).toBeGreaterThanOrEqual(0); + expect(["precision", "recall"]).toContain(knob.tightening.mustImprove); + expect(knob.tightening.autotuneEnvVar).not.toBe(knob.autotuneEnvVar); + expect(knob.tightening.eventType).not.toBe(knob.looseningEventType); + } expect(new Set(knobs.map((knob) => knob.knobId)).size).toBe(knobs.length); expect(new Set(knobs.map((knob) => knob.splitSeed)).size).toBe(knobs.length); for (const [key, knob] of Object.entries(LOOSENABLE_KNOBS)) expect(key).toBe(knob.knobId); @@ -121,6 +136,85 @@ describe("evaluateKnobLoosening on the close-confidence knob (#8159)", () => { }); }); +// ── #8225: the direction mirror — can the knob be justifiably TIGHTENED? ──────────────────────────────────── + +function aiTighteningFriendlyCorpus(): BacktestCase[] { + // Band firings at 0.94 a human REVERSED: shipped 0.93 trusts them (missed reversals — false negatives); + // the first tightening candidate 0.95 catches them — recall improves, precision holds at 1 (every + // predicted-reversed case really was reversed), so the sacrifice budget is untouched. + const cases: BacktestCase[] = []; + for (const key of visibleKeys.slice(0, AI_KNOB.minVisibleCases + 6)) cases.push(aiCase(key, 0.94, "reversed")); + for (const key of heldOutKeys.slice(0, AI_KNOB.minHeldOutCases + 3)) cases.push(aiCase(key, 0.94, "reversed")); + // Deep-low reversed anchors keep a true positive on both sides of every comparison. + cases.push(aiCase(visibleKeys[AI_KNOB.minVisibleCases + 10]!, 0.5, "reversed")); + cases.push(aiCase(heldOutKeys[AI_KNOB.minHeldOutCases + 6]!, 0.5, "reversed")); + return cases; +} + +describe("evaluateKnobTightening (#8225)", () => { + it("proposes the smallest ladder step with full evidence when both splits support it under the declared orientation", () => { + const proposal = evaluateKnobTightening(AI_KNOB, aiTighteningFriendlyCorpus()); + expect(proposal).not.toBeNull(); + expect(proposal!.knobId).toBe("ai_review_close_confidence"); + expect(proposal!.currentValue).toBe(DEFAULT_AI_REVIEW_CLOSE_CONFIDENCE); + expect(proposal!.proposedValue).toBe(0.95); + expect(proposal!.visible.verdict).toBe("improved"); + expect(proposal!.heldOut.verdict).not.toBe("regressed"); + }); + + it("returns null for a knob that declares no ladder — tightening is opt-in per knob, never implied", () => { + expect(evaluateKnobTightening(LOOSENABLE_KNOBS.satisfaction_floor!, aiTighteningFriendlyCorpus())).toBeNull(); + }); + + it("never tightens on a sample below the knob's floors", () => { + const thin = [ + ...visibleKeys.slice(0, AI_KNOB.minVisibleCases - 1).map((key) => aiCase(key, 0.94, "reversed")), + ...heldOutKeys.slice(0, AI_KNOB.minHeldOutCases + 3).map((key) => aiCase(key, 0.94, "reversed")), + ]; + expect(evaluateKnobTightening(AI_KNOB, thin)).toBeNull(); + }); + + it("refuses to step above the hard maximum even from an already-tightened current value", () => { + expect(evaluateKnobTightening(AI_KNOB, aiTighteningFriendlyCorpus(), AI_KNOB.tightening!.hardMaximum)).toBeNull(); + }); + + it("skips candidates at/below the current value: from 0.95 only 0.97 is a raise", () => { + // Band moves to 0.96 so the remaining raise (0.97) is the one the evidence supports. + const cases: BacktestCase[] = []; + for (const key of visibleKeys.slice(0, AI_KNOB.minVisibleCases + 6)) cases.push(aiCase(key, 0.96, "reversed")); + for (const key of heldOutKeys.slice(0, AI_KNOB.minHeldOutCases + 3)) cases.push(aiCase(key, 0.96, "reversed")); + cases.push(aiCase(visibleKeys[AI_KNOB.minVisibleCases + 10]!, 0.5, "reversed")); + cases.push(aiCase(heldOutKeys[AI_KNOB.minHeldOutCases + 6]!, 0.5, "reversed")); + const proposal = evaluateKnobTightening(AI_KNOB, cases, 0.95); + expect(proposal).not.toBeNull(); + expect(proposal!.proposedValue).toBe(0.97); + }); + + it("returns null when no candidate improves the win axis (uniform confirmed corpus)", () => { + const uniform = [ + ...visibleKeys.slice(0, AI_KNOB.minVisibleCases + 6).map((key) => aiCase(key, 0.99, "confirmed")), + ...heldOutKeys.slice(0, AI_KNOB.minHeldOutCases + 3).map((key) => aiCase(key, 0.99, "confirmed")), + ]; + expect(evaluateKnobTightening(AI_KNOB, uniform)).toBeNull(); + }); + + it("returns null when the visible split improves but the held-out split regresses (the transposed Pareto floor holds)", () => { + // Visible: reversed band at 0.94 (a raise wins). Held-out: CONFIRMED at 0.94 — the raise misflags them, + // an over-budget precision sacrifice on that slice. + const cases: BacktestCase[] = []; + for (const key of visibleKeys.slice(0, AI_KNOB.minVisibleCases + 6)) cases.push(aiCase(key, 0.94, "reversed")); + for (const key of heldOutKeys.slice(0, AI_KNOB.minHeldOutCases + 3)) cases.push(aiCase(key, 0.94, "confirmed")); + cases.push(aiCase(visibleKeys[AI_KNOB.minVisibleCases + 10]!, 0.5, "reversed")); + cases.push(aiCase(heldOutKeys[AI_KNOB.minHeldOutCases + 6]!, 0.5, "reversed")); + expect(evaluateKnobTightening(AI_KNOB, cases)).toBeNull(); + }); + + it("is deterministic: the same corpus and current value always produce the same proposal", () => { + const corpus = aiTighteningFriendlyCorpus(); + expect(evaluateKnobTightening(AI_KNOB, corpus)).toEqual(evaluateKnobTightening(AI_KNOB, corpus)); + }); +}); + describe("buildKnobReliabilityRecs (#8227)", () => { const status = (over: Partial<{ knobId: string; liveValue: number; reliability: { suggestion: number | null } | null }> = {}) => ({ knobId: "ai_review_close_confidence", @@ -259,6 +353,24 @@ describe("evaluateKnobDrift on the close-confidence knob (#8212)", () => { expect(report!.direction).toBe("tighter"); }); + it("#8225: a declared tightening ladder joins the drift pool — a tighter NON-shipped candidate can now dominate from shipped", () => { + // Reversed band at 0.94: live 0.93 misses them; the ladder's 0.95 catches them (recall up, precision + // held) — before #8225 the pool ended at shipped, so this stale-config shape was invisible from 0.93. + const cases: BacktestCase[] = []; + for (const key of visibleKeys.slice(0, AI_KNOB.minVisibleCases + 6)) cases.push(aiCase(key, 0.94, "reversed")); + for (const key of heldOutKeys.slice(0, AI_KNOB.minHeldOutCases + 3)) cases.push(aiCase(key, 0.94, "reversed")); + cases.push(aiCase(visibleKeys[AI_KNOB.minVisibleCases + 10]!, 0.5, "reversed")); + cases.push(aiCase(heldOutKeys[AI_KNOB.minHeldOutCases + 6]!, 0.5, "reversed")); + const report = evaluateKnobDrift(AI_KNOB, cases); + expect(report).not.toBeNull(); + expect(report!.direction).toBe("tighter"); + expect(report!.dominatingValue).toBe(0.95); + + // A ladder-less twin keeps the pre-#8225 pool: 0.95 is invisible, so this corpus reports nothing. + const { tightening: _dropped, ...ladderless } = AI_KNOB; + expect(evaluateKnobDrift(ladderless as LoosenableKnob, cases)).toBeNull(); + }); + it("is deterministic: the same corpus and live value always produce the same report", () => { expect(JSON.stringify(evaluateKnobDrift(AI_KNOB, aiLooseningFriendlyCorpus()))).toBe( JSON.stringify(evaluateKnobDrift(AI_KNOB, aiLooseningFriendlyCorpus())), diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index d5fdd90e6..0d35a11ba 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,5 +1,5 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types` (hash: 63dada085784011bb62d8a1c75f64252) +// Generated by Wrangler by running `wrangler types` (hash: 3fbc9150a68bb0e93fbf6bf660ccf637) // Runtime types generated with workerd@1.20260701.1 2026-05-28 nodejs_compat interface __BaseEnv_Env { REVIEW_AUDIT: R2Bucket; @@ -14,6 +14,7 @@ interface __BaseEnv_Env { SCORING_TIME_DECAY_ENABLED: "true"; SATISFACTION_FLOOR_AUTOTUNE_ENABLED: "false"; AI_REVIEW_CLOSE_CONFIDENCE_AUTOTUNE_ENABLED: "false"; + AI_REVIEW_CLOSE_CONFIDENCE_TIGHTEN_ENABLED: "false"; CONFIG_DRIFT_SENTINEL_ENABLED: "false"; LOOPOVER_AUTO_FILE_DRIFT_ISSUES: "false"; LOOPOVER_DRIFT_ISSUE_REPO: "JSONbored/loopover"; @@ -71,6 +72,7 @@ declare namespace NodeJS { interface ProcessEnv extends StringifyValues