fix(run-limits): count max_turns as model API calls on every harness and enforce it on claude-code

docs/TASK_DEFINITION_GUIDE.md

@@ -240,7 +240,7 @@ valid and an empty block is legal — every field defaults to "no limit".
 ```yaml
 run_limits:
   # Structural caps
-  max_turns: 20                       # hard cap on agent inner-loop turns per iteration
+  max_turns: 20                       # hard cap on model API calls per iteration
   expected_turns: 8                   # SOFT efficiency budget (visible turns) — never aborts
   task_timeout: 300                   # wall-clock cap for the full run envelope, seconds
   turn_timeout: 300                   # per-communicate() timeout, seconds
@@ -255,7 +255,7 @@ run_limits:
 
 | Field | Default | Constraint | Description |
 |-------|---------|------------|-------------|
-| `max_turns` | *unset* | `> 0` | Hard cap on agent inner-loop turns per iteration. Unset uses the SDK default. |
+| `max_turns` | *unset* | `> 0` | Hard cap on main-thread model API calls per iteration, Claude Code's turn, counted the same on every harness. The tools the last allowed call asks for still run; the turn ends when the next call begins. Unset uses the SDK default. See [HARNESS_PARITY.md](agents/HARNESS_PARITY.md). |
 | `expected_turns` | *unset* | `>= 1` | **Soft** target for cumulative visible turns. Exceeding it warns and badges the report; it never aborts. See [`expected_turns`](#expected_turns-soft-efficiency-budget). |
 | `task_timeout` | *unset* | `>= 30` | Max seconds for the full run envelope, including agent work, grading, and post-run work. |
 | `turn_timeout` | *unset* | `>= 10` | Max seconds for the agent's single `communicate()` iteration. |

docs/agents/HARNESS_PARITY.md

@@ -12,7 +12,7 @@ This page is the contract for what each run limit means per harness, plus the sh
 
 | Limit | claude-code | codex | antigravity | opencode | pi | delegate |
 |---|---|---|---|---|---|---|
-| `run_limits.max_turns` | native SDK cap (agent-loop turns) | visible-turn cap (resolved tool calls) | visible-turn cap (resolved tool calls) | native step cap (the CLI's own agent-loop steps) | native turn cap (the CLI's own `turn_start` agent-loop steps) | message-event cap (forwarded `message`-type SDK events, NOT tool calls or backend round-trips — the host exposes no round-trip boundary) |
+| `run_limits.max_turns` (main-thread model API calls on every harness, see below) | native CLI cap, plus a harness backstop when call N+1 begins | a call runs from its first item to its `thread/tokenUsage/updated`, and one that ran tools opens the next call there | a call runs from its first new MODEL step to the step carrying its usage | one `step_start` step | one `turn_start` turn | a call opens when the previous call's tools have all returned |
 | `run_limits.turn_timeout` | watchdog, SIGKILL on the CLI subprocess | watchdog + cooperative interrupt | watchdog, plus an earlier internal poll deadline at 80% of it (see below) | deadline enforced in-loop and on the final reap; SIGTERM→SIGKILL on the CLI's whole process group | deadline enforced in-loop and on the final reap; SIGTERM→SIGKILL on the CLI's whole process group | deadline checked both between reads and while blocked inside one (`asyncio.wait_for`); force-kills the host subprocess and drops the handle so the next turn respawns |
 | `run_limits.task_timeout` | orchestrator-level, agent-agnostic | orchestrator-level, agent-agnostic | orchestrator-level, agent-agnostic | orchestrator-level, agent-agnostic | orchestrator-level, agent-agnostic | orchestrator-level, agent-agnostic |
 | `run_limits.stop_early` | cooperative `should_stop` | cooperative `should_stop` | cooperative `should_stop` | cooperative `should_stop` (event granularity) | cooperative `should_stop` (event granularity — Pi streams incrementally) | cooperative `should_stop`, polled per forwarded SDK event; the host is abandoned (no interrupt command exists) and a fresh one spawns for the next turn |
@@ -531,59 +531,52 @@ needed to drive it.
   when a generation begins. Nothing in the timing accounting reads it — the
   head and tail are measured from the first and last `AssistantMessage`
   instead, which is uniform across all five — so this is recorded rather than
-  fixed. It is NOT a `max_turns` hazard: `EventCollector.visible_turn_count` is
-  `len(self._commands)`, derived from `ToolEndEvent`, and `_turn_starts` feeds
+  fixed. It is NOT a `max_turns` hazard: no cap reads it, and `_turn_starts` feeds
   only `assistant_turn_count` on the no-`AgentEndEvent` fallback path. The real
   cost of normalizing it is that the event drives the live renderers, so moving
   it changes the turn boundaries users watch during a run.
 
 All three are deliberately deferred; see `c/time-bugs-audit.md` for the
 measurements.
 
-## `max_turns` counts visible turns on Codex and Antigravity
-
-A "visible turn" is one entry in the run's timeline: one resolved tool call. It is
-the unit `result_metrics.visible_turn_count` reports and the unit that lands in
-`TurnRecord.commands`. Both backends count it live off the shared
-`EventCollector.visible_turn_count`, so one `max_turns` value means one thing on
-both.
-
-They need their own counter because a native one would be meaningless: Codex and
-Antigravity each deliver exactly **one SDK turn per `communicate()` call**, so an
-SDK-level cap would clamp at 1 no matter what the task asked for.
-
-The cap is enforced on the same loop boundary as the cooperative early stop: the
-step or notification that reaches the cap is processed whole, and the next one is
-never pulled. The in-flight turn is then cancelled server-side (best effort) so
-the cap actually stops spend. A run cut this way finalizes cleanly as
-`max_turns_exhausted` — it is not a crash, and it is not retried.
-
-**claude-code keeps its native SDK cap.** That is a real, honored cap, so it is
-left alone rather than reimplemented in a different unit. Its unit is the SDK's own
-agent-loop turn, which absorbs an arbitrary number of *parallel* tool calls, so the
-same number bounds very different amounts of work: under a prompt that encourages
-batching, a cap of N here permits many more than N tool calls, where it buys exactly
-N on the other two.
-
-**OpenCode also keeps a native unit — its stream's own steps.** Unlike Codex and
-Antigravity, `opencode run` executes a real multi-step agent loop per invocation
-and streams it (`step_start` / `step_finish`), so the natural agent-loop unit
-exists and is honored: `max_turns: N` allows N complete steps and cuts the run
-when step N+1 begins, with the completed steps' tokens intact. A step is one
-assistant generation and may carry several tool calls — so, as with claude-code,
-the same number is a looser tool-call budget than on the visible-turn backends.
-
-**Pi keeps a native unit too — its `turn_start` agent-loop steps.** Like OpenCode,
-`pi -p --mode json` runs a real multi-step agent loop per invocation and streams it
-(`turn_start` / `turn_end`), so `max_turns: N` allows N complete turns and cuts the
-run when turn N+1 begins, with the completed turns' tokens intact. Pi streams
-incrementally, so the cut genuinely stops spend mid-run. A Pi turn is one assistant
-generation and may carry several tool calls — the same looser budget as claude-code
-and OpenCode.
-
-**So holding `max_turns` constant across harnesses does not hold the budget
-constant.** If you are A/B-ing across backends and the cap is close to binding, that
-is the number to distrust.
+## `max_turns` counts model API calls on every harness
+
+One turn is one main-thread model API call, the unit Claude Code's `--max-turns`
+counts. `max_turns: N` lets the agent make N calls and still runs the tools the Nth
+call asked for. The turn ends when call N+1 begins, so a reply that finishes within
+N calls completes normally. Sub-agent calls do not count. `TurnRecord.num_turns`
+reports the same count, and a turn the cap ended reads N+1, as the Claude Code CLI
+reports it.
+
+Each harness finds the call boundary in its own stream (the table above):
+
+- **claude-code** applies the cap in the CLI, and the harness reads the CLI's
+  `error_max_turns` stop. The CLI does not apply it on every route, so the harness
+  also counts distinct main-thread `message_id`s and ends the turn itself when call
+  N+1 begins.
+- **Codex** sends `thread/tokenUsage/updated` once per call, after that call's
+  tools finish. An item starts as its tool runs, so waiting for the next call's
+  first item would let one tool of call N+1 act. A call that ran tools therefore
+  opens the next call at its `tokenUsage`, since the results always go back to the
+  model, and the cap fires before call N+1 can run anything.
+- **Antigravity** attaches `usage_metadata` to one step per call, and the next call
+  opens with a MODEL step at a new `step_index`.
+- **OpenCode** and **Pi** stream one `step_start` or `turn_start` per call.
+- **Delegate**'s SDK has no round-trip marker, and a tool-only reply streams only
+  its tool call, with no text before it. So the next call opens when every tool the
+  previous call announced has returned, since those results go back to the model,
+  and the cap fires before call N+1 can run anything. A reply that announces
+  several tools before their results counts once. If a tool never returns, the
+  next call opens at the model's next text or new tool call instead.
+
+Every harness enforces the cap on the same loop boundary as the
+cooperative early stop, then kills or cancels the in-flight turn so the cap stops
+spend. A run cut this way finalizes cleanly as `max_turns_exhausted`. It is not a
+crash, and it is not retried.
+
+One call can carry several parallel tool calls, so `max_turns` bounds model calls,
+not tool calls. A model that batches does more work per turn, on every harness
+alike.
 
 ### What a capped run looks like
 
@@ -595,12 +588,12 @@ The signals a capped run leaves behind, on every backend:
   `MAX_TURNS_EXHAUSTED` (reporting category `failed`, icon `M`). Never `ERROR`,
   and never retried.
 - `max_turns_exhausted: true` on the task record.
-- On Codex and Antigravity, the count of *resolved* tool calls the model itself
-  issued equals the cap. Two things can add a further *recorded* command, and
-  neither means the cap leaked:
-    - A tool call already in flight when the cap fires is force-closed and recorded
-      with `result_status: unknown` rather than dropped, so the trajectory shows what
-      was interrupted.
+- `num_turns` is the cap plus one.
+- The calls under the cap are recorded whole. Two things can add a further
+  *recorded* command, and neither means the cap leaked:
+    - A tool call from call N+1 that the harness saw before it stopped is
+      force-closed and recorded with `result_status: unknown` rather than dropped,
+      so the trajectory shows what was interrupted.
     - On Codex, a sub-agent's inner tool calls are recovered from its rollout after
       the pump stops, so the child's work and its tokens still reach the record. The
       cap bounds what the model was allowed to do, not what the record may explain.

src/coder_eval/agents/antigravity_agent.py

@@ -408,7 +408,7 @@ async def _drain(
                         if state.max_turns_reached():
                             state.max_turns_hit = True
                             self._log.debug(
-                                "max_turns (%s visible turns) reached; ending step loop",
+                                "max_turns (%s API calls) reached; ending step loop",
                                 state.max_turns,
                             )
                             break
@@ -438,9 +438,8 @@ async def communicate(
         conversation is cancelled (best-effort) and the turn finalizes cleanly as
         ``STOPPED_EARLY`` (``crashed=False``).
 
-        ``max_turns`` caps VISIBLE turns — resolved tool calls — enforced in-stream
-        on the same boundary as the cooperative stop: one ``communicate()`` here is
-        a single SDK turn, so a native counter would cap at 1 and mean nothing.
+        ``max_turns`` caps main-thread model API calls, Claude Code's unit,
+        enforced in-stream on the same boundary as the cooperative stop.
         See docs/agents/HARNESS_PARITY.md.
 
         Drives one logical turn: ``conversation.send(prompt)`` then iterate
@@ -723,6 +722,10 @@ def __init__(
         self.commands: list[CommandTelemetry] = []
         self._output_parts: list[str] = []
         self._assistant_turns = 0
+        # Main-thread API calls begun; one spans its first new MODEL step to its usage.
+        self.api_calls = 0
+        self._in_api_call = False
+        self._seen_steps: set[tuple[str, Any]] = set()
 
         # ToolStart on first sight of an id; ToolEnd at DONE.
         self._next_seq = 0
@@ -754,14 +757,12 @@ def ended_cleanly(self) -> bool:
         return self.stopped_early_hit or self.max_turns_hit
 
     def max_turns_reached(self) -> bool:
-        """True once this turn has produced ``max_turns`` visible turns.
+        """True once the model begins API call ``max_turns + 1``, the unit Claude Code's ``--max-turns`` caps.
 
-        Delegates to ``EventCollector.visible_turn_count``, the single
-        agent-agnostic capture path, so one ``max_turns`` means the same thing here
-        and on Codex. It counts RESOLVED tool calls, so the call that reaches the
-        cap keeps its result instead of being force-closed as unresolved.
+        The next call opens only after the previous call's tools finish, so every
+        call under the cap keeps its tool results.
         """
-        return self.max_turns is not None and self.collector.visible_turn_count >= self.max_turns
+        return self.max_turns is not None and self.api_calls > self.max_turns
 
     def _seed_first_generation_window(self, source: Any) -> None:
         """Move the first window's mark to the first observed MODEL output.
@@ -789,6 +790,11 @@ def process_step(self, step: Any) -> None:
         sstatus = _enum_value(step.status)
         ssource = _enum_value(step.source)
         self._seed_first_generation_window(ssource)
+        step_key = (getattr(step, "trajectory_id", "") or "", step.step_index)
+        if ssource == _SOURCE_MODEL and step_key not in self._seen_steps and not self._in_api_call:
+            self._in_api_call = True
+            self.api_calls += 1
+        self._seen_steps.add(step_key)
         starget = _enum_value(step.target)
         done = sstatus in (_STATUS_DONE, _STATUS_ERROR)
 
@@ -810,6 +816,9 @@ def process_step(self, step: Any) -> None:
 
         # Per-generation usage: fold into the turn total and cut an AssistantMessage.
         if step.usage_metadata is not None:
+            if not self._in_api_call:
+                self.api_calls += 1
+            self._in_api_call = False
             gen = _to_token_usage(step.usage_metadata, self.model)
             self.total_usage = self.total_usage + gen
             self._flush_generation(gen, getattr(step.usage_metadata, "thoughts_token_count", 0) or 0)
@@ -1013,7 +1022,7 @@ def finalize(self, status: AgentEndStatus, *, crashed: bool = False, crash_reaso
                 model_used=self.model,
                 assistant_turn_count=self._assistant_turns,
                 messages=self.messages,
-                num_turns=self._assistant_turns,
+                num_turns=self.api_calls,
                 crashed=crashed,
                 crash_reason=crash_reason,
                 max_turns_exhausted=status is AgentEndStatus.MAX_TURNS_EXHAUSTED,

src/coder_eval/agents/claude_code_agent.py

@@ -223,6 +223,8 @@ def __init__(
         # Set True by the in-loop cooperative-stop break (early-stop-on-criterion).
         # Distinct from timeout_hit: a clean, non-crash stop that must NOT raise.
         self.stopped_early_hit = False
+        self.max_turns_hit = False
+        self.main_turn_ids: set[str] = set()
         # Resolved by _build_claude_query, set on the state before any finalize
         # path. Stays None if we crash before setup (finalize reads it for cost
         # backfill).
@@ -410,6 +412,8 @@ def on_assistant_message(self, message: Message) -> None:
         if isinstance(message_id, str):
             self.seen_message_ids.add(message_id)
             self.last_message_had_id = True
+            if not isinstance(parent_tool_use_id, str):
+                self.main_turn_ids.add(message_id)
         else:
             self.last_message_had_id = False
 
@@ -635,6 +639,7 @@ def finalize(self, status: AgentEndStatus, *, crashed: bool = False, crash_reaso
         max_turns_exhausted = not crashed and (
             self._agent._is_max_turns_result(self.sdk_result_summary)
             or (self.max_turns is not None and self.num_turns is not None and self.num_turns > self.max_turns)
+            or self.max_turns_hit
         )
         if max_turns_exhausted and status == AgentEndStatus.COMPLETED:
             status = AgentEndStatus.MAX_TURNS_EXHAUSTED
@@ -1116,19 +1121,29 @@ async def _pump_messages(
         ruff's statement cap. ``query`` is still resolved as a module global at
         call time, so ``patch("...claude_code_agent.query", ...)`` mocks work.
 
-        Two break conditions, and the ORDER MATTERS:
+        Three break conditions, and the ORDER MATTERS:
 
         - The wall-clock guard runs at the TOP, so an over-deadline message is
           DISCARDED — no append, no events. Do NOT move it to a post-loop check.
-        - The cooperative stop runs AFTER ``state.dispatch(message)``, so a watcher
-          can flip its flag on THIS message and the next is never pulled.
+        - The max_turns backstop and the cooperative stop run AFTER
+          ``state.dispatch(message)``, so the message that trips them is recorded
+          and the next is never pulled.
         """
         async for message in query(**query_kwargs):
             if deadline is not None and time.monotonic() > deadline:
                 state.timeout_hit = True
                 self._log.warning("Turn timeout reached mid-stream; breaking out of message loop")
                 break
             state.dispatch(message)
+            # The CLI stops after max_turns main-thread API calls; one more means it ignored the cap.
+            if state.max_turns is not None and len(state.main_turn_ids) > state.max_turns:
+                state.max_turns_hit = True
+                state.num_turns = len(state.main_turn_ids)
+                self._log.warning(
+                    "CLI began API call %d past max_turns=%d; ending the turn", state.num_turns, state.max_turns
+                )
+                self._kill_transport(self._active_transport)
+                break
             if should_stop is not None and should_stop():
                 state.stopped_early_hit = True
                 self._log.debug("Cooperative stop requested; ending message loop at this boundary")
@@ -1211,11 +1226,12 @@ def _build_claude_query(
         # For later inspection: captures every field, defaults included.
         self._sdk_options_dump = dump_dataclass(options)
 
-        # Pre-constructed only under a timeout, to retain the subprocess handle
-        # for hard-kill. None otherwise, so the SDK uses its own default and tests
-        # can mock query() without a real CLI.
+        # Pre-constructed only under a timeout or turn cap, to retain the subprocess
+        # handle for hard-kill. None otherwise, so the SDK uses its own default and
+        # tests can mock query() without a real CLI. Closing the SDK's query()
+        # stream does not end the CLI: it never closes the generator it wraps.
         transport: SubprocessCLITransport | None = None
-        if timeout is not None:
+        if timeout is not None or max_turns is not None:
             transport = SubprocessCLITransport(prompt=user_input, options=options)
 
         return options, transport, effective_model