You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.
Context
src/review/auto-tune.ts holds two consumers of the same GateEvalReport: the circuit breaker
(planAutoTune) and the tuning advisor (computeTuningRecommendations, whose output feeds the
auto-apply/override path). They disagree on both of the two rules the breaker documents.
1. Sample gate: wouldMerge vs decided
planAutoTune (src/review/auto-tune.ts:103-110):
// 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. 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.decided<MIN_DECIDED){recs.push({project: r.project,severity: "info",message: `Only ${r.decided} decided PR(s) — …`});continue;}letflagged=false;// The dangerous error: would auto-merge something the human closed.if(r.mergePrecision!=null&&r.mergePrecision<RISK_MERGE_PRECISION){recs.push({…overridePayload: {confidenceFloor: TIGHTEN_FLOOR_TARGET},});
decided counts every prediction with a known outcome, holds included (src/review/parity.ts:145, r.decided += c.n; runs before the merge/close/hold split at :151-166). So the exact shape planAutoTune's comment names — 9 holds plus 1 wrong would-merge — clears decided >= 10, produces mergePrecision === 0, and emits a warn recommendation carrying an overridePayload. That payload is
auto-applicable: runAutoApplyRecommendations filters to overridePayload != null
(src/review/auto-apply.ts:401) and queues a live confidence-floor raise for the project. The breaker
refuses to act on that sample; the advisor writes config from it.
2. Precision field: weighted vs raw
GateEvalRow's own doc comment states the rule (src/review/auto-tune.ts:37-42):
// #2348: reversal-discounted variants … 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.
planAutoTune and planCloseAutoTune both read weightedMergePrecision/weightedClosePrecision. computeTuningRecommendations reads the RAW r.mergePrecision (:307), the raw r.closePrecision (:327)
and the raw r.closeFalse (:319). REVERSAL_DISCOUNT_WEIGHT is 0 (src/review/parity.ts:78), so a
project whose merges are systematically reverted has weightedMergePrecision at or near 0 while mergePrecision stays healthy — the breaker engages, the advisor stays silent, and no tightening is ever
recommended for exactly the project that needs one.
It is worse than silence downstream. RISK_MERGE_PRECISION is exported specifically so the shadow-promotion
gate can release a queued tightening once precision recovers (src/review/auto-tune.ts:283-285), and evaluateShadowPromotion compares the same raw field (src/review/auto-apply.ts:176-180, args.currentMergePrecision >= RISK_MERGE_PRECISION ⇒ "tightening no longer warranted"). A project held by
the reversal-weighted breaker therefore has its pending tightening dropped on a raw number the breaker itself
refuses to trust.
3. Dead guard arm in planCloseAutoTune
planAutoTune's comment above explains why the null check comes FIRST. Its close-side twin inverts the order
(src/review/auto-tune.ts:199):
weightedClosePrecision is non-null iff wouldClose > 0 (src/review/parity.ts:170-171), so once wouldClose >= 10 the second disjunct can never be true — an unreachable arm in a repo that gates on
branch-counted coverage.
Requirements
computeTuningRecommendations's sample gate must be r.wouldMerge < MIN_DECIDED instead of r.decided < MIN_DECIDED, matching planAutoTune's documented rule. The emitted info message must name
the would-merge count rather than decided.
computeTuningRecommendations's merge-risk test must read r.weightedMergePrecision, and its
ready-to-flip-live test (:327) must read r.weightedMergePrecision and r.weightedClosePrecision.
computeTuningRecommendations's close-side test must read r.weightedClosePrecision against AUTOTUNE_CLOSE_PRECISION_FLOOR in place of the current r.closeFalse > 0 test, so both directions gate on
the same reversal-discounted evidence the breaker uses. r.closeFalse may still be quoted in the message
text; it must no longer be the condition.
evaluateShadowPromotion's recovery check must be fed the WEIGHTED merge precision: rename AutoApplyContext.mergePrecision to weightedMergePrecision (src/review/auto-apply.ts:389) and thread it
into currentMergePrecision (:432). evaluateShadowPromotion's own parameter name and its >= RISK_MERGE_PRECISION comparison must not otherwise change.
planCloseAutoTune's guard must be reordered to r.weightedClosePrecision == null || r.wouldClose < AUTOTUNE_MIN_DECIDED,
matching planAutoTune.
Behaviour that must NOT change: MIN_DECIDED (10), AUTOTUNE_MIN_DECIDED (10), RISK_MERGE_PRECISION
(0.9), READY_MERGE_PRECISION (0.95), READY_CLOSE_PRECISION (0.9), TIGHTEN_FLOOR_TARGET, the warn/good/info ordering (:334-336), the TuningRec and OverridePayload shapes, the
tightening-only direction of isStrictlyTightening, and planAutoTune/applyAutoTune/shouldAutoClear
and their close-side twins.
⚠️ Required pattern: planAutoTune (src/review/auto-tune.ts:103-119) is the reference — its weightedMergePrecision == null || wouldMerge < AUTOTUNE_MIN_DECIDED guard and its weighted-field
comparison are exactly what the advisor must adopt. What does NOT satisfy this issue: adding a new weightedDecided field to GateEvalRow (the existing wouldMerge/wouldClose are the correct
denominators and already exist); leaving the raw fields in place and adding a second, parallel "weighted
recommendations" function; changing REVERSAL_DISCOUNT_WEIGHT; fixing only computeTuningRecommendations
and leaving auto-apply.ts still reading the raw number; a test-only PR.
Deliverables
src/review/auto-tune.ts: computeTuningRecommendations({ rows: [{ project: "o/r", decided: 10, wouldMerge: 1, mergeFalse: 1, mergePrecision: 0, weightedMergePrecision: 0, hold: 9, … }] }) returns a
single info recommendation with NO overridePayload (the statistically meaningless sample the
breaker already refuses).
src/review/auto-tune.ts: computeTuningRecommendations for a row with wouldMerge: 20, mergePrecision: 0.98, weightedMergePrecision: 0.2 returns a warn recommendation carrying overridePayload: { confidenceFloor: 0.95 }.
src/review/auto-tune.ts: planCloseAutoTune's guard is reordered so its null arm is reachable, with a
test passing weightedClosePrecision: null, wouldClose: 20.
src/review/auto-apply.ts: AutoApplyContext.weightedMergePrecision replaces mergePrecision and is
threaded into evaluateShadowPromotion's currentMergePrecision; every call site and the field's doc
comment are updated.
Tests in test/unit/auto-tune.test.ts covering: the sample-gate change (both arms), the weighted merge
test (both arms), the weighted close test (both arms), and the ready-to-flip-live test reading weighted
fields (both arms).
A test in test/unit/auto-apply.test.ts asserting that a shadow override is NOT promoted when weightedMergePrecision is below RISK_MERGE_PRECISION even though the raw precision has recovered.
A regression test at test/unit/auto-tune.test.ts named for this bug (e.g. "REGRESSION: 9 holds + 1 wrong would-merge never produces an auto-applicable tightening").
All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example
switching computeTuningRecommendations to the weighted fields without changing the decided sample gate,
or without threading the weighted precision into the shadow-promotion recovery check — does not resolve this
issue.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts and packages/loopover-engine/src/**/*.ts; both src/review/auto-tune.ts and src/review/auto-apply.ts are measured and gated. Both arms of every touched branch need a test: the wouldMerge < MIN_DECIDED gate; r.weightedMergePrecision != null and the < RISK_MERGE_PRECISION
comparison; the new weightedClosePrecision != null / < AUTOTUNE_CLOSE_PRECISION_FLOOR pair; the !flagged && … >= READY_MERGE_PRECISION && (weightedClosePrecision == null || >= READY_CLOSE_PRECISION)
conjunction (each conjunct false at least once, and the all-true case); planCloseAutoTune's reordered
guard (each disjunct); and evaluateShadowPromotion's currentMergePrecision != null and >= RISK_MERGE_PRECISION arms.
Expected Outcome
The tuning advisor and the circuit breaker judge a project on the same denominator and the same
reversal-discounted evidence, so a project whose merges are systematically reverted actually receives a
tightening recommendation, a project with 9 holds and 1 bad would-merge no longer has its live confidence
floor raised off a one-sample measurement, and a queued tightening is no longer released on a raw number the
breaker refuses to trust.
Links & Resources
src/review/auto-tune.ts:37-42 — GateEvalRow's "the circuit-breaker gates on THESE, not the raw fields"
src/review/auto-tune.ts:103-119 — planAutoTune, the reference implementation
src/review/auto-tune.ts:196-210 — planCloseAutoTune and its inverted guard
src/review/auto-tune.ts:283-334 — RISK_MERGE_PRECISION and computeTuningRecommendations
src/review/auto-apply.ts:170-182, :386-392, :425-435 — evaluateShadowPromotion and the context field
src/review/parity.ts:78, :145-171 — REVERSAL_DISCOUNT_WEIGHT and the decided/hold fold
Context
src/review/auto-tune.tsholds two consumers of the sameGateEvalReport: the circuit breaker(
planAutoTune) and the tuning advisor (computeTuningRecommendations, whose output feeds theauto-apply/override path). They disagree on both of the two rules the breaker documents.
1. Sample gate:
wouldMergevsdecidedplanAutoTune(src/review/auto-tune.ts:103-110):computeTuningRecommendations(src/review/auto-tune.ts:298-315):decidedcounts every prediction with a known outcome, holds included (src/review/parity.ts:145,r.decided += c.n;runs before themerge/close/holdsplit at:151-166). So the exact shapeplanAutoTune's comment names — 9 holds plus 1 wrong would-merge — clearsdecided >= 10, producesmergePrecision === 0, and emits awarnrecommendation carrying anoverridePayload. That payload isauto-applicable:
runAutoApplyRecommendationsfilters tooverridePayload != null(
src/review/auto-apply.ts:401) and queues a live confidence-floor raise for the project. The breakerrefuses to act on that sample; the advisor writes config from it.
2. Precision field: weighted vs raw
GateEvalRow's own doc comment states the rule (src/review/auto-tune.ts:37-42):planAutoTuneandplanCloseAutoTuneboth readweightedMergePrecision/weightedClosePrecision.computeTuningRecommendationsreads the RAWr.mergePrecision(:307), the rawr.closePrecision(:327)and the raw
r.closeFalse(:319).REVERSAL_DISCOUNT_WEIGHTis0(src/review/parity.ts:78), so aproject whose merges are systematically reverted has
weightedMergePrecisionat or near 0 whilemergePrecisionstays healthy — the breaker engages, the advisor stays silent, and no tightening is everrecommended for exactly the project that needs one.
It is worse than silence downstream.
RISK_MERGE_PRECISIONis exported specifically so the shadow-promotiongate can release a queued tightening once precision recovers (
src/review/auto-tune.ts:283-285), andevaluateShadowPromotioncompares the same raw field (src/review/auto-apply.ts:176-180,args.currentMergePrecision >= RISK_MERGE_PRECISION⇒ "tightening no longer warranted"). A project held bythe reversal-weighted breaker therefore has its pending tightening dropped on a raw number the breaker itself
refuses to trust.
3. Dead guard arm in
planCloseAutoTuneplanAutoTune's comment above explains why the null check comes FIRST. Its close-side twin inverts the order(
src/review/auto-tune.ts:199):weightedClosePrecisionis non-null iffwouldClose > 0(src/review/parity.ts:170-171), so oncewouldClose >= 10the second disjunct can never be true — an unreachable arm in a repo that gates onbranch-counted coverage.
Requirements
computeTuningRecommendations's sample gate must ber.wouldMerge < MIN_DECIDEDinstead ofr.decided < MIN_DECIDED, matchingplanAutoTune's documented rule. The emittedinfomessage must namethe would-merge count rather than
decided.computeTuningRecommendations's merge-risk test must readr.weightedMergePrecision, and itsready-to-flip-live test (
:327) must readr.weightedMergePrecisionandr.weightedClosePrecision.computeTuningRecommendations's close-side test must readr.weightedClosePrecisionagainstAUTOTUNE_CLOSE_PRECISION_FLOORin place of the currentr.closeFalse > 0test, so both directions gate onthe same reversal-discounted evidence the breaker uses.
r.closeFalsemay still be quoted in the messagetext; it must no longer be the condition.
evaluateShadowPromotion's recovery check must be fed the WEIGHTED merge precision: renameAutoApplyContext.mergePrecisiontoweightedMergePrecision(src/review/auto-apply.ts:389) and thread itinto
currentMergePrecision(:432).evaluateShadowPromotion's own parameter name and its>= RISK_MERGE_PRECISIONcomparison must not otherwise change.planCloseAutoTune's guard must be reordered tor.weightedClosePrecision == null || r.wouldClose < AUTOTUNE_MIN_DECIDED,matching
planAutoTune.MIN_DECIDED(10),AUTOTUNE_MIN_DECIDED(10),RISK_MERGE_PRECISION(0.9),
READY_MERGE_PRECISION(0.95),READY_CLOSE_PRECISION(0.9),TIGHTEN_FLOOR_TARGET, thewarn/good/infoordering (:334-336), theTuningRecandOverridePayloadshapes, thetightening-only direction of
isStrictlyTightening, andplanAutoTune/applyAutoTune/shouldAutoClearand their close-side twins.
Deliverables
src/review/auto-tune.ts:computeTuningRecommendations({ rows: [{ project: "o/r", decided: 10, wouldMerge: 1, mergeFalse: 1, mergePrecision: 0, weightedMergePrecision: 0, hold: 9, … }] })returns asingle
inforecommendation with NOoverridePayload(the statistically meaningless sample thebreaker already refuses).
src/review/auto-tune.ts:computeTuningRecommendationsfor a row withwouldMerge: 20,mergePrecision: 0.98,weightedMergePrecision: 0.2returns awarnrecommendation carryingoverridePayload: { confidenceFloor: 0.95 }.src/review/auto-tune.ts:planCloseAutoTune's guard is reordered so its null arm is reachable, with atest passing
weightedClosePrecision: null, wouldClose: 20.src/review/auto-apply.ts:AutoApplyContext.weightedMergePrecisionreplacesmergePrecisionand isthreaded into
evaluateShadowPromotion'scurrentMergePrecision; every call site and the field's doccomment are updated.
test/unit/auto-tune.test.tscovering: the sample-gate change (both arms), the weighted mergetest (both arms), the weighted close test (both arms), and the ready-to-flip-live test reading weighted
fields (both arms).
test/unit/auto-apply.test.tsasserting that a shadow override is NOT promoted whenweightedMergePrecisionis belowRISK_MERGE_PRECISIONeven though the raw precision has recovered.test/unit/auto-tune.test.tsnamed for this bug (e.g."REGRESSION: 9 holds + 1 wrong would-merge never produces an auto-applicable tightening").All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example
switching
computeTuningRecommendationsto the weighted fields without changing thedecidedsample gate,or without threading the weighted precision into the shadow-promotion recovery check — does not resolve this
issue.
Test Coverage Requirements
This repo enforces 99%+ Codecov patch coverage, branch-counted.
vitest.config.ts'scoverage.includecovers
src/**/*.tsandpackages/loopover-engine/src/**/*.ts; bothsrc/review/auto-tune.tsandsrc/review/auto-apply.tsare measured and gated. Both arms of every touched branch need a test: thewouldMerge < MIN_DECIDEDgate;r.weightedMergePrecision != nulland the< RISK_MERGE_PRECISIONcomparison; the new
weightedClosePrecision != null/< AUTOTUNE_CLOSE_PRECISION_FLOORpair; the!flagged && … >= READY_MERGE_PRECISION && (weightedClosePrecision == null || >= READY_CLOSE_PRECISION)conjunction (each conjunct false at least once, and the all-true case);
planCloseAutoTune's reorderedguard (each disjunct); and
evaluateShadowPromotion'scurrentMergePrecision != nulland>= RISK_MERGE_PRECISIONarms.Expected Outcome
The tuning advisor and the circuit breaker judge a project on the same denominator and the same
reversal-discounted evidence, so a project whose merges are systematically reverted actually receives a
tightening recommendation, a project with 9 holds and 1 bad would-merge no longer has its live confidence
floor raised off a one-sample measurement, and a queued tightening is no longer released on a raw number the
breaker refuses to trust.
Links & Resources
src/review/auto-tune.ts:37-42—GateEvalRow's "the circuit-breaker gates on THESE, not the raw fields"src/review/auto-tune.ts:103-119—planAutoTune, the reference implementationsrc/review/auto-tune.ts:196-210—planCloseAutoTuneand its inverted guardsrc/review/auto-tune.ts:283-334—RISK_MERGE_PRECISIONandcomputeTuningRecommendationssrc/review/auto-apply.ts:170-182,:386-392,:425-435—evaluateShadowPromotionand the context fieldsrc/review/parity.ts:78,:145-171—REVERSAL_DISCOUNT_WEIGHTand thedecided/holdfold