From c94c908ff51e5721d2a5f00895e3aac1abdeb871 Mon Sep 17 00:00:00 2001 From: Stanley Phoong Date: Sat, 8 Aug 2026 17:40:57 -0700 Subject: [PATCH] feat(swe-bench): eval-phase infra-vs-genuine error classifier The Pyxis sentinel only covers the agent phase; eval-phase error_ids were counted as real outcomes and never retried, which is what produced 24 of 25 permanently-bad runs on the source cluster. classify.py reads the SWE-bench report's error_ids and each instance's run_instance.log and classifies them through an ORDERED rule list, first match wins. The order is load-bearing: BuildImageError is checked before everything because its message embeds the other rules' needles, CONMON_EAGAIN and TEST_TIMEOUT precede WEDGE_EVAL, and PATCH_APPLY_FAILED is last. Anything unclassifiable is UNKNOWN and UNKNOWN is GENUINE, asserted by a membership test: a false bad-run costs one redo, a false retry biases the measurement toward optimism. Memory-kill markers are consumed by phase - an eval-phase kill is a genuine failure (an unbounded allocation is a failing patch), an agent-phase kill is recorded for audit only, since the agent merely gets an error observation and the instance still reaches a real outcome. --- .../swe_bench_distributed/__init__.py | 14 ++ .../swe_bench_distributed/classify.py | 234 ++++++++++++++++++ .../swe_bench_distributed/test_classify.py | 171 +++++++++++++ 3 files changed, 419 insertions(+) create mode 100644 src/inference_endpoint/evaluation/swe_bench_distributed/classify.py create mode 100644 tests/unit/evaluation/swe_bench_distributed/test_classify.py diff --git a/src/inference_endpoint/evaluation/swe_bench_distributed/__init__.py b/src/inference_endpoint/evaluation/swe_bench_distributed/__init__.py index 73fd2cbab..c2711f071 100644 --- a/src/inference_endpoint/evaluation/swe_bench_distributed/__init__.py +++ b/src/inference_endpoint/evaluation/swe_bench_distributed/__init__.py @@ -11,6 +11,14 @@ exactly once. """ +from .classify import ( + GENUINE_KINDS, + INFRA_KINDS, + ErrorKind, + UnitClassification, + classify_eval_log, + classify_unit, +) from .guards import HealthTerm, HealthVerdict, MemoryGuard, combine_terms, kill_by_pid from .merge import MergeRefusal, MergeResult, merge_run, verify_inventory from .queue import ( @@ -23,7 +31,10 @@ from .units import Unit, UnitPlan, plan_units __all__ = [ + "GENUINE_KINDS", + "INFRA_KINDS", "ClaimError", + "ErrorKind", "HealthTerm", "HealthVerdict", "LocalProcessLiveness", @@ -33,10 +44,13 @@ "OwnerLiveness", "SlurmStepLiveness", "Unit", + "UnitClassification", "UnitOutcome", "UnitPlan", "UnitResult", "WorkQueue", + "classify_eval_log", + "classify_unit", "combine_terms", "kill_by_pid", "merge_run", diff --git a/src/inference_endpoint/evaluation/swe_bench_distributed/classify.py b/src/inference_endpoint/evaluation/swe_bench_distributed/classify.py new file mode 100644 index 000000000..b7c06ac25 --- /dev/null +++ b/src/inference_endpoint/evaluation/swe_bench_distributed/classify.py @@ -0,0 +1,234 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Split a unit's error instances into infrastructure damage and genuine failures. + +A SWE-bench run reports three per-instance outcomes: resolved, unresolved, and +*error*. The agent phase has an infrastructure sentinel (the Pyxis +``infrastructure_failure_path``), but the eval phase has none: an instance whose +evaluation container wedged is booked as ``error``, which counts as "accounted +for", so the unit is published successful and is never retried. Those instances +silently poison a run that can then never reach a full result. + +Classification exists to catch exactly that. It reads each error instance's +``run_instance.log`` and assigns one kind. + +BIAS RULE -- this is the whole design and it is deliberately asymmetric. If an +error cannot be classified confidently it is treated as GENUINE, never as +infrastructure. A false bad-run costs one redo; a false retry silently biases +the measurement toward optimism, and an optimistic accuracy number is worse than +no number. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass, field +from enum import StrEnum +from pathlib import Path + +logger = logging.getLogger(__name__) + + +class ErrorKind(StrEnum): + """One classification of an error instance.""" + + # Infrastructure: a defect in the runtime we provided, safe to retry. + CONTAINER_EXEC_REFUSED = "container_exec_refused" + CONTAINER_FORK_EAGAIN = "container_fork_eagain" + RUNTIME_READ_TIMEOUT = "runtime_read_timeout" + IMAGE_BUILD_TIMEOUT = "image_build_timeout" + IMAGE_BUILD_ERROR = "image_build_error" + STEP_INFRASTRUCTURE_FAILURE = "step_infrastructure_failure" + ENDPOINT_CHANGED = "endpoint_changed" + + # Genuine: a real outcome of the model's patch, or unreadable. Never retried. + TEST_TIMEOUT = "test_timeout" + TEST_MEMORY_EXCEEDED = "test_memory_exceeded" + PATCH_APPLY_FAILED = "patch_apply_failed" + UNKNOWN = "unknown" + + +#: Retryable. Every member is a defect in infrastructure we control. +INFRA_KINDS: frozenset[ErrorKind] = frozenset( + { + ErrorKind.CONTAINER_EXEC_REFUSED, + ErrorKind.CONTAINER_FORK_EAGAIN, + ErrorKind.RUNTIME_READ_TIMEOUT, + ErrorKind.IMAGE_BUILD_TIMEOUT, + ErrorKind.IMAGE_BUILD_ERROR, + ErrorKind.STEP_INFRASTRUCTURE_FAILURE, + ErrorKind.ENDPOINT_CHANGED, + } +) + +#: Never retried. +#: +#: ``TEST_TIMEOUT`` is a plausible model outcome: a patch that makes the suite +#: loop is a failing patch. ``TEST_MEMORY_EXCEEDED`` is its exact parallel -- a +#: patch that makes a graded test allocate without bound is a failing patch, and +#: the alternative to killing it was never "the test passes", it was "the host +#: OOMs and the instance still never completes". ``PATCH_APPLY_FAILED`` is the +#: model emitting a diff that does not apply; SWE-bench books it as ``error`` +#: rather than ``unresolved``, but it is model behaviour. ``UNKNOWN`` is the bias +#: rule. +GENUINE_KINDS: frozenset[ErrorKind] = frozenset( + { + ErrorKind.TEST_TIMEOUT, + ErrorKind.TEST_MEMORY_EXCEEDED, + ErrorKind.PATCH_APPLY_FAILED, + ErrorKind.UNKNOWN, + } +) + +# ORDERED. First match wins, and the order is load-bearing. +# +# CONTAINER_FORK_EAGAIN and TEST_TIMEOUT come BEFORE CONTAINER_EXEC_REFUSED: a +# timed-out or fork-failed evaluation frequently *also* emits "container state +# improper" while the harness tears the container down, and reading that as a +# wedge would retry a genuine model outcome. +# +# PATCH_APPLY_FAILED is checked LAST: if a container also wedged, the wedge +# wins, because a wedged container's verdict is unreliable either way. Only a +# log with no infrastructure signature at all reaches this rule. +_RULES: tuple[tuple[ErrorKind, tuple[str, ...]], ...] = ( + ( + ErrorKind.CONTAINER_FORK_EAGAIN, + ("fork/exec /usr/bin/conmon: resource temporarily unavailable",), + ), + (ErrorKind.TEST_TIMEOUT, ("Test timed out after",)), + ( + ErrorKind.CONTAINER_EXEC_REFUSED, + ( + "can only create exec sessions on running containers", + "container state improper", + ), + ), + (ErrorKind.RUNTIME_READ_TIMEOUT, ("Read timed out. (read timeout=",)), + ( + ErrorKind.PATCH_APPLY_FAILED, + ( + "Reversed (or previously applied) patch detected", + ">>>>> Patch Apply Failed", + "hunk FAILED", + "hunk failed", + ), + ), +) + + +def classify_eval_log(text: str) -> ErrorKind: + """Classify one instance's evaluation log. + + ``BuildImageError`` is checked before the ordered rules because its message + embeds the same "Read timed out" / "500" strings the other rules look for, + so any other order misattributes a build failure. + """ + if "BuildImageError" in text: + if "Read timed out" in text: + return ErrorKind.IMAGE_BUILD_TIMEOUT + return ErrorKind.IMAGE_BUILD_ERROR + for kind, needles in _RULES: + if any(needle in text for needle in needles): + return kind + return ErrorKind.UNKNOWN + + +@dataclass(slots=True) +class UnitClassification: + """Per-kind counts for one unit's error instances.""" + + kinds: dict[ErrorKind, int] = field(default_factory=dict) + error_instance_ids: tuple[str, ...] = () + #: False when the run's report could not be read at all. "Not measured" and + #: "measured zero" are different, and conflating them once let a damaged + #: unit into a clean set. + measured: bool = False + + @property + def infra_count(self) -> int: + return sum(count for kind, count in self.kinds.items() if kind in INFRA_KINDS) + + @property + def genuine_count(self) -> int: + return sum(count for kind, count in self.kinds.items() if kind in GENUINE_KINDS) + + @property + def should_retry(self) -> bool: + return self.infra_count > 0 + + def as_counts(self) -> dict[str, int]: + return {kind.value: count for kind, count in sorted(self.kinds.items())} + + +def _find_instance_log(output_dir: Path, instance_id: str) -> Path | None: + patterns = ( + f"logs/run_evaluation/*/*/{instance_id}/run_instance.log", + f"logs/run_evaluation/*/*/*/{instance_id}/run_instance.log", + ) + for pattern in patterns: + for match in sorted(output_dir.glob(pattern)): + return match + return None + + +def memory_kill_markers(killed_dir: Path, instance_id: str) -> bool: + """True only for an *eval*-phase memory kill. + + Phase is load-bearing and the two cases must never be collapsed. An eval + kill destroyed a graded result, so the instance's error is a genuine + failure. An agent kill merely makes one tool call return an error + observation and the agent carries on, so the instance still reaches a real + outcome; that marker exists for audit and must not influence classification. + + A marker beats any log heuristic: a SIGKILLed test leaves an ambiguous log, + but the kill itself is a fact recorded before acting. + """ + return any(killed_dir.glob(f"eval.{instance_id}.*.json")) + + +def classify_unit( + output_dir: Path, + error_instance_ids: list[str] | tuple[str, ...] | None, + *, + killed_dir: Path | None = None, + infrastructure_failure: bool = False, + endpoint_changed: bool = False, +) -> UnitClassification: + """Classify every error instance of one unit. + + ``infrastructure_failure`` carries the Pyxis agent-phase sentinel, and + ``endpoint_changed`` carries a mismatch between the inference endpoint + fingerprint recorded at claim time and at publish time -- an engine + restarted under a live client produces a plausible-looking run that must not + be scored. + """ + kinds: dict[ErrorKind, int] = {} + + if infrastructure_failure: + kinds[ErrorKind.STEP_INFRASTRUCTURE_FAILURE] = ( + kinds.get(ErrorKind.STEP_INFRASTRUCTURE_FAILURE, 0) + 1 + ) + if endpoint_changed: + kinds[ErrorKind.ENDPOINT_CHANGED] = kinds.get(ErrorKind.ENDPOINT_CHANGED, 0) + 1 + + if error_instance_ids is None: + return UnitClassification(kinds=kinds, error_instance_ids=(), measured=False) + + ids = tuple(str(x) for x in error_instance_ids) + for instance_id in ids: + if killed_dir is not None and memory_kill_markers(killed_dir, instance_id): + kinds[ErrorKind.TEST_MEMORY_EXCEEDED] = ( + kinds.get(ErrorKind.TEST_MEMORY_EXCEEDED, 0) + 1 + ) + continue + log_path = _find_instance_log(output_dir, instance_id) + kind = ErrorKind.UNKNOWN + if log_path is not None: + try: + kind = classify_eval_log(log_path.read_text(errors="replace")) + except OSError: + logger.debug("could not read %s", log_path, exc_info=True) + kinds[kind] = kinds.get(kind, 0) + 1 + + return UnitClassification(kinds=kinds, error_instance_ids=ids, measured=True) diff --git a/tests/unit/evaluation/swe_bench_distributed/test_classify.py b/tests/unit/evaluation/swe_bench_distributed/test_classify.py new file mode 100644 index 000000000..2b079b315 --- /dev/null +++ b/tests/unit/evaluation/swe_bench_distributed/test_classify.py @@ -0,0 +1,171 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Infrastructure-versus-genuine classification of error instances.""" + +from __future__ import annotations + +import pytest + +from inference_endpoint.evaluation.swe_bench_distributed.classify import ( + GENUINE_KINDS, + INFRA_KINDS, + ErrorKind, + classify_eval_log, + classify_unit, +) + +pytestmark = pytest.mark.unit + + +def write_log(output_dir, instance_id: str, text: str) -> None: + log_dir = output_dir / "logs" / "run_evaluation" / "run-1" / "model" / instance_id + log_dir.mkdir(parents=True, exist_ok=True) + (log_dir / "run_instance.log").write_text(text) + + +class TestBiasRule: + def test_unknown_is_genuine_never_infra(self): + # A false bad-run costs one redo; a false retry biases the measurement + # toward optimism. Unclassifiable therefore means "keep the result". + assert ErrorKind.UNKNOWN in GENUINE_KINDS + assert ErrorKind.UNKNOWN not in INFRA_KINDS + + def test_kinds_are_partitioned(self): + assert not (INFRA_KINDS & GENUINE_KINDS) + assert INFRA_KINDS | GENUINE_KINDS == set(ErrorKind) + + def test_model_outcomes_are_genuine(self): + for kind in ( + ErrorKind.TEST_TIMEOUT, + ErrorKind.TEST_MEMORY_EXCEEDED, + ErrorKind.PATCH_APPLY_FAILED, + ): + assert kind in GENUINE_KINDS + + +class TestLogRules: + @pytest.mark.parametrize( + ("text", "expected"), + [ + ( + "fork/exec /usr/bin/conmon: resource temporarily unavailable", + ErrorKind.CONTAINER_FORK_EAGAIN, + ), + ("Test timed out after 1800s", ErrorKind.TEST_TIMEOUT), + ( + "can only create exec sessions on running containers", + ErrorKind.CONTAINER_EXEC_REFUSED, + ), + ("container state improper", ErrorKind.CONTAINER_EXEC_REFUSED), + ("Read timed out. (read timeout=60)", ErrorKind.RUNTIME_READ_TIMEOUT), + ( + "Reversed (or previously applied) patch detected", + ErrorKind.PATCH_APPLY_FAILED, + ), + ("1 out of 3 hunk FAILED", ErrorKind.PATCH_APPLY_FAILED), + ("nothing recognisable here", ErrorKind.UNKNOWN), + ], + ) + def test_each_rule(self, text, expected): + assert classify_eval_log(text) is expected + + def test_timeout_wins_over_teardown_noise(self): + # A timed-out evaluation also emits "container state improper" while the + # harness tears the container down. Reading that as a wedge would retry + # a genuine model outcome, so rule order is load-bearing. + text = "Test timed out after 1800s\ncontainer state improper\n" + assert classify_eval_log(text) is ErrorKind.TEST_TIMEOUT + + def test_fork_failure_wins_over_teardown_noise(self): + text = ( + "fork/exec /usr/bin/conmon: resource temporarily unavailable\n" + "can only create exec sessions on running containers\n" + ) + assert classify_eval_log(text) is ErrorKind.CONTAINER_FORK_EAGAIN + + def test_a_wedge_wins_over_patch_apply(self): + # A wedged container's verdict is unreliable either way, so the wedge + # decides and the unit is retried. + text = "container state improper\nhunk FAILED\n" + assert classify_eval_log(text) is ErrorKind.CONTAINER_EXEC_REFUSED + + def test_build_error_is_checked_before_every_other_rule(self): + # BuildImageError's message embeds the same needles the other rules look + # for, so any other ordering misattributes a build failure. + assert ( + classify_eval_log("BuildImageError: Read timed out. (read timeout=60)") + is ErrorKind.IMAGE_BUILD_TIMEOUT + ) + assert ( + classify_eval_log("BuildImageError: 500 Server Error") + is ErrorKind.IMAGE_BUILD_ERROR + ) + + +class TestClassifyUnit: + def test_infra_and_genuine_are_counted_separately(self, tmp_path): + write_log(tmp_path, "a-1", "container state improper") + write_log(tmp_path, "a-2", "Test timed out after 1800s") + + classification = classify_unit(tmp_path, ["a-1", "a-2"]) + + assert classification.infra_count == 1 + assert classification.genuine_count == 1 + assert classification.should_retry + + def test_only_genuine_errors_do_not_trigger_a_retry(self, tmp_path): + write_log(tmp_path, "a-1", "Test timed out after 1800s") + assert not classify_unit(tmp_path, ["a-1"]).should_retry + + def test_a_missing_log_is_unknown_and_therefore_genuine(self, tmp_path): + classification = classify_unit(tmp_path, ["absent"]) + assert classification.kinds == {ErrorKind.UNKNOWN: 1} + assert not classification.should_retry + + def test_none_error_ids_means_not_measured(self, tmp_path): + # "We did not measure" and "we measured zero" are different; conflating + # them once let a damaged unit into a clean set. + classification = classify_unit(tmp_path, None) + assert classification.measured is False + + def test_empty_error_ids_means_measured_zero(self, tmp_path): + classification = classify_unit(tmp_path, []) + assert classification.measured is True + assert classification.infra_count == 0 + + def test_eval_memory_kill_marker_is_genuine(self, tmp_path): + killed = tmp_path / "killed" + killed.mkdir() + (killed / "eval.a-1.host.999.json").write_text("{}") + write_log(tmp_path, "a-1", "container state improper") + + classification = classify_unit(tmp_path, ["a-1"], killed_dir=killed) + + # The marker beats the log: a SIGKILLed test leaves an ambiguous log, + # but the kill is a fact recorded before acting. + assert classification.kinds == {ErrorKind.TEST_MEMORY_EXCEEDED: 1} + assert not classification.should_retry + + def test_agent_phase_kill_marker_does_not_classify(self, tmp_path): + killed = tmp_path / "killed" + killed.mkdir() + (killed / "agent.a-1.host.999.json").write_text("{}") + write_log(tmp_path, "a-1", "Test timed out after 1800s") + + # An agent kill only makes one tool call return an error observation; + # the instance still reaches a real outcome, so the marker is audit-only. + classification = classify_unit(tmp_path, ["a-1"], killed_dir=killed) + assert classification.kinds == {ErrorKind.TEST_TIMEOUT: 1} + + def test_step_infrastructure_failure_forces_a_retry(self, tmp_path): + classification = classify_unit(tmp_path, [], infrastructure_failure=True) + assert classification.should_retry + assert classification.kinds == {ErrorKind.STEP_INFRASTRUCTURE_FAILURE: 1} + + def test_a_changed_endpoint_forces_a_retry(self, tmp_path): + # An engine restarted under a live client produces a plausible run that + # scores near zero and exits successfully. + classification = classify_unit(tmp_path, [], endpoint_changed=True) + assert classification.should_retry + assert classification.kinds == {ErrorKind.ENDPOINT_CHANGED: 1}