Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/clawbench/runner/batch.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import asyncio
import fnmatch
import itertools
import json
import os
import re
import shutil
Expand All @@ -17,6 +16,7 @@

import yaml

from clawbench.utils.jsonio import read_json_or_none, write_json_atomic
from clawbench.utils.paths import ASSET_ROOT, WORKSPACE_ROOT, ensure_workspace_templates


Expand Down Expand Up @@ -447,8 +447,15 @@ def print_run_stats(base_output: Path) -> None:

# Parse case and model from run-meta.json or dir name
meta_file = run_dir / "run-meta.json"
if meta_file.exists():
meta = json.loads(meta_file.read_text())
meta = read_json_or_none(meta_file) if meta_file.exists() else None
if meta is not None and not isinstance(meta, dict):
meta = None
if meta is None and meta_file.exists():
print(
f" WARNING: unreadable {meta_file}; "
"falling back to directory name for this run"
)
if meta is not None:
case = meta.get("test_case", "?")
model = meta.get("model", model_dir.name)
intercepted = meta.get("intercepted", False)
Expand Down Expand Up @@ -576,7 +583,7 @@ def write_summary_json(
for s in ("passed", "failed", "error", "skipped")
},
}
(base_output / "batch-summary.json").write_text(json.dumps(data, indent=2))
write_json_atomic(base_output / "batch-summary.json", data)


# ---------------------------------------------------------------------------
Expand Down
3 changes: 2 additions & 1 deletion src/clawbench/runner/run_support/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
)
from clawbench.runner.run_support.docker import container_engine_version, image_id
from clawbench.runner.run_support.task import normalize_extra_info
from clawbench.utils.jsonio import write_json_atomic

