Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions src/review/auto-apply.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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
Expand Down
31 changes: 20 additions & 11 deletions src/review/auto-tune.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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).`,
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/unit/auto-apply.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -381,15 +381,15 @@ 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
});

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);
});
Expand Down
60 changes: 51 additions & 9 deletions test/unit/auto-tune.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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)", () => {
Expand All @@ -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);
});
Expand All @@ -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)", () => {
Expand Down