huggingface / huggingface/lighteval
avg_at_k / avg_at_k_math / maj_at_k metric regression fixtures silently no-op (metric_class name doesn't match registered Metrics enum members)
- Dominant language
- Python
- Stars
- 2.5k
- Forks
- 555
- Avg merge
- 1d 6h
- Merged PRs (30d)
- 1
Description
### Checked other resources
- [x] This is a bug, not a usage question.
- [x] I added a clear and descriptive title that summarizes this issue.
- [x] I used the GitHub search to find a similar issue and didn't find one. Closest hits were #1054/PR #1058 (an unrelated earlier `MajAtN` argument-order bug, already fixed) and PR #1274 (open, fixes a real `MajAtN` `IndexError` on non-first-choice golds) — neither mentions that the JSON fixture meant to cover that exact `maj_at_n`/gold-position scenario never actually runs. Also checked PR #939 (which introduced this test framework) and PR #1292 (an unrelated `SamplingMetric` normalizer bug) for overlap; none.
- [x] I am sure this is a bug in LightEval rather than my code.
- [x] The bug is not resolved on the latest commit on `main`.
- [x] I posted a self-contained, minimal, reproducible example. A maintainer can copy it and run it AS IS.
### Description
`tests/unit/metrics/test_automated_metrics_pytest.py` parametrizes `TestAutomatedMetrics.test_metric_suite` over every `*.json` file under `tests/unit/metrics/test_cases/`, and each file is supposed to exercise one `Metrics` enum member end-to-end via `AutomatedMetricTester`. Three of these files use a `metric_class` value that was never a valid `Metrics` member name:
| fixture file | `metric_class` used | actually registered as |
|---|---|---|
| `tests/unit/metrics/test_cases/avg_at_k.json` | `"avg_at_k"` | `avg_at_n` |
| `tests/unit/metrics/test_cases/avg_at_k_math.json` | `"avg_at_k_math"` | `avg_at_n_math` |
| `tests/unit/metrics/test_cases/maj_at_k.json` | `"maj_at_k"` | `maj_at_n` |
(`src/lighteval/metrics/metrics.py:154` / `:434` register these as `avg_at_n` / `maj_at_n` — the "number of samples" convention used throughout the codebase — while the three fixtures were apparently written against the academic `@k` notation instead.)
`AutomatedMetricTester.run_test_case()` has a fallback for metrics that are legitimately excluded (GPU cost, broken scorer, etc. — see `SKIPPED_METRICS` at `test_metrics_automated.py:85-89`):
```python
# test_metrics_automated.py:138-147
if test_case.metric_class not in self.METRIC_CLASSES:
return {
"test_case": test_case.name,
"success": True, # Mark as success to skip
...
"skipped": True,
"skip_reason": f"Metric '{test_case.metric_class}' not available in METRIC_CLASSES",
}
```
Because `"avg_at_k"` / `"avg_at_k_math"` / `"maj_at_k"` aren't in `SKIPPED_METRICS` either, they fall into this same branch by accident: `metric.compute_sample(...)` is never called, and `success: True` is returned regardless of what `expected_output` says. `TestAutomatedMetrics.test_metric_suite` (`test_automated_metrics_pytest.py:82-104`) treats `skipped` results exactly like real passes (only reachable via a non-captured `print()`, never surfaced as `SKIPPED` in the pytest summary), so `pytest` reports these three files as green.
Net effect: 10 test cases across 3 fixture files have provided zero coverage of `AvgAtN`/`MajAtN` since this framework was added (PR #939), while looking, both in the JSON and in CI output, like they do. This is also why a real, independently-reported `MajAtN` crash (`IndexError` when the gold isn't `choices[0]`, PR #1274, currently open) went uncaught: `maj_at_k.json`'s first case (`"gold_index": [1]`) is precisely that scenario, and it has never actually run.
### Reproduction
Against `huggingface/lighteval @ 64f4f5ae173626509fad6e477ca4ee56ebb26129` (`main`) — verified byte-identical via `git hash-object` against the GitHub API blob sha for `src/lighteval/metrics/metrics_sample.py`, `src/lighteval/tasks/requests.py`, `src/lighteval/models/model_output.py`.
**Part 1 — which fixtures silently no-op:**
```python
import glob, json, sys
from pathlib import Path
sys.path.insert(0, "tests/unit/metrics")
from test_metrics_automated import AutomatedMetricTester
tester = AutomatedMetricTester()
registered = set(tester.METRIC_CLASSES.keys())
deliberately_skipped = {"faithfulness", "bert_score", "simpleqa_judge"}
for path in sorted(glob.glob("tests/unit/metrics/test_cases/*.json")):
data = json.load(open(path, encoding="utf-8"))
for suite in (data if isinstance(data, list) else [data]):
for tc in suite.get("test_cases", []):
mc = tc.get("metric_class")
if mc not in registered:
tag = "deliberate" if mc in deliberately_skipped else "NAMING MISMATCH"
print(f"{Path(path).name}: metric_class={mc!r} unregistered [{tag}]")
```
Actual output (unedited):
```
avg_at_k.json: metric_class='avg_at_k' unregistered [NAMING MISMATCH]
avg_at_k.json: metric_class='avg_at_k' unregistered [NAMING MISMATCH]
avg_at_k.json: metric_class='avg_at_k' unregistered [NAMING MISMATCH]
avg_at_k_math.json: metric_class='avg_at_k_math' unregistered [NAMING MISMATCH]
avg_at_k_math.json: metric_class='avg_at_k_math' unregistered [NAMING MISMATCH]
avg_at_k_math.json: metric_class='avg_at_k_math' unregistered [NAMING MISMATCH]
bert_score.json: metric_class='bert_score' unregistered [deliberate]
faithfulness.json: metric_class='faithfulness' unregistered [deliberate] (x3)
maj_at_k.json: metric_class='maj_at_k' unregistered [NAMING MISMATCH] (x4)
simpleqa_judge.json: metric_class='simpleqa_judge' unregistered [deliberate]
```
**Part 2 — proof the metric is genuinely never invoked**, by patching `MajAtN.compute`/`AvgAtN.compute` to unconditionally raise and re-running the exact same framework used by pytest:
```python
from unittest.mock import patch
from test_metrics_automated import AutomatedMetricTester, MetricTestSuite
from lighteval.metrics.metrics_sample import MajAtN, AvgAtN
def blow_up(*a, **k):
raise RuntimeError("metric was actually invoked")
with patch.object(MajAtN, "compute", blow_up), patch.object(AvgAtN, "compute", blow_up):
tester = AutomatedMetricTester()
for fname in ("avg_at_k.json", "avg_at_k_math.json", "maj_at_k.json"):
suite = MetricTestSuite(**json.load(open(f"tests/unit/metrics/test_cases/{fname}", encoding="utf-8")))
results = tester.run_test_suite(suite)
print(fname, "success=", sum(r["success"] for r in results), "skipped=", sum(r.get("skipped", False) for r in results))
```
Actual output (unedited — no `RuntimeError` raised, meaning `compute()` was never called even once):
```
avg_at_k.json cases= 3 reported success= 3 skipped(never called compute)= 3
avg_at_k_math.json cases= 3 reported success= 3 skipped(never called compute)= 3
maj_at_k.json cases= 4 reported success= 4 skipped(never called compute)= 4
```
**Part 3 — the actual pytest run**, confirming this is exactly what CI sees:
```
$ python -m pytest tests/unit/metrics/test_automated_metrics_pytest.py -k "avg_at_k or maj_at_k" -v
...test_cases\avg_at_k.json] PASSED [ 33%]
...test_cases\avg_at_k_math.json] PASSED [ 66%]
...test_cases\maj_at_k.json] PASSED [100%]
3 passed, 44 deselected in 25.85s
```
**Expected:** either the fixtures use the registered metric names and genuinely exercise `AvgAtN`/`MajAtN`, or, if a `metric_class` can't be resolved, the run should be reported as `SKIPPED` (visibly, in the pytest summary) rather than folded into `PASSED`. **Actual:** three fixtures worth 10 test cases silently no-op and are indistinguishable from a real pass in normal CI output.
### Suggested fix
- Rename `metric_class` in `avg_at_k.json` → `avg_at_n`, `avg_at_k_math.json` → `avg_at_n_math`, `maj_at_k.json` → `maj_at_n` (matching what the fixtures' own `expected_output` values were presumably computed against — worth double-checking each expected value once the metric is actually invoked, since it never has been).
- Separately, `run_test_case()`'s fallback conflates "deliberately excluded metric" with "unresolvable name." Consider two different states: gate deliberate exclusions with the existing `SKIPPED_METRICS` list only, and treat any other unresolved `metric_class` as a hard failure (or at minimum `pytest.skip()` so it shows up as `s` in the summary instead of `.`/`PASSED`). That would have caught this immediately.
### System Info
```
lighteval==0.13.1.dev0
Python 3.12.10
pytest==9.1.0
Windows 11 (platform-independent bug — pure test-selection logic, no I/O involved)
Commit under test: 64f4f5ae173626509fad6e477ca4ee56ebb26129 (main)
```
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with tests/unit/metrics/test_automated_metrics_pytest.py and tests/unit/metrics/test_metrics_automated.py, then inspect the three named JSON fixtures and the Metrics registrations in src/lighteval/metrics/metrics.py. Run the targeted pytest command from the reproduction. Done means the fixtures resolve to registered metrics, execute their cases, and any unresolved non-excluded name is visibly reported rather than counted as a pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100