SECRET_CONFIG_RE = re.compile(
r"(api[_-]?keys?|token|secret|password|credential)", re.IGNORECASE
Expand Down Expand Up @@ -285,4 +286,4 @@ def make_run_meta(

def write_run_meta(output_dir: Path, meta: dict[str, Any]) -> None:
output_dir.mkdir(parents=True, exist_ok=True)
(output_dir / "run-meta.json").write_text(json.dumps(meta, indent=2))
write_json_atomic(output_dir / "run-meta.json", meta)
45 changes: 45 additions & 0 deletions src/clawbench/utils/jsonio.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
"""Crash-safe JSON writes and tolerant JSON reads for run metadata."""

import json
import os
import tempfile
from pathlib import Path
from typing import Any


def write_json_atomic(path: Path, data: Any, *, indent: int = 2) -> None:
"""Write ``data`` to ``path`` so readers never observe a partial file.

The payload lands in a temporary file alongside the target, is flushed all
the way to disk, and is then moved into place with :func:`os.replace`, which
replaces atomically on POSIX and Windows alike. A crash mid-write therefore
leaves either the previous file or no file, never a truncated one.
"""
path.parent.mkdir(parents=True, exist_ok=True)
payload = json.dumps(data, indent=indent)
fd, tmp_name = tempfile.mkstemp(
dir=path.parent, prefix=f".{path.name}.", suffix=".tmp"
)
tmp = Path(tmp_name)
try:
with os.fdopen(fd, "w", encoding="utf-8") as fh:
fh.write(payload)
fh.flush()
os.fsync(fh.fileno())
os.replace(tmp, path)
except BaseException:
tmp.unlink(missing_ok=True)
raise


def read_json_or_none(path: Path) -> Any | None:
"""Return the parsed contents of ``path``, or ``None`` if it cannot be read.

Batch reporting walks run directories produced by other processes, any of
which may have been killed mid-write by the host or by an older ClawBench.
A single unreadable file must not abort reporting for the whole batch.
"""
try:
return json.loads(path.read_text(encoding="utf-8"))
except (OSError, ValueError):
return None
127 changes: 127 additions & 0 deletions tests/test_batch_stats_resilience.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
"""Atomic metadata writes and batch reporting that survives a bad run-meta.json."""

from __future__ import annotations

import importlib
import json
import shutil
from pathlib import Path

import pytest

from clawbench.runner.batch import print_run_stats
from clawbench.utils.jsonio import read_json_or_none, write_json_atomic


def _make_run(model_dir: Path, name: str, meta: str | None) -> Path:
"""Create a run directory shaped like a real one, with raw `meta` text."""
run_dir = model_dir / name
(run_dir / "data").mkdir(parents=True)
(run_dir / "data" / "actions.jsonl").write_text('{"type": "click"}\n')
if meta is not None:
(run_dir / "run-meta.json").write_text(meta, encoding="utf-8")
return run_dir


# --- write_json_atomic -------------------------------------------------------


def test_write_json_atomic_roundtrips_and_creates_parents(tmp_path: Path) -> None:
target = tmp_path / "nested" / "run-meta.json"

write_json_atomic(target, {"test_case": "001-foo", "intercepted": True})

assert json.loads(target.read_text(encoding="utf-8")) == {
"test_case": "001-foo",
"intercepted": True,
}


def test_write_json_atomic_leaves_no_temp_files(tmp_path: Path) -> None:
write_json_atomic(tmp_path / "run-meta.json", {"a": 1})

assert [p.name for p in tmp_path.iterdir()] == ["run-meta.json"]


def test_write_json_atomic_keeps_previous_file_when_serialization_fails(
tmp_path: Path,
) -> None:
"""A failed write must not truncate the file that was already there."""
target = tmp_path / "run-meta.json"
write_json_atomic(target, {"generation": 1})

with pytest.raises(TypeError):
write_json_atomic(target, {"bad": object()})

assert json.loads(target.read_text(encoding="utf-8")) == {"generation": 1}
assert [p.name for p in tmp_path.iterdir()] == ["run-meta.json"]


def test_write_run_meta_is_atomic(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
# metadata resolves a container engine at import time; see conftest note.
monkeypatch.setattr(shutil, "which", lambda cmd: cmd)
metadata = importlib.import_module("clawbench.runner.run_support.metadata")

metadata.write_run_meta(tmp_path / "out", {"test_case": "001-foo"})

written = tmp_path / "out" / "run-meta.json"
assert json.loads(written.read_text(encoding="utf-8")) == {"test_case": "001-foo"}
assert [p.name for p in (tmp_path / "out").iterdir()] == ["run-meta.json"]


# --- read_json_or_none -------------------------------------------------------


@pytest.mark.parametrize(
"raw",
['{"test_case": "001-foo"', "", "not json at all", "\udcff"],
ids=["truncated", "empty", "garbage", "undecodable"],
)
def test_read_json_or_none_returns_none_for_unreadable(
tmp_path: Path, raw: str
) -> None:
target = tmp_path / "run-meta.json"
target.write_bytes(raw.encode("utf-8", "surrogateescape"))

assert read_json_or_none(target) is None


def test_read_json_or_none_returns_none_for_missing_file(tmp_path: Path) -> None:
assert read_json_or_none(tmp_path / "nope.json") is None


# --- print_run_stats regression (issue #303) ---------------------------------


def test_print_run_stats_survives_truncated_run_meta(
tmp_path: Path, capsys: pytest.CaptureFixture[str]
) -> None:
"""One truncated run-meta.json must not abort stats for the whole batch."""
model_dir = tmp_path / "some-model"
model_dir.mkdir()
_make_run(model_dir, "run-good", json.dumps({"test_case": "001-good"}))
_make_run(model_dir, "run-truncated", '{"test_case": "002-trunc"')

print_run_stats(tmp_path)

out = capsys.readouterr().out
assert "001-good" in out
assert "run-truncated" in out
assert "WARNING: unreadable" in out


def test_print_run_stats_survives_non_object_run_meta(
tmp_path: Path, capsys: pytest.CaptureFixture[str]
) -> None:
"""Valid JSON that is not an object must not blow up on .get() either."""
model_dir = tmp_path / "some-model"
model_dir.mkdir()
_make_run(model_dir, "run-listy", "[1, 2, 3]")

print_run_stats(tmp_path)

out = capsys.readouterr().out
assert "run-listy" in out
assert "WARNING: unreadable" in out