fix(sessions): close out unanswered tool calls before the error reply (tool-batch wedge) - #1796
Merged
Aaronontheweb merged 2 commits intoAug 7, 2026
Conversation
…y wedge Reproduces the bug behind the self-hosted/DeepSeek session wedge. When a parallel tool batch partially fails, LlmSessionActor.FailCurrentTurn appends the "I encountered an error executing a tool" assistant reply without first writing a tool-result for the unanswered call. History becomes [assistant tool_calls(A,B), tool A, assistant error, tool B] — the error reply is wedged between the tool_calls message and the rest of its results. That breaks the contiguity strict OpenAI-compatible providers (DeepSeek, Qwen, vLLM) require, so every later turn fails with HTTP 400 "insufficient tool messages following tool_calls" and the session stays stuck. This is the failing (red) half of a red-green change. It drives a two-call parallel batch where one call throws in InterpretToolCall — the only pre-try seam that reaches ToolExecutionFailed, so the healthy call is recorded first — and asserts, on the history assembled for the next request, that the assistant tool_calls message is immediately followed by a contiguous run of tool-result messages answering every call id. The fix follows in a separate commit.
Fixes the tool-batch history wedge. When a parallel tool batch partially fails, the ToolExecutionFailed handler now closes out the still-unanswered call(s) with synthetic tool-results — reusing the existing ParkedToolBatchHistory / ToolBatchAbandoned machinery — BEFORE FailCurrentTurn appends the "I encountered an error executing a tool" assistant reply. History stays [assistant tool_calls(A,B), tool A, tool B, assistant error] — a contiguous tool-result run — instead of wedging the error reply between the results. Strict OpenAI-compatible providers (DeepSeek, Qwen, vLLM) reject the wedged shape with HTTP 400 "insufficient tool messages following tool_calls" on every later turn, which stuck the session. Flips ToolBatchHistoryWedgeTests from red to green; the full Sessions suite stays green.
Aaronontheweb
marked this pull request as ready for review
August 7, 2026 17:33
This was referenced Aug 7, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
This is the root cause of the self-hosted/DeepSeek session wedges from the
tool-call investigation. When a parallel tool batch partially fails,
LlmSessionActor.FailCurrentTurnappends the "I encountered an error executinga tool" assistant reply (via
SessionState.AddErrorReply) without firstwriting a tool-result for the unanswered call(s). History becomes:
Strict OpenAI-compatible providers (DeepSeek, Qwen, vLLM) require an assistant
tool_callsmessage to be immediately followed by a contiguous run oftool-result messages answering every
tool_call_id. The wedged error replybreaks that run, so every subsequent request returns HTTP 400
insufficient tool messages following tool_callsand the session is stuck untila new user turn rebuilds history.
The failing test
ToolBatchHistoryWedgeTestsdrives a two-call parallel batch where one callthrows in
InterpretToolCall— the only pre-try seam inSessionToolExecutionPipelinethat escapes toToolExecutionFailed, so thehealthy call is recorded first (a
Task.WhenAllinvariant). It then asserts, onthe history assembled for the next provider request, that the assistant
tool_callsmessage is immediately followed by tool-results for every callid.
Today it fails with:
— the
assistant(tool_calls) -> tool(A) -> assistant(error) -> tool(B)wedge,exactly as the provider would see it.
The fix (follow-up commit)
Make
FailCurrentTurnclose out the unanswered tail tool batch (synthesizetool-results for the still-open call ids, reusing the existing
ParkedToolBatchHistory/ToolBatchAbandonedmachinery) before appendingthe error reply — so history is well-formed at rest for any provider, and no
mid-batch tool failure can wedge a session.