Close six more test gaps found by a second mutation sweep

CLAUDE.md

@@ -834,13 +834,38 @@ already worked, proving it recognizes correct behavior rather than just
 matching new strings.
 
 A periodic mutation sweep is the systematic version, and it works well
-here because emission is pure template substitution — mutate an adapter,
+here because emission is pure template substitution — mutate a module,
 run `pytest tests/`, and any mutation that survives is an untested
-behavior. The sweep that produced the table above ran 18 mutations
-(retry policy, timeouts, step naming, payload threading, MCP allowlist
-narrowing, the `ANTHROPIC_API_KEY` scrub, invariant #4 and #7) and
-caught 15. Watch for **equivalent mutants** — the fan_out tier ordering
-in `fan_out_element_param` survives because it is provably unobservable,
+behavior. Two sweeps so far, 35 mutations, 26 caught on the first pass:
+
+- **Adapters** (18): retry policy, timeouts, step naming, payload
+  threading, MCP allowlist narrowing, the `ANTHROPIC_API_KEY` scrub,
+  invariants #4 and #7. Gaps found: per-node `timeout:` was ignorable on
+  both temporal and cloudflare gates with nothing failing.
+- **Drivers / inference / MCP / probe** (17): the flags that keep
+  `claude -p` cheap, `is_error` inside a zero exit, fastmcp's
+  `result.data`, probe's never-guess rule, and the cost-critical driver
+  defaults.
+
+Two lessons from the second sweep worth keeping:
+
+- **The agent-loop CLI path is the weak twin.** `_runtime_helper` carries
+  two copies of the `claude -p` flag block (judge, agent loop), and
+  mutations died on the judge copy while surviving on the loop copy —
+  `--setting-sources` and `is_error` were covered on one and not the
+  other, and `--system-prompt` on neither. That asymmetry is exactly how
+  the bare `--tools` bug shipped. When you touch one copy, test both.
+- **A symbolic assertion cannot pin a constant.**
+  `args[args.index("--model") + 1] == DEFAULT_MODEL` verifies the flag is
+  wired up and says nothing about the constant, because both sides move
+  together. `DEFAULT_MODEL` could be flipped to Opus (~5x cost) and
+  `DEFAULT_MAX_TURNS` halved (the original `error_max_turns` failure)
+  with the suite fully green. `tests/test_cost_defaults.py` pins the
+  *property* — not-Opus, `>= 60` — so a legitimate model bump passes and
+  the expensive regression fails.
+
+Watch for **equivalent mutants** — the fan_out tier ordering in
+`fan_out_element_param` survives because it is provably unobservable,
 not because it is untested; that one is commented in place.
 
 ---
@@ -1139,7 +1164,7 @@ Don't waste time debugging stubs. These are intentional.
   pipeline with cross-process gate signaling via `DBOSClient`; real
   judge usage captured through the emitted `$ROTE_USAGE_LOG` hook;
   measurements appended to `~/.local/share/rote/eval-corpus.jsonl`)
