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
60 changes: 42 additions & 18 deletions src/review/auto-tune.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ export interface GateEvalRow {
decided: number; // predictions that have a known outcome
mergePrecision: number | null;
closePrecision: number | null;
// #2348: reversal-discounted variants (src/review/parity.ts's own REVERSAL_DISCOUNT_WEIGHT — a merge/close
// later marked reversal_reverted/reversal_reopened has its confirmed-credit discounted). Denominators
// (wouldMerge/wouldClose) are unchanged, so these are always <= the raw precisions above. The circuit-
// breaker below gates on THESE, not the raw fields, so a high volume of later-reverted merges cannot keep
// the raw number artificially healthy while gaming the breaker into staying disengaged.
weightedMergeConfirmed: number;
weightedCloseConfirmed: number;
weightedMergePrecision: number | null;
weightedClosePrecision: number | null;
}

/** The gate eval report: per-project rows + a coarse "enough data to read" flag. */
Expand Down Expand Up @@ -78,28 +87,35 @@ export const AUTOCLEAR_AFTER_MS = 24 * 60 * 60 * 1000;

export interface AutoTuneAction {
project: string;
/** Raw (unweighted) merge precision, preserved for continuity with any existing log/dashboard consumer of
* this field's historical meaning. NOT what gated this action -- see weightedMergePrecision (#2348). */
mergePrecision: number;
/** #2348: the reversal-discounted precision that actually gated this action. Always <= mergePrecision. */
weightedMergePrecision: number;
decided: number;
wouldMerge: number;
message: string;
}

