From fdbd6489b8c52435446ad003a308f2ca39e09e62 Mon Sep 17 00:00:00 2001 From: Zhengyang Liu Date: Fri, 6 Nov 2026 11:55:33 +0800 Subject: [PATCH] fix(swe-smith): preserve parametrized test IDs when grading --- docs/75-example-coding-agent.md | 4 + examples/swe_smith/agents/smith_agent.py | 20 ++-- .../examples/test_swe_smith_test_statuses.py | 92 +++++++++++++++++++ 3 files changed, 110 insertions(+), 6 deletions(-) create mode 100644 tests/examples/test_swe_smith_test_statuses.py diff --git a/docs/75-example-coding-agent.md b/docs/75-example-coding-agent.md index 510416ec6..1bdab69d8 100644 --- a/docs/75-example-coding-agent.md +++ b/docs/75-example-coding-agent.md @@ -116,3 +116,7 @@ The SWE agent limits these reward-hacking paths in two ways: - **Network isolation:** we strongly recommend adding a Kubernetes network policy that denies all outbound traffic from agent pods except connections to the AGL Gateway. Without this restriction, an agent may retrieve upstream source code or other external information and obtain reward without solving the task as intended. The final reward is computed by running the task-specific `FAIL_TO_PASS` and `PASS_TO_PASS` tests inside the isolated repository environment. These controls are part of the training setup: weakening them can allow the agent to recover reference code and corrupt the reward signal. + +The evaluator matches pytest reports against the complete test IDs in `FAIL_TO_PASS` and +`PASS_TO_PASS`, including whitespace and punctuation in parametrized IDs. A required test +that is missing from the report does not count as passing. diff --git a/examples/swe_smith/agents/smith_agent.py b/examples/swe_smith/agents/smith_agent.py index aff033634..45853257b 100755 --- a/examples/swe_smith/agents/smith_agent.py +++ b/examples/swe_smith/agents/smith_agent.py @@ -28,9 +28,7 @@ # harness still reaches it via --git-dir/--work-tree. See also _forbidden_action. _HIDDEN_GIT_DIR = os.environ.get("SMITH_HIDDEN_GIT_DIR", "/opt/agl_tmp") -_STATUS_RE = re.compile( - r"(?:^|\s)(PASSED|FAILED|ERROR|SKIPPED|XFAIL|XPASS)\s+(\S+)|(\S+)\s+(PASSED|FAILED|ERROR|SKIPPED|XFAIL|XPASS)(?:\s|$)" -) +_TEST_STATUS_PATTERN = r"(PASSED|FAILED|ERROR|SKIPPED|XFAIL|XPASS)" # pytest --color=yes (or a repo that forces color) wraps PASSED in ANSI codes. # Without stripping them, parse_test_statuses never sees a bare "PASSED" token # and a fully passing suite is scored 0/N. @@ -573,10 +571,20 @@ def capture_patch() -> str: return "" -def parse_test_statuses(test_output: str) -> dict[str, str]: +def parse_test_statuses(test_output: str, test_nodes: list[str]) -> dict[str, str]: + """Match pytest reports against the complete test IDs from evaluation metadata.""" + if not test_nodes: + return {} + # Parameter IDs can contain spaces, brackets, and " - ". Match known IDs + # instead of guessing where an ID ends and pytest's failure reason begins. + nodes = "|".join(re.escape(node) for node in sorted(set(test_nodes), key=len, reverse=True)) + status_re = re.compile( + rf"(?:^|\s){_TEST_STATUS_PATTERN}\s+({nodes})(?=\s+-\s|\s*$)" + rf"|(?:^|\s)({nodes})\s+{_TEST_STATUS_PATTERN}(?:\s|$)" + ) statuses: dict[str, str] = {} for line in _ANSI_ESCAPE_RE.sub("", test_output).splitlines(): - m = _STATUS_RE.search(line) + m = status_re.search(line) if not m: continue if m.group(1): @@ -612,7 +620,7 @@ def evaluate(eval_meta: dict[str, Any], timeout: int, f2p_only: bool = True) -> ) # rc 124 == subprocess.TimeoutExpired (see _run); pytest itself never exits 124. timed_out = rc == 124 or "[timed out after" in output - statuses = parse_test_statuses(output) + statuses = parse_test_statuses(output, nodes) f2p_pass = [t for t in fail_to_pass if statuses.get(t) in ("PASSED", "XFAIL")] # only count P2P reporting PASSED/XFAIL — a missing status means it never ran. p2p_ok = [t for t in pass_to_pass if statuses.get(t) in ("PASSED", "XFAIL")] diff --git a/tests/examples/test_swe_smith_test_statuses.py b/tests/examples/test_swe_smith_test_statuses.py new file mode 100644 index 000000000..28630a9c3 --- /dev/null +++ b/tests/examples/test_swe_smith_test_statuses.py @@ -0,0 +1,92 @@ +# Copyright (c) Microsoft. All rights reserved. + +"""Match pytest reports to the full test IDs used for SWE-smith rewards.""" + +from __future__ import annotations + +import os +import sys +from pathlib import Path + +import pytest + +from examples.swe_smith.agents import smith_agent + + +@pytest.mark.parametrize("include_failure", [False, True]) +def test_evaluate_parametrized_tests_with_real_pytest( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, include_failure: bool +) -> None: + (tmp_path / "test_example.py").write_text( + "import pytest\n" + "@pytest.mark.parametrize('value', [1, 2, 3], ids=['hello world', 'a - b', 'nested [two words]'])\n" + "def test_pass(value):\n" + " assert value > 0\n" + "def test_fail():\n" + " assert False, 'failure - with [details]'\n" + ) + monkeypatch.setattr(smith_agent, "TESTBED", str(tmp_path)) + monkeypatch.setattr(smith_agent, "restore_f2p_tests", lambda *_args: None) + monkeypatch.setenv("PATH", str(Path(sys.executable).parent) + os.pathsep + os.environ.get("PATH", "")) + monkeypatch.setenv("PYTEST_DISABLE_PLUGIN_AUTOLOAD", "1") + monkeypatch.setenv("PYTEST_ADDOPTS", "") + run = smith_agent._run + + def run_without_xdist(command: str, timeout: int) -> tuple[str, int]: + if command == "python -c 'import xdist'": + return "", 1 + return run(command, timeout) + + monkeypatch.setattr(smith_agent, "_run", run_without_xdist) + fail_to_pass = ["test_example.py::test_pass[hello world]", "test_example.py::test_pass[a - b]"] + if include_failure: + fail_to_pass.append("test_example.py::test_fail") + reward, resolved, reason, timed_out = smith_agent.evaluate( + { + "instance_id": "test-instance", + "FAIL_TO_PASS": fail_to_pass, + "PASS_TO_PASS": ["test_example.py::test_pass[nested [two words]]"], + }, + timeout=30, + ) + + assert timed_out is False + assert reward == (0.0 if include_failure else 1.0) + assert resolved is (not include_failure) + assert f"FAIL_TO_PASS 2/{len(fail_to_pass)} passed" in reason + assert "PASS_TO_PASS 1/1 ok" in reason + + +@pytest.mark.parametrize( + "parameter", ["hello world", "two spaces", "a - b", "nested [two words]", "a.+? (b|c)", "status PASSED word"] +) +@pytest.mark.parametrize( + ("status", "line"), + [ + ("PASSED", "PASSED {node}"), + ("FAILED", "FAILED {node} - AssertionError: reason - with [details]"), + ("XFAIL", "{node} XFAIL (expected failure) [ 50%]"), + ("PASSED", "[gw2] [100%] \x1b[32mPASSED\x1b[0m {node}"), + ], +) +def test_statuses_preserve_full_node_ids(parameter: str, status: str, line: str) -> None: + node = f"tests/test_example.py::TestExample::test_case[{parameter}]" + assert smith_agent.parse_test_statuses(line.format(node=node), [node]) == {node: status} + + +def test_known_node_ids_do_not_match_prefixes_or_failure_reasons() -> None: + node = "tests/test_example.py::test_case" + longer_node = node + "[two words]" + output = "\n".join( + [ + f"{longer_node} PASSED [ 50%]", + f"FAILED {node} - AssertionError: report includes [{longer_node}]", + f"PASSED {node}_other", + "PASSED tests/unrelated.py::test_case", + ] + ) + assert smith_agent.parse_test_statuses(output, [node, longer_node]) == {node: "FAILED", longer_node: "PASSED"} + + +def test_no_expected_tests_returns_no_statuses() -> None: + assert smith_agent.parse_test_statuses("PASSED tests/test_example.py::test_case", []) == {}