Skip to content

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) #1304

Description

@ErenAta16

Checked other resources

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):

# 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:

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:

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.jsonavg_at_n, avg_at_k_math.jsonavg_at_n_math, maj_at_k.jsonmaj_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)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions