mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-09 03:17:54 +00:00
fix(eval): judge harness health on execution, not on how many tasks resolved
broken_incumbent_arms infers "the environment is broken" from an arm resolving zero tasks. That inference does not hold: a reviewer can be wrong about every task in a hard corpus while every process, mount and capture worked perfectly. Actions run 33962002890 is exactly that shape - 51 cells, all resolved=False with error_kind=oracle-failed, median score 0.212, and a healthy harness. Someone already knew this, and patched it by excluding review arms at the call site. That leaves the unsound inference in place for workflow and workflow_direct, and leaves review arms with no health check at all - so the run that genuinely was broken, 33912693948, where the mount made an atomic write impossible and all 41 artifacts came back empty, could not have been caught here either. So this replaces the inference rather than adding another exemption. aggregate now classifies fresh rows into execution failures (the process or its tooling did not complete), evidence failures (it completed but produced nothing trustworthy or scoreable), and admissible measurements. An arm is unhealthy only when it has fresh attempts, zero admissible measurements, and at least one execution or evidence failure. Resolution count is no longer consulted. Arms with only reused rows report current health as UNKNOWN rather than good. With the inference corrected, review arms are checked again, which is what lets the empty-artifact case be caught at all. Deliberately unchanged: comparator reuse eligibility, quality denominators, promotion thresholds, model settings, skill prompts and scheduler behaviour. Failures that stop being called infrastructure failures still surface in the counts and reasons - an agent-originated failure must not vanish from reporting because it was reclassified. broken_incumbent_arms and its tests are left in place; deleting behaviour belongs in its own change. Seven regression tests, built from both runs' shapes and labelled as reconstructed from logged observations, since 33962002890's results.jsonl did not survive the instance shutdown. They pin: a badly-scoring reviewer is healthy; an all-zero score is still a valid negative; empty artifacts are unhealthy; one admissible cell keeps an arm healthy while its failures stay visible; reused rows alone leave health unknown; reused successes do not mask fresh failures; and a parseable artifact does not excuse a failed session. 602 eval tests pass, ruff clean.
This commit is contained in:
parent
46679b4708
commit
7dfc24f8e8
2 changed files with 225 additions and 11 deletions
|
|
@ -17,7 +17,10 @@ from workflow_bench import runner
|
|||
from workflow_bench.runner import (
|
||||
aggregate,
|
||||
GraphBuildEnv,
|
||||
arm_health,
|
||||
broken_incumbent_arms,
|
||||
unhealthy_arms,
|
||||
unmeasured_arms,
|
||||
build_parser,
|
||||
infra_error_record,
|
||||
next_graph_prefetch_target,
|
||||
|
|
@ -73,6 +76,13 @@ def test_aggregate_takes_medians_and_counts_resolved():
|
|||
"resolved": 2,
|
||||
# None of these are reused, so every resolution was measured this sweep.
|
||||
"resolved_fresh": 2,
|
||||
# Health is counted separately from resolution: all three executed and
|
||||
# produced usable evidence, including the one that resolved nothing.
|
||||
"fresh_attempts": 3,
|
||||
"admissible": 3,
|
||||
"execution_failures": 0,
|
||||
"evidence_failures": 0,
|
||||
"health_reasons": [],
|
||||
"runs": 3,
|
||||
"valid_runs": 3,
|
||||
"excluded_runs": 0,
|
||||
|
|
@ -758,3 +768,95 @@ def test_packed_sweep_window_must_keep_the_pool_fed():
|
|||
outage_limit=0,
|
||||
window=2,
|
||||
)
|
||||
|
||||
|
||||
def _cell(**overrides) -> dict[str, Any]:
|
||||
"""One results.jsonl row, healthy unless told otherwise."""
|
||||
|
||||
base = record(resolved=True)
|
||||
base.update({"error_kind": None, "review_evidence_valid": True, "transcript_missing": False})
|
||||
base.update(overrides)
|
||||
return base
|
||||
|
||||
|
||||
def _arms(**by_arm) -> dict[str, dict[str, dict[str, Any]]]:
|
||||
return {"task0": {arm: aggregate(rows) for arm, rows in by_arm.items()}}
|
||||
|
||||
|
||||
def test_a_reviewer_that_scores_badly_is_not_an_unhealthy_harness():
|
||||
"""Reconstructed from Actions run 33962002890's logged observations.
|
||||
|
||||
Every completed cell was resolved=False with error_kind=oracle-failed, at a
|
||||
median score of 0.212 — the reviews ran, wrote artifacts and were scored.
|
||||
That is a valid negative for the quality gate to judge. Diagnosing it as a
|
||||
broken environment is the confusion this classification exists to end.
|
||||
"""
|
||||
|
||||
scored_but_wrong = [_cell(resolved=False, error_kind="oracle-failed") for _ in range(3)]
|
||||
results = _arms(review=scored_but_wrong, ce_review=list(scored_but_wrong))
|
||||
assert unhealthy_arms(results, {"review", "ce_review"}) == []
|
||||
health = arm_health(results, {"review"})["review"]
|
||||
assert health.admissible == 3 and health.fresh_attempts == 3
|
||||
assert (health.execution_failures, health.evidence_failures) == (0, 0)
|
||||
|
||||
|
||||
def test_an_all_zero_score_is_still_a_valid_negative():
|
||||
zeroed = [_cell(resolved=False, error_kind="oracle-failed", review_weighted_f1=0.0) for _ in range(3)]
|
||||
assert unhealthy_arms(_arms(review=zeroed), {"review"}) == []
|
||||
|
||||
|
||||
def test_artifacts_that_were_never_written_are_an_unhealthy_harness():
|
||||
"""Reconstructed from Actions run 33912693948.
|
||||
|
||||
All 41 artifacts came back 0 bytes because the mount made an atomic write
|
||||
impossible. The reviews could not produce evidence at all — the opposite of
|
||||
the case above, and the one a health check must catch. The old caller
|
||||
excluded review arms entirely, so it could not have.
|
||||
"""
|
||||
|
||||
unwritable = [_cell(resolved=False, ok=False, error_kind="review-evidence-invalid") for _ in range(3)]
|
||||
flagged = unhealthy_arms(_arms(review=unwritable), {"review"})
|
||||
assert [h.arm for h in flagged] == ["review"]
|
||||
assert flagged[0].evidence_failures == 3
|
||||
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."""
|
||||
|
||||
mixed = [
|
||||
_cell(resolved=False, error_kind="oracle-failed"),
|
||||
_cell(resolved=False, ok=False, error_kind="session-error"),
|
||||
_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.execution_failures == 2, "failures must stay visible, not be erased"
|
||||
assert health.admissible == 1
|
||||
|
||||
|
||||
def test_reused_rows_alone_leave_current_health_unknown():
|
||||
"""Historical success cannot certify this sweep's environment."""
|
||||
|
||||
reused = [_cell(reused=True) for _ in range(3)]
|
||||
results = _arms(review=reused)
|
||||
assert unmeasured_arms(results, {"review"}) == ["review"]
|
||||
assert unhealthy_arms(results, {"review"}) == []
|
||||
assert arm_health(results, {"review"})["review"].measured is False
|
||||
|
||||
|
||||
def test_reused_successes_do_not_mask_fresh_execution_failures():
|
||||
rows = [_cell(reused=True), _cell(reused=True), _cell(ok=False, error_kind="session-error")]
|
||||
flagged = unhealthy_arms(_arms(review=rows), {"review"})
|
||||
assert [h.arm for h in flagged] == ["review"]
|
||||
assert flagged[0].fresh_attempts == 1 and flagged[0].execution_failures == 1
|
||||
|
||||
|
||||
def test_a_parseable_artifact_does_not_excuse_a_failed_session():
|
||||
"""Artifact parseability must not override an execution failure."""
|
||||
|
||||
rows = [_cell(ok=False, error_kind="session-error", review_evidence_valid=True) for _ in range(2)]
|
||||
flagged = unhealthy_arms(_arms(review=rows), {"review"})
|
||||
assert [h.arm for h in flagged] == ["review"]
|
||||
assert flagged[0].execution_failures == 2
|
||||
|
|
|
|||
|
|
@ -787,6 +787,17 @@ EXCLUDED_ERROR_KINDS = REUSE_EXCLUDED_ERROR_KINDS
|
|||
# an occasional handful of cells on an aborted sweep.
|
||||
PACKED_WINDOW_MULTIPLIER = 2
|
||||
|
||||
# Health classification. These answer "did the harness work", which is a
|
||||
# different question from "did the agent get the right answer" - a review can be
|
||||
# wrong about a hard corpus while every process, mount and capture behaved.
|
||||
#
|
||||
# EXECUTION: the process or its tooling did not complete. Nothing was measured.
|
||||
# EVIDENCE: it completed, but what it produced cannot be trusted or scored.
|
||||
# Everything else - including resolved=False and a zero score - is a VALID
|
||||
# NEGATIVE: an admissible measurement that the quality gate then judges.
|
||||
EXECUTION_FAILURE_KINDS = frozenset({"session-error", "infra-error", "cleanup-failure", "cancelled"})
|
||||
EVIDENCE_FAILURE_KINDS = frozenset({"review-evidence-invalid", "evidence-unverified", "skill-not-invoked"})
|
||||
|
||||
SYSTEMIC_ERROR_KINDS = frozenset({"session-error", "infra-error", "cleanup-failure", "review-evidence-invalid"})
|
||||
DEFAULT_OUTAGE_STREAK = 5
|
||||
|
||||
|
|
@ -1466,6 +1477,26 @@ def aggregate(records: list[dict[str, Any]]) -> dict[str, Any]:
|
|||
out["cost_usd"] = (
|
||||
None if (not valid or any(cost is None for cost in valid_costs)) else statistics.median(valid_costs)
|
||||
)
|
||||
fresh = [r for r in records if not r.get("reused")]
|
||||
out["fresh_attempts"] = len(fresh)
|
||||
out["execution_failures"] = sum(1 for r in fresh if r.get("error_kind") in EXECUTION_FAILURE_KINDS)
|
||||
out["evidence_failures"] = sum(
|
||||
1
|
||||
for r in fresh
|
||||
if r.get("error_kind") in EVIDENCE_FAILURE_KINDS
|
||||
or r.get("review_evidence_valid") is False
|
||||
or r.get("transcript_missing") is True
|
||||
)
|
||||
# Admissible means the harness delivered a trustworthy measurement. It says
|
||||
# nothing about whether the answer was right, which is the whole point.
|
||||
out["admissible"] = out["fresh_attempts"] - out["execution_failures"] - out["evidence_failures"]
|
||||
out["health_reasons"] = sorted(
|
||||
{
|
||||
str(r.get("error_kind"))
|
||||
for r in fresh
|
||||
if r.get("error_kind") in EXECUTION_FAILURE_KINDS or r.get("error_kind") in EVIDENCE_FAILURE_KINDS
|
||||
}
|
||||
)
|
||||
out["resolved"] = sum(1 for r in records if r["resolved"])
|
||||
# Reused rows are last generation's measurement. The health canary below has
|
||||
# to ask whether THIS environment worked, so it needs the freshly-run count.
|
||||
|
|
@ -1527,6 +1558,71 @@ def savings(baseline: dict[str, Any], workflow: dict[str, Any]) -> dict[str, Any
|
|||
return out
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ArmHealth:
|
||||
"""What the harness observed for one arm this sweep, before any judgement."""
|
||||
|
||||
arm: str
|
||||
fresh_attempts: int
|
||||
admissible: int
|
||||
execution_failures: int
|
||||
evidence_failures: int
|
||||
reasons: tuple[str, ...]
|
||||
|
||||
@property
|
||||
def measured(self) -> bool:
|
||||
"""False when only reused rows exist - current health is UNKNOWN, not good."""
|
||||
|
||||
return self.fresh_attempts > 0
|
||||
|
||||
@property
|
||||
def unhealthy(self) -> bool:
|
||||
"""Every fresh attempt failed to execute or to produce usable evidence.
|
||||
|
||||
Deliberately not "resolved zero tasks". A reviewer can be wrong about
|
||||
every task in a hard corpus with the harness working perfectly; that is
|
||||
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
|
||||
)
|
||||
|
||||
|
||||
def arm_health(results: dict[str, dict[str, dict[str, Any]]], arms: set[str]) -> dict[str, ArmHealth]:
|
||||
"""Fold per-task aggregates into one health observation per arm."""
|
||||
|
||||
health: dict[str, ArmHealth] = {}
|
||||
for arm in sorted(arms):
|
||||
rows = [task_arms[arm] for task_arms in results.values() if arm in task_arms]
|
||||
if not rows:
|
||||
continue
|
||||
reasons: set[str] = set()
|
||||
for row in rows:
|
||||
reasons.update(row.get("health_reasons") or ())
|
||||
health[arm] = ArmHealth(
|
||||
arm=arm,
|
||||
fresh_attempts=sum(int(r.get("fresh_attempts", 0)) for r in rows),
|
||||
admissible=sum(int(r.get("admissible", 0)) for r in rows),
|
||||
execution_failures=sum(int(r.get("execution_failures", 0)) for r in rows),
|
||||
evidence_failures=sum(int(r.get("evidence_failures", 0)) for r in rows),
|
||||
reasons=tuple(sorted(reasons)),
|
||||
)
|
||||
return health
|
||||
|
||||
|
||||
def unhealthy_arms(results: dict[str, dict[str, dict[str, Any]]], arms: set[str]) -> list[ArmHealth]:
|
||||
"""Arms whose every fresh attempt failed to execute or to produce evidence."""
|
||||
|
||||
return [h for h in arm_health(results, arms).values() if h.unhealthy]
|
||||
|
||||
|
||||
def unmeasured_arms(results: dict[str, dict[str, dict[str, Any]]], arms: set[str]) -> list[str]:
|
||||
"""Arms with no fresh attempt at all - reported as unknown, never as healthy."""
|
||||
|
||||
return [h.arm for h in arm_health(results, arms).values() if not h.measured]
|
||||
|
||||
|
||||
def broken_incumbent_arms(
|
||||
results: dict[str, dict[str, dict[str, Any]]],
|
||||
incumbent_arms: set[str],
|
||||
|
|
@ -2457,18 +2553,34 @@ def _run_sweep(
|
|||
}
|
||||
(out_dir / "promotion.json").write_text(json.dumps(promotion, indent=2) + "\n")
|
||||
print(f"\n{report}\n\nWritten to {out_dir}/")
|
||||
# A reviewer may legitimately match none of a difficult hidden corpus;
|
||||
# unlike an implementation arm, zero exact resolutions is quality signal,
|
||||
# not proof that the harness failed.
|
||||
broken_incumbents = broken_incumbent_arms(results, set(CANDIDATE_ARMS.values()) - {"review"})
|
||||
if broken_incumbents:
|
||||
# Fail loudly rather than let a broken environment read as a quiet
|
||||
# "no promotion, incumbent stands."
|
||||
# Health is judged on whether fresh attempts EXECUTED and produced usable
|
||||
# evidence - never on how many tasks they resolved. Review arms used to be
|
||||
# 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] incumbent arm(s) {', '.join(broken_incumbents)} resolved zero "
|
||||
"tasks across every valid run — this looks like an environment/harness failure, "
|
||||
"not a normal candidate miss. See the errors column in report.md and error_detail "
|
||||
"in results.jsonl. Exiting non-zero rather than reporting a quiet no-promotion."
|
||||
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)
|
||||
if outage_tripped:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue