feat(review): pure config-drift evaluator over the loosenable-knob registry - #8232
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…gistry (JSONbored#8212) The loosening evaluator answers "can we safely loosen?"; nothing answered the inverse operator question from the JSONbored#8170 retro's largest wrongness source -- stale configuration: "is what is CURRENTLY live still the best-supported setting, in either direction?". Add evaluateKnobDrift(knob, cases, liveValue) beside evaluateKnobLoosening, mirroring its discipline verbatim: the knob's own split seed/fraction via splitBacktestCorpus, the same Pareto floor via compareBacktestScores (strictly improved on visible AND non-regressed on held-out), the same never-on-noise sample minimums, and the hard minimum no evidence may cross. The candidate pool is every registry candidate PLUS the shipped value -- a TIGHTER alternative dominating live is exactly the stale-config signal -- minus the live value itself, tried nearest-to-live first (minimal config change wins, mirroring smallest-step-first; equidistant ties deterministically prefer the tighter value). The report distinguishes direction ("shipped" checked first: a drifted override should revert; else looser = informational duplicate of the loosening loop, tighter = actionable staleness warning) and carries corpus sizes plus both split comparisons per the JSONbored#8121 evidence-trail convention. Null -- never a guess -- on sample-floor misses or when nothing strictly dominates. Pure evaluation only: no cron, no alert, no writes. Tests mirror the suite's membership-probe seeding: dominance in both directions, the shipped-value revert signal, no-dominance null, sample-floor null, held-out regression rejection, and byte-identical determinism.
…ub-hard-minimum filter (JSONbored#8212)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8232 +/- ##
==========================================
- Coverage 92.10% 89.55% -2.55%
==========================================
Files 776 98 -678
Lines 78328 22651 -55677
Branches 23668 3876 -19792
==========================================
- Hits 72147 20286 -51861
+ Misses 5062 2187 -2875
+ Partials 1119 178 -941
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-23 13:39:38 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk 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.
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.
|
Summary
evaluateKnobLoosening,src/services/loosening-knobs.ts) answers "can we safely loosen?"; nothing answered the inverse operator question from the calibration: backfill phase 2 — GitHub-truth reversed labels (retro successor scan) + raw-context re-fetch #8170 retro's largest identified wrongness source — stale configuration: "is what is CURRENTLY live still the best-supported setting, in either direction?"evaluateKnobDrift(knob, cases, liveValue)exported besideLOOSENABLE_KNOBS, mirroringevaluateKnobLoosening's discipline verbatim (the two named anchors): the knob's own split seed/fraction viasplitBacktestCorpus, the same Pareto floor viacompareBacktestScores(strictlyimprovedon the visible split AND non-regressedon the deterministic held-out split), the same never-on-noise sample minimums, and the hard minimum no evidence may cross.direction:"shipped"(checked first — a drifted override should revert),"looser"(informational; duplicates the loosening loop's own proposal),"tighter"(actionable staleness warning), and carries corpus sizes plus both split comparisons per the Epic: validated auto-loosening for auto-tune.ts's tuning advisor, gated on a clean backtest #8121 evidence-trail convention.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run typechecknpm run actionlintnpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
vitest run test/unit/loosening-knobs.test.ts— 14 tests green, including the 7 new drift cases mirroring the suite's membership-probe seeding technique per the issue's test requirement) plus the full rootnpm run typecheckandnpm run build --workspace @loopover/engine. The new tests cover dominance in both directions, the shipped-value revert signal, the no-dominance null, the sample-floor null, held-out regression rejection, and byte-identical determinism — both sides of every conditional in the new evaluator.actionlint/workers/mcp/ui checks are untouched surfaces; CI runs them all.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
Not applicable — pure backend evaluation function (no UI, docs, or extension surface touched; no cron, no writes, nothing visible changes until the separate wire issue consumes it).
Notes
evaluateKnobLooseningitself is untouched — the drift evaluator sits beside it sharing the privatecompareOnSlicehelper, so the two disciplines can never diverge on how a candidate is scored.