fix(engine): guard benchmark ground-truth against invalid window and duplicate roster - #9882
Conversation
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 18:58:03 UTC
Review summary Nits — 4 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.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…duplicate roster benchmarkHorizonEnd threw a bare RangeError for an unparseable frozenAt or a non-finite horizonDays (new Date(NaN).toISOString()), and deriveBenchmarkGroundTruth silently emitted two truths for a duplicated workUnitId, inflating workUnits, unresolved, and unresolvedRate's denominator. Add negated-compound guards throwing invalid_frozen_at / invalid_horizon_days in benchmarkHorizonEnd (called before any other work), and reject a duplicated roster entry with invalid_duplicate_work_unit_id rather than de-duplicating a corrupted snapshot. Production formulas, thresholds, and valid-input output are unchanged. Closes JSONbored#9644
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #9882 +/- ##
===========================================
- Coverage 91.75% 79.28% -12.48%
===========================================
Files 919 281 -638
Lines 112987 58510 -54477
Branches 27193 6777 -20416
===========================================
- Hits 103667 46387 -57280
- Misses 8034 11840 +3806
+ Partials 1286 283 -1003
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Summary
benchmarkHorizonEndandderiveBenchmarkGroundTruth(packages/loopover-engine/src/calibration/benchmark-ground-truth.ts) — the pure core that produces the labels every candidate proposal is scored against — mishandled two inputs theirPURE … STABILITYcontract does not cover:frozenAt/ non-finitehorizonDays—new Date(Date.parse(frozenAt) + horizonDays * MS_PER_DAY).toISOString()throws a bareRangeError: Invalid time valueforNaN, an uncaught exception in the harness rather than a named structural failure.workUnitId— the roster loop emitted one truth per element, so a duplicate produced two identical truths and double-counted the unit inworkUnits,unresolved, and theunresolvedRatedenominator.Fix
benchmarkHorizonEndnow uses negated-compound guards (if (!Number.isFinite(x))) throwinginvalid_frozen_at: <value>andinvalid_horizon_days: <value>, mirroringsplitBacktestCorpus'sinvalid_held_out_fractionstyle soNaNfails closed.deriveBenchmarkGroundTruthcalls the guardedbenchmarkHorizonEndbefore any other work, then rejects a duplicated roster entry withinvalid_duplicate_work_unit_id: <id>rather than silently de-duplicating a corrupted snapshot (benchmark: frozen-repo snapshot builder — leak-proof repo state at commit T #9259).No production logic changed: formulas, thresholds, vocabularies, and valid-input output are byte-identical (asserted by an additive-guards test).
Tests
Added to
packages/loopover-engine/test/benchmark-ground-truth.test.ts(theengineCodecov flag's ownnode:testsuite): namedinvalid_frozen_at/invalid_horizon_daysthrows includingNaNandInfinity, propagation throughderiveBenchmarkGroundTruth, theinvalid_duplicate_work_unit_idrejection, and an additive-guards test asserting a valid input still yields the exact sameBenchmarkGroundTruthSet. Full suite passes (16/16).Closes #9644