diff --git a/eval/tests/test_offline_sweep_integration.py b/eval/tests/test_offline_sweep_integration.py index b86c95b6b..dd3f258a1 100644 --- a/eval/tests/test_offline_sweep_integration.py +++ b/eval/tests/test_offline_sweep_integration.py @@ -262,12 +262,9 @@ def test_a_review_that_never_invoked_its_skill_is_not_a_measurement(bench, monke assert row["error_kind"] == "skill-not-invoked" assert code not in (None, 0), "the sweep must not report success on unusable evidence" - # Recorded here as the behaviour actually is, not as I first assumed. The - # row still carries a score, and aggregate() still counts it toward the arm - # median: its filter drops EXCLUDED_ERROR_KINDS and evidence_valid=False, - # and "skill-not-invoked" is neither. The health guard is what stops the - # sweep, so a single-run sweep cannot promote on it - but in a mixed run the - # median for an arm would include a cell whose skill never ran. Pinned so - # the behaviour cannot change silently either way. + # The row still carries its own score - the artifact was well formed - but + # aggregate() now keeps it out of the arm's QUALITY median, since a cell + # whose skill never ran did not measure that skill. It still counts for + # cost, because the session ran and was billed. assert row["review_weighted_f1"] == 1.0 assert row["review_evidence_valid"] is True diff --git a/eval/tests/test_workflow_bench.py b/eval/tests/test_workflow_bench.py index a53b2b8cd..b8b70c0d6 100644 --- a/eval/tests/test_workflow_bench.py +++ b/eval/tests/test_workflow_bench.py @@ -1052,3 +1052,26 @@ def test_a_raising_packed_cell_still_persists_its_settled_siblings(): ) assert (1, "review") in folded, "the sibling that completed was never recorded" + + +def test_an_uninvoked_skill_does_not_move_the_quality_median_but_still_costs(): + """An arm measures a SKILL; a cell where the skill never ran did not measure it. + + The row's evidence is well formed, so the old filter kept it and its score + moved the arm's quality median - an arm could be credited for a review it + never performed with the skill under test. Cost and duration still count: + that session really ran and really was billed. + """ + + good = record(review_weighted_f1=1.0, cost_usd=2.0) + uninvoked = record( + review_weighted_f1=0.0, cost_usd=4.0, error_kind="skill-not-invoked", skill_invoked=False + ) + agg = aggregate([good, uninvoked]) + + assert agg["review_weighted_f1"] == 1.0, "the uninvoked cell must not drag quality" + assert agg["cost_usd"] == 3.0, "but it was still billed, so it counts for cost" + + # A wrong-but-valid review is a quality result and must still count. + wrong = record(review_weighted_f1=0.0, cost_usd=2.0, resolved=False, error_kind="oracle-failed") + assert aggregate([good, wrong])["review_weighted_f1"] == 0.5 diff --git a/eval/workflow_bench/runner.py b/eval/workflow_bench/runner.py index a2223a20a..25a2d5f5a 100644 --- a/eval/workflow_bench/runner.py +++ b/eval/workflow_bench/runner.py @@ -1591,11 +1591,17 @@ def aggregate(records: list[dict[str, Any]]) -> dict[str, Any]: "review_category_accuracy", "review_grounded_evidence", ) - if any("review_weighted_f1" in record for record in valid): + # QUALITY metrics only: a cell whose skill never ran did not measure the + # skill, so its score must not move the arm's quality median. It stays in + # `valid` for cost and duration, because that session really did run and + # really was billed - and it stays visible to the promotion gate, which has + # its own vocabulary for a candidate that never loaded its skill. + scored = [record for record in valid if record.get("error_kind") != "skill-not-invoked"] + if any("review_weighted_f1" in record for record in scored): for metric in review_metrics: - values = [record[metric] for record in valid if record.get(metric) is not None] + values = [record[metric] for record in scored if record.get(metric) is not None] reducer = min if metric == "review_blocker_recall" else statistics.median - out[metric] = reducer(values) if values and len(values) == len(valid) else None + out[metric] = reducer(values) if values and len(values) == len(scored) else None verdicts = [record.get("review_verdict_correct") for record in valid] out["review_verdict_correct"] = ( all(value is True for value in verdicts)