Skip to content

npm run evaluate passes on an empty benchmarks/cases.json: NaN thresholds disable the ranking gate #461

Description

@aryamthecodebreaker

npm run evaluate is the ranking-quality gate in npm run ci. If benchmarks/cases.json is empty or truncated to [], both hit rates become NaN, every threshold comparison is false, and the gate exits 0 — reporting success having measured nothing.

scripts/benchmark-savings.mjs guards against exactly this and evaluate.mjs does not, so this reads as an oversight rather than a decision.

Reproduction

echo "[]" > benchmarks/cases.json
npm run evaluate
{
  "cases": 0,
  "hits": 0,
  "top1Hits": 0,
  "top1HitRate": null,
  "top3HitRate": null,
  "thresholds": {
    "top1": 0.5,
    "top3": 0.8
  },
  "results": []
}
EXIT=0

The recorded output is null rather than an error, because JSON.stringify(NaN) is null — so a downstream consumer reading top1HitRate sees "no data" and nothing anywhere says the suite was empty.

Threshold logic in isolation, for the three shapes:

cases=10  top1Rate=0.6    top3Rate=0.9    -> PASSES (exit 0)
cases=10  top1Rate=0      top3Rate=0.1    -> FAILS  (exit 1)
cases=0   top1Rate=NaN    top3Rate=NaN    -> PASSES (exit 0)

Cause

scripts/evaluate.mjs:19-20, 32-37:

const top3HitRate = hits / results.length;      // 0/0 -> NaN
const top1HitRate = top1Hits / results.length;  // 0/0 -> NaN
...
if (top1HitRate < summary.thresholds.top1 || top3HitRate < summary.thresholds.top3) {
  process.exit(1);
}

Every comparison against NaN is false, so the failure branch is unreachable and the script falls through to exit 0.

There is no assertion on cases.length. Compare scripts/benchmark-savings.mjs:45-47, which pins its dataset size explicitly:

if (dataset.cases.length !== 16) {
  throw new Error(`Expected the frozen 16-case regression dataset, found ${dataset.cases.length}.`);
}

Impact

The suite is currently 8 cases. A bad merge, a truncated write, a JSON edit that leaves [], or a path change that resolves to a different file all produce a green npm run ci with the ranking gate silently disabled — while the summary JSON still gets recorded and published as evidence.

This repository takes evaluation integrity seriously enough to have withdrawn a baseline claim that did not survive a fair comparison (#449). A gate that passes on zero cases is the same class of problem one level down: the number is absent rather than wrong, and absence currently reads as success.

Suggested fix

Assert the case count, matching the sibling script:

if (!Array.isArray(cases) || cases.length === 0) {
  throw new Error(`benchmarks/cases.json must contain at least one case; found ${Array.isArray(cases) ? 0 : typeof cases}.`);
}

Pinning the exact expected count, as benchmark-savings.mjs does, would additionally catch a partial truncation that leaves some cases behind — worth considering, though it does mean touching the script whenever a case is added.

Guarding the threshold comparison itself (!Number.isFinite(top1HitRate) || ...) would be belt-and-braces and costs one line.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingevaluationBenchmarks, datasets, and ranking quality

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions