diff --git a/src/review/auto-apply.ts b/src/review/auto-apply.ts index d1776aea6..33e196659 100644 --- a/src/review/auto-apply.ts +++ b/src/review/auto-apply.ts @@ -382,11 +382,12 @@ export interface AutoApplyContext { baseScopeCap?: { files: number; lines: number }; /** This project's decided-sample count from the gate eval (drives the promotion evidence gate). */ decided: number; - /** This project's freshly-computed merge precision from THIS tick's gate eval (the same field - * computeTuningRecommendations reads). Threaded into evaluateShadowPromotion so a shadow-queued tightening - * cannot be promoted once the precision that originally warranted it has since recovered. Optional/nullable - * because a project can have no would-merge samples yet (GateEvalRow.mergePrecision is null in that case). */ - mergePrecision?: number | null; + /** This project's freshly-computed reversal-WEIGHTED merge precision from THIS tick's gate eval (the same + * field computeTuningRecommendations gates on, #10014). Threaded into evaluateShadowPromotion so a + * shadow-queued tightening cannot be promoted once the weighted precision that originally warranted it has + * since recovered -- feeding the RAW number here would clear a hold the reversal-weighted breaker still + * holds. Optional/nullable because a project can have no would-merge samples yet (it is null in that case). */ + weightedMergePrecision?: number | null; /** The tuning advisor's recommendations for this project (only ones with an overridePayload are applied). */ recs: TuningRec[]; /** Current wall-clock (ms) — injected for determinism in tests. */ @@ -429,7 +430,7 @@ export async function runAutoApplyRecommendations(env: StorageEnv, ctx: AutoAppl decided: ctx.decided, validatedUntilIso: shadow.validatedUntil, nowIso, - ...(ctx.mergePrecision !== undefined ? { currentMergePrecision: ctx.mergePrecision } : {}), + ...(ctx.weightedMergePrecision !== undefined ? { currentMergePrecision: ctx.weightedMergePrecision } : {}), }); if (gate.promote) { // Audit BEFORE the mutation — see applyOverrideRecommendation's force branch for why this ordering diff --git a/src/review/auto-tune.ts b/src/review/auto-tune.ts index d6cf730dd..8e27658b8 100644 --- a/src/review/auto-tune.ts +++ b/src/review/auto-tune.ts @@ -196,7 +196,9 @@ export interface CloseAutoTuneAction { export function planCloseAutoTune(report: GateEvalReport): CloseAutoTuneAction[] { const actions: CloseAutoTuneAction[] = []; for (const r of report.rows) { - if (r.wouldClose < AUTOTUNE_MIN_DECIDED || r.weightedClosePrecision == null) continue; + // #10014: null-check FIRST, mirroring planAutoTune -- weightedClosePrecision is non-null iff wouldClose > 0, + // so once wouldClose >= 10 the old trailing `== null` disjunct was unreachable (a dead branch arm). + if (r.weightedClosePrecision == null || r.wouldClose < AUTOTUNE_MIN_DECIDED) continue; if (r.weightedClosePrecision < AUTOTUNE_CLOSE_PRECISION_FLOOR) { actions.push({ project: r.project, @@ -298,37 +300,44 @@ const pct = (x: number | null): string => (x == null ? "—" : `${Math.round(x * export function computeTuningRecommendations(report: GateEvalReport): TuningRec[] { const recs: TuningRec[] = []; for (const r of report.rows) { - if (r.decided < MIN_DECIDED) { - recs.push({ project: r.project, severity: "info", message: `Only ${r.decided} decided PR(s) — collect more shadow data before judging accuracy or flipping live.` }); + // #10014: gate on wouldMerge, not decided, matching planAutoTune's documented rule -- precision is measured + // over WOULD-MERGE predictions, so a project of many holds + one wrong would-merge (9 holds + 1) is a + // statistically meaningless sample the breaker already refuses; gating on `decided` let it clear the floor. + if (r.wouldMerge < MIN_DECIDED) { + recs.push({ project: r.project, severity: "info", message: `Only ${r.wouldMerge} would-merge PR(s) — collect more shadow data before judging accuracy or flipping live.` }); continue; } let flagged = false; - // The dangerous error: would auto-merge something the human closed. - if (r.mergePrecision != null && r.mergePrecision < RISK_MERGE_PRECISION) { + // The dangerous error: would auto-merge something the human closed. #10014: read the reversal-WEIGHTED + // precision the breaker gates on (weightedMergePrecision is non-null iff wouldMerge > 0), so a project whose + // merges are systematically reverted -- weighted at ~0 while raw stays healthy -- is flagged, not silent. + if (r.weightedMergePrecision != null && r.weightedMergePrecision < RISK_MERGE_PRECISION) { recs.push({ project: r.project, severity: "warn", - message: `Would have auto-merged ${r.mergeFalse} PR(s) the human CLOSED (merge precision ${pct(r.mergePrecision)} over ${r.wouldMerge}). Tighten guardrails / raise the confidence floor — do NOT flip live yet.`, + message: `Would have auto-merged ${r.mergeFalse} PR(s) the human CLOSED (weighted merge precision ${pct(r.weightedMergePrecision)} over ${r.wouldMerge}). Tighten guardrails / raise the confidence floor — do NOT flip live yet.`, // Auto-applicable TIGHTENING: raise the floor to the ready bar. Strictly safe-ward (a higher floor can // only HOLD more would-merges, never add a bad one), so the apply path can promote it. (#275) overridePayload: { confidenceFloor: TIGHTEN_FLOOR_TARGET }, }); flagged = true; } - // The other error: would auto-close something the human merged. - if (r.closeFalse > 0) { + // The other error: would auto-close something the human merged. #10014: gate on the weighted close precision + // against the same floor the close breaker uses, not the raw closeFalse count (which the weighted number + // discounts). closeFalse is still named in the message, but no longer the condition. + if (r.weightedClosePrecision != null && r.weightedClosePrecision < AUTOTUNE_CLOSE_PRECISION_FLOOR) { recs.push({ project: r.project, severity: "warn", - message: `Would have auto-closed ${r.closeFalse} PR(s) the human MERGED (close precision ${pct(r.closePrecision)}). Loosen the area/scope rules before going live.`, + message: `Would have auto-closed ${r.closeFalse} PR(s) the human MERGED (weighted close precision ${pct(r.weightedClosePrecision)}). Loosen the area/scope rules before going live.`, }); flagged = true; } - if (!flagged && r.mergePrecision != null && r.mergePrecision >= READY_MERGE_PRECISION && (r.closePrecision == null || r.closePrecision >= READY_CLOSE_PRECISION)) { + if (!flagged && r.weightedMergePrecision != null && r.weightedMergePrecision >= READY_MERGE_PRECISION && (r.weightedClosePrecision == null || r.weightedClosePrecision >= READY_CLOSE_PRECISION)) { recs.push({ project: r.project, severity: "good", - message: `Merge precision ${pct(r.mergePrecision)} over ${r.decided} decided PR(s) with no false closes — looks ready to flip live (shadow:false).`, + message: `Merge precision ${pct(r.weightedMergePrecision)} over ${r.decided} decided PR(s) with no false closes — looks ready to flip live (shadow:false).`, }); } } diff --git a/test/unit/auto-apply.test.ts b/test/unit/auto-apply.test.ts index 131995c5e..1328582fb 100644 --- a/test/unit/auto-apply.test.ts +++ b/test/unit/auto-apply.test.ts @@ -381,7 +381,7 @@ describe("runAutoApplyRecommendations (#278 — closes the loop: queue tightenin it("refuses to promote a stale shadow tightening once the project's precision has since recovered", async () => { const { env, tables } = fakeEnv(); tables.shadow.set("g", { confidence_floor: 0.95, scope_cap_files: null, scope_cap_lines: null, validated_until: "2026-06-19T00:00:00Z" }); - await runAutoApplyRecommendations(env, ctx({ recs: [], mergePrecision: 0.92 })); + await runAutoApplyRecommendations(env, ctx({ recs: [], weightedMergePrecision: 0.92 })); expect(tables.live.has("g")).toBe(false); // NOT promoted expect(tables.shadow.has("g")).toBe(true); // stays queued rather than being silently dropped }); @@ -389,7 +389,7 @@ describe("runAutoApplyRecommendations (#278 — closes the loop: queue tightenin it("still promotes a soaked shadow override when the fresh precision has NOT recovered", async () => { const { env, tables } = fakeEnv(); tables.shadow.set("g", { confidence_floor: 0.95, scope_cap_files: null, scope_cap_lines: null, validated_until: "2026-06-19T00:00:00Z" }); - await runAutoApplyRecommendations(env, ctx({ recs: [], mergePrecision: 0.5 })); + await runAutoApplyRecommendations(env, ctx({ recs: [], weightedMergePrecision: 0.5 })); expect(tables.live.get("g")?.confidence_floor).toBe(0.95); expect(tables.shadow.has("g")).toBe(false); }); diff --git a/test/unit/auto-tune.test.ts b/test/unit/auto-tune.test.ts index 8df920124..df43b6c10 100644 --- a/test/unit/auto-tune.test.ts +++ b/test/unit/auto-tune.test.ts @@ -237,6 +237,12 @@ describe("planCloseAutoTune (#close-precision-breaker) — tightening-only, clos it("does NOT engage when close precision is null (no would-close predictions with a known outcome)", () => { expect(planCloseAutoTune(report([row({ project: "p", decided: 30, wouldClose: 0, closeConfirmed: 0, closePrecision: null })]))).toHaveLength(0); }); + it("#10014: the reordered guard's null arm is reachable — null weighted precision with wouldClose >= 10 skips (not a crash on the <-comparison)", () => { + // Before the reorder the `wouldClose < 10` arm short-circuited first, so once wouldClose >= 10 the trailing + // `== null` disjunct was dead. Null-checking FIRST (mirroring planAutoTune) makes this arm reachable: a + // null weighted precision at wouldClose=20 continues (no action), never reaching the `< FLOOR` comparison. + expect(planCloseAutoTune(report([row({ project: "p", decided: 30, wouldClose: 20, closeConfirmed: 0, closePrecision: 0.5, weightedClosePrecision: null })]))).toHaveLength(0); + }); it("does NOT engage when close precision is healthy (it only ever tightens, never loosens)", () => { expect(planCloseAutoTune(report([row({ project: "p", decided: 30, wouldClose: 30, closeConfirmed: 29, closePrecision: 0.97 })]))).toHaveLength(0); }); @@ -421,15 +427,25 @@ describe("computeTuningRecommendations (#self-improve)", () => { expect(recs[0]?.severity).toBe("warn"); }); - it("renders an em-dash for a NULL close precision in the loosen-warn message (pct null branch)", () => { - // closeFalse > 0 but closePrecision is null (e.g. no would-close predictions had a known outcome): the - // loosen-warn message interpolates pct(null) → "—" rather than a percentage. Exercises pct's null side. + it("#10014: the loosen-warn fires on a weighted close precision below the floor, not on a raw closeFalse count", () => { + // The close-side gate now reads weightedClosePrecision against AUTOTUNE_CLOSE_PRECISION_FLOOR (0.85), the + // same reversal-discounted evidence the close breaker uses -- not the raw closeFalse count the weighted + // number discounts. A below-floor weighted precision emits a loosen-warn naming that weighted percentage. const recs = computeTuningRecommendations( - report([row({ project: "p", decided: 15, wouldMerge: 10, mergeConfirmed: 10, mergePrecision: 1.0, closeFalse: 2, closePrecision: null })]), + report([row({ project: "p", decided: 15, wouldMerge: 10, mergeConfirmed: 10, mergePrecision: 1.0, weightedMergePrecision: 1.0, closeFalse: 2, wouldClose: 6, closePrecision: 0.9, weightedClosePrecision: 0.2 })]), ); const loosen = recs.find((r) => r.severity === "warn" && /loosen/i.test(r.message)); expect(loosen).toBeDefined(); - expect(loosen?.message).toContain("close precision —"); + expect(loosen?.message).toContain("weighted close precision 20%"); + }); + + it("#10014: a NULL weighted close precision no longer triggers a loosen-warn (the raw closeFalse path is gone)", () => { + // Previously closeFalse > 0 with a null closePrecision emitted a loosen-warn; now a null weighted close + // precision (no would-close outcomes) fires nothing, so a merge-clean project reads as ready, not held. + const recs = computeTuningRecommendations( + report([row({ project: "p", decided: 15, wouldMerge: 12, mergeConfirmed: 12, mergePrecision: 1.0, weightedMergePrecision: 1.0, closeFalse: 2, closePrecision: null, weightedClosePrecision: null })]), + ); + expect(recs.find((r) => r.severity === "warn" && /loosen/i.test(r.message))).toBeUndefined(); }); it("says READY when close precision is also high and non-null (ready close-precision threshold branch)", () => { @@ -444,11 +460,12 @@ describe("computeTuningRecommendations (#self-improve)", () => { expect(recs[0]?.message).toMatch(/ready to flip live/i); }); - it("does NOT say ready when a non-null close precision is below the ready bar (close arm fails)", () => { - // Same ready conditions but closePrecision below READY_CLOSE_PRECISION (0.9) and no false closes → neither - // warn nor good fires, so the project yields no recommendation. Asserts the close arm gates 'good'. + it("#10014: does NOT say ready when the weighted close precision is below the ready bar (close arm gates 'good' on the weighted field)", () => { + // High merge precision but weightedClosePrecision at 0.88 — below the 0.9 ready bar yet at/above the 0.85 + // loosen floor, so NEITHER the good arm nor the loosen-warn fires: the project yields no recommendation. + // Proves the ready guard's close arm now reads weightedClosePrecision, not the raw closePrecision. const recs = computeTuningRecommendations( - report([row({ project: "borderline", decided: 20, wouldMerge: 18, mergeConfirmed: 18, mergePrecision: 1.0, wouldClose: 4, closeConfirmed: 3, closeFalse: 0, closePrecision: 0.8 })]), + report([row({ project: "borderline", decided: 20, wouldMerge: 18, mergeConfirmed: 18, mergePrecision: 1.0, weightedMergePrecision: 1.0, wouldClose: 4, closeConfirmed: 3, closeFalse: 0, closePrecision: 0.95, weightedClosePrecision: 0.88 })]), ); expect(recs).toHaveLength(0); }); @@ -464,6 +481,31 @@ describe("computeTuningRecommendations (#self-improve)", () => { ); expect(recs.map((r) => r.project)).toEqual(["alpha", "zeta"]); }); + + it("REGRESSION #10014: the sample gate reads wouldMerge, not decided — 9 holds + 1 wrong would-merge is info-only, no overridePayload", () => { + // The exact shape planAutoTune's comment names: decided=10 clears the old `decided < MIN_DECIDED` gate, but + // it is 9 holds + 1 would-merge — a statistically meaningless sample the breaker already refuses. Under the + // fix it gates on wouldMerge (1 < 10) and emits an INFO rec with NO auto-applicable overridePayload, rather + // than a warn that queued a live confidence-floor raise off a single prediction. + const recs = computeTuningRecommendations( + report([row({ project: "o/r", decided: 10, wouldMerge: 1, mergeFalse: 1, mergePrecision: 0, weightedMergePrecision: 0, hold: 9 })]), + ); + expect(recs).toHaveLength(1); + expect(recs[0]?.severity).toBe("info"); + expect(recs[0]?.overridePayload).toBeUndefined(); + expect(recs[0]?.message).toContain("would-merge"); + }); + + it("REGRESSION #10014: a healthy RAW merge precision with a failing WEIGHTED one still warns with a tightening overridePayload", () => { + // A project whose merges are systematically reverted: mergePrecision 0.98 looks healthy, but the + // reversal-weighted precision the breaker gates on is 0.2. The advisor must read the weighted field and flag + // it (with the auto-applicable tightening payload), not stay silent on the raw number the breaker distrusts. + const recs = computeTuningRecommendations( + report([row({ project: "o/r", decided: 20, wouldMerge: 20, mergeConfirmed: 20, mergeFalse: 0, mergePrecision: 0.98, weightedMergePrecision: 0.2 })]), + ); + const warn = recs.find((r) => r.severity === "warn"); + expect(warn?.overridePayload).toEqual({ confidenceFloor: 0.95 }); + }); }); describe("T3 byte-stability pins (#8225 migration map)", () => {