Skip to content

benchmark(ground-truth): benchmarkHorizonEnd throws on an unparseable `frozenAt #9644

Description

@JSONbored

⚠️ 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

deriveBenchmarkGroundTruth (packages/loopover-engine/src/calibration/benchmark-ground-truth.ts:120) is the
pure core that produces the labels every candidate proposal is scored against. Its header states the contract:
"PURE: no clock, no IO, no randomness" and "STABILITY. Ground truth for a given (snapshot, horizon) must not
depend on WHEN extraction runs"
. Two inputs break it in ways the current code does not handle.

1. An unparseable frozenAt throws rather than failing structurally.
benchmarkHorizonEnd (line 101) does:

return new Date(Date.parse(frozenAt) + horizonDays * MS_PER_DAY).toISOString();

Date.parse returns NaN for a malformed string, and new Date(NaN).toISOString() throws RangeError: Invalid time value. The same happens for a non-finite horizonDays. This module's sibling validator
validateBenchmarkProposal / validateBenchmarkTask
(packages/loopover-engine/src/calibration/benchmark-proposal.ts:185, 213) is explicitly built to the opposite
contract — "pure, never throws for ordinarily-invalid input, one error per failing field path" — and it is
BenchmarkTask, not the ground-truth input, that gets validated there. deriveBenchmarkGroundTruth takes a
free-form object (snapshotRef, frozenAt, horizonDays, workUnitIds, events, reversals) that no
validator on the path covers, so a bad frozenAt reaching it is an uncaught exception in the harness rather
than a named failure.

2. A duplicated work-unit id silently corrupts every denominator.
Line 159 iterates input.workUnitIds directly and pushes one truth per element. The doc calls workUnitIds
"the authoritative roster from the snapshot", and coverage.workUnits (line 180) is
input.workUnitIds.length. A roster containing the same id twice therefore emits two identical truths, counts
the unit twice in workUnits, counts it twice in unresolved if unresolved, and yields an unresolvedRate
computed over an inflated denominator. scoreableGroundTruths (line 199) — documented as "The work units a
scorer may divide by … the single definition of that rule so no scorer can re-implement it slightly
differently"
— returns the duplicate twice, so a scorer built on it double-weights that work unit. Nothing in
the module rejects or collapses the duplicate.

Both matter more than usual here because this is the ground truth a public leaderboard's numbers are divided
by (#9262 requirement 3, #9265's commitments block).

Requirements

  • benchmarkHorizonEnd must throw a NAMED, descriptive error rather than a bare RangeError when frozenAt
    is unparseable or horizonDays is not a finite number: throw new Error(\invalid_frozen_at: ${frozenAt}`)andthrow new Error(`invalid_horizon_days: ${horizonDays}`), using the negated-compound guard form (if (!Number.isFinite(x))) so NaNfails closed — matchingsplitBacktestCorpus (packages/loopover-engine/src/calibration/backtest-split.ts:26-28) and computeReliabilityCurve (packages/loopover-engine/src/calibration/reliability-curve.ts:77-90`).
  • deriveBenchmarkGroundTruth must throw invalid_duplicate_work_unit_id: <id> when workUnitIds contains a
    duplicate. It must NOT silently de-duplicate: a roster with a duplicate is a bug in the snapshot builder
    (benchmark: frozen-repo snapshot builder — leak-proof repo state at commit T #9259) and hiding it would let a corrupted snapshot score normally.
  • deriveBenchmarkGroundTruth must call the guarded benchmarkHorizonEnd before any other work, so an invalid
    window is rejected before partial results are computed.
  • The module header must document these three rejection cases alongside its existing three rules, in the same
    prose style.

⚠️ Required pattern: mirror splitBacktestCorpus's guard style at
packages/loopover-engine/src/calibration/backtest-split.ts:25-28 — a negated compound condition and a
snake_case_reason: value error message, described in its own doc comment as a caller bug. What does NOT
satisfy this issue: returning a result object with an error field (this module's siblings throw for caller
bugs and return values for data outcomes); silently de-duplicating workUnitIds; clamping an invalid
horizonDays; or adding the guards without extending the module header.

Deliverables

  • benchmarkHorizonEnd throws invalid_frozen_at: … for an unparseable frozenAt and
    invalid_horizon_days: … for a non-finite horizonDays, asserted by tests for each including the NaN
    input.
  • deriveBenchmarkGroundTruth throws invalid_duplicate_work_unit_id: … for a duplicated roster entry,
    asserted by a test.
  • A test asserting the existing valid-input behaviour is byte-identical after the change (same
    BenchmarkGroundTruthSet for an existing fixture), so the guards are additive.
  • The module header documents all three rejection cases.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example adding
the frozenAt guard while leaving the duplicate-roster path unguarded — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted, and packages/loopover-engine/src/**/*.ts IS
inside coverage.include — this file is measured and gated exactly like src/**, so the patch-coverage gate
applies in full. Every new guard needs a test for the throwing arm AND the passing arm (a valid frozenAt, a
finite horizonDays, a duplicate-free roster), which is also the 100%-branch discipline
packages/loopover-engine/test/ already holds for the sibling calibration modules. The module's own header
calls its invariant tests the deliverable; treat them as such.

Expected Outcome

A malformed snapshot window or a duplicated work-unit roster fails with a named, actionable error at the pure
core, instead of throwing an opaque RangeError from the harness or silently double-weighting one work unit in
every score derived from the set.

Links & Resources

packages/loopover-engine/src/calibration/benchmark-ground-truth.ts:97-203,
packages/loopover-engine/src/calibration/backtest-split.ts:20-39,
packages/loopover-engine/src/calibration/reliability-curve.ts:72-90,
packages/loopover-engine/src/calibration/benchmark-proposal.ts:180-242. Related: #9259, #9261, #9262.

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions