Skip to content

docs(specs): canonical specs 087/088 state rules their modules no longer implement #2353

Description

@widecloud

#2348 named one canonical spec per multiply-specified module — benchmark/repo_task_mean.py → spec
088, benchmark/tie_order_share.py → spec 087. Both documents predate guards that were later added
to the modules they specify, so in three places the canonical spec now states a rule its module does
not implement. Because these are the documents a change to those modules is reviewed against, the
drift is in exactly the text a reviewer is meant to trust.

Each claim below was checked by running the module, not by reading the sibling specs.

Evidence

1. Oversized task counts are skipped, not counted (spec 088).

Spec 088, Partition stats, requires:

A repo row SHALL count toward the mean only when its tasks value passes _is_int AND is > 0

repo_task_mean._is_task_count additionally probes float(value) and rejects a count that raises
OverflowError, so a 10**400 count — which passes _is_int and is > 0 — is logged and skipped:

>>> summarize_repo_task_mean({"per_repo": [{"tasks": 10**400}, {"tasks": 4}]})
{'kind': 'multi', 'scored_repos': 1, 'total_tasks': 4, 'mean_tasks_per_repo': 4.0, ...}
# spec 088 requires scored_repos == 2

The same clause governs the single-artifact branch, which spec 088 says yields
mean_tasks_per_repo == float(tasks) for any positive int; float(10**400) raises, and the
module reports zero counts with a None mean instead.

2. An unrepresentable mean becomes None (spec 088).

Spec 088 makes None exclusive to the empty case:

WHEN scored_repos > 0 THEN mean_tasks_per_repo SHALL be round(total_tasks / scored_repos, 3);
WHEN scored_repos == 0 THEN it SHALL be None.

_safe_mean also returns None — with a logger.warning — when the division overflows, which can
happen with scored_repos > 0 because a sum can exceed float range even when each row fit.

3. The generalization gate keys on the share, not on the counts (specs 087 and 041).

Spec 087, Artifact-kind branches, requires:

WHEN kind is generalization AND both partitions report an int total and an int tie
THEN the overall total/tie SHALL be their sums …
WHEN … either partition's total or tie is not an int (a malformed partition) THEN the
overall total/tie/tie_order_share SHALL all be None

benchmark/order_share.py gates on each partition's derived share being non-None, which is
strictly stronger: a zero-task partition has integer counts but no share, so it withholds the
overall even though both partitions report int counts.

>>> Z = {"agree": 0, "disagree": 0, "tie": 0, "single": 0, "offline": 0}
>>> H = {"agree": 4, "disagree": 0, "tie": 1, "single": 0, "offline": 0}
>>> s = summarize_tie_order_share({"generalization_gap": 0.0,
...     "tuned": {"judge_order_stats": Z}, "held_out": {"judge_order_stats": H}})
>>> {k: s[k] for k in ("total", "tie", "tie_order_share")}
{'total': None, 'tie': None, 'tie_order_share': None}
# both partitions report int total and int tie (0/0 and 5/1);
# spec 087 requires the sums — total 5, tie 1, share 0.2

Spec 041 (superseded by 087) carries the same clause, and the gate is shared by all six
make_order_share bindings, so the drift covers agree_/disagree_/single_/dual_order_share
and offline_share wherever their specs state the same rule.

Impact

Review-against-spec is part of the contribution rubric, and #2348 made these two documents the ones
that review runs against. A reviewer applying spec 088 literally would read _is_task_count's
overflow probe as an unspecified extra condition and ask for its removal — reinstating the
OverflowError crash the guard exists to prevent. The order_share gate is the same shape: its
comment cites sibling fixes in scored_fraction (#1274), skip_share (#1272) and
dual_order_coverage (#1280), so the specs are the only artifacts still describing the pre-fix
behavior. The drift is also invisible to the contract tests, which assert the clauses each spec
does state, so nothing currently fails.

Suggested fix

Amend the canonical specs to state the as-built behavior; the guards read as deliberate (the
_is_task_count docstring cites how weight_integrity rejects oversized ints, and the gate cites
three sibling fixes by issue number), so the modules are the source of truth here.

  • Spec 088: add the representability condition to the counted-row criterion, and allow None from
    _safe_mean on overflow with the warning it emits.
  • Spec 087 (and 041's mirrored clause): restate the generalization branch in terms of each
    partition's share rather than its counts.
  • Extend the affected contract tests to cover the newly stated criteria, so the specs stay pinned.

Changing the modules to match the specs instead would reintroduce the crashes those guards were
added for, and is not recommended.

This touches specs/** and tests/**, so it needs benchmark-change-approved before a PR is
opened.

Metadata

Metadata

Assignees

No one assigned

    Labels

    benchmarkEvaluation harnessbugSomething isn't workingdocumentationImprovements or additions to documentation

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions