From b02d98795cafa48a2d670e0463fe62aec03ef91b Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 26 Oct 2026 21:49:05 -0400 Subject: [PATCH] fix: validate repeated pass query cohorts before reporting --- .github/workflows/devdex-eval.yml | 2 +- benchmark/run_benchmark.py | 8 ++- devdex/scorer/report_metrics.py | 25 +++++++- devdex/tests/test_pass_cohort_integrity.py | 74 ++++++++++++++++++++++ 4 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 devdex/tests/test_pass_cohort_integrity.py diff --git a/.github/workflows/devdex-eval.yml b/.github/workflows/devdex-eval.yml index d554749..35b4a5a 100644 --- a/.github/workflows/devdex-eval.yml +++ b/.github/workflows/devdex-eval.yml @@ -59,7 +59,7 @@ jobs: - name: Install run: | python -m pip install --upgrade pip - pip install pytest + pip install pytest numpy - name: Dataset + scorer tests # the DIRECTORY, not a hand-listed set -- a new test file must not need a CI edit to run run: python -m pytest devdex/tests -q diff --git a/benchmark/run_benchmark.py b/benchmark/run_benchmark.py index 07c55c7..a407f88 100644 --- a/benchmark/run_benchmark.py +++ b/benchmark/run_benchmark.py @@ -102,16 +102,18 @@ def _ci(values, rng): def report(engine, out=None): """Read the run records back and score them with the published scorer.""" - from devdex.scorer.report_metrics import score_records + from devdex.scorer.report_metrics import check_pass_cohort, score_records rng = np.random.default_rng(0) rows, results = [], {} for t in TRACKS: files = sorted(glob.glob(str(ROOT / f"devdex/runs/*ext-{engine}-{t}/run*/tasks/*.json"))) if not files: continue - recs = [] + recs, cohort = [], None for f in files: - recs += json.load(open(f)) + records = json.load(open(f)) + cohort = check_pass_cohort(records, cohort, f) + recs += records is_gold, pool_test = _scorer_for(t) m = score_records(recs, t, is_gold, pool_test) if not m: diff --git a/devdex/scorer/report_metrics.py b/devdex/scorer/report_metrics.py index b396242..f5b21d6 100644 --- a/devdex/scorer/report_metrics.py +++ b/devdex/scorer/report_metrics.py @@ -399,6 +399,28 @@ def _mean_cells(cells): return out +def check_pass_cohort(records, expected, cell): + """Validate one row per qid and the same query set in every repeated pass. + + A single smoke subset is valid. A partial or different repeat is not an independent + measurement of that same subset and cannot be averaged into its score. + """ + seen = set() + for record in records: + qid = record.get("qid") + if not isinstance(qid, str) or not qid: + raise SystemExit(f"{cell}: every record needs a nonempty string qid") + if qid in seen: + raise SystemExit(f"{cell}: duplicate qid {qid!r} within one pass") + seen.add(qid) + if expected is not None and seen != expected: + raise SystemExit( + f"{cell}: inconsistent query cohort across repeated passes " + f"({len(seen)} items versus {len(expected)}); " + "finish the incomplete pass or report separate experiments separately") + return seen + + # ------------------------------------------------------------------ report # Docs runs may carry any docs_v3.x GT: the versions share one item set and differ only in how # wide canonical_sources is. Scoring them all against the widest is deliberate. What is NOT safe @@ -466,9 +488,10 @@ def _docs_pool(record, engine): files = run_files(runs_dir, pass_label, token, arm, ".json") if not files: continue - per_pass = [] + per_pass, cohort = [], None for f in files: records = json.load(open(f)) + cohort = check_pass_cohort(records, cohort, f) if family == "docs" and records: check_docs_coverage(records, docs_meta, f"{track}/{arm}") one = score_records(records, family, gold_test(family), pool_test(family)) diff --git a/devdex/tests/test_pass_cohort_integrity.py b/devdex/tests/test_pass_cohort_integrity.py new file mode 100644 index 0000000..a70b166 --- /dev/null +++ b/devdex/tests/test_pass_cohort_integrity.py @@ -0,0 +1,74 @@ +"""Reports must not call different query sets repeated measurements of the same items.""" +import json + +import pytest + +from devdex.scorer import report_metrics + + +def row(qid, hit=True): + return {"qid": qid, "gold_accept": ["gold"], "citations": ["gold"] if hit else [], + "committed": hit, "search_log": [{"results": [{"rank": 0, "ref": "gold"}]}]} + + +def save(root, records, number, public): + label = "ext-example-repo" if public else "p1-opus-repo-fc-mcp" + prefix = root / "devdex/runs" if public else root + tasks = prefix / f"20261026T120000-{label}" / f"run{number}/tasks" + tasks.mkdir(parents=True) + (tasks / f"p{number}_repo_arm.json").write_text(json.dumps(records)) + + +def report(root, public, monkeypatch): + if public: + from benchmark import run_benchmark + monkeypatch.setattr(run_benchmark, "ROOT", root) + return run_benchmark.report("example", root / "results.json") + return report_metrics.build(str(root), "p1") + + +@pytest.mark.parametrize("public", [False, True]) +@pytest.mark.parametrize("second", [[row("a")], [row("c"), row("d")], []]) +def test_different_or_incomplete_repeat_cannot_publish_a_pooled_score(tmp_path, monkeypatch, + public, second): + save(tmp_path, [row("a"), row("b", False)], 1, public) + save(tmp_path, second, 2, public) + with pytest.raises(SystemExit, match="cohort"): + report(tmp_path, public, monkeypatch) + assert not (tmp_path / "results.json").exists() + + +@pytest.mark.parametrize("public", [False, True]) +def test_duplicate_query_within_one_pass_is_rejected(tmp_path, monkeypatch, public): + save(tmp_path, [row("a"), row("a")], 1, public) + with pytest.raises(SystemExit, match="duplicate"): + report(tmp_path, public, monkeypatch) + + +@pytest.mark.parametrize("public", [False, True]) +def test_reordered_equal_cohort_and_repeated_queries_across_passes_are_valid(tmp_path, monkeypatch, + public): + save(tmp_path, [row("a"), row("b", False)], 1, public) + save(tmp_path, [row("b", False), row("a")], 2, public) + result = report(tmp_path, public, monkeypatch) + if public: + assert result == 0 + assert json.loads((tmp_path / "results.json").read_text())["tracks"]["repo"]["recall@10"] == 0.5 + else: + cells, failures = result + assert failures == [] + assert cells["repo"]["fc-mcp"]["n"] == 2 + assert cells["repo"]["fc-mcp"]["passes"] == 2 + assert cells["repo"]["fc-mcp"]["recall"] == 0.5 + + +def test_one_smoke_subset_pass_remains_reportable(tmp_path, monkeypatch): + save(tmp_path, [row("smoke-only")], 1, True) + assert report(tmp_path, True, monkeypatch) == 0 + + +@pytest.mark.parametrize("public", [False, True]) +def test_missing_query_identity_is_rejected(tmp_path, monkeypatch, public): + save(tmp_path, [row(None)], 1, public) + with pytest.raises(SystemExit, match="qid"): + report(tmp_path, public, monkeypatch)