Skip to content

fix(review): gate the tuning advisor on wouldMerge and weighted precision, matching the breaker - #10142

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
shin-core:fix/auto-tune-weighted-precision-gates-10014
Jul 31, 2026
Merged

fix(review): gate the tuning advisor on wouldMerge and weighted precision, matching the breaker#10142
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
shin-core:fix/auto-tune-weighted-precision-gates-10014

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What & why

src/review/auto-tune.ts has two consumers of the same GateEvalReport: the circuit breaker (planAutoTune) and the tuning advisor (computeTuningRecommendations, whose output feeds the auto-apply/override path). They disagreed on both rules the breaker documents.

1. Sample gate: decided vs wouldMerge. The advisor gated on r.decided < MIN_DECIDED; decided counts every prediction with a known outcome, holds included. So the exact shape planAutoTune's comment names — 9 holds + 1 wrong would-merge — clears decided >= 10, produces mergePrecision === 0, and emits a warn carrying an overridePayload. That payload is auto-applicable: runAutoApplyRecommendations queues a live confidence-floor raise off a single prediction the breaker already refuses as statistically meaningless.

2. Precision field: raw vs weighted. GateEvalRow's doc states the breaker gates on the reversal-discounted weighted* fields "so a high volume of later-reverted merges cannot keep the raw number artificially healthy while gaming the breaker into staying disengaged." The advisor read the raw mergePrecision/closePrecision/closeFalse. REVERSAL_DISCOUNT_WEIGHT is 0, so a project whose merges are systematically reverted has weightedMergePrecision ≈ 0 while mergePrecision stays healthy — the breaker engages, the advisor stays silent, and no tightening is recommended for exactly the project that needs one. Worse: evaluateShadowPromotion's recovery check was fed the raw number, so a project held by the reversal-weighted breaker had its pending tightening dropped on a figure the breaker itself distrusts.

3. Dead guard arm. planCloseAutoTune's guard was r.wouldClose < AUTOTUNE_MIN_DECIDED || r.weightedClosePrecision == null — but weightedClosePrecision is non-null iff wouldClose > 0, so once wouldClose >= 10 the == null disjunct was unreachable (a dead branch under branch-counted coverage).

The fix

  • computeTuningRecommendations's sample gate is r.wouldMerge < MIN_DECIDED (message names the would-merge count); its merge-risk and ready tests read r.weightedMergePrecision; its close-side warn reads r.weightedClosePrecision against AUTOTUNE_CLOSE_PRECISION_FLOOR (closeFalse stays in the message text, no longer the condition).
  • AutoApplyContext.mergePrecision is renamed to weightedMergePrecision and threaded into evaluateShadowPromotion's currentMergePrecision, so the recovery check reads the weighted number.
  • planCloseAutoTune's guard is reordered to null-check first (mirroring planAutoTune), making the null arm reachable.

Unchanged: every threshold constant (MIN_DECIDED 10, RISK_MERGE_PRECISION 0.9, READY_MERGE_PRECISION 0.95, READY_CLOSE_PRECISION 0.9, AUTOTUNE_CLOSE_PRECISION_FLOOR 0.85, TIGHTEN_FLOOR_TARGET), the warn/good/info ordering, the TuningRec/OverridePayload shapes, isStrictlyTightening's tightening-only direction, and planAutoTune/applyAutoTune/shouldAutoClear and their close twins.

Tests (test/unit/auto-tune.test.ts, test/unit/auto-apply.test.ts)

  • REGRESSION: 9 holds + 1 wrong would-merge (decided: 10, wouldMerge: 1) is info-only with no overridePayload; a healthy raw merge precision (0.98) with a failing weighted one (0.2) warns with overridePayload: { confidenceFloor: 0.95 }.
  • planCloseAutoTune's reordered null arm is reachable (weightedClosePrecision: null, wouldClose: 20 → no action).
  • The two existing tests that encoded the old raw-field/closeFalse behaviour are updated to the weighted-gate contract; the shadow-promotion recovery tests now pass weightedMergePrecision.
  • All new assertions fail on main.

Validation

  • Diff coverage on both src/review/auto-tune.ts and src/review/auto-apply.ts is 100% line and branch.
  • npm run typecheck clean for these files (the rename is caught end-to-end — the sole call site, selftune-wire.ts, does not pass the field, and the test call sites are updated); npm run engine-parity:drift-check passes (neither file is a twin); npm run dead-exports:check clean; both suites (158 tests) green.
  • git diff --check clean; no schema/migration/generated-artifact change.

Closes #10014

…sion, matching the breaker

computeTuningRecommendations disagreed with its sibling circuit breaker on both
rules the breaker documents. It gated the sample on decided (holds included)
instead of wouldMerge, so 9 holds + 1 wrong would-merge cleared decided>=10,
produced mergePrecision 0, and emitted a warn carrying an auto-applicable
overridePayload -- queuing a live confidence-floor raise off a single prediction
the breaker already refuses. And it read the RAW mergePrecision/closePrecision
while the breaker gates on the reversal-WEIGHTED fields, so a project whose merges
are systematically reverted (weighted ~0, raw healthy) got no tightening
recommended -- exactly the project that needs one -- and evaluateShadowPromotion
dropped its queued tightening on the raw number the breaker itself distrusts.

