diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f4398b4..f8ed6585 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/). - Changed the default Harbor version to `0.22.0`. ### Fixed +- Isolate `clawbench-reproduce` downloads in a per-invocation cache directory so cleanup preserves existing work-directory files and removes only owned downloads, including on failure. - Align public discovery metadata with the canonical repository and shipping corpus, label historical V1 scores in both READMEs, and correct the v0.10.0 citation release date. - Host-timeout container termination now uses the lazy container-engine resolver. - Added host-side container and batch-job timeouts so a wedged run cannot stall a batch indefinitely. diff --git a/src/clawbench/eval/reproduce.py b/src/clawbench/eval/reproduce.py index e947bd9c..d56dcd98 100644 --- a/src/clawbench/eval/reproduce.py +++ b/src/clawbench/eval/reproduce.py @@ -18,7 +18,9 @@ import argparse import json -import shutil +import tempfile +from collections.abc import Iterator +from contextlib import contextmanager import subprocess import sys from pathlib import Path @@ -112,6 +114,21 @@ def verdict( return ok, "\n".join(lines) +@contextmanager +def download_cache(work_dir: Path, keep_cache: bool) -> Iterator[Path]: + """Own only a unique child directory, never the caller's work directory.""" + work_dir.mkdir(parents=True, exist_ok=True) + if keep_cache: + cache_dir = Path(tempfile.mkdtemp(prefix="clawbench-", dir=work_dir)) + print(f" Cache retained at: {cache_dir}") + yield cache_dir + else: + with tempfile.TemporaryDirectory(prefix="clawbench-", dir=work_dir) as tmp: + cache_dir = Path(tmp) + print(f" Temporary cache: {cache_dir}") + yield cache_dir + + def main() -> int: p = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter @@ -139,7 +156,7 @@ def main() -> int: "--work-dir", type=Path, default=Path("./reproduce-cache"), - help="Local dir for HF download (default ./reproduce-cache)", + help="Parent directory for an isolated download cache (default ./reproduce-cache)", ) p.add_argument( "--keep-cache", @@ -157,66 +174,64 @@ def main() -> int: return 2 published = PUBLISHED_V2_HERMES[args.model] - args.work_dir.mkdir(parents=True, exist_ok=True) - print( - f"== Reproducing {args.model} (n={published[3]}, tolerance ±{args.tolerance}pp) ==\n" - ) - print("[1/3] Download trace subset from HF ...") - batch_dir = download(args.model, args.work_dir) - - # Need a model batch root; HF subset puts task dirs under - # batch-aligned-...//batch-...// → walk down. - candidates = [p for p in batch_dir.rglob("batch-*") if p.is_dir()] - if not candidates: - # No nested batch-* dir? The batch_dir itself is the root. - candidates = [batch_dir] - inner_batch = candidates[0] - # If there's a model sub-dir inside, use it - sub = [ - c - for c in inner_batch.iterdir() - if c.is_dir() and not c.name.startswith("batch-logs") - ] - if sub and any( - ( - c / next(c.iterdir(), Path("/dev/null")) / "data" / "interception.json" - ).exists() - for c in sub - ): - inner_batch = sub[0] - print(f" → batch root: {inner_batch}") - - print(f"\n[2/3] Re-judge with {args.judge_model} (rubric={args.rubric}) ...") - summary = rescore(inner_batch, args.judge_model, args.rubric) - - n = summary["n_total"] - observed_icpt = 100.0 * summary["n_intercepted"] / n if n else 0.0 - observed_lenient = 100.0 * summary.get("reward_pct_lenient", 0) - observed_strict = 100.0 * summary.get("reward_pct_strict", 0) - observed = (observed_icpt, observed_lenient, observed_strict, n) - - print("\n[3/3] Compare to published row ...") - ok, table = verdict(observed, published, args.tolerance) - print(table) - print() - if ok: + with download_cache(args.work_dir, args.keep_cache) as cache_dir: print( - f"✓ PASS — reproduction within ±{args.tolerance} pp of published numbers." + f"== Reproducing {args.model} (n={published[3]}, tolerance ±{args.tolerance}pp) ==\n" ) - else: - print(f"✗ FAIL — at least one metric deviates more than ±{args.tolerance} pp.") - print(" Possible causes:") - print(" - Different judge model (we use deepseek-v4-pro on OpenRouter).") - print( - " - Different rubric (our prompts in src/clawbench/runner/judge_llm.py)." - ) - print(" - HF dataset rev drift — try `hf download --revision `.") - - if not args.keep_cache: - shutil.rmtree(args.work_dir, ignore_errors=True) - print(f" (deleted {args.work_dir}; pass --keep-cache to keep traces)") - - return 0 if ok else 1 + print("[1/3] Download trace subset from HF ...") + batch_dir = download(args.model, cache_dir) + + # Need a model batch root; HF subset puts task dirs under + # batch-aligned-...//batch-...// → walk down. + candidates = [p for p in batch_dir.rglob("batch-*") if p.is_dir()] + if not candidates: + # No nested batch-* dir? The batch_dir itself is the root. + candidates = [batch_dir] + inner_batch = candidates[0] + # If there's a model sub-dir inside, use it + sub = [ + c + for c in inner_batch.iterdir() + if c.is_dir() and not c.name.startswith("batch-logs") + ] + if sub and any( + ( + c / next(c.iterdir(), Path("/dev/null")) / "data" / "interception.json" + ).exists() + for c in sub + ): + inner_batch = sub[0] + print(f" → batch root: {inner_batch}") + + print(f"\n[2/3] Re-judge with {args.judge_model} (rubric={args.rubric}) ...") + summary = rescore(inner_batch, args.judge_model, args.rubric) + + n = summary["n_total"] + observed_icpt = 100.0 * summary["n_intercepted"] / n if n else 0.0 + observed_lenient = 100.0 * summary.get("reward_pct_lenient", 0) + observed_strict = 100.0 * summary.get("reward_pct_strict", 0) + observed = (observed_icpt, observed_lenient, observed_strict, n) + + print("\n[3/3] Compare to published row ...") + ok, table = verdict(observed, published, args.tolerance) + print(table) + print() + if ok: + print( + f"✓ PASS — reproduction within ±{args.tolerance} pp of published numbers." + ) + else: + print( + f"✗ FAIL — at least one metric deviates more than ±{args.tolerance} pp." + ) + print(" Possible causes:") + print(" - Different judge model (we use deepseek-v4-pro on OpenRouter).") + print( + " - Different rubric (our prompts in src/clawbench/runner/judge_llm.py)." + ) + print(" - HF dataset rev drift — try `hf download --revision `.") + + return 0 if ok else 1 if __name__ == "__main__": diff --git a/tests/test_reproduce_cache.py b/tests/test_reproduce_cache.py new file mode 100644 index 00000000..562b11f3 --- /dev/null +++ b/tests/test_reproduce_cache.py @@ -0,0 +1,89 @@ +"""The reproduction CLI must never own its caller's entire work directory.""" + +from pathlib import Path + +import pytest + +from clawbench.eval import reproduce + + +@pytest.mark.parametrize( + "outcome", ["pass", "fail", "download_error", "judge_error", "interrupt"] +) +@pytest.mark.parametrize("keep_cache", [False, True]) +def test_cli_preserves_user_files_and_cleans_only_its_cache( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, outcome: str, keep_cache: bool +) -> None: + work_dir = tmp_path / "existing-work" + work_dir.mkdir() + sentinel = work_dir / "unrelated.txt" + sentinel.write_text("user data") + previous_cache = work_dir / "clawbench-previous" + previous_cache.mkdir() + (previous_cache / "trace.json").write_text("previous run") + destinations = [] + + def download(model: str, dest: Path) -> Path: + destinations.append(dest) + assert dest.parent == work_dir + assert dest != previous_cache + (dest / "download.txt").write_text("owned download") + if outcome == "download_error": + raise SystemExit("download failed") + batch = dest / "traces" + batch.mkdir() + return batch + + def rescore(batch_dir: Path, judge_model: str, rubric: str) -> dict: + if outcome == "judge_error": + raise RuntimeError("judge failed") + if outcome == "interrupt": + raise KeyboardInterrupt + return { + "n_total": 129, + "n_intercepted": 4 if outcome == "pass" else 129, + "reward_pct_lenient": 3 / 129, + "reward_pct_strict": 0, + } + + monkeypatch.setattr(reproduce, "download", download) + monkeypatch.setattr(reproduce, "rescore", rescore) + argv = [ + "clawbench-reproduce", + "--model", + "deepseek-v4-flash", + "--work-dir", + str(work_dir), + ] + if keep_cache: + argv.append("--keep-cache") + monkeypatch.setattr("sys.argv", argv) + errors = { + "download_error": SystemExit, + "judge_error": RuntimeError, + "interrupt": KeyboardInterrupt, + } + if outcome in errors: + with pytest.raises(errors[outcome]): + reproduce.main() + else: + assert reproduce.main() == (0 if outcome == "pass" else 1) + + assert sentinel.read_text() == "user data" + assert (previous_cache / "trace.json").read_text() == "previous run" + assert len(destinations) == 1 + assert destinations[0].exists() is keep_cache + if keep_cache: + assert (destinations[0] / "download.txt").read_text() == "owned download" + + +def test_overlapping_invocations_have_independent_caches(tmp_path: Path) -> None: + with reproduce.download_cache(tmp_path, False) as first: + (first / "active.txt").write_text("first run") + with reproduce.download_cache(tmp_path, False) as second: + assert second != first + assert first.is_dir() and second.is_dir() + assert not second.exists() + assert (first / "active.txt").read_text() == "first run" + assert not first.exists() + assert tmp_path.is_dir()