From 0a96ea995289ef7e3a83ddd1223b1820482a6155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Thu, 29 Oct 2026 16:23:21 +0000 Subject: [PATCH] feat(eval): benchmark deferred MCP tool loading (#3765) * feat(eval): benchmark deferred MCP tools Refs #3609 * fix(eval): address benchmark CodeQL findings * fix(ci): execute MCP loading canaries (#3765) --------- Co-authored-by: Gergo Magyar --- .github/workflows/ci-tests.yml | 1 + .github/workflows/release-evaluation.yml | 46 ++++++- eval/tests/test_mcp_loading_report.py | 97 +++++++++++++ eval/tests/test_mcp_tool_loading.py | 152 +++++++++++++++++++++ eval/tests/test_pr_evaluation.py | 5 + eval/tests/test_workflow_bench.py | 1 + eval/workflow_bench/README.md | 34 +++++ eval/workflow_bench/mcp_loading_report.py | 158 ++++++++++++++++++++++ eval/workflow_bench/mock_provider.py | 12 +- eval/workflow_bench/pr_evaluation.py | 6 +- eval/workflow_bench/runner.py | 33 ++++- 11 files changed, 535 insertions(+), 10 deletions(-) create mode 100644 eval/tests/test_mcp_loading_report.py create mode 100644 eval/tests/test_mcp_tool_loading.py create mode 100644 eval/workflow_bench/mcp_loading_report.py diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index ec5659ed3..5b58082de 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -1551,6 +1551,7 @@ jobs: tests/test_ce_plugin_runtime.py tests/test_offline_sweep_integration.py tests/test_mock_provider.py + tests/test_mcp_tool_loading.py tests/test_release_evaluation_smoke.py tests/test_evolve.py::test_outer_runner_pid_namespace_kills_setsid_descendant tests/test_oracle_assets.py::test_hidden_vitest_config_executes_sibling_oracle_against_candidate_checkout diff --git a/.github/workflows/release-evaluation.yml b/.github/workflows/release-evaluation.yml index 94f1e01aa..c38f20715 100644 --- a/.github/workflows/release-evaluation.yml +++ b/.github/workflows/release-evaluation.yml @@ -17,6 +17,10 @@ on: description: Start, probe, and stop EC2 without paid model calls type: boolean default: false + mcp_tool_loading: + description: Add candidate deferred-MCP experiment (12 extra paid cells) + type: boolean + default: false candidate_ref: description: 'Full commit SHA or version tag to evaluate; latest-rc resolves the newest published RC' type: string @@ -380,6 +384,14 @@ jobs: --gitnexus-root "$GITHUB_WORKSPACE/$runtime" \ --claude-bin "$RUNNER_TEMP/claude-canary/node_modules/@anthropic-ai/claude-code-linux-x64/claude" done + - name: Require offline deferred-tool compatibility before paid sessions + if: inputs.mcp_tool_loading == true + working-directory: eval + env: + GITNEXUS_REQUIRE_CLAUDE_CANARY: '1' + CLAUDE_CANARY_BIN: ${{ runner.temp }}/claude-canary/node_modules/@anthropic-ai/claude-code-linux-x64/claude + run: >- + uv run --locked --extra dev python -m pytest tests/test_mcp_tool_loading.py -q - name: Recheck PR opt-in and pinned head immediately before paid sessions if: inputs.candidate_pr != '' working-directory: eval @@ -406,7 +418,7 @@ jobs: uv run --locked --extra dev python -m workflow_bench.runner \ --tasks "$RUNNER_TEMP/release-evaluation/$runtime/tasks.yaml" \ --gitnexus-root "$GITHUB_WORKSPACE/$runtime" \ - --arms "${arms[@]}" --runs 3 --include-expensive \ + --arms "${arms[@]}" --runs 3 --include-expensive --mcp-tool-loading eager \ --workers 3 --timeout 3600 --model "$MODEL" --effort "$EFFORT" \ --claude-bin "$RUNNER_TEMP/claude-canary/node_modules/@anthropic-ai/claude-code-linux-x64/claude" \ --out "$RUNNER_TEMP/release-evaluation/$runtime/raw" @@ -433,6 +445,25 @@ jobs: --workers 3 --timeout 3600 --model "$MODEL" --effort "$EFFORT" \ --claude-bin "$RUNNER_TEMP/claude-canary/node_modules/@anthropic-ai/claude-code-linux-x64/claude" \ --out "$RUNNER_TEMP/release-evaluation/candidate/cli-raw" + - name: Measure candidate with deferred MCP descriptions + if: inputs.mcp_tool_loading == true && inputs.candidate_pr == '' + timeout-minutes: 360 + working-directory: eval + env: + GITNEXUS_BENCH_OPENAI_API_KEY: ${{ secrets.GITNEXUS_BENCH_OPENAI_API_KEY }} + GITNEXUS_BENCH_ANTHROPIC_API_KEY: ${{ secrets.GITNEXUS_BENCH_ANTHROPIC_API_KEY }} + run: | + set -euo pipefail + remaining=$(( ${{ needs.start-runner.outputs.stop_deadline_epoch }} - $(date +%s) - 5400 )) + (( remaining > 0 )) || { echo '::error::No time remains for deferred sessions before the evidence reserve.'; exit 1; } + timeout --signal=TERM --kill-after=60 "${remaining}s" \ + uv run --locked --extra dev python -m workflow_bench.runner \ + --tasks "$RUNNER_TEMP/release-evaluation/candidate/tasks.yaml" \ + --gitnexus-root "$GITHUB_WORKSPACE/candidate" \ + --arms baseline --mcp-tool-loading deferred --runs 3 --include-expensive \ + --workers 3 --timeout 3600 --model "$MODEL" --effort "$EFFORT" \ + --claude-bin "$RUNNER_TEMP/claude-canary/node_modules/@anthropic-ai/claude-code-linux-x64/claude" \ + --out "$RUNNER_TEMP/release-evaluation/candidate/deferred" - name: Summarize complete or incomplete measurements if: ${{ always() && steps.resolve.outputs.sha != '' }} working-directory: eval @@ -469,6 +500,15 @@ jobs: --stable "$RUNNER_TEMP/release-evaluation/stable/public/agent-evaluation.json" \ --stable-sha '${{ steps.resolve.outputs.stable_sha }}' \ --out "$RUNNER_TEMP/release-evaluation/public" + - name: Summarize matched MCP loading experiment + if: ${{ always() && inputs.mcp_tool_loading == true && steps.resolve.outputs.sha != '' && inputs.candidate_pr == '' }} + working-directory: eval + run: | + uv run --locked --extra dev python -m workflow_bench.mcp_loading_report \ + --eager "$RUNNER_TEMP/release-evaluation/candidate/raw/results.jsonl" \ + --deferred "$RUNNER_TEMP/release-evaluation/candidate/deferred/results.jsonl" \ + --metadata "$RUNNER_TEMP/release-evaluation/candidate/metadata.json" \ + --out "$RUNNER_TEMP/release-evaluation/public" - name: Collect sanitized evidence and publish summary if: ${{ always() && steps.resolve.outputs.sha != '' }} run: | @@ -485,7 +525,7 @@ jobs: if [[ -f "$candidate" ]]; then cp "$candidate" "$public/agent-evaluation.$extension"; fi if [[ -f "$stable" ]]; then cp "$stable" "$public/stable-agent-evaluation.$extension"; fi done - for report in release-quality-gate agent-evaluation; do + for report in release-quality-gate agent-evaluation mcp-tool-loading; do if [[ -f "$public/$report.md" ]]; then cat "$public/$report.md" >> "$GITHUB_STEP_SUMMARY"; fi done - name: Summarize the opt-in CLI-only arm @@ -511,6 +551,8 @@ jobs: ${{ runner.temp }}/release-evaluation/public/stable-agent-evaluation.md ${{ runner.temp }}/release-evaluation/public/release-quality-gate.json ${{ runner.temp }}/release-evaluation/public/release-quality-gate.md + ${{ runner.temp }}/release-evaluation/public/mcp-tool-loading.json + ${{ runner.temp }}/release-evaluation/public/mcp-tool-loading.md if-no-files-found: error retention-days: 30 - name: Upload the opt-in CLI-only arm comparison diff --git a/eval/tests/test_mcp_loading_report.py b/eval/tests/test_mcp_loading_report.py new file mode 100644 index 000000000..1fb7b3b8c --- /dev/null +++ b/eval/tests/test_mcp_loading_report.py @@ -0,0 +1,97 @@ +"""A partial cheap arm is not evidence of savings.""" + +import copy +import json +import sys + +import pytest + +from tests.test_release_report import metadata, rows +from workflow_bench import mcp_loading_report +from workflow_bench.mcp_loading_report import COUNTERS, compare_loading, render_markdown + + +def measurements(): + eager = rows() + for row in eager: + row.update({field: 10 for field in COUNTERS}) + row.update( + mcp_tool_loading="none" if row["arm"] == "baseline_nomcp" else "eager", + task_prompt_digest="1" * 64, + sanitized_graph_snapshot_digest="2" * 64 if row["arm"] == "baseline" else None, + sanitized_task_sha="3" * 40, + runtime_sha=metadata()["runtime_sha"], + harness_sha=metadata()["harness_sha"], + ) + deferred = [dict(row, mcp_tool_loading="deferred", cost_usd=0.9) for row in eager if row["arm"] == "baseline"] + return eager, deferred + + +def test_complete_comparison_keeps_failed_solutions_and_costs(): + report = compare_loading(*measurements(), metadata()) + assert report["complete"] is True + assert report["arms"]["deferred"]["solved"] == 2 + assert report["arms"]["deferred"]["cells"] == 3 + assert report["paired"]["deferred_vs_eager"]["mean_cost_change_pct"] == pytest.approx(-28) + assert report["paired"]["deferred_vs_none"]["solve_delta"] == -1 + assert len(report["per_run"]) == 9 + assert all("internal_path" not in row for row in report["per_run"]) + assert "2/3" in render_markdown(report) + + +def test_fresh_graph_bytes_can_differ_when_source_and_runtime_match(): + eager, deferred = measurements() + for row in deferred: + row["sanitized_graph_snapshot_digest"] = "f" * 64 + report = compare_loading(eager, deferred, metadata()) + assert report["complete"] is True + assert report["per_run"][-1]["sanitized_graph_snapshot_digest"] == "f" * 64 + + +@pytest.mark.parametrize("mutation", [ + lambda e, d: d.pop(), + lambda e, d: d.append(copy.deepcopy(d[0])), + lambda e, d: d.append(None), + lambda e, d: d.append({"arm": []}), + lambda e, d: d[0].update(mcp_tool_loading="eager"), + lambda e, d: e[0].pop("mcp_tool_loading"), + lambda e, d: d[0].update(model="different-model"), + lambda e, d: d[0].update(effort="low"), + lambda e, d: d[0].update(task_base_sha="f" * 40), + lambda e, d: d[0].update(oracle_digest="f" * 64), + lambda e, d: d[0].update(sandbox_dependency_content_digest="f" * 64), + lambda e, d: d[0].update(sanitized_graph_snapshot_digest=None), + lambda e, d: d[0].update(sanitized_task_sha="f" * 40), + lambda e, d: d[0].update(runtime_sha="f" * 40), + lambda e, d: d[0].update(harness_sha="f" * 40), + lambda e, d: d[0].update(task_prompt_digest="f" * 64), + lambda e, d: d[0].update(ok=False, error_kind="session-error"), + lambda e, d: d[0].update(transcript_missing=True), + lambda e, d: d[0].update(cost_usd=float("nan")), + lambda e, d: d[0].update(cache_read_input_tokens=-1), + lambda e, d: d[0].update(num_turns=True), + lambda e, d: d[0].update(arm="baseline_nomcp"), +]) +def test_incomplete_or_confounded_comparison_never_reports_savings(mutation): + eager, deferred = measurements() + mutation(eager, deferred) + report = compare_loading(eager, deferred, metadata()) + assert report["complete"] is False and report["problems"] + assert report["arms"] == report["paired"] == {} + assert "no savings estimate" in render_markdown(report) + json.dumps(report, allow_nan=False) + + +def test_report_command_writes_failure_receipt_for_missing_deferred_file(tmp_path, monkeypatch): + eager, _ = measurements() + (tmp_path / "eager.jsonl").write_text("\n".join(json.dumps(row) for row in eager)) + (tmp_path / "metadata.json").write_text(json.dumps(metadata())) + monkeypatch.setattr(sys, "argv", [ + "report", "--eager", str(tmp_path / "eager.jsonl"), "--deferred", str(tmp_path / "missing.jsonl"), + "--metadata", str(tmp_path / "metadata.json"), "--out", str(tmp_path / "out"), + ]) + with pytest.raises(SystemExit, match="1"): + mcp_loading_report.main() + report = json.loads((tmp_path / "out/mcp-tool-loading.json").read_text()) + assert report["complete"] is False + assert report["arms"] == {} diff --git a/eval/tests/test_mcp_tool_loading.py b/eval/tests/test_mcp_tool_loading.py new file mode 100644 index 000000000..2e0f0866d --- /dev/null +++ b/eval/tests/test_mcp_tool_loading.py @@ -0,0 +1,152 @@ +"""Prove selective discovery at the provider boundary before paid experiments.""" + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest +import yaml + +from workflow_bench import model_gateway, runner, runner_sessions +from workflow_bench.mock_provider import MockProvider, Reply +from tests.test_workflow_bench_sessions import _probe_cell_provisioning, bench_args, fake_sandbox, session_record + + +@pytest.mark.parametrize("mode", ["eager", "deferred"]) +@pytest.mark.parametrize("arm", ["baseline", "baseline_nomcp", "baseline_cli", "workflow_direct"]) +def test_loading_switch_reaches_only_the_baseline_session(monkeypatch, tmp_path, mode, arm): + captured = [] + monkeypatch.setenv("ENABLE_TOOL_SEARCH", "ambient-value-must-not-leak") + monkeypatch.setattr(runner, "run_claude", lambda *a, **kw: captured.append(kw) or session_record()) + monkeypatch.setattr(runner, "run_verify", lambda *a, **kw: (True, "ok")) + runner.run_arm(arm, {"prompt": "Fix foo", "verify": "true"}, tmp_path, + bench_args(mcp_tool_loading=mode), sandbox=fake_sandbox(tmp_path)) + session = captured[0] + if arm == "baseline": + assert session["env"]["ENABLE_TOOL_SEARCH"] == ("true" if mode == "deferred" else "false") + assert session["allowed_tools"] == [ + *runner_sessions.allowed_agent_tools(implementation=True), + *(["ToolSearch"] if mode == "deferred" else []), + ] + else: + assert "ENABLE_TOOL_SEARCH" not in session["env"] + assert "ToolSearch" not in session["allowed_tools"] + if arm in ("baseline_cli", "baseline_nomcp"): + assert json.loads(session["mcp_config_json"]) == {"mcpServers": {}} + + +def test_loading_default_preserves_eager_catalog(): + parser = runner.build_parser() + assert parser.parse_args(["--tasks", "tasks.yaml", "--model", "gpt-5.6-sol"]).mcp_tool_loading == "eager" + + +@pytest.mark.parametrize("arm, expected", [("baseline", "deferred"), ("baseline_nomcp", "none")]) +def test_host_receipts_survive_failed_provisioning(monkeypatch, tmp_path, arm, expected): + ctx, *_ = _probe_cell_provisioning(monkeypatch, tmp_path) + ctx.args.mcp_tool_loading = "deferred" + ctx.args.measurement_revisions = {"runtime_sha": "a" * 40, "harness_sha": "b" * 40} + row = runner.run_cell(ctx, 0, arm) + assert row["ok"] is False + assert row["mcp_tool_loading"] == expected + assert row["runtime_sha"] == "a" * 40 and row["harness_sha"] == "b" * 40 + + +@pytest.mark.parametrize("mode", ["eager", "deferred"]) +def test_pinned_cli_discovers_only_selected_mcp_tool_through_gateway(tmp_path, monkeypatch, mode): + cli = os.environ.get("CLAUDE_CANARY_BIN") + if not cli or not Path(cli).is_file(): + if os.environ.get("GITNEXUS_REQUIRE_CLAUDE_CANARY") == "1": + pytest.fail("the required pinned Claude CLI is missing") + pytest.skip("CLAUDE_CANARY_BIN supplies the pinned real client") + root = Path(__file__).resolve().parents[2] + manifest = json.loads((root / ".github/claude-canary-runtime/package.json").read_text()) + version = manifest["dependencies"]["@anthropic-ai/claude-code"] + actual_version = subprocess.check_output([cli, "--version"], text=True).strip() + assert actual_version == f"{version} (Claude Code)" + mcp = tmp_path / "mcp.py" + mcp.write_text('''import json, sys +for line in sys.stdin: + request = json.loads(line) + if 'id' not in request: + continue + method = request.get('method') + if method == 'initialize': + result = {'protocolVersion': '2025-06-18', 'capabilities': {'tools': {}}, + 'serverInfo': {'name': 'offline-probe', 'version': '1'}} + elif method == 'tools/list': + result = {'tools': [ + {'name': name, 'description': marker * 200, + 'inputSchema': {'type': 'object', 'properties': {'name': {'type': 'string'}}, 'required': ['name']}} + for name, marker in [('context', 'SELECTED_DESCRIPTION '), ('unused', 'UNUSED_DESCRIPTION ')] + ]} + elif method == 'tools/call': + result = {'content': [{'type': 'text', 'text': 'FOUND_' + request['params']['name']}]} + else: + result = {} + print(json.dumps({'jsonrpc': '2.0', 'id': request['id'], 'result': result}), flush=True) +''') + + class ScriptedProvider(MockProvider): + def next_reply(self): + # The Responses mock returns JSON, so the CLI may retry a streaming + # request as non-streaming. Identical requests need identical steps. + called = {item.get("name") for item in self.requests[-1].body.get("input", []) + if isinstance(item, dict) and item.get("type") == "function_call"} + if mode == "deferred" and "ToolSearch" not in called: + return Reply(tools=[{"name": "ToolSearch", "input": {"query": "select:mcp__gitnexus__context"}}]) + if "mcp__gitnexus__context" not in called: + return Reply(tools=[{"name": "mcp__gitnexus__context", "input": {"name": "foo"}}]) + return Reply(text="Finished.") + + with ScriptedProvider() as provider: + original = model_gateway.write_openai_litellm_config + + def config(path, names): + original(path, names) + document = yaml.safe_load(path.read_text()) + for entry in document["model_list"]: + entry["litellm_params"]["api_base"] = provider.base_url + "/v1" + path.write_text(yaml.safe_dump(document)) + return path + + monkeypatch.setattr(model_gateway, "write_openai_litellm_config", config) + with model_gateway.OpenAIGateway( + openai_api_key="offline-key", model_names=["gpt-5.6-sol"], + work_dir=tmp_path / "gateway", ready_timeout_s=60, + ) as gateway: + env = { + "PATH": os.environ["PATH"], "HOME": str(tmp_path), + "CLAUDE_CONFIG_DIR": str(tmp_path / ".claude"), + "ANTHROPIC_API_KEY": gateway.auth_token, "ANTHROPIC_BASE_URL": gateway.base_url, + "ENABLE_TOOL_SEARCH": "true" if mode == "deferred" else "false", + "CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC": "1", "DISABLE_AUTOUPDATER": "1", + "HTTPS_PROXY": "http://127.0.0.1:9", "HTTP_PROXY": "http://127.0.0.1:9", + "NO_PROXY": "127.0.0.1,localhost", + } + tools = ["Read", "mcp__gitnexus__context", "mcp__gitnexus__unused"] + if mode == "deferred": + tools.append("ToolSearch") + mcp_config = json.dumps({"mcpServers": {"gitnexus": { + "type": "stdio", "command": sys.executable, "args": [str(mcp)], + }}}) + completed = subprocess.run( + [cli, "-p", "Inspect foo", "--model", "gpt-5.6-sol", "--output-format", "stream-json", + "--verbose", "--max-turns", "5", "--strict-mcp-config", "--mcp-config", mcp_config, + "--tools", *tools, "--allowedTools", *tools], + cwd=tmp_path, env=env, capture_output=True, text=True, timeout=60, + ) + events = [json.loads(line) for line in completed.stdout.splitlines() if line.strip()] + assert completed.returncode == 0, completed.stdout[-2000:] + assert events[-1]["type"] == "result" and events[-1]["is_error"] is False + assert any("FOUND_context" in json.dumps(event.get("tool_use_result")) for event in events) + bodies = [request.body for request in provider.requests] + assert bodies and all(request.path.endswith("/responses") for request in provider.requests) + first = json.dumps(bodies[0]) + if mode == "deferred": + assert "SELECTED_DESCRIPTION" not in first + assert "UNUSED_DESCRIPTION" not in json.dumps(bodies) + assert any("SELECTED_DESCRIPTION" in json.dumps(body) for body in bodies[1:]) + else: + assert "SELECTED_DESCRIPTION" in first and "UNUSED_DESCRIPTION" in first diff --git a/eval/tests/test_pr_evaluation.py b/eval/tests/test_pr_evaluation.py index 0a4c3da15..c480b934a 100644 --- a/eval/tests/test_pr_evaluation.py +++ b/eval/tests/test_pr_evaluation.py @@ -463,6 +463,11 @@ def test_workflow_protocol_ignores_opt_in_arms_after_the_paid_step(workflow_prot (None, {"if": "env.CLI_ARM == 'true'", "run": "python -m workflow_bench.runner\npython -m workflow_bench.runner"}, "must follow"), (None, {"if": "env.OTHER == 'true'", "run": "python -m workflow_bench.runner"}, "one paid"), + (0, {"if": "inputs.mcp_tool_loading == true && inputs.candidate_pr == ''", + "run": "python -m workflow_bench.runner"}, "must follow"), + (None, {"if": "inputs.mcp_tool_loading == true && inputs.candidate_pr == ''", + "run": "python -m workflow_bench.runner\npython -m workflow_bench.runner"}, "must follow"), + (None, {"if": "inputs.mcp_tool_loading == true", "run": "python -m workflow_bench.runner"}, "one paid"), ], ) def test_workflow_protocol_rejects_misplaced_or_unlisted_extra_arms(workflow_protocol_pair, position, step, message): diff --git a/eval/tests/test_workflow_bench.py b/eval/tests/test_workflow_bench.py index 9e5714296..e72ba4e19 100644 --- a/eval/tests/test_workflow_bench.py +++ b/eval/tests/test_workflow_bench.py @@ -277,6 +277,7 @@ def test_eval_ci_uses_locked_uv_and_blocking_native_containment_jobs(): # Carries the real-CLI identity probe, which needs CLAUDE_CANARY_BIN - # set only on this job. Omitted from this list it skipped everywhere. "tests/test_mock_provider.py", + "tests/test_mcp_tool_loading.py", # Paired prepare/session/oracle/report composition uses that same native # CLI and containment; it must execute here rather than skip everywhere. "tests/test_release_evaluation_smoke.py", diff --git a/eval/workflow_bench/README.md b/eval/workflow_bench/README.md index ede2a0b13..7b64b6729 100644 --- a/eval/workflow_bench/README.md +++ b/eval/workflow_bench/README.md @@ -205,6 +205,40 @@ every repetition, solve counts, cost and agent wall time. Failed solutions stay in the denominator. Infrastructure/session failures, missing costs, incomplete pairs or failed containment make evidence incomplete. +To isolate MCP description overhead, enable the manual `mcp_tool_loading` +input (off by default). The normal candidate/stable pairs retain eager loading; +an additional candidate sweep runs `baseline` with `--mcp-tool-loading deferred` +(12 extra paid cells for the current four-task suite). Before any paid session, +an offline canary drives the pinned Claude CLI through the real LiteLLM gateway: +the initial request must omit MCP descriptions, discovery must load and execute +the selected tool, and an unused tool's description must remain absent. + +The switch explicitly sets `ENABLE_TOOL_SEARCH` and includes `ToolSearch` in +both CLI tool lists. Merely improving agent instructions cannot change an eager +catalog. Hooks remain disabled and prompts, read-only MCP tools, model, effort, +and outputs remain the same. Each sweep rebuilds its graph from the same +sanitized task commit using the same runtime. The additional discovery turn may +offset catalog savings; the benchmark measures the net result. + +The separate `mcp-tool-loading.json` / `.md` report requires all three modes' +task/run cells, matching runtime/harness revisions, prompts, dependencies and +sanitized task commits. It retains failed repairs and rejects missing cells, +duplicate cells, missing counters and infrastructure failures before calculating +savings. It reports solved counts, costs, time, turns and cache/output tokens. +This is a descriptive experiment, not release approval or statistical proof. +The workflow runs the deferred sweep after the existing pairs; repeat with +reversed order before interpreting a small difference as a reliable gain. + +For local paired sweeps, use `--arms baseline_nomcp baseline --mcp-tool-loading +eager` followed by `--arms baseline --mcp-tool-loading deferred` with the same +pinned inputs and separate output directories. Summarize with: + +```bash +uv run --locked --extra dev python -m workflow_bench.mcp_loading_report \ + --eager candidate/raw/results.jsonl --deferred candidate/deferred/results.jsonl \ + --metadata candidate/metadata.json --out public +``` + Stable publishing requires a successful default-branch evaluation for that **exact commit, measured by main's current evaluator**, no more than seven days old. The publish job loads `eval/` from main's head rather than the release diff --git a/eval/workflow_bench/mcp_loading_report.py b/eval/workflow_bench/mcp_loading_report.py new file mode 100644 index 000000000..a66a4d153 --- /dev/null +++ b/eval/workflow_bench/mcp_loading_report.py @@ -0,0 +1,158 @@ +"""Matched no-MCP/eager/deferred experiment; never release approval evidence.""" + +from __future__ import annotations + +import argparse +import json +import re +import statistics +from pathlib import Path +from typing import Any + +from .release_report import META_FIELDS, build_report + +COUNTERS = ("num_turns", "input_tokens", "cache_creation_input_tokens", "cache_read_input_tokens", "output_tokens") +BINDINGS = ("task_prompt_digest", "sandbox_dependency_content_digest") + + +def compare_loading(eager: list[Any], deferred: list[Any], metadata: dict[str, Any]) -> dict[str, Any]: + """Reuse release cell validation, then require the same task/run and assets.""" + control = build_report(eager, metadata) + no_mcp = [row for row in eager if isinstance(row, dict) and row.get("arm") == "baseline_nomcp"] + # Do not filter the experimental input: an unexpected arm or duplicate must + # invalidate the comparison, not silently disappear from its denominator. + experiment = build_report([*no_mcp, *deferred], metadata) + problems = [f"eager: {p}" for p in control["problems"]] + problems += [f"deferred: {p}" for p in experiment["problems"]] + report = {key: metadata[key] for key in META_FIELDS} + report.update( + schema="gitnexus.mcp-tool-loading/v1", + generated_at=control["generated_at"], + complete=False, problems=problems, arms={}, paired={}, per_run=[], + ) + groups: dict[str, list[dict[str, Any]]] = {"none": no_mcp, "eager": [], "deferred": []} + for label, rows, public_rows in ( + ("eager", eager, control["per_run"]), + ("deferred", deferred, experiment["per_run"][len(no_mcp):]), + ): + dictionaries = [row for row in rows if isinstance(row, dict)] + for row, public in zip(dictionaries, public_rows, strict=True): + expected_mode = "none" if label == "eager" and row.get("arm") == "baseline_nomcp" else label + actual_mode = row.get("mcp_tool_loading") + public = dict(public, mcp_tool_loading=actual_mode if actual_mode in ("none", "eager", "deferred") else None) + for field in (*BINDINGS, "sanitized_task_sha", "sanitized_graph_snapshot_digest", "runtime_sha", "harness_sha"): + value = row.get(field) + if isinstance(value, str) and re.fullmatch(r"[0-9a-f]{40}|[0-9a-f]{64}", value): + public[field] = value + report["per_run"].append(public) + if row.get("mcp_tool_loading") != expected_mode: + problems.append(f"{label}: missing or incorrect host loading receipt") + for field in ("runtime_sha", "harness_sha"): + if row.get(field) != metadata[field]: + problems.append(f"{label}: mismatched {field}") + for field in COUNTERS: + value = row.get(field) + if type(value) is not int or value < 0: + problems.append(f"{label}: invalid {field}") + else: + public[field] = value + if expected_mode != "none": + groups[label].append(row) + if problems: + return report + cells = {mode: {(row["task"], row["run"]): row for row in rows} for mode, rows in groups.items()} + for key, row in cells["eager"].items(): + for field in (*BINDINGS, "sanitized_task_sha"): + value = row.get(field) + pattern = r"[0-9a-f]{40}" if field == "sanitized_task_sha" else r"[0-9a-f]{64}" + if not isinstance(value, str) or re.fullmatch(pattern, value) is None: + problems.append(f"{key}: missing {field}") + elif any(cells[mode][key].get(field) != value for mode in groups): + problems.append(f"{key}: mismatched {field}") + # Each sweep builds a fresh graph from the same sanitized commit with + # the same pinned runtime. Database bytes/timestamps need not match. + for mode in ("eager", "deferred"): + digest = cells[mode][key].get("sanitized_graph_snapshot_digest") + if not isinstance(digest, str) or re.fullmatch(r"[0-9a-f]{64}", digest) is None: + problems.append(f"{key}/{mode}: missing graph receipt") + if problems: + return report + report["complete"] = True + for mode, rows in groups.items(): + report["arms"][mode] = { + "cells": len(rows), "solved": sum(row["resolved"] for row in rows), + **{f"mean_{field}": statistics.mean(row[field] for row in rows) + for field in ("cost_usd", "duration_s", *COUNTERS)}, + } + for mode in ("none", "eager"): + baseline = report["arms"][mode] + treatment = report["arms"]["deferred"] + report["paired"][f"deferred_vs_{mode}"] = { + "solve_delta": treatment["solved"] - baseline["solved"], + "mean_cost_change_pct": ( + 100 * (treatment["mean_cost_usd"] / baseline["mean_cost_usd"] - 1) + if baseline["mean_cost_usd"] else None + ), + } + return report + + +def render_markdown(report: dict[str, Any]) -> str: + lines = [ + "## MCP tool-loading experiment", "", + f"Runtime `{report['runtime_sha']}`; harness `{report['harness_sha']}`.", + f"Model `{report['model']}`, effort `{report['effort']}`, n={report['runs']} per task and mode.", + "Hooks disabled; prompts, MCP backend and outputs unchanged. Descriptive evidence only; no release approval.", + "", + ] + if not report["complete"]: + return "\n".join([*lines, "**Incomplete or mismatched evidence; no savings estimate.**", "", + *[f"- {p}" for p in report["problems"]], ""]) + lines += [ + "| Mode | Solved | Mean cost | Mean seconds | Mean turns | Mean cache-read tokens | Mean output tokens |", + "| --- | --- | --- | --- | --- | --- | --- |", + ] + for mode, arm in report["arms"].items(): + lines.append( + f"| {mode} | {arm['solved']}/{arm['cells']} | ${arm['mean_cost_usd']:.4f} | " + f"{arm['mean_duration_s']:.1f} | {arm['mean_num_turns']:.1f} | " + f"{arm['mean_cache_read_input_tokens']:.0f} | {arm['mean_output_tokens']:.0f} |" + ) + for label, pair in report["paired"].items(): + change = pair["mean_cost_change_pct"] + cost = f"{change:+.1f}%" if change is not None else "undefined (zero control cost)" + lines += ["", f"{label}: cost {cost}; solved delta {pair['solve_delta']:+d}."] + lines += ["", "All completed repairs, including failed solutions, remain in the denominator. " + + "Three repetitions per task are descriptive, not a significance claim.", ""] + return "\n".join(lines) + + +def read_rows(path: Path) -> list[Any]: + rows = [] + for line in path.read_text().splitlines() if path.is_file() else []: + if not line.strip(): + continue + try: + rows.append(json.loads(line)) + except json.JSONDecodeError: + rows.append(None) + return rows + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--eager", required=True, type=Path) + parser.add_argument("--deferred", required=True, type=Path) + parser.add_argument("--metadata", required=True, type=Path) + parser.add_argument("--out", required=True, type=Path) + args = parser.parse_args() + report = compare_loading(read_rows(args.eager), read_rows(args.deferred), json.loads(args.metadata.read_text())) + args.out.mkdir(parents=True, exist_ok=True) + (args.out / "mcp-tool-loading.json").write_text(json.dumps(report, indent=2, allow_nan=False) + "\n") + (args.out / "mcp-tool-loading.md").write_text(render_markdown(report)) + if not report["complete"]: + raise SystemExit(1) + + +if __name__ == "__main__": + main() diff --git a/eval/workflow_bench/mock_provider.py b/eval/workflow_bench/mock_provider.py index dcc328251..d9f4de7a3 100644 --- a/eval/workflow_bench/mock_provider.py +++ b/eval/workflow_bench/mock_provider.py @@ -33,6 +33,7 @@ from __future__ import annotations import json import threading import time +import uuid from collections import deque from dataclasses import dataclass, field from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer @@ -185,8 +186,11 @@ def _openai_response(reply: Reply) -> dict[str, Any]: cache_read = reply.cache_read_input_tokens or 0 cache_write = reply.cache_creation_input_tokens or 0 total_input = reply.input_tokens + cache_read + cache_write + # Reusing call IDs across turns makes the CLI discard later tool results. + # Discovery followed by an MCP call must be two distinct calls. + response_id = uuid.uuid4().hex return { - "id": "resp_mock", + "id": f"resp_{response_id}", "object": "response", "created_at": int(time.time()), "status": "completed", @@ -194,7 +198,7 @@ def _openai_response(reply: Reply) -> dict[str, Any]: "error": None, "output": [ { - "id": "msg_mock", + "id": f"msg_{response_id}", "type": "message", "role": "assistant", "status": "completed", @@ -207,10 +211,10 @@ def _openai_response(reply: Reply) -> dict[str, Any]: # been wrong about the wire on the one path that matters most. *( { - "id": f"fc_mock_{index}", + "id": f"fc_{response_id}_{index}", "type": "function_call", "status": "completed", - "call_id": f"call_mock_{index}", + "call_id": f"call_{response_id}_{index}", "name": tool["name"], "arguments": json.dumps(tool.get("input", {})), } diff --git a/eval/workflow_bench/pr_evaluation.py b/eval/workflow_bench/pr_evaluation.py index 3ea3d2dc3..b106aa236 100644 --- a/eval/workflow_bench/pr_evaluation.py +++ b/eval/workflow_bench/pr_evaluation.py @@ -31,7 +31,11 @@ PREVIEW_PATHS = { } EVALUATION_WORKFLOW = ".github/workflows/release-evaluation.yml" # Exact `if:` gates of the dispatch-only descriptive arms (never PR previews). -OPT_IN_ARM_CONDITIONS = frozenset({"env.CLI_ARM == 'true'", "env.CODE_ARM == 'true'"}) +OPT_IN_ARM_CONDITIONS = frozenset({ + "env.CLI_ARM == 'true'", + "env.CODE_ARM == 'true'", + "inputs.mcp_tool_loading == true && inputs.candidate_pr == ''", +}) MEASUREMENT_PATHS = ("eval/", ".github/claude-canary-runtime/", EVALUATION_WORKFLOW) diff --git a/eval/workflow_bench/runner.py b/eval/workflow_bench/runner.py index 0368f2f7d..cf8748271 100644 --- a/eval/workflow_bench/runner.py +++ b/eval/workflow_bench/runner.py @@ -100,7 +100,7 @@ from .oracle_assets import ( staged_task_oracle, with_hidden_harness_apply_exclude, ) -from .process_control import cancellation_scope, ManagedProcessError +from .process_control import cancellation_scope, ManagedProcessError, run_checked from .promotion_apply import committed_destination_base_digests from .review_scoring import ( REVIEW_OUTPUT, @@ -475,6 +475,14 @@ def run_arm( model=args.model, build_sandbox_environment=environment_builder, ) + baseline_tools = allowed_agent_tools(implementation=True) + if arm == "baseline": + # Explicit host setting: never inherit ENABLE_TOOL_SEARCH from the + # operator. ToolSearch must be in --tools as well as --allowedTools. + deferred = getattr(args, "mcp_tool_loading", "eager") == "deferred" + env["ENABLE_TOOL_SEARCH"] = "true" if deferred else "false" + if deferred: + baseline_tools.append("ToolSearch") # All arms load ordinary repository context (CLAUDE.md) through the same # startup mode. The no-MCP arm instead disables skills/commands and # restricts its exact tool surface, with an empty MCP config and no @@ -680,7 +688,7 @@ def run_arm( BASELINE_PROMPT.format(task=task["prompt"]), worktree, disallowed_tools=["Skill"], - **{**common, "allowed_tools": allowed_agent_tools(implementation=True)}, + **{**common, "allowed_tools": baseline_tools}, ) ) record = sum_sessions(sessions) @@ -1452,6 +1460,11 @@ def run_cell(ctx: TaskCellContext, run_idx: int, arm: str) -> dict[str, Any]: "run": run_idx, "sandbox_backend": ctx.sandbox_backend, "runtime_exposure": runtime_exposure, + **getattr(args, "measurement_revisions", {}), + "mcp_tool_loading": ( + getattr(args, "mcp_tool_loading", "eager") if execution_arm == "baseline" + else "none" if execution_arm == "baseline_nomcp" else None + ), "task_asset_snapshot_digest": (ctx.asset_snapshot.digest if ctx.asset_snapshot is not None else None), "task_asset_manifest_digest": ( ctx.asset_snapshot.manifest_digest if ctx.asset_snapshot is not None else None @@ -2005,6 +2018,12 @@ def build_parser() -> argparse.ArgumentParser: help="arms to compare (default: workflow, workflow_direct, baseline, baseline_nomcp)", ) parser.add_argument("--claude-bin", default="claude") + parser.add_argument( + "--mcp-tool-loading", + choices=("eager", "deferred"), + default="eager", + help="MCP catalog loading for the baseline arm only (default: eager); other arms are unchanged", + ) parser.add_argument( "--gitnexus-root", type=Path, @@ -2195,7 +2214,15 @@ def main() -> None: sandbox_backend = "bwrap" require_claude_sandbox_helpers() runtime_mounts = trusted_gitnexus_runtime_mounts(root=args.gitnexus_root) - except SandboxError as exc: + args.measurement_revisions = {} + for field, root in (("runtime_sha", args.gitnexus_root), ("harness_sha", HARNESS_ROOT)): + sha = run_checked( + ["git", "-C", str(root.expanduser().resolve()), "rev-parse", "HEAD^{commit}"], timeout=60, + ).stdout_tail.strip() + if not re.fullmatch(r"[0-9a-f]{40}", sha): + raise SandboxError(f"cannot pin {field} for measurement") + args.measurement_revisions[field] = sha + except (SandboxError, ManagedProcessError) as exc: parser.error(str(exc)) raise AssertionError("ArgumentParser.error() returned unexpectedly") gateway = attach_openai_gateway(args)