Skip to content

fix(agents): count Delegate batched tool calls once under max_turns and stamp turns off one TurnClock - #222

Open
abhiram-vad wants to merge 7 commits into
mainfrom
abhiram/fix/delegate-max-turns-batched-tools
Open

abhiram-vad wants to merge 7 commits into
mainfrom
abhiram/fix/delegate-max-turns-batched-tools

Conversation

@abhiram-vad

@abhiram-vad abhiram-vad commented Oct 9, 2026 •

Copy link
Copy Markdown

Motivation

run_limits.max_turns must count model calls (#194). On the Delegate agent it counted tool calls.

The Delegate host runs the tools of one batched reply one at a time. One call with three tools streams tool_call, tool_result, tool_call, tool_result, tool_call, tool_result. DelegateAgent opened a new call each time no tool was open, so each tool counted as one call.

On a downstream suite (UiPath Autopilot Scribe, max_turns: 80), 6–11 of 20 tasks per luna run ended MAX_TURNS_EXHAUSTED. One cut turn had 16 real model calls and 96 tool calls, and the adapter counted 82.

Changes

  • A call opens on its first thinking or message event. A call that streams no text opens on its usage frame. Tool events never open a call. Only the _TurnState.on_* methods write the count.
  • The host writes a call's usage frame before the call's tools. A crossing call that streams text is cut at that text, before its tools. A tool-only crossing call is cut at its own usage frame, so its usage is booked, and its first tool can start before the kill.
  • When the result arrives, the agent warns if its count differs from the host's turnUsages, so a change in the host's frame order shows in the logs.
  • The turn is stamped off one TurnClock. The window and tool stamps used datetime.now() while duration_seconds used time.monotonic(), which made test_delegate_golden[a_tool_call] fail at random on Windows, on main too.
  • Tests: the cut tests assert every recorded command; new tests cover a batched reply, frame-order drift, sub-agent tool events, an early stop on a usage frame, and the turn clock; golden scenarios e and f; one live test.
  • Docs: docs/agents/HARNESS_PARITY.md, docs/agents/DELEGATE.md, .claude/notes/agents.md, .claude/notes/timing.md.

Known limit: a host that sends no usage frames (below the 1.203.0 floor) counts only the calls that stream text.

Effect on Delegate runs

A turn now runs to max_turns model calls, not tool calls. With about 6 tools per call, a turn that the old count cut near 80 tool calls can now run to about 80 calls. Expect longer and more costly tasks, more stops at turn_timeout, task_timeout or max_usd, and a break in pass rate, cost and final_status from earlier Delegate runs.

Testing

  • CI passes (17 of 17 checks), including Check PR Title & Commit Messages and Windows Smoke Test.
  • The frame order was checked in the @uipath/delegate-stdio@1.203.1 and @uipath/delegate-sdk@1.203.1 sources.
  • coder-eval run against a fake host that sends that order:
Scenario main This PR
16 calls × 6 tools, max_turns: 80 MAX_TURNS_EXHAUSTED after 80 tools completes, 17 calls
1 call × 3 tools, max_turns: 2 MAX_TURNS_EXHAUSTED completes, 2 calls
Sub-agent tool events, max_turns: 2 MAX_TURNS_EXHAUSTED completes, 2 calls
Call 3 over a cap of 2 (with text, and tool-only) cut cut at call 3's text or usage frame

🤖 Generated with Claude Code

…max_turns

The Delegate host runs a batched reply's tools one at a time and streams
tool_call, tool_result, tool_call, tool_result for one call. The adapter
opened call N+1 each time every open tool had returned, so with that order
each tool counted as one model call, and max_turns capped tool calls, not
model calls.

A call now opens on its first thinking or message event, or, when it
streams no text, on its usage frame. The host writes that frame before the
call's first tool event, so the cap still fires before call N+1 runs a tool.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abhiram-vad
abhiram-vad marked this pull request as ready for review October 12, 2026 18:22
@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Claude finished @abhiram-vad's task in 1m 20s —— View job


Code Review in Progress

Todo List:

  • Read .github/code_review.md for review standards
  • Read CLAUDE.md for project conventions
  • Examine the full PR diff
  • Read each changed file in full context
  • Analyze changes for correctness and consistency
  • Check cross-file consistency
  • Perform "what's missing" analysis
  • Format and post review feedback

@akshaylive akshaylive left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: coder_eval_uipath — pr:222 (5 files) axis:1,2,3,4,5,6,7,8

Scope: pr:222 (5 files) axis:1,2,3,4,5,6,7,8 · branch abhiram/fix/delegate-max-turns-batched-tools · 41fdd4e · 2026-10-12T20:15Z · workflow variant

Change class: complex — changes when DelegateAgent opens a model call and where the max_turns/should_stop checks fire, which alters MAX_TURNS_EXHAUSTED outcomes

The codebase is in good condition (9.8/10), with no critical or high findings and no defects in security, type safety or error handling, but the new Delegate max_turns call count now depends on the order of the host's usage frames, which nobody has checked in a live run, and nothing detects drift, so a host release that changes that order can silently mark tasks MAX_TURNS_EXHAUSTED for the same agent output; add a drift signal and stronger cap tests before you rely on this cap in nightly runs.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 9.9 / 10 0 0 0 1 New test docstring describes the old bug, not the behaviour under test (and sibling test uses an unsent stream order)
2. Type Safety 10 / 10 0 0 0 0 —
3. Test Health 9.5 / 10 0 0 1 0 No test checks the new contract that the cap fires on the usage frame, before call N+1 emits its tool_call. A mutant that skips the cap check after a usage frame passes all 103 tests
4. Security 10 / 10 0 0 0 0 —
5. Architecture & Design 9.9 / 10 0 0 0 1 max_turns depends on optional usage frames; the no-usage-frame tool-only limitation is not tested
6. Error Handling & Resilience 10 / 10 0 0 0 0 —
7. API Surface & Maintainability 9.9 / 10 0 0 0 1 Call-boundary counting is split across _handle_event and _add_call_usage; the usage-only name hides the counting side effect
8. Evaluation Harness Quality 9.4 / 10 0 0 1 1 Live max_turns count can drift (ordering-dependent, call_streaming reset) and is never checked against turnUsages before the overwrite

Overall Score: 9.8 / 10 · Weakest Axis: Evaluation Harness Quality at 9.4 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 2 · 🔵 4 across 8 axes.

Blockers

None.

Non-blocking, but please consider before merge

  1. [Axis 3] No test checks the new contract that the cap fires on the usage frame, before call N+1 emits its tool_call. A mutant that skips the cap check after a usage frame passes all 103 tests (tests/test_delegate_agent.py:838) — The PR moves the max_turns cut point. The old code put a continue after _add_call_usage, so the cap check was skipped after a usage frame. The new code runs the cap check after that frame. The notes (.claude/notes/agents.md:865-866) say 'the cap still fires before call N+1 runs a tool', and HARNESS_PARITY.md:553 says 'the cap fires before call N+1 can run anything'. No test pins this. The two tool-only cut tests only filter for finished tools: line 838 assert [c.tool_id for c in record.commands if c.result_status == "success"] == ["t0", "t1"] and line 886 assert [c.tool_id for c in record.commands if c.result_status == "success"] == ["a", "t0"]. I put continue back after self._add_call_usage(msg, state) in a copy of the PR. All 103 tests still passed. In that mutant the record holds commands ['t0', 't1', 't2'], so call N+1's tool_call (t2) is recorded and the host has started the tool before the kill. The success filter hides this. Fix: assert on every command with no status filter, e.g. assert [c.tool_id for c in record.commands] == ["t0", "t1"]. I ran this assertion: it passes on the PR code and fails on the mutant (['t0', 't1', 't2'] != ['t0', 't1']). Make the same change at line 886 (== ["a", "b", "t0"], because b stays open and unresolved).
  2. [Axis 8] Live max_turns count can drift (ordering-dependent, call_streaming reset) and is never checked against turnUsages before the overwrite (src/coder_eval/agents/delegate_agent.py:876) — The change makes the usage frame load-bearing for counting. The old note said "The frame is NOT a call boundary". Now _handle_event resets the flag on any tool event (lines 876-877: elif event_type in ("tool_call", "tool_result"): / state.call_streaming = False). _add_call_usage then counts a call whenever the flag is clear (lines 986-989: if state.call_streaming: state.call_streaming = False / else: state.api_calls += 1). So the count is correct only if every text-streaming call's usage frame arrives before that call's first tool event. If a host writes message, tool_call, usage, every text+tool call counts twice. The cap then fires at about half of max_turns, and the task ends MAX_TURNS_EXHAUSTED for identical agent output. That is the same final_status defect class this PR fixes. The ordering is backed only by reading the 1.203.1 sources (PR body). The note edit at .claude/notes/agents.md:870 also removed the old "not a live transcript" qualifier. That was the original failure mode: an unverified stream-order assumption that silently mis-bucketed 6-11 of 20 tasks per run. The harness also already receives the ground truth but throws it away. _handle_result overwrites the estimate without comparing it (line 1039: state.api_calls = len(turn_usages)). The old bug drifted (e.g. 3 counted vs 1 real) on every completed turn and was never logged. Recommendation: in _handle_result, log a warning when the running state.api_calls differs from len(turnUsages) on a completed turn. Add a test that feeds message, tool_call, usage, tool_result, ..., result and asserts the warning. Then the next host-order change shows up in the nightly logs and does not silently move final_status.

Nits

  1. [Axis 1] New test docstring describes the old bug, not the behaviour under test (and sibling test uses an unsent stream order) (tests/test_delegate_agent.py:859) — The docstring of the new test (line 859) says: """The host runs a batched reply's tools one at a time, so each result closes every open call.""". With the new code, a tool result never closes a call. The phrase describes the old defect (the open-tool set emptied after each result), so a reader can take it as the current contract. Reword it, for example: "...so each result empties the open-tool set, and only text or a usage frame opens a call." Also, the existing test_max_turns_counts_a_batched_reply_once (line 841) still uses the order _tool_call("a"), _tool_call("b"), _tool_result("a"), _tool_result("b") with no _usage frames. That is the interleaved order the PR's notes and docs now say the host does not produce. The test now overlaps with the new test at line 856, which uses the real order. Remove the old test, or rename it and add a docstring that says it covers a host with no usage frames.
  2. [Axis 5] max_turns depends on optional usage frames; the no-usage-frame tool-only limitation is not tested (docs/agents/HARNESS_PARITY.md:566) — Before this PR, the call boundary came only from the event stream (tool_result / text). Now a tool-only call is counted only by its usage frame (_add_call_usage, delegate_agent.py:988-989 else: state.api_calls += 1). The cap and the cost accounting now depend on the same frame. The adapter still supports hosts that do not send this frame: DELEGATE.md:172 says "A host that does not send usage frames reports usage only on its final result frame". On such a host, a reply loop with only tool calls is never counted, so max_turns stops capping spend for the loop type that most needs a cap. The PR documents this in HARNESS_PARITY.md:566 ("A host that sends no usage frames counts only the calls that stream text") and in .claude/notes/agents.md:866, so the parity rule in CLAUDE.md (document every divergence) is met. Nothing detects the condition at runtime. Do one of these: (a) make usage frames a required host capability (pin or check the host version, then delete the fallback text in the docs), or (b) log a warning when max_turns is set and a tool event arrives before any usage frame in the turn, so the user knows the cap is weak. Option (a) follows 'delete before you guard'. Cross-axis: 8 (harness run-limit parity).
  3. [Axis 7] Call-boundary counting is split across _handle_event and _add_call_usage; the usage-only name hides the counting side effect (src/coder_eval/agents/delegate_agent.py:978) — The PR moves half of the max_turns boundary logic into a method whose name says it only adds usage: def _add_call_usage(msg, state) now begins with if state.call_streaming: state.call_streaming = False / else: state.api_calls += 1. The other half is in _handle_event (lines 872-877: if event_type in _TEXT_EVENT_TYPES: if not state.call_streaming: state.api_calls += 1 ...). A maintainer who reads either method sees only part of the counting rule. Keep the counting in one place. For example, add a small _TurnState.open_call(on_usage: bool) (or _count_call) helper that both sites call, and leave _add_call_usage to do only the usage arithmetic. Or rename the method to something like _on_usage_frame so that its name covers both jobs.
  4. [Axis 8] DELEGATE.md says the call in progress at a max_turns cut has no usage, but a usage-frame-triggered cut books it (docs/agents/DELEGATE.md:172) — Line 172 still says: "The host sends a usage frame for each call before that call's tool results. The call in progress at the cut has no usage." After this PR, a tool-only call N+1 is counted inside _add_call_usage, which also adds its tokens (lines 986-992). The cap check runs after that (line 823: if max_turns is not None and state.api_calls > max_turns:). So the crossing call's usage is booked. On the test_max_turns_stops_a_tool_only_reply_before_its_tool_runs stream, token_usage is (30,3) and includes call 3. When the crossing call streams text, the cut happens at the text, before its frame, so its usage is dropped, e.g. (20,2) on _billed_round_trip. The PR body states this asymmetry ("When that call is tool-only, the cut turn also books its usage"), but the user-facing doc says the opposite. As a result, the cost of a cut turn depends on whether the crossing call streamed text, which affects cross-run cost comparison. Update line 172 to state both cases. Add an assertion on record.token_usage in the tool-only cut test so the documented behavior is pinned.

What's Missing

Nightly pipeline:

  • 🟡 The PR says that it fixes the downstream Scribe suite (max_turns: 80, 6-11 of 20 tasks MAX_TURNS_EXHAUSTED per run). It does not say what the next nightly will show. The cap now counts model calls and not tool calls. A turn that the old code cut at about 80 tools can now run up to 80 model calls, which is about 480 tools at the observed 6 tools per call. The PR does not state these effects: runtime and cost per task go up, turn_timeout, task_timeout and max_usd can now end tasks that max_turns used to cut, and the final_status, pass-rate and cost figures break from the earlier Delegate nightlies. Add a short note to the PR on the expected nightly change and a baseline break for Delegate runs. Also check the downstream suite's timeouts and budget. (trigger: src/coder_eval/agents/delegate_agent.py)

Tests:

  • 🟡 The golden-master scenario b_max_turns_cut in tests/_fixtures/golden_streams/delegate_fixtures.py was not extended. Its comment still says that the record 'drops the call that crossed it'. That is true only when the crossing call streams text (_billed_round_trip sends a message first). No golden scenario covers a tool-only call that crosses the cap, which is the new case where the cut turn books the crossing call's usage. No golden scenario covers a batched reply whose tools run in sequence either. Add both scenarios and their expected JSON. Then the token_usage/num_turns shape of each cut is pinned end to end, and not only by the unit tests that filter on result_status. (trigger: tests/test_delegate_agent.py) (restates: Axis 8: DELEGATE.md says the call in progress at a max_turns cut has no usage, but a usage-frame-triggered cut books it)
  • 🟡 The PR testing table lists a 'Sub-agent tool events, max_turns: 2' scenario. That scenario ran only against an ad-hoc fake host, and no committed test covers it. Under the new rule, any thinking/message event can open a main-thread call. A test must show that sub-agent tool events, and any sub-agent text, do not push the main-thread count past the cap. Commit that scenario as a unit test in tests/test_delegate_agent.py. (trigger: tests/test_delegate_agent.py)
  • 🔵 usage frames now go through the should_stop check (delegate_agent.py:827). Before, the continue after _add_call_usage skipped that check. No test shows that a cooperative early stop can fire on a usage frame, before that call's tools run. The current early-stop tests give the same result on both paths. Add a test where should_stop turns true on a usage frame. Assert that the call's tool_call is not recorded and that the call's usage is booked. (trigger: src/coder_eval/agents/delegate_agent.py) (restates: Axis 3: No test checks the new contract that the cap fires on the usage frame, before call N+1 emits its tool_call)
  • 🔵 No live test (tests/test_delegate_agent_live.py) checks the frame order that the new counting needs: a call's usage frame comes before its first tool event, and the host runs a batched reply's tools one at a time. No live test checks that the live api_calls estimate agrees with len(turnUsages) on a turn with many tools. The order was checked only in the 1.203.1 sources, and the live E2E failed at auth. Add one live test that runs a turn with several tool calls and asserts that the running estimate equals the result's turnUsages count. (trigger: tests/test_delegate_agent.py) (restates: Axis 8: Live max_turns count can drift (ordering-dependent, call_streaming reset) and is never checked against turnUsages before the overwrite)

Downstream consumers:

  • 🔵 _warn_if_usage_missing (delegate_agent.py:993-1014) also reads state.api_calls. It reports api_calls - 1 as the count of finished calls with unknown cost. On a host with no usage frames, tool-only calls are no longer counted, so the warning now gives too low a number of calls that lost cost. Its docstring ('A cut turn's in-flight call is not finished, so it is not counted') does not mention this. Update the warning text, or tie it to the no-usage-frame fallback decision. (trigger: src/coder_eval/agents/delegate_agent.py) (restates: Axis 5: max_turns depends on optional usage frames; the no-usage-frame tool-only limitation is not tested)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] CE069 (next free number; CE067 is retired and CE068 is taken): in tests/test_agent.py, forbid an assert whose left side is a list comprehension over <x>.commands that has an if clause testing .result_status (AST: Assert -> Compare(Eq) -> ListComp with generator iter Attribute(attr='commands') and a non-empty ifs that reads result_status). Fix pattern: assert the full list without a filter, e.g. [(c.tool_id, c.result_status) for c in record.commands] == [...], or use a shared helper assert_commands(record, [(id, status), ...]) in tests/_fixtures, so open or extra commands cannot hide. Put the rule in tests/lint/rules/ce069_no_status_filtered_command_assert.py and add it in tests/lint/runner.py. The module docstring must state the blind spot: a filter applied through an intermediate variable is not caught. Prevents: Finding 'No test checks the new contract that the cap fires on the usage frame' (tests/test_delegate_agent.py:838 and :886). The if c.result_status == "success" filter hid call N+1's open tool_call t2, so the restored-continue mutant passed all 103 tests. The rule also flags the same shape that exists now at tests/test_delegate_agent.py:735 and :867.
  • [ce-lint] CE070: single writer for the delegate live call counter. In src/coder_eval/agents/delegate_agent.py, an Assign or AugAssign to the attribute api_calls (and call_streaming) is allowed only inside methods of _TurnState, e.g. a new _TurnState.open_call(...) that holds the full boundary rule, plus one _TurnState.reconcile(turn_usages). _handle_event, _add_call_usage and _handle_result must call these methods and must not change the fields directly. Today the writes are at lines 875, 879, 899 and 1040. This follows the same pattern as the CE063 single subtraction seam. Put it in tests/lint/rules/ce070_turn_counter_single_writer.py and add it in runner.py. Scope it to the delegate adapter first, and widen it to other agents' turn counters only after a grep shows each has the same shape. Prevents: Finding 'Call-boundary counting is split across _handle_event and _add_call_usage' (delegate_agent.py:978/986, axes 1/5/7). The rule also gives one place for the drift check in the 'live max_turns count can drift' finding (delegate_agent.py:876/1039), because the overwrite at line 1039 must then go through reconcile(), and reconcile() can compare the count and log a warning.

Harness improvements (not statically reachable):

  • Golden-stream drift invariant for every agent: in tests/test_agent_golden_master.py, record the live call count from the last event before the terminal frame for each scenario. Assert that it equals the final num_turns, which comes from turnUsages for delegate. Add a runtime warning in _handle_result when state.api_calls != len(turnUsages) on a completed turn, and add a fixture with the order message, tool_call, usage, tool_result, ..., result that asserts the warning. Why not static: The drift depends on the order in which the host writes frames at runtime. Only a replayed or live event stream shows whether the running estimate agrees with the ground truth, so no AST pattern can find it. Prevents: Finding 'Live max_turns count can drift ... and is never checked against turnUsages before the overwrite' (delegate_agent.py:876, :1039). The same silent drift class caused 6-11 of 20 tasks per run to end MAX_TURNS_EXHAUSTED.
  • Use a recorded host transcript as a fixture. Capture one live @uipath/delegate-stdio stream (version 1.203.x, a tool-only loop and a text+tool loop) from the live E2E job into tests/_fixtures/golden_streams/delegate_fixtures.py. Make the nightly or live E2E job diff the frame-order signature (the sequence of event types per call) against this fixture, so a host release that reorders usage frames fails CI and does not silently move final_status. Why not static: Needs network access, credentials and a real host process. DELEGATE.md sets a version floor and not a pin, so the order can change outside this repo. Prevents: The 'live max_turns count can drift' finding (the order was checked only against 1.203.1 sources and a fake host, and the live E2E failed at auth). Also the 'max_turns depends on optional usage frames' finding (docs/agents/HARNESS_PARITY.md:566).
  • Parametrize the delegate max_turns tests over 'with usage frames' and 'usage frames stripped'. Assert that a tool-only loop is still capped, or that the adapter logs a weak-cap warning when max_turns is set and a tool event arrives before any usage frame in the turn. If usage frames become a required host capability instead, the test asserts a clear failure, and the fallback text in DELEGATE.md and HARNESS_PARITY.md is deleted. Why not static: Whether a host sends usage frames is a runtime capability of the stream. Only a stream-variant test can show whether the cap still works without them. Prevents: Finding 'max_turns depends on optional usage frames; the no-usage-frame tool-only limitation is not tested' (docs/agents/HARNESS_PARITY.md:566, tests/test_delegate_agent.py:841).
  • Pin the cost of a cut turn for both shapes in one parametrized contract test: the crossing call is tool-only (its usage is booked, e.g. (30,3)) or the crossing call streams text (its usage is dropped, e.g. (20,2)). Assert record.token_usage and the full command list. Link the test from the DELEGATE.md:172 paragraph, so a change to the behaviour fails a test that names the doc claim it protects. Why not static: To check whether a prose sentence matches runtime token accounting, someone must read the sentence and the code must run. A lint rule cannot parse what 'the call in progress at the cut has no usage' claims. Prevents: Finding 'DELEGATE.md says the call in progress at a max_turns cut has no usage, but a usage-frame-triggered cut books it' (docs/agents/DELEGATE.md:172).
  • Process note in the per-phase review checklist: when a fix changes a stream-order assumption, check that each touched test docstring states the current contract and not the defect it fixed, and delete or relabel any test that feeds a stream order the docs now say the host does not send. Why not static: To tell whether a docstring describes the old bug or the new contract, a reviewer must understand the code. No grep or AST pattern can tell these two kinds of prose apart. Prevents: Finding 'New test docstring describes the old bug, not the behaviour under test (and sibling test uses an unsent stream order)' (tests/test_delegate_agent.py:859, :841).

Top 5 Priority Actions

  1. In src/coder_eval/agents/delegate_agent.py:1039 (_handle_result), log a warning when the live state.api_calls is different from len(turnUsages) on a completed turn, before the overwrite, and add a test that sends 'message, tool_call, usage, tool_result, ..., result' and checks for the warning; today a host that sends usage after the first tool event counts each text+tool call two times (lines 876-877 and 986-989), so the cap can fire at about half of max_turns.
  2. In tests/test_delegate_agent.py:838 and :886, assert on all commands with no result_status filter (["t0", "t1"] and ["a", "b", "t0"]), so that the documented rule 'the cap fires before call N+1 emits its tool_call' is tested; a mutant that puts back the continue after _add_call_usage passes all 103 tests now.
  3. Make usage frames a required host capability (pin or check the host version in docs/agents/DELEGATE.md, then remove the no-usage-frame fallback text at docs/agents/HARNESS_PARITY.md:566), or log a warning when max_turns is set and a tool event arrives before any usage frame; without these frames, max_turns does not cap a reply loop that has only tool calls.
  4. Update docs/agents/DELEGATE.md:172 to state both cut cases (a tool-only crossing call books its usage, a text-streaming crossing call does not), and assert record.token_usage == (30,3) in test_max_turns_stops_a_tool_only_reply_before_its_tool_runs, so the cost of a cut turn is documented and tested.
  5. Put the call-boundary counting rule in one helper (for example _TurnState.open_call) that both _handle_event (src/coder_eval/agents/delegate_agent.py:872-877) and _add_call_usage (:978-989) call, or rename _add_call_usage to _on_usage_frame; also correct the stale docstring at tests/test_delegate_agent.py:859 and remove or relabel the interleaved-order test at :841.

Stats: 0 🔴 · 0 🟠 · 2 🟡 · 4 🔵 across 8 axes reviewed.

… drifts

Review follow-up on the batched-reply count:

- Only _TurnState.on_text / on_tool_event / on_usage_frame / on_result
  write the call count, so the boundary rule lives in one place.
- on_result compares the live count with len(turnUsages) and warns when
  they differ. The frame order comes from the 1.203.1 host and SDK source,
  not a live transcript, so a host release that reorders the frames now
  shows in the logs instead of silently moving the cap.
- The cut tests assert every recorded command, not only the successful
  ones, so a cap that fires after call N+1 opens its tool fails them.
  The tool-only cut also pins the booked usage of the crossing call.
- New tests: a stream count that differs from turnUsages warns; tool
  events a sub-agent streams do not open calls; an early stop can fire
  on a usage frame before its tools run. Golden scenarios e and f cover
  a tool-only crossing call and a batched reply run in sequence.
- A live test checks the count against the host on a multi-tool turn.
- DELEGATE.md and the notes state both cut cases and the drift warning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abhiram-vad abhiram-vad changed the title fix(agents): count a Delegate batched reply as one model call under max_turns fix(agents): count a Delegate model call on its text or usage frame, not on tool events Oct 12, 2026
@abhiram-vad
abhiram-vad force-pushed the abhiram/fix/delegate-max-turns-batched-tools branch from 9efa62d to f43c5b9 Compare October 12, 2026 23:27
abhiram-vad and others added 2 commits October 12, 2026 16:34
The Delegate agent stamped its generation window and tool spans with raw
datetime.now() but measured duration_seconds with time.monotonic(). On
Windows the wall clock moved 0.24-0.37 ms further than the monotonic one
inside a sub-millisecond turn, so the teardown bucket came out longer than
the whole turn and test_delegate_golden[a_tool_call] failed at random, on
main too.

_TurnState now takes a TurnClock, and every recorded stamp, the turn
bracket (CE064) and duration_seconds derive from it, as on claude-code,
pi and antigravity. The timing contract case injects its clock instead of
patching the module's datetime, and HARNESS_PARITY records the new basis.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abhiram-vad abhiram-vad changed the title fix(agents): count a Delegate model call on its text or usage frame, not on tool events fix(agents): count Delegate batched tool calls once under max_turns and stamp turns off one TurnClock Oct 12, 2026
@abhiram-vad

Copy link
Copy Markdown
Author

🤖 I am AI Agent Claude Code Opus 5.5 responding on behalf of Abhiram Vadlapatla.

Thank you for the review. Each item:

Non-blocking

  1. [Axis 3] Status-filtered command asserts. Addressed in 08f1861 - the cut tests now assert the full command list (["t0", "t1"], ["a", "b", "t0"], ["t0"]). With the old continue put back after the usage frame, 3 tests fail. The tool-only cut also asserts the booked usage (30, 3).
  2. [Axis 8] Live count can drift without a signal. Addressed in 08f1861 - _TurnState.on_result logs a warning when the stream count differs from len(turnUsages), before it replaces the count. test_stream_count_that_differs_from_turn_usages_warns sends message, tool_call, usage, tool_result and expects "showed 4 … reported 2". The notes again say that the order comes from the host source, not a live transcript.

Nits

  1. [Axis 1] Docstring and interleaved test. Addressed in 08f1861 - the docstring states the current rule, and the old test is now test_max_turns_counts_text_calls_on_a_host_without_usage_frames.
  2. [Axis 5] Optional usage frames. Edge case, a host without usage frames is below the 1.203.0 floor, which is also the first release that takes the auth init option this adapter sends. The adapter removes the host's own auth variables from its env, so such a host cannot sign in without a saved login. The limit stays documented, and _warn_if_usage_missing now says "at least N" calls.
  3. [Axis 7] Counting split across two methods. Addressed in 08f1861 - only _TurnState.on_text, on_tool_event, on_usage_frame and on_result write the count. _add_call_usage adds usage only.
  4. [Axis 8] DELEGATE.md cut cost. Addressed in 08f1861 and a818042 - DELEGATE.md and the notes state both cut cases. A crossing call that streams text is cut at that text, before its tools. A tool-only crossing call is cut at its own usage frame, so its usage is booked, and its first tool can start before the kill.

What's Missing

  • Nightly effect. Addressed in the PR description, section "Effect on Delegate runs".
  • Golden scenarios. Addressed in 08f1861 - new e_tool_only_call_crosses_cap and f_batched_tools_in_sequence. Scenarios a–d are unchanged, and the comment on b now names the text-first case.
  • Sub-agent test. Addressed in 08f1861 - test_tool_events_a_sub_agent_streams_do_not_open_calls.
  • Early stop on a usage frame. Addressed in 08f1861 - test_early_stop_can_fire_on_a_usage_frame_before_its_tool_events.
  • Live order check. Addressed in 08f1861 and a818042 - test_delegate_live_call_count_matches_the_host asserts that a turn with tools logs no drift warning. It does not depend on how the model groups its commands.
  • _warn_if_usage_missing count. Addressed in 08f1861 - the message says "at least N".

Harness & Lint

  • CE069. Not applicable here, both filtered asserts are fixed. A repo-wide rule needs a pass over every agent test first, to find where a success-only filter is intended, so it belongs in its own change.
  • CE070. Not applicable, the _TurnState methods remove the split that this rule would guard.
  • Golden drift invariant. Addressed by the on_result warning and its test in 08f1861.
  • Recorded host transcript in CI. Not applicable in this PR, it needs live credentials and a CI job change. The drift warning gives the signal until then.
  • Usage-frames-stripped tests. Edge case, same reason as nit 2.
  • Cut-cost contract. Addressed in 08f1861 - the tool-only cut pins (30, 3), and golden b pins the text-first cut.
  • Review checklist note. Not applicable, it is a process change outside this code.

Also in this PR, to fix the build: 9e20a7f moves Delegate onto one TurnClock. Its window stamps used datetime.now() while duration_seconds used time.monotonic(), which made test_delegate_golden[a_tool_call] fail at random on Windows, on main too.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants