scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-10-03 02:21:44 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 8

fix(eval): pin the health guard below the breaker, and stop calling mixed runs healthy

Browse source 
Two corrections to the health-classification patch.

The regression I wrote could not have proved what it claimed. A fixture of 41
empty artifacts aborts through the outage breaker long before finalization:
review-evidence-invalid is in SYSTEMIC_ERROR_KINDS and the limit is 5, so it
trips at cell 5 through the pre-existing path. It demonstrated failure
detection, not the new guard. The decisive test now uses ONE fresh unusable
cell, asserts the streak stays under the breaker threshold, and only then
requires finalization to abort - leaving the new check as the only thing that
can catch it. Removing the call makes that test fail; restoring it passes.

The accurate defect statement is narrower than the last message claimed. Review
arms were excluded from the final incumbent-health check while the consecutive-
failure breaker gave them separate, partial coverage. They were not unguarded.

Second: "one admissible cell plus two execution failures" was asserted as
healthy. That converts "not wholly unusable" into "ran reliably", which is how
a partly-broken sweep passes review. Arms now report UNKNOWN, OBSERVED_OK,
DEGRADED or UNUSABLE. Only UNUSABLE is fatal, so eligibility and promotion are
untouched - this changes what is reported, not what is allowed.

The guard is extracted as enforce_measurement_health so it can be driven
directly, and it now reports a status line per arm. It names no cause: an empty
artifact establishes that evidence is unusable, not that a mount rejected the
write, so it prints cause=undetermined rather than guessing EROFS. It still
runs after report.md and promotion.json are written, so a failing sweep leaves
its evidence behind.

ce_review is named explicitly at the call site. It is a comparator rather than
a candidate, so it is absent from CANDIDATE_ARMS.values(), and dropping the
review exclusion alone would have left it unclassified.

The wiring test reads _run_sweep's compiled code object for the referenced
global rather than matching source text. It is honest about its limit: it
proves the call exists and would catch its removal, but no test here drives
_run_sweep end to end, which needs bwrap and a sandbox.

broken_incumbent_arms is marked LEGACY and NON-AUTHORITATIVE with removal
tracked. It has no production caller.

608 eval tests pass, ruff clean. The two test_model_gateway.py failures are
test_locked_litellm_translates_messages_to_offline_responses and
test_openai_gateway_never_leaves_proxy_output_on_an_undrained_pipe; both fail
identically on origin/main in this environment, checked directly rather than
carried forward as an inherited label.
This commit is contained in:
Gergo Magyar 2026-09-07 12:28:51 +00:00
parent 7dfc24f8e8
commit 46ce838e8f
2 changed files with 152 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

89
eval/tests/test_workflow_bench.py
View file

@ -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

95
eval/workflow_bench/runner.py
View file

@ -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.