Gate the advisor's sample on wouldMerge < MIN_DECIDED, its risk/ready tests on
weightedMergePrecision/weightedClosePrecision, and its close-side warn on the
weighted close precision against AUTOTUNE_CLOSE_PRECISION_FLOOR (closeFalse stays
in the message, no longer the condition). Thread the weighted merge precision into
evaluateShadowPromotion by renaming AutoApplyContext.mergePrecision to
weightedMergePrecision. Reorder planCloseAutoTune's guard to null-check first,
matching planAutoTune, so its formerly-dead null arm is reachable. Every threshold
constant, severity ordering, and the tightening-only direction are unchanged.

Closes JSONbored#10014
@shin-core
shin-core requested a review from JSONbored as a code owner July 31, 2026 09:02
@loopover-orb

loopover-orb Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-31 09:33:26 UTC

4 files · 1 AI reviewer · no blockers · readiness 95/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR aligns computeTuningRecommendations with the documented breaker contract in planAutoTune/planCloseAutoTune: it gates the sample-size check on wouldMerge instead of decided, switches the risk/ready comparisons to the reversal-weighted precision fields, threads weightedMergePrecision (renamed from mergePrecision) through AutoApplyContext into evaluateShadowPromotion, and reorders planCloseAutoTune's null-check to make the previously-dead guard arm reachable. The change is well-traced to the actual defect (verified against auto-tune.ts's own doc comments on GateEvalRow and planAutoTune) and is backed by regression tests that reproduce the exact 9-holds+1-wrong-would-merge and healthy-raw/failing-weighted scenarios described in the PR body. One correctness nuance: the close-side loosen-warn gate changed from `closeFalse > 0` to `weightedClosePrecision < AUTOTUNE_CLOSE_PRECISION_FLOOR`, which is a defensible alignment with the close breaker but is a broader behavioral change than the PR title implies (it silently drops the previous closeFalse>0-with-null-precision warning path), and is tested but worth a second look given it's bundled with the two headline fixes.

Nits — 5 non-blocking
  • src/review/auto-tune.ts's computeTuningRecommendations loosen-warn condition change (closeFalse>0 → weightedClosePrecision<floor) is a third behavioral change bundled into a PR titled around wouldMerge/weighted-precision gating — worth calling out explicitly in the PR description since it changes what triggers a loosen-warn, not just which field it reads.
  • The PR description says orb(auto-tune): the tuning advisor gates on decided and RAW precision, the two things its own sibling breaker documents as wrong #10014 but the external history note flags 'partial' issue-link coverage — worth double-checking the issue fully scopes all three fixes (sample gate, weighted precision, dead guard) before merge.
  • test/unit/auto-tune.test.ts's renamed/added tests are thorough but rely heavily on comments to explain intent (orb(auto-tune): the tuning advisor gates on decided and RAW precision, the two things its own sibling breaker documents as wrong #10014 tags) rather than descriptive test-only helper names — fine as-is but a nit on density.
  • Consider splitting the PR description to explicitly enumerate the loosen-warn condition change as a third, separate fix alongside the two headline gating fixes, since it changes trigger semantics not just field selection.
  • In auto-apply.ts, the file is now ~436 lines per the size-smell note; if more auto-apply logic is anticipated, consider splitting the pure helpers (sanitize/merge/tightening/promotion-gate) from the D1-backed store functions into a separate module.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #10014
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 78 registered-repo PR(s), 60 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor shin-core; Gittensor profile; 78 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: significant
Linked issue satisfaction

Addressed
The diff implements all stated requirements: computeTuningRecommendations now gates on wouldMerge instead of decided, reads weighted merge/close precision in place of raw fields, the close-side test uses weightedClosePrecision against AUTOTUNE_CLOSE_PRECISION_FLOOR, AutoApplyContext.mergePrecision is renamed to weightedMergePrecision and threaded into evaluateShadowPromotion, and planCloseAutoTune

Review context
  • Author: shin-core
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: TypeScript, JavaScript, Solidity, Dart, Python, CSS, PHP, Rust
  • Official Gittensor activity: 78 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 31, 2026
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.02%. Comparing base (98a2564) to head (27ec3dc).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main   #10142       +/-   ##
===========================================
- Coverage   91.99%   80.02%   -11.97%     
===========================================
  Files         931      284      -647     
  Lines      114000    58944    -55056     
  Branches    27523     8754    -18769     
===========================================
- Hits       104877    47172    -57705     
- Misses       7823    11480     +3657     
+ Partials     1300      292     -1008     
Flag Coverage Δ
backend 99.07% <100.00%> (+3.39%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/review/auto-apply.ts 99.22% <100.00%> (ø)
src/review/auto-tune.ts 98.85% <100.00%> (-1.15%) ⬇️

... and 780 files with indirect coverage changes

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 4ce09ed into JSONbored:main Jul 31, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

orb(auto-tune): the tuning advisor gates on decided and RAW precision, the two things its own sibling breaker documents as wrong

1 participant