refactor(benchmarks): inline the web-flag negation per review

Drop _closed_book_for(): the negation has one call site, and a helper
introduced purely as a test seam is not worth the indirection.

The regression is now pinned where the bug lived. The test calls
run_eval() with resolve_closed_book() monkeypatched as a spy that records
the override it receives and raises a sentinel to stop before task
execution, asserting False/True/None for --web, --no-web and no flag.
That covers the faulty handoff itself rather than a helper wrapping it.

Author: Samurai007AK

benchmarks/public/runner/run_subprocess.py

@@ -203,25 +203,6 @@ async def run_task_subprocess(
 
 # ── One-run evaluator ────────────────────────────────────────────────
 
-def _closed_book_for(args: argparse.Namespace) -> bool:
-    """Book policy for this run, honouring ``--web`` / ``--no-web``.
-
-    The two are *open-book* flags: ``--web`` sets ``args.web`` True, ``--no-web``
-    sets it False, and absence leaves None. ``resolve_closed_book`` takes the
-    opposite polarity — its ``override`` is the *closed-book* decision, returned
-    verbatim. Feeding ``args.web`` in unnegated makes both flags do the reverse of
-    their help text, and exports an inverted ``REACT_NO_WEB``/``SWARM_NO_WEB`` to
-    every worker, so the run scores under the book policy nobody asked for.
-
-    None stays None so an unflagged run still falls through to the benchmark's
-    own declaration.
-    """
-    from benchmarks.public.sandbox_profiles import resolve_closed_book
-
-    web = getattr(args, "web", None)
-    return resolve_closed_book(args.benchmark, None if web is None else not web)
-
-
 async def run_eval(
     args: argparse.Namespace,
     *,
@@ -241,10 +222,20 @@ async def run_eval(
     # Both names are set because the toggle is read per workflow — stateful
     # reads REACT_NO_WEB, agent-team reads SWARM_NO_WEB — and a benchmark should
     # get the same policy whichever workflow runs it.
-    closed = _closed_book_for(args)
+    # --web/--no-web are open-book flags; resolve_closed_book's override is the
+    # closed-book decision, returned verbatim. Negate, or both flags do the
+    # reverse of their help text. None falls through to the benchmark's own
+    # declaration.
+    from benchmarks.public.sandbox_profiles import resolve_closed_book
+
+    web = getattr(args, "web", None)
+    closed = resolve_closed_book(
+        args.benchmark,
+        None if web is None else not web,
+    )
     os.environ["REACT_NO_WEB"] = "1" if closed else "0"
     os.environ["SWARM_NO_WEB"] = "1" if closed else "0"
-    source = "--web/--no-web" if getattr(args, "web", None) is not None else "benchmark default"
+    source = "--web/--no-web" if web is not None else "benchmark default"
     logger.info(
         "Book policy: %s (%s) — web tools %s",
         "closed-book" if closed else "open-book", source,

tests/test_benchmark_sandbox_profiles.py

@@ -92,28 +92,39 @@ def test_book_policy_override_wins_both_ways():
     assert resolve_closed_book("browsecomp", True) is True    # --no-web
 
 
-def test_runner_does_not_invert_the_web_flags() -> None:
-    """``--no-web`` must reach the workers as closed-book, and vice versa.
+async def test_runner_does_not_invert_the_web_flags(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """``--no-web`` must reach ``resolve_closed_book`` as closed-book, and vice versa.
 
     ``resolve_closed_book`` is polarity-correct on its own, so its unit tests
     above stay green even when the runner hands it an un-negated *open-book*
-    flag. That is exactly how the flags shipped inverted, so the regression
-    has to be pinned at the runner's translation step.
+    flag. That is exactly how the flags shipped inverted, so the regression has
+    to be pinned on the handoff inside ``run_eval``.
     """
     import argparse
 
-    from benchmarks.public.runner.run_subprocess import _closed_book_for
+    from benchmarks.public import sandbox_profiles
+    from benchmarks.public.runner import run_subprocess
 
-    # Explicit flags win over the benchmark's own declaration, both ways.
-    assert _closed_book_for(argparse.Namespace(benchmark="browsecomp", web=False)) is True
-    assert _closed_book_for(argparse.Namespace(benchmark="officeqa", web=True)) is False
+    class _Stop(Exception):
+        """Sentinel: end the run once the override has been observed."""
 
-    # No flag: the benchmark default decides and must be left alone.
-    assert _closed_book_for(argparse.Namespace(benchmark="browsecomp", web=None)) is False
-    assert _closed_book_for(argparse.Namespace(benchmark="officeqa", web=None)) is True
+    seen: list[bool | None] = []
 
-    # An args namespace without the attribute at all still resolves.
-    assert _closed_book_for(argparse.Namespace(benchmark="officeqa")) is True
+    def _spy(benchmark: str, override: bool | None = None) -> bool:
+        seen.append(override)
+        raise _Stop
+
+    monkeypatch.setattr(sandbox_profiles, "resolve_closed_book", _spy)
+
+    for web in (True, False, None):
+        args = argparse.Namespace(benchmark="browsecomp", pipeline=None, web=web)
+        with pytest.raises(_Stop):
+            await run_subprocess.run_eval(args, out_dir=tmp_path, seed=0)
+
+    # --web is open-book, --no-web is closed-book, no flag defers to the benchmark.
+    assert seen == [False, True, None]
 
 
 class _FakeResourceManager: