fix(reproduce): preserve user files when cleaning download caches

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.

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-.../<model>/batch-.../<model>/ → 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 <commit>`.")
-
-    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-.../<model>/batch-.../<model>/ → 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 <commit>`.")
+
+        return 0 if ok else 1
 
 
 if __name__ == "__main__":

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()