fix(scorer): exclude unscored samples from accuracy() and mean() denominator

[goktugozkanmd]

Summary

Score.unscored() is documented as "excluded from metrics and reducers" — the reducer layer honors this by skipping NaN-at-root values. However, accuracy() and mean() pass every sample through value_to_float() without checking for NaN, so a single unscored sample forces the entire metric to NaN.

This is part of the work tracked in #4286 (scoring metrics silently dropping / mishandling inconclusive and errored samples).

Changes

accuracy()

• Skips samples whose Score.value is NaN (the Score.unscored() sentinel)
• Computes the ratio over the scored subset only
• Logs coverage (scored / total) at INFO level when below 100%, so the effective evaluation scope is visible alongside the headline
• Clamps the result to [0, 1] with a WARNING if out of range (prevents silently reading 1.2 or -0.3 as accuracy)

mean()

• Filters out NaN values before calling np.mean()
• Returns 0.0 when no samples are scored

Compatibility

• All 33 existing metric tests pass unchanged
• Behaviour for fully scored evals (C/I/P/N strings, numeric values) is identical
• Only changes behaviour when Score.unscored() or raw float(\"nan\") values are present — these are now correctly excluded

Related

• Issue Scoring metrics silently drop inconclusive/errored samples from the denominator and don't bound their range #4286
• PR fix(scorer): mark model_graded grade-parse failure as unscored (not INCORRECT) #4048 (model_graded_qa grade-parse failure → unscored)
• PR fix(scorer): tag math() answer-extraction failures with reason metadata (#4026) #4091 (math() returns NOANSWER instead of INCORRECT)

Follow-up

Full three-bucket accounting (scored / inconclusive / errored) and a formal ScoreStatus enum are left as future work — this PR only fixes the immediate metric-level NaN corruption.

[goktugozkanmd]

@dragonstyle, this is ready for maintainer review. I rebased it onto current main and resolved the mean.py conflict without carrying over the unrelated clamp/log changes. uv run pytest tests/scorer -q passes: 347 passed, 157 optional-backend skips. Ruff also passes on the touched files. Could you take a look when you have bandwidth?

[MattFisher]

Thanks for looking at this area; metric handling of unscored samples is exactly where I've been spending time lately, so I want to flag something about the premise before this takes up a maintainer review cycle.

As far as I can tell, the NaN corruption described in the summary can't occur through the eval pipeline. NaN scores are filtered out before metrics are computed, in _eval/task/results.py: scorer_for_metrics filters NaN-at-root for scalar-valued scorers, and scorers_from_metric_dict does the same for dict-valued scorers, including NaN inside dict values, per metric key. These filters are what produce the scored_samples / unscored_samples counts on EvalScore. So by the time accuracy() or mean() runs, there are no NaN values left to skip. I've verified this end-to-end: a run with one CORRECT and one Score.unscored() sample reports accuracy = 1.0, scored_samples = 1, unscored_samples = 1 — no NaN in the metric.

That's consistent with the tests here: they invoke accuracy()(scores) directly on a hand-constructed SampleScore list, which bypasses the pipeline filter. That demonstrates the metric-level behaviour, but not a user-reachable bug.

Given that, I think the options are:

1. If you have a reproduction where a real eval run produces a NaN metric, please share it (task + settings + log), because it would mean the results-layer filter has a gap worth understanding before patching around it.
2. If the motivation is defense-in-depth for code that calls the metric functions directly, the PR should say that explicitly. In this case a couple of things would need tightening: the description still mentions coverage logging and [0,1] clamping that the rebase removed, and _is_unscored is private to _reducer.reducer, so importing it into the metrics modules creates a private cross-module dependency (it would want promoting to a shared location first).

On the broader visibility concern from #4286 (unscored/errored samples silently narrowing the effective denominator): that's now tracked as a presentation change in #4481, which proposes surfacing scored/errored/unscored counts next to headline metrics in the terminal and viewer. The data is already on EvalScore, so no metric-internals change is needed for it.

[goktugozkanmd]

Thanks, this is a fair distinction. I do not have an end-to-end reproduction where a normal eval run feeds `NaN` through to `accuracy()` / `mean()` and produces a `NaN` metric. The reproduction I had in mind is the broader coverage problem from #4286: samples can leave the effective denominator (`errored` or `unscored`) while the headline metric still looks clean/high. That is a real user-facing reporting problem, but it is not the same as the metric-level `NaN` corruption described in this PR. Given your explanation of `_eval/task/results.py`, I agree this PR should not be framed as fixing a user-reachable pipeline bug. I think the cleaner path is: 1. Treat this PR, if kept, only as defense-in-depth for direct callers of metric functions, and tighten it accordingly — including not importing a private `_is_unscored` helper from the reducer module. 2. Move the main user-facing work to #4481, where the actual issue is clearer: headline metrics should surface `scored / unscored / errored` coverage so readers can see what the metric was computed over. So I will stop presenting #4458 as the fix for #4286's main user-facing problem. If maintainers prefer, I am also fine with closing this PR in favor of #4481 and handling the direct-metric guard separately.

[goktugozkanmd]

Closing this myself in favor of #4481.

Matt's point is correct: I do not have an end-to-end reproduction where a normal eval run lets NaN propagate into accuracy() / mean(). The real user-facing problem is denominator visibility for scored / unscored / errored samples, and #4481 scopes that directly without changing metric internals or adding a private cross-module dependency.

If a direct-metric defense-in-depth guard is still useful later, it should be reopened as a narrower PR with the helper promoted to a shared public location first.