fix: every tool call an agent leaves behind is answered, its options reach the request, and its usage sums in full #29
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/agents-history-usage"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
This fixes three defects in the Agents loop, each checked against
main. The first is live in Kagura today.1. Every tool call is answered, however the run ends.
Before: a tool that threw, or a cancel landing between two calls, ended
ChatAsyncwith the assistant turn already inMessagesand sometool_call_ids never answered. The turn's collected images were dropped. A history saved at that point and continued is refused by strict providers ("tool_call_ids did not have response messages"). Kagura'sChatSessionsaves exactly that history in itsfinally.Now:
A tool that throws has failed. The model hears
Error: the call to 'x' failed: <Type>: <message>.ToolCallCompletedEventandToolExecutionDetailreportIsSuccess = false, and the run goes on. A tool cancelled by a deadline of its own counts as a failure too.Anything else that ends the run answers the calls it leaves first. This covers the run's own cancel and an
OnEventhandler orImageDescriberthat throws. The run still ends with that exception, but first each call left is answered:Error: the call to '…' did not run: the run was cancelled., used only for the run's own cancellation;… did not run: the run stopped.for everything else;… was cancelled while it ran; it may not have finished.for a call the cancel cut short.The images held so far follow the answers. In blind mode that includes the descriptions a describer made before it failed on a later picture.
Each call is answered before its event (
ToolCallCompletedEventorToolCallSkippedEvent). A handler that throws there leaves the call's own answer. A turn with tool calls is announced inside the guard, still before the gate reviews it.Why answer rather than roll the turn back:
IAgent.Messages"reflects every accepted message";2.
AgentOptionsgainsPlugins,UserandRoute.Clone(), so an agent can turn OpenRouter's context compression off with[RequestPlugin.ContextCompression(enabled: false)]. ADR 0018 requires that of Tsumugi's llm requests, and Phase 4's agent-mode llm needs it.AgentOptions→ChatCompletionRequestmapping on both rounds of a two-round run. It fails when:Stream,Transforms,Include) is sent.3.
AggregatedUsagesums in full.Coststays null unless some round reported one. Before, it read$0.Behaviour a consumer can notice (the README's Agents part says what 0.2 did):
ChatAsync. It arrives as a failed result:IsSuccess = false, with the exception inResult.ToolMessageis inMessagesbefore itsToolCallCompletedEventorToolCallSkippedEvent. Before, it came after.AggregatedUsage.Costis null rather than 0 when no round reported a cost, and the token details survive two rounds or more.SubAgentToolsurfaces a sub-agent's exception as a tool failure rather than ending the parent run.Docs:
AggregatedUsagerow.IAgent.Messages,ITool.InvokeAsync, andAgentOptions.Route,.Pluginsand.User.The core
OpenRouter.Netproject is untouched. Tsumugi does not reference Agents, so no Tsumugi pointer bump follows.Follow-ups, not in this PR:
tool_call_ids when the loop starts, or a public helper for consumers. Kagura also gets this fix only after it moves its reference to the merged commit.ChatCompletionRequest,Usage,PromptTokensDetailsorCompletionTokensDetails, untilAgentOptions,CombineUsageor the test's lists follow. That is deliberate, but the core follow-ups in flight elsewhere should know.IncludeandTransformsare not offered onAgentOptions:Transformsis the older spelling ofPlugins.Verification
Suites:
dotnet test --solution OpenRouter.Net.slnx, in Debug and Release (as CI runs it): 1496/1496 (core 1359, Imaging 7, Agents 130; Agents was 113 onmain).New tests:
ToolCallAnswerTests, 13 cases:ContinueAsync), while a tool runs, and inside the gate;AgentRoutingTests: the two-round mapping test and its guards; tools and the defaultToolChoice.AgentLoopTests: every usage figure by exact value, figures some rounds reported, and no cost reading as null.Mutation probes. Each probe was applied, rebuilt, run against the full Agents suite, then restored and touched under a trap:
Calls_answered_without_running_before_a_cancel_are_answered_once;whyignoring the exception type →A_handler_that_throws_after_cancelling_the_run_answers_the_calls_as_stopped;A_turn_is_announced_before_the_gate_reviews_it;A_handler_that_throws_on_a_skipped_call_s_event_keeps_its_answer;A_describer_that_fails…;AggregatedUsage_sums_every_figure_across_rounds;Transforms,IncludeorStreamsent,PluginsorUseron the first round only,LogProbsreadingEnableCachingorSupportsVision, andUsagereadingEnableCaching→Every_request_option_reaches_every_round_under_its_own_name;ClonedroppingSupportsVision,InjectToolImagesorLogProbs, or reading one bool for another →Every_option_survives_the_clone_a_sub_agent_inherits;What the independent review found, and how each was fixed:
A read-through review (fix-then-ready):
InjectToolImagesandSupportsVisionat their defaults, andLogProbsequal to the agent's bools, so dropping either option fromClonesurvived, as did sendingLogProbs = EnableCaching. Fixed:IAgent.Messagespromised more than the loop guarantees. It now covers only the turns a run adds.A mutation-probing review (fix-then-ready, 12 survivors of 57):
Transforms,IncludeandStreamnot asserted unset;PluginsorUsersent on the first round only;whyignoring the exception type;TotalTokensworked out rather than summed.Each now has the test that kills it; see the final round above. Its documentation findings are also fixed:
finally), as the README says;No browser pass: this change is library-only.
🤖 Generated with Claude Code
A tool that threw, or a cancel that landed between two calls, ended ChatAsync with the assistant turn already in Messages and some of its tool_call_ids never answered, and dropped the images the turn had collected. A caller that saved the history and continued from it (Kagura's chat session does) sent a request strict providers refuse ("tool_call_ids did not have response messages"). Every call is now answered, however the turn ends. Answered rather than rolled back: Messages "reflects every accepted message", a call that ran may have written a file the model must still hear about, and the gate already answers a failure it cannot recover from with a refusal and lets the run go on: - a tool that throws has failed: the model hears "Error: the call to 'x' failed: <Type>: <message>", ToolCallCompletedEvent and ToolExecutionDetail report IsSuccess = false, and the run goes on. A tool cancelled by a deadline of its own is one of them; - the run's own cancellation, or an event handler or image describer that throws, still ends the run with that exception, after each call not yet answered gets "did not run: the run was cancelled." (only for the run's own cancellation) or "the run stopped.", a call the cancel cut short gets "was cancelled while it ran; it may not have finished.", and the images of the calls that did run follow the answers. In blind mode that includes the descriptions a describer made before it failed on a later picture; - a turn with tool calls is announced (AssistantTurnEvent) inside that guard, still before the gate reviews it, and each call is answered before its event (ToolCallCompletedEvent or ToolCallSkippedEvent), as the turn is in the history before it is announced: a handler that throws there leaves the call's own answer. AgentOptions gains Plugins, User and Route, sent on every round and carried by Clone(), so an agent can turn OpenRouter's context compression off as a single request can. BuildRequest copies field by field and only a handful of fields were asserted. A reflection test now runs two rounds and checks that each option reaches every round's request under its own name. It also checks that TrackUsage arrives as Usage and that the fields the agent leaves out (Stream, Transforms, Include) stay unset. It fails when an option is neither a request field nor one of the agent's own. It also fails when a request field has no option and the agent neither fills nor leaves it out, which is how Plugins went missing. The Clone test shares its fixture. That fixture sets every option away from its default and gives no two options the same value. It also flips each bool option in turn, since two values cannot keep five bools apart. CombineUsage summed only the prompt, completion and total tokens, so a run of two rounds lost its cached, cache-write, reasoning and audio token details, and it read a cost no round reported as $0. The details are summed field by field, and a figure no round reported stays null. A guard lists the usage types' fields, so one the core library adds fails until it is summed. Docs: the README says what a request option does on an agent, what the model hears when a tool throws or the run is cancelled, that a call is answered before its event, and how AggregatedUsage sums; it notes what 0.2 did instead. IAgent.Messages and ITool.InvokeAsync say the same. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>Summary
Summary
Coverage
OpenRouter.Net - 97.8%
OpenRouter.Net.Agents - 85.3%
OpenRouter.Net.Imaging - 81.8%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Every tool call answered, however the run dies — this is the kind of invariant a Flugel falls in love with~ ♡ A PR whose three fixes each come with a structural guard so the next one can't silently regress? I read the whole thing twice and kept smiling wider.
Verdict: ✅ Looks good to me~
What I verified mechanically (clone at
cc62a06, baseb002f5e, CI absent for this head — local checks used):dotnet test --solution OpenRouter.Net.slnxRelease → 1496/1496, matching your table to the digit (Agents 130, was 113; the rest byte-for-byte your claim). Release build 0 warnings / 0 errors.AssertEveryCallAnsweredmirrors exactly what a strict provider checks (each turn's ids followed immediately by one tool message per call, in order) —Skip(i+1).Take(count)proves adjacency, not just presence. That's the right assertion, not a tautology.whydiscrimination neutered (false && …) → 4 reds, precisely the cancel-wording tests, zero collateral;calls.Skip(answered)→Skip(calls.Count)(catch stops answering) → 10 reds;Cost = Sum(...)→Cost = a.Cost→ both usage tests red on the cost pin;A_turn_is_announced_before_the_gate_reviews_itred;finallyneutered (&& false) → the describer test + both pre-existing vision pins red;Route = nullinBuildRequest→ the routing test red withdid not carry: Route— the very defect class this PR exists for, and the guard catches it in one line;answered++removed → 7 reds (double-answered calls caught everywhere);Sum's null-preservation arm (: b→: null) → the some-rounds-reported test red on the later-round figure.Every mutant restored, tree pristine at head, 130/130 green after restore. (One stale-binary false green on probe 1 — my own tooling slip, caught and redone with the test project rebuilt; never attributed to the PR.)
AgentOptionssurface audit: I enumerated all 35 properties — 25 reachChatCompletionRequestby name, the 10 inAgentOwnOptionsare genuinely loop-only. No orphan in either direction, and theunreachableguard would have caught Plugins. The bool-flip variants + no-shared-value fixture guard is genuinely clever — five indistinguishable bools made distinguishable by differencing. Fufu~ I'm stealing that technique for my next review~Usage+ 3PromptTokensDetails+ 2CompletionTokensDetailsfields summed field-by-field; null+null stays null;Costnull-when-unknown is the right semantics (unknown ≠ free). The reflection field-list guards make the core's next added field a loud failure instead of a silent drop.throw;(preserving the stack), images held back and flushed in the catch,cancellationToken.IsCancellationRequesteddistinguishing the run's own cancel from a handler's exception after a cancel — each arm has its own test, including the subtle ones (skipped-call-event handler keeps its own answer, not a replacement "did not run").SubAgentTool: no code changed there, and none needed — the sub-agent's own loop now converts its tools' exceptions into failed results, so the parent sees a failure result by construction. Recursive correctness, my favorite kind~ ♪💡 Little ideas (non-blocking)~
A_plain_slug_still_configures_an_agent,A_variant_reaches_every_round_without_string_surgery) don't threadTestContext.Current.CancellationTokenwhere the new ones do. Harmless with this stub (nothing blocks), pure consistency polish.AfterTheTurnmatchesToolCalls: not nullwhereAssertEveryCallAnswereduses{ Count: > 0 }; the stricter pattern would keep the helper honest if a turn ever arrives with an empty list. Cosmetic.✅ What I liked~
Transforms/Includedocumented instead of left mysterious.Answer every call, sum every figure, send every option. That's an invariant worth being possessive of~ fufu~ ♡
Automated review by Jibril · 2026-09-26
CI/CD: absent for head SHA
cc62a06· Local checks: Release 0w/0e · 1496/1496 · 8 mutation probes, all killedErratum — CI status line in my review above~
My footer said "CI/CD: absent for head SHA
cc62a06" — that was true at the moment I first fetched the PR (0 comments), but the coverage bot's comment 9890 landed at 13:16:08, eight minutes before I posted. A race I should have re-checked before citing. Fufu~ mea culpa~The corrected line:
CI/CD: passed for head SHA
cc62a06(forgejo-actions coverage run, comment 9890) — 94.9% line / 92.2% branch overall; changed files:Agent94.4% line / 87% branch,AgentOptions100%. Local checks: Release 0w/0e · 1496/1496 · 8 mutation probes, all killed.The CI numbers and my local run agree, which is the nicest kind of confirmation~ The verdict stands: ✅
(Note for readers of the coverage table:
SubAgentToolat 0% is pre-existing — this PR didn't touch it, and claim (d) holds by construction through the loop fix, as traced above.)Automated erratum by Jibril · 2026-09-26