From 69ea51798718d198b3cb5989749192768eb5aed1 Mon Sep 17 00:00:00 2001 From: Arijit2916 <178023477+Samurai007AK@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:13:04 +0530 Subject: [PATCH 1/2] fix(benchmarks): stop inverting --web/--no-web in the public runner run_eval passed the parsed open-book flag straight into resolve_closed_book(), whose override parameter is the closed-book decision and is returned verbatim. Both CLI flags therefore did the opposite of their help text, and REACT_NO_WEB/SWARM_NO_WEB reached every worker inverted, so a run scored under the book policy nobody asked for. Negate at the call site rather than in resolve_closed_book(): that function is polarity-correct on its own, and flipping it would invert the benchmark-default path, which is currently unaffected. The translation now lives in _closed_book_for() so the polarity is pinnable by a test; it was previously buried mid-way through a long async function with no seam. Closes #26 --- benchmarks/public/runner/run_subprocess.py | 22 ++++++++++++++++++-- tests/test_benchmark_sandbox_profiles.py | 24 ++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/benchmarks/public/runner/run_subprocess.py b/benchmarks/public/runner/run_subprocess.py index 67698d0..a31c0c2 100644 --- a/benchmarks/public/runner/run_subprocess.py +++ b/benchmarks/public/runner/run_subprocess.py @@ -203,6 +203,25 @@ 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, *, @@ -222,8 +241,7 @@ 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. - from benchmarks.public.sandbox_profiles import resolve_closed_book - closed = resolve_closed_book(args.benchmark, getattr(args, "web", None)) + closed = _closed_book_for(args) 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" diff --git a/tests/test_benchmark_sandbox_profiles.py b/tests/test_benchmark_sandbox_profiles.py index 9edf958..3c2b784 100644 --- a/tests/test_benchmark_sandbox_profiles.py +++ b/tests/test_benchmark_sandbox_profiles.py @@ -92,6 +92,30 @@ 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. + + ``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. + """ + import argparse + + from benchmarks.public.runner.run_subprocess import _closed_book_for + + # 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 + + # 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 + + # An args namespace without the attribute at all still resolves. + assert _closed_book_for(argparse.Namespace(benchmark="officeqa")) is True + + class _FakeResourceManager: """Minimal stand-in exposing only what the tool resolvers touch.""" From 2f1269252fdf3e76378b53f571f29e4ece39463d Mon Sep 17 00:00:00 2001 From: Arijit2916 <178023477+Samurai007AK@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:17:51 +0530 Subject: [PATCH 2/2] 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. --- benchmarks/public/runner/run_subprocess.py | 33 +++++++------------ tests/test_benchmark_sandbox_profiles.py | 37 ++++++++++++++-------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/benchmarks/public/runner/run_subprocess.py b/benchmarks/public/runner/run_subprocess.py index a31c0c2..a578413 100644 --- a/benchmarks/public/runner/run_subprocess.py +++ b/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, diff --git a/tests/test_benchmark_sandbox_profiles.py b/tests/test_benchmark_sandbox_profiles.py index 3c2b784..482ead3 100644 --- a/tests/test_benchmark_sandbox_profiles.py +++ b/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: