diff --git a/README.md b/README.md index 8342c92..698eece 100644 --- a/README.md +++ b/README.md @@ -227,6 +227,7 @@ python -m twine upload dist/* 1. **评测运行器显式授权**:只有受信任的 SWE-bench runner 实例能调用危险 shell 的授权入口;环境变量不能关闭普通用户的安全检查,forbidden 命令始终拒绝。 2. **Prompt 收紧**:强制最小改动、禁止安装依赖/修改配置、要求验证后再结束。 3. **合规修正**:移除 goal description 中的 `FAIL_TO_PASS` 测试名泄露,agent 只看 issue 描述,验收测试由评估 harness 在不可见情况下运行。 +4. **可恢复结果**:每个系统开始和结束时都会原子保存独立状态;环境故障不会计入答错,恢复运行时优先重新验收已保存的 patch,不重复调用模型。 ### 复现 diff --git a/agent/atomic_io.py b/agent/atomic_io.py new file mode 100644 index 0000000..e5e8a46 --- /dev/null +++ b/agent/atomic_io.py @@ -0,0 +1,45 @@ +"""Crash-safe helpers for small state and report files.""" + +from __future__ import annotations + +import json +import os +import tempfile +from pathlib import Path +from typing import Any + + +def atomic_write_text(path: str | Path, content: str, *, mode: int = 0o600) -> None: + """Replace *path* atomically after flushing the new contents to disk.""" + target = Path(path) + target.parent.mkdir(parents=True, exist_ok=True) + fd, temporary_name = tempfile.mkstemp(prefix=f".{target.name}.", dir=target.parent) + temporary = Path(temporary_name) + try: + with os.fdopen(fd, "w", encoding="utf-8") as stream: + stream.write(content) + stream.flush() + os.fsync(stream.fileno()) + temporary.chmod(mode) + os.replace(temporary, target) + try: + directory_fd = os.open(target.parent, os.O_RDONLY) + try: + os.fsync(directory_fd) + finally: + os.close(directory_fd) + except OSError: + # Some filesystems do not support fsync on directories. The file + # replacement is still atomic there. + pass + except BaseException: + temporary.unlink(missing_ok=True) + raise + + +def atomic_write_json(path: str | Path, value: Any, *, mode: int = 0o600) -> None: + atomic_write_text( + path, + json.dumps(value, indent=2, ensure_ascii=False, default=str) + "\n", + mode=mode, + ) diff --git a/agent/direct_agent.py b/agent/direct_agent.py index 7691ea2..fc8af97 100644 --- a/agent/direct_agent.py +++ b/agent/direct_agent.py @@ -9,6 +9,8 @@ import json import logging +import os +import time from pathlib import Path from typing import Any @@ -143,6 +145,8 @@ def _log_event(self, event: dict[str, Any]) -> None: try: with self.log_path.open("a", encoding="utf-8") as f: f.write(json.dumps(event, ensure_ascii=False, default=str) + "\n") + f.flush() + os.fsync(f.fileno()) except Exception: logger.exception("failed to write trace event") @@ -163,6 +167,20 @@ def run(self, goal_description: str, max_steps: int = 50) -> str: ] tools_schema = build_tools_payload(self.tools) + started_at = time.monotonic() + total_tokens = 0 + + def finish(status: str, message: str, step: int) -> str: + self._log_event( + { + "type": "run_end", + "status": status, + "step": step, + "total_tokens": total_tokens, + "duration_seconds": time.monotonic() - started_at, + } + ) + return message self._log_event( { @@ -174,13 +192,12 @@ def run(self, goal_description: str, max_steps: int = 50) -> str: } ) - total_tokens = 0 max_tokens = self.llm.config.max_total_tokens_per_turn for step in range(1, max_steps + 1): if step > 1 and total_tokens >= max_tokens: message = f"Reached token budget ({max_tokens}) without final answer." self._log_event({"type": "token_budget_reached", "max_tokens": max_tokens}) - return message + return finish("token_budget_reached", message, step - 1) messages = self._compact_messages(messages, max_turns=20) logger.info("step %d/%d: calling LLM", step, max_steps) try: @@ -194,7 +211,7 @@ def run(self, goal_description: str, max_steps: int = 50) -> str: "error": str(exc), } ) - return f"LLM error at step {step}: {exc}" + return finish("llm_error", f"LLM error at step {step}: {exc}", step) total_tokens += response.usage.total_tokens self._log_event( @@ -202,6 +219,8 @@ def run(self, goal_description: str, max_steps: int = 50) -> str: "type": "llm_response", "step": step, "content": response.content, + "usage": response.usage.model_dump(), + "cumulative_tokens": total_tokens, "tool_calls": [ { "id": c.id, @@ -231,7 +250,7 @@ def run(self, goal_description: str, max_steps: int = 50) -> str: "content": response.content, } ) - return response.content or "" + return finish("completed", response.content or "", step) # Execute tool calls in sequence (model may request parallel, we # execute sequentially for simplicity — same as Claude Code) @@ -306,4 +325,8 @@ def run(self, goal_description: str, max_steps: int = 50) -> str: "max_steps": max_steps, } ) - return f"Reached maximum steps ({max_steps}) without final answer." + return finish( + "max_steps_reached", + f"Reached maximum steps ({max_steps}) without final answer.", + max_steps, + ) diff --git a/scripts/compare_three_systems.py b/scripts/compare_three_systems.py index e90853f..3cab1b4 100644 --- a/scripts/compare_three_systems.py +++ b/scripts/compare_three_systems.py @@ -26,6 +26,7 @@ REPO_ROOT = Path(__file__).resolve().parents[1] sys.path.insert(0, str(REPO_ROOT)) +from agent.atomic_io import atomic_write_json, atomic_write_text # noqa: E402 from agent.config import Config, load_config # noqa: E402 from agent.llm import LLMClient, Message # noqa: E402 from swe_bench.dataset import SWEBenchDataset, SWEBenchTask # noqa: E402 @@ -75,6 +76,62 @@ class ComparisonResult: swe_agent_resolved: bool | None swe_agent_duration: float | None swe_agent_error: str | None + direct_status: str = "pending" + claude_status: str = "pending" + swe_agent_status: str = "pending" + + +def save_comparison(path: Path, result: ComparisonResult) -> None: + """Checkpoint one task without exposing a partially-written JSON file.""" + atomic_write_json(path, asdict(result)) + + +def save_system_result( + result: ComparisonResult, + system: str, + outcome: dict[str, Any], + *, + infrastructure_failure: bool = False, +) -> None: + """Copy a system outcome into the comparison with explicit result state.""" + setattr(result, f"{system}_duration", outcome["duration"]) + setattr(result, f"{system}_error", outcome["error"]) + if infrastructure_failure: + setattr(result, f"{system}_resolved", None) + setattr(result, f"{system}_status", "infrastructure_error") + else: + setattr(result, f"{system}_resolved", outcome["resolved"]) + setattr(result, f"{system}_status", "completed") + + +def reevaluate_existing_patch( + task: SWEBenchTask, + task_output_dir: Path, + system: str, + workspace: Path, + previous_duration: float | None, +) -> dict[str, Any] | None: + """Retry only evaluation after infrastructure failure, without another LLM run.""" + system_output_dir = task_output_dir / system + candidates = ( + system_output_dir / "agent.patch", + system_output_dir / task.id / "agent.patch", + ) + patch_path = next((path for path in candidates if path.is_file()), None) + if patch_path is None: + return None + patch = patch_path.read_text(encoding="utf-8") + if not patch.strip(): + return None + logger.info("re-evaluating saved %s patch for %s without an LLM call", system, task.id) + outcome = evaluate_patch( + task, + workspace, + patch, + system_output_dir / "docker_reeval", + ) + outcome["duration"] = previous_duration + return outcome def load_tasks(dataset_path: str, task_ids: list[str]) -> list[SWEBenchTask]: @@ -552,6 +609,17 @@ def evaluate_patch( def render_table(results: list[ComparisonResult]) -> str: + def cell(resolved: bool | None, status: str) -> str: + if status == "infrastructure_error": + return "infra" + if status == "budget_exhausted": + return "budget" + if status == "running": + return "running" + if resolved is None: + return "-" + return str(resolved) + lines = [ "| task | direct | Claude | SWE-agent |", "|------|--------|--------|-----------|", @@ -559,20 +627,22 @@ def render_table(results: list[ComparisonResult]) -> str: for r in results: lines.append( f"| {r.task_id} | " - f"{r.direct_resolved if r.direct_resolved is not None else '-'} | " - f"{r.claude_resolved if r.claude_resolved is not None else '-'} | " - f"{r.swe_agent_resolved if r.swe_agent_resolved is not None else '-'} |" + f"{cell(r.direct_resolved, r.direct_status)} | " + f"{cell(r.claude_resolved, r.claude_status)} | " + f"{cell(r.swe_agent_resolved, r.swe_agent_status)} |" ) lines.append("") - lines.append( - f"**direct resolved:** {sum(1 for r in results if r.direct_resolved)}/{len(results)}" - ) - lines.append( - f"**Claude resolved:** {sum(1 for r in results if r.claude_resolved)}/{len(results)}" - ) - lines.append( - f"**SWE-agent resolved:** {sum(1 for r in results if r.swe_agent_resolved)}/{len(results)}" - ) + for label, field in ( + ("direct", "direct"), + ("Claude", "claude"), + ("SWE-agent", "swe_agent"), + ): + completed = [r for r in results if getattr(r, f"{field}_status") == "completed"] + resolved = sum(getattr(r, f"{field}_resolved") is True for r in completed) + unfinished = len(results) - len(completed) + lines.append( + f"**{label} resolved:** {resolved}/{len(completed)} completed ({unfinished} unfinished)" + ) return "\n".join(lines) @@ -698,39 +768,66 @@ def main() -> int: for key, value in existing.items(): if hasattr(r, key): setattr(r, key, value) + for system in ("direct", "claude", "swe_agent"): + if ( + getattr(r, f"{system}_status") == "pending" + and getattr(r, f"{system}_resolved") is not None + ): + setattr(r, f"{system}_status", "completed") logger.info("loaded partial result for %s", task.id) except Exception: - pass + logger.warning("ignoring unreadable partial result %s", comparison_path) # Prepare one workspace per system so they don't interfere. if args.mode in ("direct", "all") and ( r.direct_resolved is None or (args.rerun_failed and r.direct_resolved is not True) ): + retrying_evaluation = r.direct_status == "infrastructure_error" + r.direct_status = "running" + save_comparison(comparison_path, r) workspace = task_output_dir / "direct_workspace" prepare_workspace(task, workspace) - logger.info("running direct for %s", task.id) - direct = run_direct( - task, - task_output_dir / "direct", - workspace, - config, # type: ignore[arg-type] - args.model, - timeout_seconds=args.direct_timeout, - max_steps=args.direct_max_steps, - token_budget=args.direct_token_budget, + direct = ( + reevaluate_existing_patch( + task, task_output_dir, "direct", workspace, r.direct_duration + ) + if retrying_evaluation + else None + ) + if direct is None: + logger.info("running direct for %s", task.id) + direct = run_direct( + task, + task_output_dir / "direct", + workspace, + config, # type: ignore[arg-type] + args.model, + timeout_seconds=args.direct_timeout, + max_steps=args.direct_max_steps, + token_budget=args.direct_token_budget, + ) + budget_exhausted = not direct["resolved"] and (direct["error"] or "").startswith( + "Reached token budget" ) - r.direct_resolved = direct["resolved"] - r.direct_duration = direct["duration"] - r.direct_error = direct["error"] + direct_infra = not direct["resolved"] and is_infra_error(direct["error"]) + save_system_result( + r, + "direct", + direct, + infrastructure_failure=budget_exhausted or direct_infra, + ) + if budget_exhausted: + r.direct_status = "budget_exhausted" + save_comparison(comparison_path, r) logger.info("direct %s -> resolved=%s", task.id, r.direct_resolved) - if not r.direct_resolved and (r.direct_error or "").startswith("Reached token budget"): + if budget_exhausted: logger.error( "direct benchmark budget exhausted for %s: %s. Aborting batch.", task.id, r.direct_error, ) infrastructure_failure = True - elif not r.direct_resolved and is_infra_error(r.direct_error): + elif direct_infra: logger.error( "direct evaluation infrastructure failure for %s: %s. Aborting batch.", task.id, @@ -743,36 +840,46 @@ def main() -> int: and args.mode in ("claude", "all") and (r.claude_resolved is None or (args.rerun_failed and r.claude_resolved is not True)) ): + retrying_evaluation = r.claude_status == "infrastructure_error" + r.claude_status = "running" + save_comparison(comparison_path, r) workspace = task_output_dir / "claude_workspace" prepare_workspace(task, workspace) - logger.info("running Claude Code for %s", task.id) - claude = run_claude( - task, - task_output_dir / "claude", - workspace, - args.model, - timeout_seconds=args.claude_timeout, + claude = ( + reevaluate_existing_patch( + task, task_output_dir, "claude", workspace, r.claude_duration + ) + if retrying_evaluation + else None + ) + if claude is None: + logger.info("running Claude Code for %s", task.id) + claude = run_claude( + task, + task_output_dir / "claude", + workspace, + args.model, + timeout_seconds=args.claude_timeout, + ) + claude_infra = not claude["resolved"] and ( + (claude["error"] or "").startswith(("claude timed out", "claude exit code")) + or is_infra_error(claude["error"]) ) - r.claude_resolved = claude["resolved"] - r.claude_duration = claude["duration"] - r.claude_error = claude["error"] + save_system_result( + r, + "claude", + claude, + infrastructure_failure=claude_infra, + ) + save_comparison(comparison_path, r) logger.info("Claude %s -> resolved=%s", task.id, r.claude_resolved) - if not r.claude_resolved and (r.claude_error or "").startswith( - ("claude timed out", "claude exit code") - ): + if claude_infra: logger.error( "Claude infrastructure failure for %s: %s. Aborting batch.", task.id, r.claude_error, ) infrastructure_failure = True - elif not r.claude_resolved and is_infra_error(r.claude_error): - logger.error( - "Claude evaluation infrastructure failure for %s: %s. Aborting batch.", - task.id, - r.claude_error, - ) - infrastructure_failure = True if ( not infrastructure_failure @@ -782,23 +889,39 @@ def main() -> int: or (args.rerun_failed and r.swe_agent_resolved is not True) ) ): + retrying_evaluation = r.swe_agent_status == "infrastructure_error" + r.swe_agent_status = "running" + save_comparison(comparison_path, r) workspace = task_output_dir / "swe_agent_workspace" prepare_workspace(task, workspace) - logger.info("running SWE-agent for %s", task.id) - swe = run_swe_agent( - task, - task_output_dir / "swe_agent", - workspace, - args.model, - timeout_seconds=args.swe_agent_timeout, - max_steps=args.swe_agent_max_steps, - timeout_per_command=args.swe_agent_timeout_per_command, + swe = ( + reevaluate_existing_patch( + task, task_output_dir, "swe_agent", workspace, r.swe_agent_duration + ) + if retrying_evaluation + else None ) - r.swe_agent_resolved = swe["resolved"] - r.swe_agent_duration = swe["duration"] - r.swe_agent_error = swe["error"] + if swe is None: + logger.info("running SWE-agent for %s", task.id) + swe = run_swe_agent( + task, + task_output_dir / "swe_agent", + workspace, + args.model, + timeout_seconds=args.swe_agent_timeout, + max_steps=args.swe_agent_max_steps, + timeout_per_command=args.swe_agent_timeout_per_command, + ) + swe_infra = not swe["resolved"] and is_infra_error(swe["error"]) + save_system_result( + r, + "swe_agent", + swe, + infrastructure_failure=swe_infra, + ) + save_comparison(comparison_path, r) logger.info("SWE-agent %s -> resolved=%s", task.id, r.swe_agent_resolved) - if not r.swe_agent_resolved and is_infra_error(r.swe_agent_error): + if swe_infra: logger.error( "SWE-agent infrastructure failure detected for %s: %s. " "Aborting batch to avoid wasting time/token.", @@ -807,11 +930,7 @@ def main() -> int: ) infrastructure_failure = True - # Save incremental result. - (task_output_dir / "comparison.json").write_text( - json.dumps(asdict(r), indent=2, ensure_ascii=False), - encoding="utf-8", - ) + save_comparison(comparison_path, r) if infrastructure_failure: break @@ -827,11 +946,8 @@ def main() -> int: }, "tasks": [asdict(r) for r in results], } - (output_dir / "report.json").write_text( - json.dumps(report, indent=2, ensure_ascii=False), - encoding="utf-8", - ) - (output_dir / "report.md").write_text(render_table(results), encoding="utf-8") + atomic_write_json(output_dir / "report.json", report) + atomic_write_text(output_dir / "report.md", render_table(results) + "\n") logger.info("report saved to %s", output_dir) print(render_table(results)) return 0 diff --git a/swe_bench/reporter.py b/swe_bench/reporter.py index fe15dfc..34a34c3 100644 --- a/swe_bench/reporter.py +++ b/swe_bench/reporter.py @@ -10,6 +10,8 @@ from pydantic import BaseModel, Field +from agent.atomic_io import atomic_write_json, atomic_write_text + logger = logging.getLogger("swe_bench.reporter") @@ -80,20 +82,14 @@ class JSONReporter: @staticmethod def render(report: BenchmarkReport, path: Path) -> None: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text( - json.dumps(report.model_dump(), indent=2, ensure_ascii=False, default=str), - encoding="utf-8", - ) + atomic_write_json(path, report.model_dump()) logger.info("wrote JSON report to %s", path) @staticmethod def render_task_result(result: TaskResult, path: Path) -> None: """Write a single task result as JSON.""" path.parent.mkdir(parents=True, exist_ok=True) - path.write_text( - json.dumps(result.model_dump(), indent=2, ensure_ascii=False, default=str), - encoding="utf-8", - ) + atomic_write_json(path, result.model_dump()) @staticmethod def load_task_result(path: Path) -> TaskResult: @@ -139,5 +135,5 @@ def render(report: BenchmarkReport, path: Path) -> None: ) lines.append("") - path.write_text("\n".join(lines), encoding="utf-8") + atomic_write_text(path, "\n".join(lines) + "\n") logger.info("wrote Markdown report to %s", path) diff --git a/tests/test_atomic_io.py b/tests/test_atomic_io.py new file mode 100644 index 0000000..eed13b8 --- /dev/null +++ b/tests/test_atomic_io.py @@ -0,0 +1,30 @@ +import json +import os + +import pytest + +from agent.atomic_io import atomic_write_json, atomic_write_text + + +def test_atomic_write_json_replaces_complete_document(tmp_path): + target = tmp_path / "state.json" + atomic_write_json(target, {"status": "running"}) + atomic_write_json(target, {"status": "completed", "resolved": True}) + + assert json.loads(target.read_text()) == {"status": "completed", "resolved": True} + assert not list(tmp_path.glob(".state.json.*")) + + +def test_atomic_write_keeps_previous_file_when_replace_fails(tmp_path, monkeypatch): + target = tmp_path / "state.json" + atomic_write_text(target, "old") + + def fail_replace(_source, _target): + raise OSError("simulated crash before replace") + + monkeypatch.setattr(os, "replace", fail_replace) + with pytest.raises(OSError, match="simulated crash"): + atomic_write_text(target, "new") + + assert target.read_text() == "old" + assert not list(tmp_path.glob(".state.json.*")) diff --git a/tests/test_direct_agent.py b/tests/test_direct_agent.py new file mode 100644 index 0000000..759e73c --- /dev/null +++ b/tests/test_direct_agent.py @@ -0,0 +1,31 @@ +import json +from types import SimpleNamespace + +from agent.direct_agent import DirectAgent +from agent.llm.schema import AssistantResponse, Usage + + +class FakeLLM: + config = SimpleNamespace(max_total_tokens_per_turn=100) + + def chat(self, _messages, tools): + assert tools == [] + return AssistantResponse(content="done", usage=Usage(total_tokens=7)) + + +def test_direct_agent_records_durable_run_summary(tmp_path): + trace = tmp_path / "agent.log" + agent = DirectAgent(FakeLLM(), tmp_path, "test", allowed_tools=[], log_path=trace) + + assert agent.run("fix it") == "done" + + events = [json.loads(line) for line in trace.read_text().splitlines()] + response = next(event for event in events if event["type"] == "llm_response") + finished = events[-1] + assert response["usage"]["total_tokens"] == 7 + assert response["cumulative_tokens"] == 7 + assert finished["type"] == "run_end" + assert finished["status"] == "completed" + assert finished["step"] == 1 + assert finished["total_tokens"] == 7 + assert finished["duration_seconds"] >= 0 diff --git a/tests/test_swe_agent_local_runner.py b/tests/test_swe_agent_local_runner.py index c50e811..702184e 100644 --- a/tests/test_swe_agent_local_runner.py +++ b/tests/test_swe_agent_local_runner.py @@ -3,18 +3,23 @@ import shlex import sys from pathlib import Path -from types import SimpleNamespace +from types import ModuleType, SimpleNamespace import pytest import yaml from agent.config import Config from scripts.compare_three_systems import ( + ComparisonResult, _activate_command_venv, _claude_environment, build_goal_description, preflight_claude_endpoint, + reevaluate_existing_patch, + render_table, run_direct, + save_comparison, + save_system_result, ) from swe_agent_local_runner import ( LocalSWEEnv, @@ -68,6 +73,84 @@ def test_docker_evaluator_does_not_misclassify_test_failure(): assert detect_infrastructure_failure("FAILED tests/test_feature.py::test_answer") is None +def test_comparison_checkpoints_infrastructure_failure_without_counting_wrong(tmp_path): + result = ComparisonResult( + task_id="demo", + direct_resolved=None, + direct_duration=None, + direct_error=None, + claude_resolved=None, + claude_duration=None, + claude_error=None, + swe_agent_resolved=None, + swe_agent_duration=None, + swe_agent_error=None, + ) + result.claude_status = "running" + save_comparison(tmp_path / "comparison.json", result) + save_system_result( + result, + "claude", + {"resolved": False, "duration": 12.5, "error": "infrastructure failure"}, + infrastructure_failure=True, + ) + save_comparison(tmp_path / "comparison.json", result) + + saved = json.loads((tmp_path / "comparison.json").read_text()) + assert saved["claude_resolved"] is None + assert saved["claude_status"] == "infrastructure_error" + assert saved["claude_duration"] == 12.5 + + +def test_comparison_report_excludes_unfinished_results_from_denominator(): + result = ComparisonResult( + task_id="demo", + direct_resolved=True, + direct_duration=1.0, + direct_error=None, + claude_resolved=None, + claude_duration=1.0, + claude_error="infrastructure failure", + swe_agent_resolved=None, + swe_agent_duration=None, + swe_agent_error=None, + direct_status="completed", + claude_status="infrastructure_error", + ) + + report = render_table([result]) + + assert "| demo | True | infra | - |" in report + assert "**direct resolved:** 1/1 completed (0 unfinished)" in report + assert "**Claude resolved:** 0/0 completed (1 unfinished)" in report + + +def test_infrastructure_retry_reuses_saved_patch_without_model_call(tmp_path, monkeypatch): + patch_dir = tmp_path / "claude" + patch_dir.mkdir() + (patch_dir / "agent.patch").write_text("diff --git a/a.py b/a.py\n") + observed = {} + + def fake_evaluate(task, workspace, patch, output_dir): + observed.update( + task=task, + workspace=workspace, + patch=patch, + output_dir=output_dir, + ) + return {"resolved": True, "duration": None, "error": None, "patch": patch} + + monkeypatch.setattr("scripts.compare_three_systems.evaluate_patch", fake_evaluate) + workspace = tmp_path / "workspace" + outcome = reevaluate_existing_patch(_task(), tmp_path, "claude", workspace, 42.0) + + assert outcome is not None + assert outcome["resolved"] is True + assert outcome["duration"] == 42.0 + assert observed["patch"] == "diff --git a/a.py b/a.py\n" + assert observed["output_dir"] == patch_dir / "docker_reeval" + + def test_local_env_preserves_command_exit_status(tmp_path): env = LocalSWEEnv(tmp_path, _task(), timeout=5) try: @@ -232,7 +315,9 @@ def __init__(self, *, config, **_kwargs): def run_task(self, _task): return SimpleNamespace(resolved=False, patch_path=None, error="expected") - monkeypatch.setattr("swe_bench.runner.SWEBenchRunner", FakeRunner) + runner_module = ModuleType("swe_bench.runner") + runner_module.SWEBenchRunner = FakeRunner # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "swe_bench.runner", runner_module) config = Config() original = ( config.llm.model,