/** PURE: which projects' merge precision has dropped enough to warrant engaging the circuit-breaker? */
/** PURE: which projects' merge precision has dropped enough to warrant engaging the circuit-breaker?
* Gates on the reversal-WEIGHTED precision (#2348), not the raw one -- see GateEvalRow's own doc comment
* for why: a high volume of later-reverted merges must not keep the raw number artificially healthy. */
export function planAutoTune(report: GateEvalReport): AutoTuneAction[] {
const actions: AutoTuneAction[] = [];
for (const r of report.rows) {
// Gate on wouldMerge, NOT decided: precision is measured over WOULD-MERGE predictions, so a project with many
// holds/closes but few would-merges (e.g. 9 holds + 1 wrong would-merge) must not trip the breaker on a
// statistically meaningless sample. mergePrecision is non-null iff wouldMerge > 0, so check it FIRST to keep
// both arms of the guard reachable.
if (r.mergePrecision == null || r.wouldMerge < AUTOTUNE_MIN_DECIDED) continue;
if (r.mergePrecision < AUTOTUNE_MERGE_PRECISION_FLOOR) {
// statistically meaningless sample. weightedMergePrecision is non-null iff wouldMerge > 0 (same nullability
// as the raw field it discounts), so check it FIRST to keep both arms of the guard reachable.
if (r.weightedMergePrecision == null || r.wouldMerge < AUTOTUNE_MIN_DECIDED) continue;
if (r.weightedMergePrecision < AUTOTUNE_MERGE_PRECISION_FLOOR) {
actions.push({
project: r.project,
mergePrecision: r.mergePrecision,
mergePrecision: r.mergePrecision ?? r.weightedMergePrecision,
weightedMergePrecision: r.weightedMergePrecision,
decided: r.decided,
wouldMerge: r.wouldMerge,
message: `Auto-merge DISABLED for ${r.project}: merge precision ${Math.round(r.mergePrecision * 100)}% over ${r.wouldMerge} would-merge PR(s) (< ${Math.round(AUTOTUNE_MERGE_PRECISION_FLOOR * 100)}%). Would-merges now HOLD for review. Investigate, then clear holdonly:${r.project}.`,
message: `Auto-merge DISABLED for ${r.project}: weighted merge precision ${Math.round(r.weightedMergePrecision * 100)}% (raw ${Math.round((r.mergePrecision ?? r.weightedMergePrecision) * 100)}%) over ${r.wouldMerge} would-merge PR(s) (< ${Math.round(AUTOTUNE_MERGE_PRECISION_FLOOR * 100)}%). Would-merges now HOLD for review. Investigate, then clear holdonly:${r.project}.`,
});
}
}
Expand All @@ -124,7 +140,9 @@ export async function applyAutoTune(flags: FlagStore, report: GateEvalReport): P

/** PURE: should an auto-engaged breaker for `project` be cleared now? True when the per-project holdonly flag
* was set ≥ AUTOCLEAR_AFTER_MS ago AND merge precision is no longer failing (recovered, or no recent merge
* predictions to judge). Never considers a human-set global holdonly (no per-project row → setAt is null). */
* predictions to judge). Never considers a human-set global holdonly (no per-project row → setAt is null).
* Reads the same weighted precision the breaker engaged on (#2348) -- a project can never clear on a raw
* number recovering while the reversal-discounted one it was actually held for is still failing. */
export function shouldAutoClear(report: GateEvalReport, project: string, setAtIso: string | null, nowMs: number): boolean {
if (!setAtIso) return false; // not auto-engaged for THIS project (global breaker is human-only)
// SQLite CURRENT_TIMESTAMP is "YYYY-MM-DD HH:MM:SS" in UTC (no zone). Normalize to ISO and FORCE a UTC zone
Expand All @@ -135,7 +153,7 @@ export function shouldAutoClear(report: GateEvalReport, project: string, setAtIs
const setMs = Date.parse(hasZone ? t : `${t}Z`);
if (!Number.isFinite(setMs) || nowMs - setMs < AUTOCLEAR_AFTER_MS) return false; // still in cooldown
const row = report.rows.find((r) => r.project === project);
const stillFailing = !!row && row.mergePrecision != null && row.wouldMerge >= AUTOTUNE_MIN_DECIDED && row.mergePrecision < AUTOTUNE_MERGE_PRECISION_FLOOR;
const stillFailing = !!row && row.weightedMergePrecision != null && row.wouldMerge >= AUTOTUNE_MIN_DECIDED && row.weightedMergePrecision < AUTOTUNE_MERGE_PRECISION_FLOOR;
return !stillFailing; // cooldown elapsed + precision recovered (or no signal) → clear and let it retry
}

Expand All @@ -162,25 +180,31 @@ export async function maybeAutoClearHoldOnly(flags: FlagStore, report: GateEvalR

export interface CloseAutoTuneAction {
project: string;
/** Raw (unweighted) close precision, preserved for continuity with any existing log/dashboard consumer of
* this field's historical meaning. NOT what gated this action -- see weightedClosePrecision (#2348). */
closePrecision: number;
/** #2348: the reversal-discounted precision that actually gated this action. Always <= closePrecision. */
weightedClosePrecision: number;
decided: number;
wouldClose: number;
message: string;
}

/** PURE: which projects' CLOSE precision has dropped enough to warrant engaging the close-side breaker?
* Mirrors planAutoTune, testing closePrecision against AUTOTUNE_CLOSE_PRECISION_FLOOR. */
* Mirrors planAutoTune, testing the reversal-WEIGHTED closePrecision (#2348) against
* AUTOTUNE_CLOSE_PRECISION_FLOOR -- not the raw one, for the same anti-gaming reason as the merge breaker. */
export function planCloseAutoTune(report: GateEvalReport): CloseAutoTuneAction[] {
const actions: CloseAutoTuneAction[] = [];
for (const r of report.rows) {
if (r.wouldClose < AUTOTUNE_MIN_DECIDED || r.closePrecision == null) continue;
if (r.closePrecision < AUTOTUNE_CLOSE_PRECISION_FLOOR) {
if (r.wouldClose < AUTOTUNE_MIN_DECIDED || r.weightedClosePrecision == null) continue;
if (r.weightedClosePrecision < AUTOTUNE_CLOSE_PRECISION_FLOOR) {
actions.push({
project: r.project,
closePrecision: r.closePrecision,
closePrecision: r.closePrecision ?? r.weightedClosePrecision,
weightedClosePrecision: r.weightedClosePrecision,
decided: r.decided,
wouldClose: r.wouldClose,
message: `Auto-CLOSE DISABLED for ${r.project}: close precision ${Math.round(r.closePrecision * 100)}% over ${r.wouldClose} would-close PR(s) (< ${Math.round(AUTOTUNE_CLOSE_PRECISION_FLOOR * 100)}%). Would-closes now HOLD for review. Investigate, then clear closehold:${r.project}.`,
message: `Auto-CLOSE DISABLED for ${r.project}: weighted close precision ${Math.round(r.weightedClosePrecision * 100)}% (raw ${Math.round((r.closePrecision ?? r.weightedClosePrecision) * 100)}%) over ${r.wouldClose} would-close PR(s) (< ${Math.round(AUTOTUNE_CLOSE_PRECISION_FLOOR * 100)}%). Would-closes now HOLD for review. Investigate, then clear closehold:${r.project}.`,
});
}
}
Expand All @@ -205,17 +229,17 @@ export async function applyCloseAutoTune(flags: FlagStore, report: GateEvalRepor
}

/** PURE: should an auto-engaged CLOSE breaker for `project` be cleared now? Mirrors shouldAutoClear but tests
* closePrecision. True when the per-project closehold flag was set ≥ AUTOCLEAR_AFTER_MS ago AND close precision
* is no longer failing (recovered, or no recent close predictions to judge). Never considers a human-set global
* closehold (no per-project row → setAt is null). */
* the weighted closePrecision (#2348). True when the per-project closehold flag was set ≥ AUTOCLEAR_AFTER_MS
* ago AND close precision is no longer failing (recovered, or no recent close predictions to judge). Never
* considers a human-set global closehold (no per-project row → setAt is null). */
export function shouldAutoClearClose(report: GateEvalReport, project: string, setAtIso: string | null, nowMs: number): boolean {
if (!setAtIso) return false; // not auto-engaged for THIS project (global breaker is human-only)
const t = setAtIso.includes("T") ? setAtIso : setAtIso.replace(" ", "T");
const hasZone = t.endsWith("Z") || /[+-]\d\d:?\d\d$/.test(t);
const setMs = Date.parse(hasZone ? t : `${t}Z`);
if (!Number.isFinite(setMs) || nowMs - setMs < AUTOCLEAR_AFTER_MS) return false; // still in cooldown
const row = report.rows.find((r) => r.project === project);
const stillFailing = !!row && row.closePrecision != null && row.wouldClose >= AUTOTUNE_MIN_DECIDED && row.closePrecision < AUTOTUNE_CLOSE_PRECISION_FLOOR;
const stillFailing = !!row && row.weightedClosePrecision != null && row.wouldClose >= AUTOTUNE_MIN_DECIDED && row.weightedClosePrecision < AUTOTUNE_CLOSE_PRECISION_FLOOR;
return !stillFailing; // cooldown elapsed + precision recovered (or no signal) → clear and let it retry
}

Expand Down
11 changes: 10 additions & 1 deletion src/review/selftune-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export const SELFTUNE_BASE_CONFIDENCE_FLOOR = 0;
export function evalRowFromCalibration(project: string, positive: number, negative: number): GateEvalRow {
const wouldMerge = positive + negative; // resolved recommendation outcomes graded against the human's call
const decided = wouldMerge;
const mergePrecision = wouldMerge > 0 ? positive / wouldMerge : null;
return {
project,
wouldMerge,
Expand All @@ -77,8 +78,16 @@ export function evalRowFromCalibration(project: string, positive: number, negati
closeFalse: 0, // held at 0 by construction: the only loosening branch of the advisor is unreachable
hold: 0,
decided,
mergePrecision: wouldMerge > 0 ? positive / wouldMerge : null,
mergePrecision,
closePrecision: null,
// #2348: recommendation-outcome calibration (agent_recommendation_outcomes) carries no reversal signal at
// all — it is not derived from review_audit, so there is nothing for parity.ts's reversal-discount formula
// to discount BY. weighted === raw here by construction (no data to distinguish them), mirroring how a
// review_audit row with zero reversals also naturally produces weighted === raw.
weightedMergeConfirmed: positive,
weightedCloseConfirmed: 0,
weightedMergePrecision: mergePrecision,
weightedClosePrecision: null,
};
}

Expand Down
147 changes: 133 additions & 14 deletions test/unit/auto-tune.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,33 @@ import {
shouldAutoClearClose,
} from "../../src/review/auto-tune";

const row = (over: Partial<GateEvalRow>): GateEvalRow => ({
project: "p",
wouldMerge: 0,
mergeConfirmed: 0,
mergeFalse: 0,
wouldClose: 0,
closeConfirmed: 0,
closeFalse: 0,
hold: 0,
decided: 0,
mergePrecision: null,
closePrecision: null,
...over,
});
const row = (over: Partial<GateEvalRow>): GateEvalRow => {
const mergeConfirmed = over.mergeConfirmed ?? 0;
const closeConfirmed = over.closeConfirmed ?? 0;
const mergePrecision = over.mergePrecision ?? null;
const closePrecision = over.closePrecision ?? null;
return {
project: "p",
wouldMerge: 0,
mergeConfirmed,
mergeFalse: 0,
wouldClose: 0,
closeConfirmed,
closeFalse: 0,
hold: 0,
decided: 0,
mergePrecision,
closePrecision,
// #2348: default weighted === raw (no reversal signal in a bare fixture), so every pre-#2348 test that
// only sets the raw fields keeps exercising the breaker meaningfully unchanged. A test proving the
// anti-gaming property overrides weightedMergePrecision/weightedClosePrecision explicitly to diverge.
weightedMergeConfirmed: mergeConfirmed,
weightedCloseConfirmed: closeConfirmed,
weightedMergePrecision: mergePrecision,
weightedClosePrecision: closePrecision,
...over,
};
};
const report = (rows: GateEvalRow[]): GateEvalReport => ({ rows, hasSignal: rows.some((r) => r.decided >= 10) });

/** A stub FlagStore (the injected seam — the live D1-backed store is deferred infra). */
Expand Down Expand Up @@ -69,6 +82,42 @@ describe("planAutoTune (#self-improve) — circuit-breaker is one-directional",
});
});

describe("planAutoTune — #2348 weighted anti-gaming cutover", () => {
it("engages on a WEIGHTED precision failure even though RAW precision is healthy (reversal-discounted merges cannot keep the raw number artificially healthy to dodge the breaker)", () => {
const a = planAutoTune(
report([
row({
project: "gamed",
decided: 20,
wouldMerge: 20,
mergeConfirmed: 19,
mergePrecision: 0.95, // raw looks healthy...
weightedMergePrecision: 0.5, // ...but most of those "confirmed" merges were later reverted
}),
]),
);
expect(a).toHaveLength(1);
expect(a[0]?.mergePrecision).toBe(0.95); // raw preserved for log continuity, NOT what gated this
expect(a[0]?.weightedMergePrecision).toBe(0.5); // weighted is what actually gated it
expect(a[0]?.message).toContain("weighted merge precision 50%");
expect(a[0]?.message).toContain("raw 95%");
});
it("does NOT engage when weighted precision is healthy, proving the gate reads weightedMergePrecision (not mergePrecision) — these fields are structurally independent on GateEvalRow even though weighted <= raw always holds in real parity.ts data", () => {
expect(
planAutoTune(
report([row({ project: "p", decided: 20, wouldMerge: 20, mergeConfirmed: 10, mergePrecision: 0.5, weightedMergePrecision: 0.9 })]),
),
).toHaveLength(0);
});
it("falls back to the weighted precision for the action's raw mergePrecision field when raw is null (defensive fallback; not expected from live parity.ts data, whose weighted/raw nullability always match)", () => {
const a = planAutoTune(
report([row({ project: "p", decided: 20, wouldMerge: 20, mergeConfirmed: 18, mergePrecision: null, weightedMergePrecision: 0.5 })]),
);
expect(a).toHaveLength(1);
expect(a[0]?.mergePrecision).toBe(0.5);
});
});

describe("applyAutoTune", () => {
it("sets the holdonly flag for a newly-flagged project", async () => {
const { flags, setFlag } = stubFlags();
Expand Down Expand Up @@ -151,6 +200,23 @@ describe("shouldAutoClear (#272 recovery-gated breaker auto-clear)", () => {
});
});

describe("shouldAutoClear — #2348 weighted anti-gaming cutover", () => {
const now = Date.parse("2026-06-20T12:00:00Z");
const past = new Date(now - AUTOCLEAR_AFTER_MS - 3_600_000).toISOString(); // >24h ago
it("stays engaged after cooldown when RAW precision recovered but WEIGHTED precision is still failing (reversals keep it held; raw recovery alone cannot clear it)", () => {
const stillGamed = report([
row({ project: "g", decided: 20, wouldMerge: 20, mergeConfirmed: 20, mergePrecision: 1.0, weightedMergePrecision: 0.5 }),
]);
expect(shouldAutoClear(stillGamed, "g", past, now)).toBe(false);
});
it("clears after cooldown once WEIGHTED precision recovers, proving the recovery check reads weightedMergePrecision", () => {
const recoveredWeighted = report([
row({ project: "g", decided: 20, wouldMerge: 20, mergeConfirmed: 20, mergePrecision: 1.0, weightedMergePrecision: 0.9 }),
]);
expect(shouldAutoClear(recoveredWeighted, "g", past, now)).toBe(true);
});
});

// ── CLOSE-precision circuit-breaker (symmetric mirror of the merge breaker) ─────────────────────────────────

describe("planCloseAutoTune (#close-precision-breaker) — tightening-only, close direction", () => {
Expand All @@ -175,6 +241,42 @@ describe("planCloseAutoTune (#close-precision-breaker) — tightening-only, clos
});
});

describe("planCloseAutoTune — #2348 weighted anti-gaming cutover", () => {
it("engages on a WEIGHTED close-precision failure even though RAW close precision is healthy", () => {
const a = planCloseAutoTune(
report([
row({
project: "gamed",
decided: 20,
wouldClose: 20,
closeConfirmed: 19,
closePrecision: 0.95,
weightedClosePrecision: 0.5,
}),
]),
);
expect(a).toHaveLength(1);
expect(a[0]?.closePrecision).toBe(0.95); // raw preserved for log continuity, NOT what gated this
expect(a[0]?.weightedClosePrecision).toBe(0.5); // weighted is what actually gated it
expect(a[0]?.message).toContain("weighted close precision 50%");
expect(a[0]?.message).toContain("raw 95%");
});
it("does NOT engage when weighted close precision is healthy, proving the gate reads weightedClosePrecision (not closePrecision)", () => {
expect(
planCloseAutoTune(
report([row({ project: "p", decided: 20, wouldClose: 20, closeConfirmed: 10, closePrecision: 0.5, weightedClosePrecision: 0.9 })]),
),
).toHaveLength(0);
});
it("falls back to the weighted close precision for the action's raw closePrecision field when raw is null (defensive fallback)", () => {
const a = planCloseAutoTune(
report([row({ project: "p", decided: 20, wouldClose: 20, closeConfirmed: 18, closePrecision: null, weightedClosePrecision: 0.5 })]),
);
expect(a).toHaveLength(1);
expect(a[0]?.closePrecision).toBe(0.5);
});
});

describe("applyCloseAutoTune", () => {
it("sets the closehold flag for a newly-flagged project", async () => {
const { flags, setFlag } = stubFlags();
Expand Down Expand Up @@ -258,6 +360,23 @@ describe("shouldAutoClearClose (#close-precision-breaker recovery-gated auto-cle
});
});

describe("shouldAutoClearClose — #2348 weighted anti-gaming cutover", () => {
const now = Date.parse("2026-06-20T12:00:00Z");
const past = new Date(now - AUTOCLEAR_AFTER_MS - 3_600_000).toISOString(); // >24h ago
it("stays engaged after cooldown when RAW close precision recovered but WEIGHTED close precision is still failing", () => {
const stillGamed = report([
row({ project: "g", decided: 20, wouldClose: 20, closeConfirmed: 20, closePrecision: 1.0, weightedClosePrecision: 0.5 }),
]);
expect(shouldAutoClearClose(stillGamed, "g", past, now)).toBe(false);
});
it("clears after cooldown once WEIGHTED close precision recovers, proving the recovery check reads weightedClosePrecision", () => {
const recoveredWeighted = report([
row({ project: "g", decided: 20, wouldClose: 20, closeConfirmed: 20, closePrecision: 1.0, weightedClosePrecision: 0.9 }),
]);
expect(shouldAutoClearClose(recoveredWeighted, "g", past, now)).toBe(true);
});
});

describe("computeTuningRecommendations (#self-improve)", () => {
it("says READY when merge precision is high over a real sample with no false closes", () => {
const recs = computeTuningRecommendations(report([row({ project: "gittensory", decided: 20, wouldMerge: 18, mergeConfirmed: 18, mergePrecision: 1.0 })]));
Expand Down
Loading