Skip to content

engine(calibration): phase7-calibration-loop's normalizeCompositeWeights lacks the invalid-vs-zero-weight distinction its siblings implement #10316

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

packages/loopover-engine/src/phase7-calibration-loop.ts's normalizeCompositeWeights (lines
173-193) has a comment claiming its zero-total branch is "reachable ONLY when both weights were
explicitly 0":

function normalizeCompositeWeights(config: Phase7CalibrationConfig): { historicalReplay: number; prOutcome: number } {
  const raw = {
    historicalReplay: finiteNonNegative(config.historicalReplayWeight, DEFAULT_CONFIG.historicalReplayWeight),
    prOutcome: finiteNonNegative(config.prOutcomeWeight, DEFAULT_CONFIG.prOutcomeWeight),
  };
  const total = raw.historicalReplay + raw.prOutcome;
  if (total <= 0) {
    // #8644: ... This branch is reachable ONLY when both weights were explicitly 0 --
    // `finiteNonNegative` floors any invalid input to a positive default, which cannot sum to 0 ...
    return { historicalReplay: 0, prOutcome: 0 };
  }
  ...
}

That claim is contradicted by finiteNonNegative itself (lines 121-125), two lines away in the same
file:

function finiteNonNegative(value: number | undefined, fallback: number): number {
  if (value === undefined) return fallback;
  if (!Number.isFinite(value) || value < 0) return 0;
  return value;
}

finiteNonNegative does NOT float invalid input to "a positive default" — it returns 0 (not the
fallback) for a NaN/negative value. It only returns the fallback when value === undefined. So
if config.historicalReplayWeight and config.prOutcomeWeight are both NaN or negative (not
undefined), raw is {0, 0}, total is 0, and the "explicit all-zero" branch fires — even
though the caller never explicitly asked for {0, 0}, they supplied invalid weights.

This matters because computePhase7CalibrationLoop (lines 358-361) has a fast path that bypasses
sanitization entirely:

const config =
  input.config && "phase7LoopEnabled" in input.config
    ? (input.config as Phase7CalibrationConfig)
    : resolvePhase7CalibrationConfig(input.config);

When the caller passes an object that already has a phase7LoopEnabled key (i.e. treated as an
already-resolved Phase7CalibrationConfig), resolvePhase7CalibrationConfig's own sanitization
(which DOES correctly recover NaN/"heavy"-style invalid weights to their defaults, per
test/unit/phase7-calibration-loop.test.ts's "warns on malformed values and falls back safely"
test) is skipped entirely, and the raw, unsanitized config object flows straight into
normalizeCompositeWeights.

Every sibling calibration composer in this package handles this correctly:
gate-verdict-calibration.ts::normalizeCompositeWeights (line 309),
finding-severity-calibration.ts::normalizeCompositeWeights (line 356), and
reviewer-consensus-calibration.ts::normalizeCompositeWeights (line ~395) all call a private
isInvalidWeight() helper first: when the summed weights are <= 0, they distinguish an explicit,
legitimate all-zero weighting (preserve {0,0,0}) from a genuinely-invalid input (recover to
DEFAULT_COMPOSITE_WEIGHTS, i.e. the 50/50-equivalent default). phase7-calibration-loop.ts's
normalizeCompositeWeights has no such distinction — it always returns {0, 0} for any non-positive
total, silently forcing combinedAccuracy: null forever for a caller who passed invalid (not
explicitly-zero) weights via the pre-resolved-config fast path, instead of recovering to the 50/50
default the sibling composers would.

packages/loopover-engine/test/phase7-calibration-loop.test.ts's only test of this branch (line
497, #8644) exercises the explicit-{0,0}-weight case only; no test exercises the pre-resolved-
config-with-NaN/negative-weight path.

Requirements

  • Add an isInvalidWeight helper to phase7-calibration-loop.ts, matching the shape and semantics
    of the existing isInvalidWeight in gate-verdict-calibration.ts (or finding-severity- calibration.ts/reviewer-consensus-calibration.ts — all three are equivalent): it must return
    true for undefined, NaN, and negative values, and false for a legitimate 0 or any
    positive finite number.
  • In normalizeCompositeWeights, when total <= 0, check isInvalidWeight(config.historicalReplayWeight) || isInvalidWeight(config.prOutcomeWeight) against the RAW config values (not the already-floored
    raw values) before deciding the outcome:
    • If either raw weight is invalid per isInvalidWeight, recompute raw using
      DEFAULT_CONFIG.historicalReplayWeight/DEFAULT_CONFIG.prOutcomeWeight (this file's existing
      0.5/0.5 defaults, declared at lines 108-109) in place of the invalid value(s), then normalize the
      same way the non-zero-total branch below already does (historicalReplay / total, prOutcome / total) so the result is the module's existing 50/50-equivalent default split, not {0, 0}.
    • Otherwise (both raw weights are legitimately, explicitly 0), preserve today's {historicalReplay: 0, prOutcome: 0} behavior — this is the #8644 explicit-zero-preservation case and must not
      regress.
  • Correct the misleading comment claiming finiteNonNegative "floors any invalid input to a positive
    default" — it does not; state accurately what finiteNonNegative actually does and why the new
    isInvalidWeight check is needed as a result.

Deliverables

  • normalizeCompositeWeights({ ...DEFAULT_CONFIG, historicalReplayWeight: Number.NaN, prOutcomeWeight: -3 }) returns the same normalized weights as
    normalizeCompositeWeights(DEFAULT_CONFIG) (recovers to the default split), not {0, 0}.
  • normalizeCompositeWeights({ ...DEFAULT_CONFIG, historicalReplayWeight: 0, prOutcomeWeight: 0 }) still returns {historicalReplay: 0, prOutcome: 0} (the #8644 explicit-zero case is
    unchanged).
  • The misleading comment about finiteNonNegative's behavior is corrected.

All three Deliverables are required in this one PR — there is no narrower scope for this issue.

Test Coverage Requirements

packages/loopover-engine/src/** is measured by Codecov via two separate uploads whose hits are
unioned
— root test/** AND packages/loopover-engine/test/**. Add regression tests to
packages/loopover-engine/test/phase7-calibration-loop.test.ts (this file's existing test location)
covering: (1) normalizeCompositeWeights/computePhase7CalibrationLoop called via the
pre-resolved-config fast path (an object containing phase7LoopEnabled) with a NaN or negative
weight recovers to the default split rather than zeroing out, and (2) the existing #8644
explicit-all-zero-weight case still preserves {0, 0}. Target 100% branch coverage of the new
isInvalidWeight check, both the invalid-input branch and the legitimate-explicit-zero branch.

Expected Outcome

A caller that bypasses resolvePhase7CalibrationConfig's sanitization by passing an already-shaped
Phase7CalibrationConfig object with invalid (NaN/negative) weights gets the same safe recovery
behavior the three sibling calibration composers already provide, instead of a silently-permanent
combinedAccuracy: null.

Links & Resources

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