diff --git a/.claude/notes/agents.md b/.claude/notes/agents.md index 12de3f84..b36815af 100644 --- a/.claude/notes/agents.md +++ b/.claude/notes/agents.md @@ -61,14 +61,12 @@ intentionally brief and out of scope; trimming for DISPLAY belongs in the render - **Harness run-limit parity**: a shared `BaseAgentConfig` field must mean the same thing on every backend, so a divergence is either fixed or documented — never silent. - **`run_limits.max_turns` on Codex/Antigravity counts VISIBLE turns** (resolved tool - calls, read live off the shared `EventCollector.visible_turn_count`, the same list - `TurnRecord.commands` holds) because one `communicate()` is a single SDK turn on both, - so a native counter would clamp at 1; claude-code keeps its native SDK cap, whose unit - (an agent-loop turn) absorbs arbitrarily many parallel calls — the same number is NOT - the same budget across harnesses. OpenCode and Pi each keep a native unit too, because - their CLIs stream a real multi-step loop per `communicate()` - (`step_start`/`step_finish`, `turn_start`/`turn_end`). The cap is enforced on the same + **`run_limits.max_turns` counts main-thread model API calls on every harness**, per + iteration (each retry and dialog exchange starts at zero). Codex and Antigravity run one + SDK turn per `communicate()`, so each counts calls from its own stream (Codex + `thread/tokenUsage/updated`, Antigravity MODEL steps). claude-code keeps the CLI's + `--max-turns` plus a backstop that counts main-thread `message_id`s. OpenCode and Pi + stream one `step_start` / `turn_start` per call. The cap is enforced on the same loop boundary as the cooperative early stop and finalizes cleanly as `max_turns_exhausted` (no crash, no retry); on Antigravity that boundary lives in `_drain()`, so the background-work poll loop honors it too. diff --git a/docs/DIALOG_MODE.md b/docs/DIALOG_MODE.md index 9fff5dc9..a5cf05ad 100644 --- a/docs/DIALOG_MODE.md +++ b/docs/DIALOG_MODE.md @@ -121,13 +121,15 @@ After each exchange the driver evaluates the stop conditions **in this order**, 2. **`stop_on_criteria_pass`** (`criteria_passed`) — every success criterion passes. Requires per-turn checking (`check_criteria: every_turn` or `both`); pairing it with the default `end_of_dialog` is rejected at load time, since there would be nothing to check against. -3. **`max_turns`** (`max_turns`) — the hard cap on exchanges. The agent exhausting its *own* inner - `max_turns` mid-exchange ends the dialog with the same reason. +3. **`max_turns`** (`max_turns`) — the hard cap on exchanges. 4. **`max_total_tokens`** (`budget`) — the dialog-wide budget across simulator **and** agent. The dialog ends and the task is **still scored** — unlike [`run_limits.max_total_tokens`](TASK_DEFINITION_GUIDE.md#run-limits), which covers the subject agent only and aborts. -5. **`stop_token`** (`stop_token`) — only if none of the above fired is the simulator asked for +5. **`run_limits.max_turns`** (`agent_max_turns`): the agent used up its own model-call cap inside + one exchange. That cap restarts on every exchange, so this reason means one exchange ran long, not + that the dialog ran out of exchanges. +6. **`stop_token`** (`stop_token`) — only if none of the above fired is the simulator asked for another message; the sentinel token in *that fresh utterance* ends the dialog. This is the workhorse in practice — the simulator decides, in character, that it got what it wanted — but it is evaluated **last**, so a turn that trips `max_turns` or the budget never gets the chance to diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index 70789dba..d0a30010 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/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,8 +255,8 @@ run_limits: | Field | Default | Constraint | Description | |-------|---------|------------|-------------| -| `max_turns` | *unset* | `> 0` | Hard cap on agent inner-loop turns per iteration. Unset uses the SDK default. | -| `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). | +| `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. Each retry and each dialog exchange starts a fresh count. Unset uses the SDK default. See [HARNESS_PARITY.md](agents/HARNESS_PARITY.md). | +| `expected_turns` | *unset* | `>= 1` | **Soft** target for visible turns (tool calls plus the final reply) summed over the whole task, a different unit from `max_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. | | `max_input_tokens` | *unset* | `>= 1` | Max cumulative input (prompt) tokens. | @@ -327,8 +327,8 @@ that did: a budgeted task that failed counts as over budget, while tasks with no `expected_turns` budget are excluded entirely (success or fail). The count compared against the budget is **visible turns** — one per tool call -plus one for the agent's final reply — *not* the SDK's `total_turns` (which -counts assistant messages and can bundle several tool calls into one). +plus one for the agent's final reply. It is *not* `total_turns`, which counts +model API calls (the `max_turns` unit), and one call can batch several tool calls. Set it to the number of turns a competent agent should need for the task. Pick budgets consistently across a suite — the headline % is only comparable when @@ -1721,7 +1721,7 @@ The simulator runs as a tools-disabled Claude Code agent on its own resolved `Ap **Semantics:** - The task's `initial_prompt` is the user's *opening* message; the simulator picks up from turn 2. -- `max_turns` is the intra-dialog cap (the worst-case agent call budget per trial). Use `n_trials` for variance sampling. +- `max_turns` caps exchanges. Each exchange also gets a fresh `run_limits.max_turns` of model API calls, so the worst case per trial is the product of the two. Use `n_trials` for variance sampling. - The `reference` solution, if present, is hidden from the simulator (same security posture as for the coding agent). - When `n_trials > 1`, each trial becomes its own `ResolvedTask` with its own zero-padded replicate directory (`runs/////`) and its own `task.json` — the same fan-out mechanism as experiment `repeats`, which `n_trials` takes precedence over when simulation is enabled. Trial-level metadata appears under `simulation.replicate_index` / `simulation.n_trials` on the `EvaluationResult`. diff --git a/docs/agents/ANTIGRAVITY.md b/docs/agents/ANTIGRAVITY.md index 87505869..fa84e82e 100644 --- a/docs/agents/ANTIGRAVITY.md +++ b/docs/agents/ANTIGRAVITY.md @@ -190,8 +190,9 @@ as every other agent. 5. **`allowed_tools` / `disallowed_tools` are not read.** The harness runs with its full builtin tool set, so an Antigravity run has tools (web search, subagents, URL fetch) that the same task file denies on Claude Code and Codex. -6. **`max_turns` counts visible turns.** One `communicate()` is a single SDK turn here, - so the cap counts resolved tool calls instead, enforced on the step loop. See +6. **`max_turns` is counted by the harness.** One `communicate()` is a single SDK turn + here, so the harness counts model API calls itself (a MODEL step at a new + `step_index` opens one) and enforces the cap on the step loop. See [Run-Limit Parity](HARNESS_PARITY.md). 7. **Shell commands over ~10s are moved to the background.** The localharness has a 10-second maximum synchronous wait; past it the command becomes a background task diff --git a/docs/agents/CODEX.md b/docs/agents/CODEX.md index 2d6a131c..29a98517 100644 --- a/docs/agents/CODEX.md +++ b/docs/agents/CODEX.md @@ -217,7 +217,7 @@ The Codex SDK is synchronous. The agent uses `_run_async()` helper to detect and | **Session Resume** | `--resume {session_id}` | Via thread ID | | **Permissions** | `permission_mode` + `allowed_tools` | `permission_mode` → sandbox/approval + `allowed_tools`/`disallowed_tools` → thread config | | **Tool Enforcement** | Not enforced by Coder Eval wrapper | `enabled_tools` honored; `disabled_tools` NOT enforced by the SDK | -| **`max_turns`** | Native SDK turn cap (assistant messages) | Visible-turn cap (tool calls), enforced on the notification pump | +| **`max_turns`** | Model API calls: the CLI's `--max-turns`, plus a harness backstop | Model API calls, counted per `thread/tokenUsage/updated` and enforced on the notification pump | | **Early stop** | Supported (cooperative `should_stop`, polled between messages) | Supported — polled after each streamed notification; the in-flight turn is interrupted best-effort | Run-limit semantics per harness: [Run-Limit Parity](HARNESS_PARITY.md). diff --git a/docs/agents/HARNESS_PARITY.md b/docs/agents/HARNESS_PARITY.md index a6874a4f..f473be81 100644 --- a/docs/agents/HARNESS_PARITY.md +++ b/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,8 +531,7 @@ 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. @@ -540,50 +539,49 @@ needed to drive it. 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. + +The count is per iteration: each retry and each dialog exchange starts at zero. A +dialog whose agent hits the cap inside an exchange ends with `stop_reason: +agent_max_turns`. That is distinct from `max_turns`, the simulator's cap on +exchanges. ### What a capped run looks like @@ -595,12 +593,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. diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 1923c474..6e3f1edb 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/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, diff --git a/src/coder_eval/agents/claude_code_agent.py b/src/coder_eval/agents/claude_code_agent.py index bd1fb002..ecde1c38 100644 --- a/src/coder_eval/agents/claude_code_agent.py +++ b/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 @@ -982,7 +987,7 @@ def capture_stderr(line: str) -> None: try: options, transport, effective_model = self._build_claude_query( - user_input, timeout, max_turns, capture_stderr + user_input, timeout, max_turns, capture_stderr, can_stop=should_stop is not None ) # Set on the state BEFORE the AgentStart emit and any finalize path # (finalize reads it for cost backfill); stays None if setup crashed. @@ -1116,12 +1121,13 @@ 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: @@ -1129,9 +1135,19 @@ async def _pump_messages( 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") + self._kill_transport(self._active_transport) break def _build_claude_query( @@ -1140,11 +1156,14 @@ def _build_claude_query( timeout: float | None, max_turns: int | None, stderr_callback: Callable[[str], None], + *, + can_stop: bool = False, ) -> tuple[ClaudeAgentOptions, SubprocessCLITransport | None, str | None]: - """Build the SDK options (+ a timeout-only transport) for one turn. + """Build the SDK options (+ a killable transport) for one turn. - ``transport`` is None unless a ``timeout`` is set: it is pre-constructed - only so the watchdog can hard-kill the subprocess. ``effective_model`` may + ``transport`` is None unless a timeout, a turn cap or a cooperative stop + (``can_stop``) can end the turn: it is pre-constructed only so the harness + can hard-kill the subprocess. ``effective_model`` may be None on a DirectRoute with no configured model. ``stderr_callback`` is wired in here but owned by ``communicate``. """ @@ -1211,11 +1230,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 when the harness may end the turn, 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 or can_stop: transport = SubprocessCLITransport(prompt=user_input, options=options) return options, transport, effective_model diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index 5611038e..fbb150e9 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -87,6 +87,9 @@ # the actual files regardless. _FILE_CHANGE_FAILURE_STATUSES = frozenset({"failed", "declined"}) +# Items that open no model API call: the prompt, hook injections, and compaction. +_NON_MODEL_ITEM_TYPES = frozenset({"userMessage", "hookPrompt", "contextCompaction"}) + # Thread-item types carrying transcript CONTENT or session metadata rather than a # tool call. Everything ELSE streamed as item/started+item/completed is treated as # a tool call, so a new Codex tool kind is captured automatically instead of being @@ -365,6 +368,10 @@ def __init__( # Text-less reasoning blocks, resolved at flush once reasoning tokens known. self.reasoning_placeholders: list[ContentBlock] = [] self.gen_index = 0 + # Main-thread API calls begun; one spans its first item to its tokenUsage event. + self.api_calls = 0 + self.in_api_call = False + self.call_ran_tools = False # Finalize inputs, COMMITTED by communicate after a clean pump return. # Defaults are the crash values (no terminal usage; format from messages). @@ -505,15 +512,17 @@ 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`` rather than - ``self.commands``, which SKIPS items whose telemetry the SDK does not - resolve; the collector counts every emitted tool end, which is what lands - in ``TurnRecord.commands``. Codex delivers one SDK turn per - ``communicate()``, so a native counter would cap at 1. + Codex closes a call with its tokenUsage event only after that call's tools + finish, so the calls before the cap run whole. """ - 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 _open_api_call(self) -> None: + self.api_calls += 1 + self.in_api_call = True + self.call_ran_tools = False def dispatch(self, notification: Any) -> bool: """Route a notification to its handler. Returns True on ``turn/completed`` @@ -550,8 +559,11 @@ def on_item_started(self, notification: Any) -> None: if item_id is not None and started_at_ms is not None: self.start_ms_by_id[item_id] = started_at_ms root_type = getattr(root, "type", None) + if not self.in_api_call and root_type not in _NON_MODEL_ITEM_TYPES: + self._open_api_call() # Any item that isn't transcript content is a tool call (generic capture). if root_type is not None and root_type not in _CONTENT_ITEM_TYPES: + self.call_ran_tools = True tool_id = item_id or f"{root_type}_{self.next_sequence}" self.seq_by_id[tool_id] = self.next_sequence # Recorded on the START telemetry too: close_open_tools publishes @@ -669,6 +681,13 @@ def on_agent_message_delta(self, notification: Any) -> None: def on_token_usage_updated(self, notification: Any) -> None: """One per generation → cut a message. Carries `total` (cumulative over the whole THREAD, i.e. every turn so far) and `last` (this generation's delta).""" + if not self.in_api_call: + self.api_calls += 1 + self.in_api_call = False + # Tool results always go back to the model, so the next call begins here, + # before Codex can run the next call's tools (an item starts as its tool runs). + if self.call_ran_tools: + self._open_api_call() if notification.payload: self.latest_token_usage = getattr(notification.payload, "token_usage", None) self._flush_message(getattr(self.latest_token_usage, "last", None)) @@ -745,7 +764,7 @@ def finalize(self, status: AgentEndStatus, *, crashed: bool = False, crash_reaso model_used=model_used, assistant_turn_count=1, messages=self.messages, - num_turns=1, + num_turns=max(self.api_calls, 1), crashed=crashed, crash_reason=crash_reason, max_turns_exhausted=status is AgentEndStatus.MAX_TURNS_EXHAUSTED, @@ -873,11 +892,9 @@ async def communicate( user_input: The message/prompt to send stream_callback: Optional callback for real-time event streaming timeout: Hard wall-clock deadline in seconds - max_turns: Hard cap on VISIBLE turns — tool calls, the unit - ``result_metrics.visible_turn_count`` counts — enforced in-stream on - the same pump boundary as the cooperative stop. Codex delivers one - SDK turn per ``communicate()``, so a native turn counter would cap - at 1; see docs/agents/HARNESS_PARITY.md. + max_turns: Hard cap on main-thread model API calls, Claude Code's + unit, enforced in-stream on the same pump boundary as the + cooperative stop; see docs/agents/HARNESS_PARITY.md. should_stop: Cooperative early-stop callback, polled after each dispatched notification. When it returns True the pump breaks, the in-flight turn is interrupted (best-effort) and the turn @@ -1514,7 +1531,7 @@ async def _run_turn_with_streaming( # stop, so an armed early-stop wins a tie. if state.max_turns_reached(): state.max_turns_hit = True - self._log.debug("max_turns (%s visible turns) reached; ending notification pump", state.max_turns) + self._log.debug("max_turns (%s API calls) reached; ending notification pump", state.max_turns) self._interrupt_active_turn() # best-effort; stops server-side spend break finally: diff --git a/src/coder_eval/agents/delegate_agent.py b/src/coder_eval/agents/delegate_agent.py index 05e65d74..9d71738d 100644 --- a/src/coder_eval/agents/delegate_agent.py +++ b/src/coder_eval/agents/delegate_agent.py @@ -105,6 +105,10 @@ _TEXT_EVENT_TYPES = frozenset({"thinking", "message"}) +def _tool_id(msg: dict[str, Any]) -> str: + return str(msg.get("toolId") or msg.get("id") or msg.get("callId") or "") + + def _env(bare_name: str) -> str | None: """Read a ``DELEGATE_``-namespaced auth var, falling back to the bare name. @@ -270,6 +274,13 @@ def __init__(self, *, iteration: int, user_input: str, model: str | None) -> Non self.open_tools: dict[str, CommandTelemetry] = {} self.sequence = 0 self.message_events = 0 + # Backend round-trips begun. A tool-only reply streams no text, so each call + # after the first opens once the previous call's tools have all returned. + self.api_calls = 0 + # A result arrived while other tools were still open, so the next call is not + # counted yet. If the model speaks or calls a new tool first, those tools never + # returned and the next call has begun. + self.results_incomplete = False self.model_used: str | None = model self.usage: TokenUsage | None = None @@ -659,7 +670,7 @@ def emit(event: StreamEvent) -> None: self._handle_event(msg, state, emit) - if max_turns is not None and state.message_events >= max_turns: + if max_turns is not None and state.api_calls > max_turns: state.max_turns_exhausted = True await self._abandon_host_after_loop_exit() break @@ -713,6 +724,15 @@ def _handle_event(self, msg: dict[str, Any], state: _TurnState, emit: Callable[[ if usage is not None: state.usage = usage + if state.api_calls == 0 and (event_type in _TEXT_EVENT_TYPES or event_type == "tool_call"): + state.api_calls = 1 + elif state.results_incomplete and ( + event_type in _TEXT_EVENT_TYPES or (event_type == "tool_call" and _tool_id(msg) not in state.open_tools) + ): + self._close_open_tools(state, emit) + state.api_calls += 1 + state.results_incomplete = False + if event_type in _TEXT_EVENT_TYPES: text = msg.get("content") if isinstance(text, str) and text: @@ -731,6 +751,9 @@ def _handle_event(self, msg: dict[str, Any], state: _TurnState, emit: Callable[[ self._handle_tool_call(msg, state, emit) elif event_type == "tool_result": self._handle_tool_result(msg, state, emit) + state.results_incomplete = bool(state.open_tools) + if not state.open_tools: + state.api_calls += 1 elif event_type == "error": message = msg.get("message") or msg.get("content") or "unknown error" state.error_message = str(message) @@ -742,7 +765,7 @@ def _handle_event(self, msg: dict[str, Any], state: _TurnState, emit: Callable[[ def _handle_tool_call(self, msg: dict[str, Any], state: _TurnState, emit: Callable[[StreamEvent], None]) -> None: # UNVERIFIED: exact id-field spelling. - tool_id = str(msg.get("toolId") or msg.get("id") or msg.get("callId") or uuid.uuid4()) + tool_id = _tool_id(msg) or str(uuid.uuid4()) tool_name = str(msg.get("toolName") or msg.get("tool") or "unknown") parameters = msg.get("input") parameters = parameters if isinstance(parameters, dict) else {} @@ -764,7 +787,7 @@ def _handle_tool_call(self, msg: dict[str, Any], state: _TurnState, emit: Callab emit(ToolStartEvent(task_id=self.task_id, turn_id=state.turn_id, tool=telemetry)) def _handle_tool_result(self, msg: dict[str, Any], state: _TurnState, emit: Callable[[StreamEvent], None]) -> None: - tool_id = str(msg.get("toolId") or msg.get("id") or msg.get("callId") or "") + tool_id = _tool_id(msg) telemetry = state.open_tools.pop(tool_id, None) if telemetry is None: # A result with no matching open call (id mismatch or unknown shape). @@ -896,7 +919,7 @@ def _finalize_turn( model_used=state.model_used, assistant_turn_count=max(state.message_events, 1) if not crashed else state.message_events, messages=messages, - num_turns=None if crashed else max(state.message_events, 1), + num_turns=None if crashed else max(state.api_calls, 1), max_turns_exhausted=state.max_turns_exhausted, result_summary=ResultSummary( is_error=crashed, diff --git a/src/coder_eval/models/limits.py b/src/coder_eval/models/limits.py index 48bdb48c..bd026f8c 100644 --- a/src/coder_eval/models/limits.py +++ b/src/coder_eval/models/limits.py @@ -32,18 +32,20 @@ class RunLimits(BaseModel): max_turns: int | None = Field( default=None, gt=0, - description="Max agent inner-loop turns per iteration. None = SDK default.", + description=( + "Max main-thread model API calls per iteration, on every harness. Each retry and each " + "dialog exchange starts a fresh count. None = SDK default." + ), ) expected_turns: int | None = Field( default=None, ge=1, description=( - "Soft target for cumulative visible turns across a task. A 'turn' is one " - "entry in the Turn timeline: each tool call contributes 1, plus 1 for the " - "final reply when present. " - "When the running total exceeds this, the orchestrator logs a one-shot " - "warning and the report renders a badge — the run is NOT aborted " - "(use max_turns for a hard cap). None disables the check." + "Soft target for visible turns summed over the whole task: each tool call counts 1, " + "plus 1 for the final reply when present. This is a different unit and scope from " + "max_turns, which caps model API calls per iteration. When the running total exceeds " + "this, the orchestrator logs a one-shot warning and the report renders a badge; the " + "run is NOT aborted. None disables the check." ), ) task_timeout: int | None = Field( diff --git a/src/coder_eval/models/results.py b/src/coder_eval/models/results.py index 8e04a0ff..867c1c24 100644 --- a/src/coder_eval/models/results.py +++ b/src/coder_eval/models/results.py @@ -395,9 +395,9 @@ class TurnRecord(BaseModel): num_turns: int | None = Field( default=None, description=( - "Number of inner-loop turns the SDK reported for this communicate() call " - "(from ResultMessage.num_turns). None when the SDK did not emit a " - "ResultMessage (e.g. crash partial before the final message arrived)." + "Main-thread model API calls in this communicate() call, the unit max_turns caps; " + "max_turns + 1 when the cap ended the turn. None when the agent crashed before " + "reporting it (e.g. a Claude Code partial before its ResultMessage arrived)." ), ) max_turns_exhausted: bool = Field( @@ -457,6 +457,7 @@ class SimulationTelemetry(BaseModel): "criteria_passed", "stop_token", "max_turns", + "agent_max_turns", "budget", "error", "run_limit_exceeded", diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index e1909e64..eccfd7f1 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -2646,7 +2646,7 @@ async def _simulation_dialog_loop(self, initial_prompt: str | None, sandbox_dir: if turn_record.max_turns_exhausted: self.result.max_turns_exhausted = True - stop_reason = DialogStopReason.MAX_TURNS + stop_reason = DialogStopReason.AGENT_MAX_TURNS logger.warning( "Agent exhausted its inner max_turns during simulation turn %s; ending dialog.", turns_completed, diff --git a/src/coder_eval/reports/html.py b/src/coder_eval/reports/html.py index ebe97853..c9fb8ee2 100644 --- a/src/coder_eval/reports/html.py +++ b/src/coder_eval/reports/html.py @@ -1068,7 +1068,8 @@ def _render_installed_tools(result: EvaluationResult) -> str: _SIMULATION_STOP_REASON_LABELS = { "criteria_passed": ("success", "criteria passed"), "stop_token": ("neutral", "simulator ended dialog"), - "max_turns": ("failure", "turn cap reached"), + "max_turns": ("failure", "exchange cap reached"), + "agent_max_turns": ("failure", "agent max_turns reached"), "budget": ("failure", "token budget exhausted"), "error": ("failure", "simulator error"), } diff --git a/src/coder_eval/simulation/termination.py b/src/coder_eval/simulation/termination.py index d5610bda..4cc96f04 100644 --- a/src/coder_eval/simulation/termination.py +++ b/src/coder_eval/simulation/termination.py @@ -14,6 +14,7 @@ class DialogStopReason(StrEnum): CRITERIA_PASSED = "criteria_passed" STOP_TOKEN = "stop_token" MAX_TURNS = "max_turns" + AGENT_MAX_TURNS = "agent_max_turns" BUDGET = "budget" ERROR = "error" RUN_LIMIT_EXCEEDED = "run_limit_exceeded" diff --git a/src/coder_eval/streaming/collector.py b/src/coder_eval/streaming/collector.py index 61b03300..33c9d1ba 100644 --- a/src/coder_eval/streaming/collector.py +++ b/src/coder_eval/streaming/collector.py @@ -81,22 +81,6 @@ def on_event(self, event: StreamEvent) -> None: elif isinstance(event, AgentEndEvent): self._agent_end = event - @property - def visible_turn_count(self) -> int: - """Visible timeline entries observed so far — one per resolved tool call. - - The live, in-stream counterpart of ``result_metrics.visible_turn_count``, - which counts the very same list once the turn is a finished - ``TurnRecord`` (minus its trailing final-reply entry, which cannot exist - while the turn is still running). - - Agents whose SDK has no meaningful native turn counter (Codex, - Antigravity) enforce ``run_limits.max_turns`` against this, so the cap - means the same thing on both. Keying on ``tool_id`` means a re-emitted - end event cannot double-count. - """ - return len(self._commands) - def _ordered_commands(self) -> list[CommandTelemetry]: return sorted(self._commands.values(), key=lambda c: c.sequence_number) diff --git a/tests/_fixtures/golden_streams/expected/codex_b_command_execution.json b/tests/_fixtures/golden_streams/expected/codex_b_command_execution.json index b57169b8..685fe7cf 100644 --- a/tests/_fixtures/golden_streams/expected/codex_b_command_execution.json +++ b/tests/_fixtures/golden_streams/expected/codex_b_command_execution.json @@ -70,7 +70,7 @@ } ], "model_used": "gpt-5-codex", - "num_turns": 1, + "num_turns": 2, "result_summary": null, "timestamp": "", "token_usage": { diff --git a/tests/_fixtures/golden_streams/expected/codex_d_cross_flush_is_error.json b/tests/_fixtures/golden_streams/expected/codex_d_cross_flush_is_error.json index e239e539..8a316c13 100644 --- a/tests/_fixtures/golden_streams/expected/codex_d_cross_flush_is_error.json +++ b/tests/_fixtures/golden_streams/expected/codex_d_cross_flush_is_error.json @@ -61,7 +61,7 @@ } ], "model_used": "gpt-5-codex", - "num_turns": 1, + "num_turns": 2, "result_summary": null, "timestamp": "", "token_usage": { diff --git a/tests/test_agent.py b/tests/test_agent.py index 07a5b5a5..ef975c0c 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -202,7 +202,7 @@ def __init__(self) -> None: self.content = "ok" self.model = "mock-model" - async def mock_query(prompt, options): + async def mock_query(prompt, options, transport=None): captured_options.append(options) yield AssistantMessage() yield ResultMessage() @@ -1718,6 +1718,116 @@ async def mock_query(prompt, options, transport=None): assert turn_record.result_summary.subtype == "error_max_turns" +def _api_call(n, parent_tool_use_id=None): + """One API call as the SDK streams it: one message per content block, one shared id.""" + from tests._fixtures.golden_streams.claude_fixtures import AssistantMessage, ThinkingBlock, ToolUseBlock + + mid = f"{'sub' if parent_tool_use_id else 'msg'}-{n}" + return [ + AssistantMessage([ThinkingBlock("planning")], message_id=mid, parent_tool_use_id=parent_tool_use_id), + AssistantMessage( + [ToolUseBlock(f"{mid}-tool", "Bash", {"command": "echo hi"})], + message_id=mid, + parent_tool_use_id=parent_tool_use_id, + ), + ] + + +@pytest.mark.asyncio +async def test_claude_agent_max_turns_backstop_ends_a_turn_the_cli_did_not_cap(): + """A CLI that ignores --max-turns is cut when it begins API call max_turns + 1.""" + from tests._fixtures.golden_streams.claude_fixtures import ResultMessage + + agent = ClaudeCodeAgent(parse_agent_config(type=AgentKind.CLAUDE_CODE, permission_mode="acceptEdits")) + pulled = 0 + + passed_transport = [] + + async def mock_query(prompt, options, transport=None): + nonlocal pulled + passed_transport.append(transport) + for n in range(200): + for message in _api_call(n): + pulled += 1 + yield message + yield ResultMessage(num_turns=200) + + with tempfile.TemporaryDirectory() as tmpdir: + await agent.start(tmpdir) + with ( + patch("coder_eval.agents.claude_code_agent.query", mock_query), + patch.object(ClaudeCodeAgent, "_kill_transport") as kill, + ): + turn_record = await agent.communicate("loop forever", max_turns=3) + + # No turn timeout here, so the cap alone must give the backstop a process to kill. + assert passed_transport[0] is not None + kill.assert_called_once_with(passed_transport[0]) + assert pulled == 3 * 2 + 1 + assert turn_record.crashed is False + assert turn_record.max_turns_exhausted is True + assert turn_record.num_turns == 4 + assert len(turn_record.commands) == 3 + + +@pytest.mark.asyncio +async def test_claude_agent_cooperative_stop_kills_the_cli(): + """A cooperative stop with no timeout or cap still kills the CLI instead of leaving it running.""" + agent = ClaudeCodeAgent(parse_agent_config(type=AgentKind.CLAUDE_CODE, permission_mode="acceptEdits")) + pulled = 0 + passed_transport = [] + + async def mock_query(prompt, options, transport=None): + nonlocal pulled + passed_transport.append(transport) + for n in range(200): + for message in _api_call(n): + pulled += 1 + yield message + + with tempfile.TemporaryDirectory() as tmpdir: + await agent.start(tmpdir) + with ( + patch("coder_eval.agents.claude_code_agent.query", mock_query), + patch.object(ClaudeCodeAgent, "_kill_transport") as kill, + ): + turn_record = await agent.communicate("loop forever", should_stop=lambda: pulled >= 4) + + assert passed_transport[0] is not None + kill.assert_called_once_with(passed_transport[0]) + assert pulled == 4 + assert turn_record.crashed is False + assert turn_record.max_turns_exhausted is False + + +@pytest.mark.asyncio +async def test_claude_agent_max_turns_backstop_ignores_emissions_and_subagent_calls(): + """Per-block emissions share one API call, and sub-agent calls have their own cap.""" + from tests._fixtures.golden_streams.claude_fixtures import ResultMessage + + agent = ClaudeCodeAgent(parse_agent_config(type=AgentKind.CLAUDE_CODE, permission_mode="acceptEdits")) + + async def mock_query(prompt, options, transport=None): + for message in _api_call(0): + yield message + for n in range(5): + for message in _api_call(n, parent_tool_use_id="msg-0-tool"): + yield message + for message in _api_call(1): + yield message + yield ResultMessage(num_turns=2) + + with tempfile.TemporaryDirectory() as tmpdir: + await agent.start(tmpdir) + with patch("coder_eval.agents.claude_code_agent.query", mock_query): + turn_record = await agent.communicate("delegate", max_turns=2) + + assert turn_record.max_turns_exhausted is False + assert turn_record.num_turns == 2 + assert turn_record.result_summary is not None + assert turn_record.result_summary.subtype == "success" + + def test_setting_sources_default_is_project(): """When config.setting_sources is None, it defaults to ['project'] at runtime.""" config = parse_agent_config( diff --git a/tests/test_antigravity_agent.py b/tests/test_antigravity_agent.py index 681985e3..db1b45c0 100644 --- a/tests/test_antigravity_agent.py +++ b/tests/test_antigravity_agent.py @@ -1355,46 +1355,70 @@ async def __aexit__(self, *exc): assert [p.kind for p in configs[0].policies] == ["allow_all"] -# --- max_turns visible-turn cap ----------------------------------------------------- +# --- max_turns cap ------------------------------------------------------------------- # -# max_turns was accepted and never read on this backend, so a task capping turns ran -# uncapped here while the same file capped on Claude Code. The cap counts VISIBLE -# turns (tool calls — result_metrics.visible_turn_count's unit), enforced on the same -# step-loop boundary as the cooperative stop. +# max_turns caps main-thread model API calls, Claude Code's unit. A call opens at its +# first new MODEL step and closes with its usage, and the cap fires when call +# max_turns + 1 opens, on the same step-loop boundary as the cooperative stop. -def _tool_steps(count: int) -> list: - """`count` complete tool calls, each an ACTIVE step followed by its DONE step.""" +def _tool_steps(count: int, first_index: int = 1) -> list: + """`count` API calls that each run one tool: a new MODEL step carrying the call's usage, then its DONE step.""" steps = [] for i in range(count): call = _tc("run_command", f"t{i}", {"command_line": f"echo {i}"}) - steps.append(_step("TOOL_CALL", "ACTIVE", target="TARGET_ENVIRONMENT", tool_calls=[call])) + steps.append( + _step( + "TOOL_CALL", + "ACTIVE", + target="TARGET_ENVIRONMENT", + tool_calls=[call], + usage=_usage(10, 0, 5, 0), + step_index=first_index + i, + ) + ) done = _tc("run_command", f"t{i}", {"command_line": f"echo {i}", "exit_code": 0, "combined_output": str(i)}) - steps.append(_step("TOOL_CALL", "DONE", target="TARGET_ENVIRONMENT", tool_calls=[done])) + steps.append( + _step("TOOL_CALL", "DONE", target="TARGET_ENVIRONMENT", tool_calls=[done], step_index=first_index + i) + ) return steps -async def test_max_turns_caps_visible_turns(): - """The stream offers 5 tool calls; max_turns=2 keeps 2 and never pulls the rest.""" +def _resolved(record) -> list[str]: + return [c.tool_id for c in record.commands if c.result_status != "unknown"] + + +async def test_max_turns_caps_api_calls(): + """The stream offers 5 calls; max_turns=2 stops as the third opens and never pulls the rest.""" agent = _agent_with_steps(_tool_steps(5)) record = await agent.communicate("go", max_turns=2) - assert len(record.commands) == 2 + assert _resolved(record) == ["t0", "t1"] assert record.max_turns_exhausted is True + assert record.num_turns == 3 -async def test_max_turns_keeps_the_deciding_step_whole(): - """The tool call that reaches the cap is completed, not cut mid-flight.""" +async def test_max_turns_keeps_the_last_allowed_call_whole(): + """The last allowed call keeps its tool result: the cap fires only once the next call opens.""" agent = _agent_with_steps(_tool_steps(3)) record = await agent.communicate("go", max_turns=1) - assert len(record.commands) == 1 assert record.commands[0].result_status == "success" assert record.commands[0].result_summary == "0" +async def test_a_final_reply_on_the_last_allowed_call_completes(): + reply = _step("TEXT_RESPONSE", "DONE", content="done", complete=True, usage=_usage(10, 0, 5, 0), step_index=2) + agent = _agent_with_steps([*_tool_steps(1), reply]) + + record = await agent.communicate("go", max_turns=2) + + assert record.max_turns_exhausted is False + assert record.num_turns == 2 + + async def test_under_the_cap_completes_normally(): agent = _agent_with_steps(_tool_steps(2)) @@ -1402,6 +1426,7 @@ async def test_under_the_cap_completes_normally(): assert len(record.commands) == 2 assert record.max_turns_exhausted is False + assert record.num_turns == 2 async def test_no_max_turns_is_uncapped(): @@ -1417,11 +1442,16 @@ async def test_no_max_turns_is_uncapped(): async def test_cooperative_stop_outranks_the_cap(): """Both firing on the same step reports STOPPED_EARLY — the more specific reason.""" agent = _agent_with_steps(_tool_steps(5)) + polls: list[int] = [] - record = await agent.communicate("go", max_turns=1, should_stop=lambda: True) + def should_stop() -> bool: + polls.append(1) + return len(polls) >= 3 # the poll after call 2 opens, where max_turns=1 also fires + + record = await agent.communicate("go", max_turns=1, should_stop=should_stop) assert record.max_turns_exhausted is False - assert len(record.commands) == 1 + assert _resolved(record) == ["t0"] async def test_cap_reached_on_a_poll_redrain_stops_polling(monkeypatch): @@ -1437,9 +1467,13 @@ async def test_cap_reached_on_a_poll_redrain_stops_polling(monkeypatch): monkeypatch.setattr(antigravity_agent.asyncio, "sleep", _no_sleep) bg = _tc("run_command", "bg1", {"command_line": "sleep 999"}) - batch1 = [_step("TOOL_CALL", "ACTIVE", target="TARGET_ENVIRONMENT", tool_calls=[bg])] - # The re-drain kicks off a SECOND background job, then closes the first and runs - # one more call — reaching the cap (2) with an orphan still ACTIVE. Both exit + batch1 = [ + _step( + "TOOL_CALL", "ACTIVE", target="TARGET_ENVIRONMENT", tool_calls=[bg], usage=_usage(10, 0, 5, 0), step_index=1 + ) + ] + # The re-drain kicks off a SECOND background job in call 2, closes the first, and + # opens call 3, which reaches the cap (2) with an orphan still ACTIVE. Both exit # conditions are live at once, and the cap has to win: otherwise the loop keeps # polling out a background job on a run that is already over. batch2 = [ @@ -1448,6 +1482,8 @@ async def test_cap_reached_on_a_poll_redrain_stops_polling(monkeypatch): "ACTIVE", target="TARGET_ENVIRONMENT", tool_calls=[_tc("run_command", "bg2", {"command_line": "sleep 999"})], + usage=_usage(10, 0, 5, 0), + step_index=2, ), _step( "TOOL_CALL", @@ -1456,21 +1492,21 @@ async def test_cap_reached_on_a_poll_redrain_stops_polling(monkeypatch): tool_calls=[ _tc("run_command", "bg1", {"command_line": "sleep 999", "exit_code": 0, "combined_output": "x"}) ], + step_index=1, ), - *_tool_steps(1), + *_tool_steps(1, first_index=3), ] - batch3 = _tool_steps(2) # must never be drained + batch3 = _tool_steps(2, first_index=4) # must never be drained agent = _agent_with_steps([batch1, batch2, batch3]) conv = agent._sdk_agent.conversation record = await agent.communicate("go", max_turns=2) assert record.max_turns_exhausted is True - # The cap counts RESOLVED calls. The still-open bg2 is force-closed and recorded - # as unresolved rather than dropped, so the trajectory shows what was interrupted. - resolved = [c for c in record.commands if c.result_status != "unknown"] - assert [c.tool_id for c in resolved] == ["bg1", "t0"] - assert [c.tool_id for c in record.commands if c.result_status == "unknown"] == ["bg2"] + # The still-open calls are force-closed and recorded as unresolved rather than + # dropped, so the trajectory shows what was interrupted. + assert _resolved(record) == ["bg1"] + assert [c.tool_id for c in record.commands if c.result_status == "unknown"] == ["bg2", "t0"] assert conv.receive_steps_call_count == 2 # initial drain + one poll re-drain, then stop assert conv.cancel_call_count == 1 diff --git a/tests/test_codex_agent.py b/tests/test_codex_agent.py index 8f35de22..a02a5e30 100644 --- a/tests/test_codex_agent.py +++ b/tests/test_codex_agent.py @@ -2027,18 +2027,17 @@ def test_zsh_login_shell_restores_mock_prepend_end_to_end(self, monkeypatch, tmp agent._cleanup_login_shell_home() -class TestMaxTurnsVisibleTurnCap: - """``max_turns`` was documented as "unused for Codex single-turn" and dropped. +class TestMaxTurnsApiCallCap: + """``max_turns`` caps main-thread model API calls, Claude Code's unit. - Codex delivers one SDK turn per ``communicate()``, so a native turn counter would - cap at 1 and mean nothing; the cap therefore counts VISIBLE turns (completed tool - calls — the unit ``result_metrics.visible_turn_count`` sums) and is enforced on the - same pump boundary as the cooperative stop. + A call runs from its first item to its tokenUsage event, and a call that ran tools + opens the next one there, so the pump stops as soon as the last allowed call's + tools finish, before the next call can run anything. """ @staticmethod def _cmd_notifications(count: int) -> list: - """`count` completed shell commands, then the terminal turn/completed.""" + """`count` API calls that each think and run one shell command, then a final-reply call.""" notifications = [] for i in range(count): root = SimpleNamespace( @@ -2049,21 +2048,30 @@ def _cmd_notifications(count: int) -> list: aggregated_output=f"step-{i}\n", duration_ms=5, ) + reasoning = _reasoning_item("plan", item_id=f"r{i}") + notifications.append(_item_notification("item/started", reasoning)) + notifications.append(_item_notification("item/completed", reasoning)) notifications.append(_item_notification("item/started", root)) notifications.append(_item_notification("item/completed", root)) + notifications.append(_token_usage(inp=10, out=5, cached=0)) + reply = SimpleNamespace(type="agentMessage", id="m1", text="done") + notifications.append(_item_notification("item/started", reply)) + notifications.append(_item_notification("item/completed", reply)) + notifications.append(_token_usage(inp=10, out=5, cached=0)) notifications.append(_turn_completed()) return notifications - async def test_cap_stops_the_pump_at_the_limit(self): + async def test_cap_stops_before_the_next_call_runs_a_tool(self): agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), self._cmd_notifications(5)) record = await agent.communicate("go", max_turns=2) assert len(record.commands) == 2 assert record.max_turns_exhausted is True + assert record.num_turns == 3 async def test_cap_keeps_the_deciding_call_complete(self): - """Counting COMPLETED calls means the one that reaches the cap keeps its result.""" + """The last allowed call keeps its tool result: the cap fires only once the next call opens.""" agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), self._cmd_notifications(3)) record = await agent.communicate("go", max_turns=1) @@ -2071,6 +2079,14 @@ async def test_cap_keeps_the_deciding_call_complete(self): assert len(record.commands) == 1 assert record.commands[0].result_status == "success" + async def test_a_final_reply_on_the_last_allowed_call_completes(self): + agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), self._cmd_notifications(1)) + + record = await agent.communicate("go", max_turns=2) + + assert record.max_turns_exhausted is False + assert record.num_turns == 2 + async def test_cap_interrupts_the_in_flight_turn(self): """Best-effort server-side interrupt, so the cap actually stops spend.""" agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), self._cmd_notifications(5)) @@ -2086,6 +2102,7 @@ async def test_under_the_cap_completes_normally(self): assert len(record.commands) == 2 assert record.max_turns_exhausted is False + assert record.num_turns == 3 async def test_no_cap_consumes_the_whole_stream(self): """None must preserve the pre-existing behavior exactly.""" @@ -2099,10 +2116,16 @@ async def test_no_cap_consumes_the_whole_stream(self): async def test_cooperative_stop_outranks_the_cap(self): """Both firing on the same notification reports STOPPED_EARLY.""" agent = _started_agent(parse_agent_config(type=AgentKind.CODEX), self._cmd_notifications(5)) + polls: list[int] = [] + + def should_stop() -> bool: + polls.append(1) + return len(polls) >= 5 # the poll after call 1's tokenUsage, where max_turns=1 also fires - record = await agent.communicate("go", max_turns=1, should_stop=lambda: True) + record = await agent.communicate("go", max_turns=1, should_stop=should_stop) assert record.max_turns_exhausted is False + assert len(record.commands) == 1 async def test_capped_turn_still_folds_sub_agent_tokens(self, monkeypatch, tmp_path): """A capped turn must not lose the child threads' spend. @@ -2127,7 +2150,7 @@ async def test_capped_turn_still_folds_sub_agent_tokens(self, monkeypatch, tmp_p ) spawn = _collab_call("spawnAgent", call_id="call_spawn", model="gpt-5.5", child_thread=child) wait = _collab_call("wait", call_id="call_wait", result="5050", child_thread=child) - # The cap fires on the wait, before turn/completed is ever dispatched. + # The cap fires once the second call's tools finish, before turn/completed is dispatched. notifications = [ _item_notification("item/started", spawn), _item_notification("item/completed", spawn), diff --git a/tests/test_delegate_agent.py b/tests/test_delegate_agent.py index 436243ed..faa8f29b 100644 --- a/tests/test_delegate_agent.py +++ b/tests/test_delegate_agent.py @@ -380,14 +380,85 @@ async def test_timeout_elapsing_mid_read_still_raises_turn_timeout_error(self, p record = await agent.communicate("hi again") assert record.agent_output == "clean turn" + @staticmethod + def _round_trip(n: int) -> list[bytes]: + """One backend round-trip: an empty reply, then its tool call and result.""" + return [ + _line({"type": "message", "content": ""}), + _line({"type": "tool_call", "toolId": f"t{n}", "toolName": "shell", "input": {}}), + _line({"type": "tool_result", "toolId": f"t{n}", "output": "ok"}), + ] + async def test_max_turns_exhausted(self, patch_exec, tmp_path): + events = [*self._round_trip(0), *self._round_trip(1), *self._round_trip(2)] + agent, _proc = await _started_agent(patch_exec, events, tmp_path) + record = await agent.communicate("hi", max_turns=1) + assert record.max_turns_exhausted is True + assert [c.tool_id for c in record.commands if c.result_status == "success"] == ["t0"] + assert record.num_turns == 2 + + async def test_max_turns_counts_round_trips_not_text_chunks(self, patch_exec, tmp_path): + """The SDK streams a reply as several message events; they are one round-trip.""" events = [ - _line({"type": "message", "content": "one"}), - _line({"type": "message", "content": "two"}), + *self._round_trip(0), + _line({"type": "thinking", "content": "wrap up"}), + _line({"type": "message", "content": "all "}), + _line({"type": "message", "content": "done"}), + _line({"type": "send_ok", "result": "all done"}), ] agent, _proc = await _started_agent(patch_exec, events, tmp_path) - record = await agent.communicate("hi", max_turns=1) + record = await agent.communicate("hi", max_turns=2) + assert record.max_turns_exhausted is False + assert record.num_turns == 2 + + async def test_max_turns_stops_a_tool_only_reply_before_its_tool_runs(self, patch_exec, tmp_path): + """A tool-only reply streams no text event, only its tool call.""" + events = [ + _line({"type": "message", "content": "on it"}), + *[ + _line({"type": kind, "toolId": f"t{n}", "toolName": "shell", "output": "ok"}) + for n in range(3) + for kind in ("tool_call", "tool_result") + ], + ] + agent, _proc = await _started_agent(patch_exec, events, tmp_path) + record = await agent.communicate("hi", max_turns=2) + assert record.max_turns_exhausted is True + assert [c.tool_id for c in record.commands if c.result_status == "success"] == ["t0", "t1"] + assert record.num_turns == 3 + + async def test_max_turns_counts_a_batched_reply_once(self, patch_exec, tmp_path): + events = [ + _line({"type": "message", "content": ""}), + _line({"type": "tool_call", "toolId": "a", "toolName": "shell"}), + _line({"type": "tool_call", "toolId": "b", "toolName": "shell"}), + _line({"type": "tool_result", "toolId": "a", "output": "ok"}), + _line({"type": "tool_result", "toolId": "b", "output": "ok"}), + _line({"type": "message", "content": "done"}), + _line({"type": "send_ok", "result": "done"}), + ] + agent, _proc = await _started_agent(patch_exec, events, tmp_path) + record = await agent.communicate("hi", max_turns=2) + assert record.max_turns_exhausted is False + assert record.num_turns == 2 + + async def test_max_turns_still_counts_after_a_tool_never_returns(self, patch_exec, tmp_path): + """Tool b never returns, so the next call opens on its first new tool call.""" + events = [ + _line({"type": "tool_call", "toolId": "a", "toolName": "shell"}), + _line({"type": "tool_call", "toolId": "b", "toolName": "shell"}), + _line({"type": "tool_result", "toolId": "a", "output": "ok"}), + *[ + _line({"type": kind, "toolId": f"t{n}", "toolName": "shell", "output": "ok"}) + for n in range(3) + for kind in ("tool_call", "tool_result") + ], + ] + agent, _proc = await _started_agent(patch_exec, events, tmp_path) + record = await agent.communicate("hi", max_turns=2) assert record.max_turns_exhausted is True + assert [c.tool_id for c in record.commands if c.result_status == "success"] == ["a", "t0"] + assert record.num_turns == 3 async def test_communicate_before_start_raises(self): agent = DelegateAgent(_config()) diff --git a/tests/test_run_limits_orchestrator.py b/tests/test_run_limits_orchestrator.py index 86760d48..7e0feeb6 100644 --- a/tests/test_run_limits_orchestrator.py +++ b/tests/test_run_limits_orchestrator.py @@ -633,6 +633,50 @@ async def test_warning_fires_when_single_simulation_turn_exceeds(self, tmp_path, assert orch._expected_turns_warning_emitted is True +@pytest.mark.asyncio +async def test_agent_max_turns_ends_dialog_with_its_own_reason(tmp_path): + from coder_eval.models import SimulationConfig + + sim = SimulationConfig( + enabled=True, + persona="user", + goal="get the agent to do x", + max_turns=5, + check_criteria="end_of_dialog", + ) + task = _make_task(run_limits=RunLimits(max_turns=2)) + task = task.model_copy(update={"simulation": sim, "initial_prompt": "first message"}) + + orch = _make_orchestrator(task, tmp_path) + turn = _make_turn(commands=2).model_copy(update={"max_turns_exhausted": True}) + orch.agent = AsyncMock() + orch.agent.communicate = AsyncMock(return_value=turn) + + mock_checker = MagicMock() + mock_checker.check_all_async = AsyncMock( + return_value=[CriterionResult(criterion_type="file_exists", description="x", score=0.0)] + ) + orch.success_checker = mock_checker + + mock_simulator = MagicMock() + mock_simulator.model = DEFAULT_SIMULATOR_MODEL + mock_simulator.start = AsyncMock() + mock_simulator.stop = AsyncMock() + mock_simulator.next_user_message = AsyncMock() + + with ( + patch("coder_eval.orchestrator.UserSimulator", return_value=mock_simulator), + patch("coder_eval.orchestrator.resolve_reference_dir", return_value=None), + ): + await orch._simulation_dialog_loop("first message", tmp_path / "sandbox") + + assert orch.result.simulation is not None + assert orch.result.simulation.stop_reason == "agent_max_turns" + assert orch.result.simulation.total_turns == 1 + assert orch.result.max_turns_exhausted is True + mock_simulator.next_user_message.assert_not_called() + + class TestBuildSimulationTelemetry: """Direct field-mapping tests for the _build_simulation_telemetry SSOT builder.""" diff --git a/tests/test_visible_turn_cap.py b/tests/test_visible_turn_cap.py deleted file mode 100644 index 93cf5f09..00000000 --- a/tests/test_visible_turn_cap.py +++ /dev/null @@ -1,68 +0,0 @@ -"""``run_limits.max_turns`` must mean the same thing on Codex and Antigravity. - -Neither SDK can express the cap natively — each delivers exactly one SDK turn per -``communicate()`` call, so a native counter would clamp at 1 no matter what the task -asked for. Both therefore count VISIBLE turns (resolved tool calls) off one shared -definition, ``EventCollector.visible_turn_count``, rather than two per-agent counters -that happen to agree. See docs/agents/HARNESS_PARITY.md. - -Per-agent enforcement (where the cap fires in the loop, and how the run finalizes) -is covered in test_codex_agent.py and test_antigravity_agent.py. -""" - -from datetime import datetime - -import pytest - -from coder_eval.agents.antigravity_agent import AntigravityAgent -from coder_eval.agents.codex_agent import CodexAgent -from coder_eval.models import CommandTelemetry -from coder_eval.streaming.collector import EventCollector -from coder_eval.streaming.events import ToolEndEvent, ToolEndStatus - - -def _tool_end(collector: EventCollector, tool_id: str) -> None: - collector.on_event( - ToolEndEvent( - task_id="t", - turn_id="turn-1", - tool=CommandTelemetry(tool_name="Bash", tool_id=tool_id, timestamp=datetime.now(), sequence_number=0), - status=ToolEndStatus.OK, - ) - ) - - -def test_collector_visible_turn_count_counts_resolved_tool_calls(): - """The single definition Codex and Antigravity both cap against.""" - collector = EventCollector() - assert collector.visible_turn_count == 0 - - _tool_end(collector, "a") - _tool_end(collector, "b") - - assert collector.visible_turn_count == 2 - - -def test_collector_visible_turn_count_does_not_double_count_a_tool_id(): - """Keyed on tool_id, so a re-emitted end event cannot inflate the count past the cap.""" - collector = EventCollector() - - _tool_end(collector, "a") - _tool_end(collector, "a") - - assert collector.visible_turn_count == 1 - - -def test_collector_visible_turn_count_matches_the_built_record(): - """It is the live view of exactly the list ``TurnRecord.commands`` ends up holding.""" - collector = EventCollector() - for tool_id in ("a", "b", "c"): - _tool_end(collector, tool_id) - - assert collector.visible_turn_count == len(collector.build_turn_record().commands) - - -@pytest.mark.parametrize("agent_cls", [CodexAgent, AntigravityAgent]) -def test_both_capped_agents_declare_cooperative_stop(agent_cls): - """The turn cap reuses the cooperative-stop boundary, so both must support it.""" - assert agent_cls.supports_cooperative_stop is True