diff --git a/eval/tests/test_workflow_bench.py b/eval/tests/test_workflow_bench.py index 2bb0990a3..91786e213 100644 --- a/eval/tests/test_workflow_bench.py +++ b/eval/tests/test_workflow_bench.py @@ -14,6 +14,7 @@ import yaml from typing import Any from workflow_bench import runner +from workflow_bench.evolution import CANDIDATE_ARMS from workflow_bench.runner import ( aggregate, GraphBuildEnv, @@ -821,8 +822,12 @@ def test_artifacts_that_were_never_written_are_an_unhealthy_harness(): assert "review-evidence-invalid" in flagged[0].reasons -def test_one_admissible_cell_keeps_an_arm_healthy(): - """A mixed sweep is not a broken environment; the failures still surface.""" +def test_one_admissible_cell_leaves_an_arm_degraded_not_healthy(): + """Mixed outcomes are DEGRADED. One usable measurement does not erase two failures. + + Not fatal - the sweep still produced evidence - but calling it healthy is + how a partly-broken environment passes review. + """ mixed = [ _cell(resolved=False, error_kind="oracle-failed"), @@ -830,8 +835,9 @@ def test_one_admissible_cell_keeps_an_arm_healthy(): _cell(resolved=False, ok=False, error_kind="infra-error"), ] results = _arms(review=mixed) - assert unhealthy_arms(results, {"review"}) == [] health = arm_health(results, {"review"})["review"] + assert health.status == "DEGRADED" + assert unhealthy_arms(results, {"review"}) == [], "degraded is diagnostic, not fatal" assert health.execution_failures == 2, "failures must stay visible, not be erased" assert health.admissible == 1 @@ -860,3 +866,80 @@ def test_a_parseable_artifact_does_not_excuse_a_failed_session(): flagged = unhealthy_arms(_arms(review=rows), {"review"}) assert [h.arm for h in flagged] == ["review"] assert flagged[0].execution_failures == 2 + + +def test_a_single_unusable_review_is_caught_below_the_breaker_threshold(): + """The decisive regression for the finalization guard. + + A fixture of 41 empty artifacts would abort through the outage breaker - + review-evidence-invalid is systemic and the limit is 5 - so it proves + nothing about this path. One fresh unusable cell is under that threshold, + which leaves the finalization check as the only thing that can catch it. + """ + + streak = 0 + for _ in range(1): + streak = runner.systemic_outage_streak("review-evidence-invalid", streak) + assert streak < runner.DEFAULT_OUTAGE_STREAK, "fixture must not reach the breaker" + + results = _arms(review=[_cell(resolved=False, ok=False, error_kind="review-evidence-invalid")]) + with pytest.raises(SystemExit) as exc: + runner.enforce_measurement_health(results, {"review"}) + assert exc.value.code == 1 + + +def test_finalization_reports_every_arm_and_names_no_cause(capsys): + """Status for each arm; an empty artifact does not become an EROFS diagnosis.""" + + results = _arms( + review=[_cell(resolved=False, ok=False, error_kind="review-evidence-invalid")], + ce_review=[_cell(resolved=False, error_kind="oracle-failed")], + ) + with pytest.raises(SystemExit): + runner.enforce_measurement_health(results, {"review", "ce_review"}) + out = capsys.readouterr().out + assert "review: UNUSABLE" in out + assert "ce_review: OBSERVED_OK" in out + assert "cause=undetermined" in out + assert "EROFS" not in out and "mount" not in out + + +def test_valid_negatives_do_not_abort_finalization(capsys): + """The 16h run's shape must survive the real guard, not just the classifier.""" + + scored_but_wrong = [_cell(resolved=False, error_kind="oracle-failed") for _ in range(3)] + health = runner.enforce_measurement_health( + _arms(review=scored_but_wrong, ce_review=list(scored_but_wrong)), {"review", "ce_review"} + ) + assert {h.status for h in health.values()} == {"OBSERVED_OK"} + assert "UNUSABLE" not in capsys.readouterr().out + + +def test_reused_only_arm_is_reported_unknown_by_finalization(capsys): + runner.enforce_measurement_health(_arms(review=[_cell(reused=True)]), {"review"}) + assert "review: UNKNOWN" in capsys.readouterr().out + + +def test_run_sweep_calls_the_health_guard_and_not_the_legacy_helper(): + """Pins the wiring the caller correction exposed. + + Reads the compiled code object's global references rather than the source + text: deleting the call removes the name and fails this test, which is the + mutation check. It does NOT prove the guard runs end to end - _run_sweep + needs bwrap and a sandbox, so no test here drives it. + """ + + referenced = runner._run_sweep.__code__.co_names + assert "enforce_measurement_health" in referenced + assert "broken_incumbent_arms" not in referenced + + +def test_ce_review_is_classified_even_though_it_is_not_a_candidate_arm(): + """ce_review is a comparator, absent from CANDIDATE_ARMS. + + Dropping the `- {"review"}` exclusion alone would have left it unchecked. + """ + + assert "ce_review" not in set(CANDIDATE_ARMS.values()) + health = arm_health(_arms(ce_review=[_cell()]), {"review", "ce_review"}) + assert "ce_review" in health diff --git a/eval/workflow_bench/runner.py b/eval/workflow_bench/runner.py index 18ffcd88f..d83059d6a 100644 --- a/eval/workflow_bench/runner.py +++ b/eval/workflow_bench/runner.py @@ -1575,6 +1575,26 @@ class ArmHealth: return self.fresh_attempts > 0 + @property + def status(self) -> str: + """UNKNOWN / OBSERVED_OK / DEGRADED / UNUSABLE. + + DEGRADED is the distinction that matters: an arm with both admissible + measurements and observed failures produced usable evidence but did not + run reliably. Reporting that as healthy is how a partly-broken sweep + looks fine. It is diagnostic here - only UNUSABLE is fatal - so this + patch changes what is reported, not what is eligible. + """ + + if not self.measured: + return "UNKNOWN" + failures = self.execution_failures + self.evidence_failures + if self.admissible == 0 and failures > 0: + return "UNUSABLE" + if failures > 0: + return "DEGRADED" + return "OBSERVED_OK" + @property def unhealthy(self) -> bool: """Every fresh attempt failed to execute or to produce usable evidence. @@ -1584,9 +1604,7 @@ class ArmHealth: a valid negative and belongs to the quality gate, not here. """ - return self.measured and self.admissible == 0 and ( - self.execution_failures > 0 or self.evidence_failures > 0 - ) + return self.status == "UNUSABLE" def arm_health(results: dict[str, dict[str, dict[str, Any]]], arms: set[str]) -> dict[str, ArmHealth]: @@ -1623,11 +1641,52 @@ def unmeasured_arms(results: dict[str, dict[str, dict[str, Any]]], arms: set[str return [h.arm for h in arm_health(results, arms).values() if not h.measured] +def enforce_measurement_health( + results: dict[str, dict[str, dict[str, Any]]], arms: set[str] +) -> dict[str, ArmHealth]: + """Report every arm's measurement status; abort only on UNUSABLE. + + Runs after report.md and promotion.json are written, so a failing sweep + still leaves its evidence behind. Reports cause as undetermined: an empty + artifact establishes that evidence is unusable, not why - naming a mount + failure here would be a guess the recorded rows do not support. + """ + + health = arm_health(results, arms) + for arm in sorted(health): + observed = health[arm] + reasons = f" reason={','.join(observed.reasons)}" if observed.reasons else "" + print( + f"[measurement-health] {arm}: {observed.status} " + f"fresh_attempts={observed.fresh_attempts} admissible={observed.admissible} " + f"execution_failures={observed.execution_failures} " + f"evidence_failures={observed.evidence_failures}{reasons}" + ) + unusable = [h for h in health.values() if h.unhealthy] + if unusable: + detail = "; ".join(f"{h.arm} ({h.fresh_attempts} fresh attempt(s))" for h in unusable) + print( + f"[measurement-health] {detail} produced no usable measurement this sweep. " + "cause=undetermined — see error_detail in results.jsonl. Exiting non-zero rather " + "than reporting a quiet no-promotion." + ) + raise SystemExit(1) + return health + + def broken_incumbent_arms( results: dict[str, dict[str, dict[str, Any]]], incumbent_arms: set[str], ) -> list[str]: - """Incumbent arms that resolved nothing across every task they ran. + """LEGACY, NON-AUTHORITATIVE. Superseded by ``enforce_measurement_health``. + + Kept only so its historical behaviour stays documented and testable while + the replacement settles; it has no production caller. Do not wire it into a + health decision - it infers a broken environment from a resolution count, + which a reviewer facing a hard corpus falsifies. Remove once the + measurement-health path has run in CI. + + Incumbent arms that resolved nothing across every task they ran. An incumbent arm is the currently-shipped, presumably-working skill: if it resolves NOTHING across every task it ran, that reads as an environment or @@ -2558,31 +2617,9 @@ def _run_sweep( # excluded here because "resolved zero" is quality signal for a reviewer # facing a hard corpus; with the inference corrected they are included # again, which is what lets an all-artifacts-empty run be caught at all. - checked_arms = set(CANDIDATE_ARMS.values()) | {"review", "ce_review"} - unmeasured = unmeasured_arms(results, checked_arms) - if unmeasured: - print( - f"[harness-health] arm(s) {', '.join(sorted(unmeasured))} have no freshly-run cell " - "this sweep, so current execution health is UNKNOWN rather than good. Their rows " - "came from reuse; a paid cell per incumbent arm is what makes this measurable." - ) - unhealthy = unhealthy_arms(results, checked_arms) - if unhealthy: - # Fail loudly rather than let a broken environment read as a quiet - # "no promotion, incumbent stands." A low score never reaches here. - detail = "; ".join( - f"{h.arm}: {h.execution_failures} execution / {h.evidence_failures} evidence " - f"failure(s) over {h.fresh_attempts} fresh attempt(s)" - f"{' — ' + ', '.join(h.reasons) if h.reasons else ''}" - for h in unhealthy - ) - print( - f"[harness-health] {detail}. Every fresh attempt failed to execute or to produce " - "usable evidence, which is an environment/harness failure rather than a candidate " - "miss. See error_detail in results.jsonl. Exiting non-zero rather than reporting a " - "quiet no-promotion." - ) - raise SystemExit(1) + # ce_review is named explicitly: it is a comparator, not a candidate, so it + # is absent from CANDIDATE_ARMS and would otherwise go unclassified. + enforce_measurement_health(results, set(CANDIDATE_ARMS.values()) | {"review", "ce_review"}) if outage_tripped: # Non-zero exit so a driver (evolve.py) treats the partial benchmark as a # failed run and halts instead of proposing from outage-truncated evidence.