-- 1189 tests (1162 fast + 27 slow). Run with `pytest tests/` (fast
+- 1197 tests (1170 fast + 27 slow). Run with `pytest tests/` (fast
   only — what runs by default). Slow tests cover the runtime e2e
   suites (Temporal, Cloudflare, DBOS, DBOS-TS, Inngest,
   MCP-over-stdio); the TS ones require a Node toolchain, DBOS-TS

pyproject.toml

@@ -138,7 +138,11 @@ extend-exclude = ["examples"]
 select = ["E", "F", "I", "UP", "B", "SIM"]
 
 [tool.pytest.ini_options]
-addopts = "-m 'not slow'"
+# -rs: always print the REASON for every skip. A conditional skip whose
+# condition silently becomes permanent is invisible behind a bare
+# "1 skipped" count — that is how `test_per_tool_override_changes_payload`
+# went from a real test to dead weight without anyone noticing.
+addopts = "-m 'not slow' -rs"
 markers = [
     "slow: tests that need an external toolchain (Node/npm/tsc) or take >5s. Run with `pytest -m slow` to include.",
 ]

tests/test_cost_defaults.py

@@ -0,0 +1,79 @@
+"""Defaults that cost real money when they change.
+
+Two compiler-driver constants have burned this project before, and both
+are documented in CLAUDE.md as "don't change this" — but documentation
+is not a test. A mutation sweep confirmed it: flipping the default model
+to Opus and halving the turn budget both left the entire suite green.
+
+The existing driver tests compare against the constants *symbolically*
+(``args[args.index("--model") + 1] == DEFAULT_MODEL``), which is right
+for checking the flag is wired up and useless for catching a change to
+the constant itself — both sides move together.
+
+These tests pin the *property* rather than the literal, so a legitimate
+upgrade (sonnet-4-6 → sonnet-5) passes while the expensive regression
+fails.
+"""
+
+from __future__ import annotations
+
+import re
+
+from rote.compiler.drivers.anthropic_api import DEFAULT_MODEL as API_DEFAULT_MODEL
+from rote.compiler.drivers.claude import DEFAULT_MAX_TURNS
+from rote.compiler.drivers.claude import DEFAULT_MODEL as CLI_DEFAULT_MODEL
+
+#: Compiling BDR ran ~$3.50 per attempt on Opus and exhausted a Claude
+#: Max "extra usage" budget in two runs. Sonnet follows the structured
+#: rubric perfectly well; Opus is ~5x the price for no measured gain.
+_OPUS_RE = re.compile(r"opus", re.IGNORECASE)
+
+#: BDR-scale skills need ~25 tool calls minimum and realistically 40-50
+#: with exploration. The original default of 30 produced a hard
+#: `error_max_turns` failure on the first real run.
+_MIN_MAX_TURNS = 60
+
+
+def test_compiler_drivers_do_not_default_to_opus() -> None:
+    """Both drivers default to a non-Opus model.
+
+    Asserted as "not Opus" rather than an exact model id so bumping the
+    Sonnet generation stays a one-line change, while the regression this
+    exists to prevent still fails.
+    """
+    for name, model in (
+        ("ClaudeDriver", CLI_DEFAULT_MODEL),
+        ("AnthropicApiDriver", API_DEFAULT_MODEL),
+    ):
+        assert not _OPUS_RE.search(model), (
+            f"{name}.DEFAULT_MODEL is {model!r}. Opus is ~5x Sonnet's price "
+            f"and Sonnet follows the compiler rubric fine — two Opus runs of "
+            f"BDR exhausted a Max extra-usage budget. If you have specific "
+            f"evidence a skill needs Opus, pass model= explicitly rather than "
+            f"changing the default for everyone."
+        )
+
+
+def test_both_drivers_share_a_default_model() -> None:
+    """The subprocess and in-process drivers must not drift apart.
+
+    They are two ways to run the same compiler agent; a different default
+    on each makes compile cost depend on which driver happened to be
+    selected, which is invisible to the user.
+    """
+    assert CLI_DEFAULT_MODEL == API_DEFAULT_MODEL
+
+
+def test_max_turns_leaves_headroom_for_bdr_scale_skills() -> None:
+    """The turn budget stays >= 60.
+
+    Not an equality check: raising it is harmless, lowering it silently
+    truncates a compile into `error_max_turns` after the run has already
+    spent most of its money.
+    """
+    assert DEFAULT_MAX_TURNS >= _MIN_MAX_TURNS, (
+        f"DEFAULT_MAX_TURNS is {DEFAULT_MAX_TURNS}. BDR-scale skills need "
+        f"~25 tool calls minimum and 40-50 with exploration; the original "
+        f"default of 30 caused a hard error_max_turns failure on the first "
+        f"real run. Don't reduce it without measuring."
+    )

tests/test_eval_estimate.py

@@ -19,7 +19,7 @@
 from rote.eval.priors import Priors
 from rote.eval.sidecar import EvalEstimates, StepEstimate, TurnRange
 from rote.eval.tokens import HeuristicTokenCounter
-from rote.ir import NodeKind, Pipeline, load_pipeline
+from rote.ir import Node, NodeKind, Pipeline, load_pipeline
 
 REPO_ROOT = Path(__file__).resolve().parent.parent
 BDR_PIPELINE = REPO_ROOT / "examples" / "bdr-outreach" / "expected" / "pipeline.yaml"
@@ -312,19 +312,53 @@ def test_external_call_payload_sums_footprint(bdr_pipeline: Pipeline) -> None:
     assert payload == n_external * priors.tokens_per_external_call_result
 
 
-def test_per_tool_override_changes_payload(bdr_pipeline: Pipeline) -> None:
-    """A pinned per-tool payload beats the default constant for that tool."""
+def test_per_tool_override_changes_payload() -> None:
+    """A pinned per-tool payload beats the default constant for that tool.
+
+    Built on a purpose-made fixture rather than the BDR example. This
+    test previously took its mcp-bound node from BDR and skipped when
+    there wasn't one — and there never was: BDR's five required MCP
+    servers all come from its two agent loops' `tool_servers`, while its
+    four external_call nodes carry no `mcp:` binding at all. So it
+    silently never ran. A test whose subject is an example's incidental
+    shape is a test that can quietly stop testing.
+    """
     from rote.eval.estimate import external_call_payload_tokens
 
-    external = [n for n in bdr_pipeline.nodes if n.kind is NodeKind.EXTERNAL_CALL and n.mcp]
-    if not external:
-        pytest.skip("BDR fixture has no MCP-bound external_call to override")
-    tool = external[0].mcp.tool
-    base = external_call_payload_tokens(bdr_pipeline, Priors())
+    pinned = Node(
+        id="fetch_pinned",
+        kind=NodeKind.EXTERNAL_CALL,
+        description="d",
+        impl="m.py:fetch",
+        mcp={"server": "slack", "tool": "slack_get_messages"},
+    )
+    unpinned = Node(
+        id="fetch_other",
+        kind=NodeKind.EXTERNAL_CALL,
+        description="d",
+        impl="m.py:other",
+        mcp={"server": "gmail", "tool": "gmail_search"},
+    )
+    pipeline = Pipeline(
+        name="payloads",
+        input={"type": "In", "required": [], "optional": []},
+        nodes=[pinned, unpinned],
+        edges=[{"from": "fetch_pinned", "to": "fetch_other"}],
+        entry_nodes=["fetch_pinned"],
+        exit_nodes=["fetch_other"],
+    )
+
+    priors = Priors()
+    default_each = priors.tokens_per_external_call_result
+    assert external_call_payload_tokens(pipeline, priors) == 2 * default_each
+
     bumped = external_call_payload_tokens(
-        bdr_pipeline, Priors(payload_tokens_per_tool={tool: 99_000.0})
+        pipeline, Priors(payload_tokens_per_tool={"slack_get_messages": 99_000.0})
     )
-    assert bumped > base
+    # Exactly one node is pinned: the other keeps the default. Asserting
+    # `bumped > base` alone would also pass if the override leaked onto
+    # every external_call.
+    assert bumped == 99_000.0 + default_each
 
 
 def test_priors_from_overrides_scalars_and_per_tool() -> None:

tests/test_inference.py

@@ -633,3 +633,120 @@ def resolve_url(name: str, _pipeline_url: None) -> str:
     # better to go on, and a name no server provides never resolves anyway.
     _servers, unpinned = helper._mcp_config_for(tools, None)
     assert len(unpinned) == 4
+
+
+# ───────── The overhead-control flags, on BOTH CLI paths ─────────
+#
+# `claude -p` defaults to shipping Claude Code's entire coding-agent
+# system prompt: ~37k cache-creation tokens and 11.5s for a one-sentence
+# judge. `--system-prompt <one line>` + `--tools ""` + `--setting-sources
+# ""` bring the identical call to ~740 tokens and ~3.5s.
+#
+# A mutation sweep found `--system-prompt` untested on BOTH paths and
+# `--setting-sources` / `is_error` untested on the agent-loop path — the
+# same asymmetry that let the bare `--tools` flag ship broken, since the
+# agent-loop CLI path had no coverage at all.
+
+
+def _cli_flag(command: list[str], flag: str) -> str:
+    assert flag in command, f"{flag} missing from the invocation"
+    return command[command.index(flag) + 1]
+
+
+def test_judge_cli_replaces_the_default_coding_agent_prompt(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """A judge must carry its own short system prompt.
+
+    Asserting presence would pass with an empty value; asserting the
+    exact text would break on any wording change. The load-bearing
+    property is that it is *short and custom* — dropping the flag is
+    what restores the 37k-token default.
+    """
+    monkeypatch.setattr(helper, "_mcp_config_for", lambda *_a: ({}, []))
+    seen = _capture_cli(monkeypatch, _cli_envelope(structured_output={"grade": 9}))
+
+    helper.call_judge(
+        node_id="grade_essay",
+        client="anthropic",
+        prompt="Grade it.",
+        output_schema={"type": "object", "properties": {"grade": {"type": "integer"}}},
+        model="claude-sonnet-4-6",
+    )
+
+    prompt = _cli_flag(seen["command"], "--system-prompt")
+    assert prompt.strip(), "an empty system prompt restores Claude Code's default"
+    assert len(prompt) < 400, (
+        f"the judge system prompt is {len(prompt)} chars — it is meant to be a "
+        f"one-liner; the whole point of the flag is not shipping a large prompt"
+    )
+
+
+def test_agent_loop_cli_replaces_the_default_coding_agent_prompt(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Same contract on the agent-loop path, which had no coverage of it."""
+    monkeypatch.setattr(helper, "_mcp_config_for", lambda *_a: ({}, []))
+    seen = _capture_cli(monkeypatch, _agent_envelope())
+
+    helper.run_agent_loop(
+        node_id="target_research",
+        description="Research the account.",
+        task='{"account": "acme"}',
+        model="claude-sonnet-4-6",
+        tools=["bright_data_search"],
+        max_iterations=6,
+    )
+
+    prompt = _cli_flag(seen["command"], "--system-prompt")
+    assert prompt.strip(), "an empty system prompt restores Claude Code's default"
+    assert len(prompt) < 400
+
+
+def test_agent_loop_cli_does_not_inherit_user_settings(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """`--setting-sources ""` on the agent loop, as on the judge.
+
+    Without it the loop picks up whatever hooks, MCP servers and
+    permissions the *developer's* machine has configured — so an emitted
+    pipeline behaves differently per laptop, which is the opposite of
+    what compiling a skill is for.
+    """
+    monkeypatch.setattr(helper, "_mcp_config_for", lambda *_a: ({}, []))
+    seen = _capture_cli(monkeypatch, _agent_envelope())
+
+    helper.run_agent_loop(
+        node_id="target_research",
+        description="Research the account.",
+        task='{"account": "acme"}',
+        model="claude-sonnet-4-6",
+        tools=["bright_data_search"],
+        max_iterations=6,
+    )
+
+    assert _cli_flag(seen["command"], "--setting-sources") == ""
+
+
+def test_agent_loop_cli_surfaces_the_envelopes_error(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """A failed agent loop must not read as a success.
+
+    `claude -p` reports API failures *inside a zero exit* — `is_error`
+    in the envelope — so returncode alone calls a dead loop a win and
+    the workflow proceeds on a garbage result. Covered on the judge
+    path; this is the loop half.
+    """
+    monkeypatch.setattr(helper, "_mcp_config_for", lambda *_a: ({}, []))
+    _capture_cli(monkeypatch, _agent_envelope(is_error=True, result="rate limited"))
+
+    with pytest.raises(RuntimeError, match="rate limited"):
+        helper.run_agent_loop(
+            node_id="target_research",
+            description="Research the account.",
+            task='{"account": "acme"}',
+            model="claude-sonnet-4-6",
+            tools=["bright_data_search"],
+            max_iterations=6,
+        )