fix(review): gate the auto-tune breaker on reversal-weighted precision (#2348)#5088
Merged
Merged
Conversation
#2348) planAutoTune/shouldAutoClear and their close-side mirrors now engage and clear on GateEvalRow's reversal-discounted weightedMergePrecision / weightedClosePrecision (already computed by parity.ts's computeGateEval, PR #4169) instead of the raw precision fields. A high volume of later- reverted merges can no longer keep the raw number artificially healthy to dodge the breaker. The raw fields are preserved on AutoTuneAction / CloseAutoTuneAction for log continuity and surfaced alongside the weighted number in the alert message. computeTuningRecommendations (the separate advisory-only tuning surface) is unchanged. evalRowFromCalibration's synthetic rows (selftune-wire.ts, sourced from agent_recommendation_outcomes) carry no reversal signal, so their weighted fields mirror the raw ones by construction.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5088 +/- ##
=======================================
Coverage 94.20% 94.20%
=======================================
Files 470 470
Lines 39722 39726 +4
Branches 14493 14497 +4
=======================================
+ Hits 37421 37425 +4
Misses 1645 1645
Partials 656 656
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
planAutoTune/shouldAutoClearand their close-side mirrors (planCloseAutoTune/shouldAutoClearClose) now gate onGateEvalRow's reversal-discountedweightedMergePrecision/weightedClosePrecision(already computed byparity.ts'scomputeGateEval, landed in feat(review): discount reverted/reopened outcomes in the gate-eval fold #4169) instead of the raw precision fields — a high volume of later-reverted merges can no longer keep the raw number artificially healthy to dodge the breaker.AutoTuneAction/CloseAutoTuneActionfor log continuity, and both numbers are surfaced in the alert message (weighted merge precision 50% (raw 95%) ...) so an operator can see the gap at a glance.computeTuningRecommendations(the separate advisory-only tuning surface, not "the breaker") is deliberately unchanged — out of this issue's named scope.evalRowFromCalibration(selftune-wire.ts, sourced fromagent_recommendation_outcomes) has no reversal signal in its data source, so its weighted fields mirror the raw ones by construction — documented inline.Closes #2348.
Re-verification of the issue's second downstream consumer (orb collector)
The issue's deliverable 4 names two downstream consumers to re-verify: the auto-tune breaker (this PR) and the orb collector (
src/orb/analytics.ts). I checked — the orb collector'sfoldInstancedoes not consumeparity.ts'sGateEvalRow/computeGateEvalat all; it folds its own independentmergePrecisiondirectly from fleet-submittedorb_signalscells, and already excludes areversal_flag !== "reverted"merge frommergeConfirmed(see the field's own doc comment:P(merged & not reverted | gate said merge)). It predates and is structurally separate from this weighted-fold work, and was already reversal-aware by construction. No change needed there.Scope
type(scope): short summaryConventional Commit format.src/files, 3 test files) and does not mix unrelated changes.CONTRIBUTING.md; nosite//CNAME/VitePress touched.Validation
npm run typecheck— clean.npm run test:coverage(full unsharded) —src/review/auto-tune.ts100% lines (83/83) / 100% branches (79/79).src/review/selftune-wire.ts100% lines (30/30); its one uncovered branch (line 123,isAgentConfiguredguard in the pre-existing, untouchedselfTuneRepos) is outside this diff — confirmed via rawlcov.infoBRDAinspection that both new lines (including the newwouldMerge > 0 ? ... : nullternary) are fully hit.auto-tune.test.ts+outcomes-wire.test.ts+selftune-wiring.test.ts+selftune-readback.test.ts— 136/136 passing, including new tests proving the actual anti-gaming property (raw precision healthy + weighted precision failing → breaker now engages where it previously wouldn't; and the inverse recovery case forshouldAutoClear/shouldAutoClearClose).npm run test:workers/npm run build:mcp/npm run ui:*— not run locally; no MCP, worker-binding, wrangler, or UI surface touched by this diff (puresrc/review/**logic + tests), left to CI per this session's established "typecheck + affected tests locally, trust CI for the rest" practice.npm audit --audit-level=moderate— no dependency changes in this PR.??raw-precision fallback on both the merge and close side) and the core anti-gaming property have dedicated tests, not just line coverage.Safety
Notes
git log -S weightedMergePrecisionthat PR feat(review): discount reverted/reopened outcomes in the gate-eval fold #4169 already built the weighted fold + its own regression tests inparity.ts(deliverables 1–3), explicitly deferring "the auto-tune.ts breaker cutover" as future maintainer work — this PR is that deferred cutover (deliverable 4).