fix(core): drop tool parts when a failed turn loses its reasoning block - #40157
Open
asjad3 wants to merge 1 commit into
Open
fix(core): drop tool parts when a failed turn loses its reasoning block#40157asjad3 wants to merge 1 commit into
asjad3 wants to merge 1 commit into
Conversation
Replaying an errored assistant turn could send Anthropic a tool_use with no preceding thinking block, which the API rejects with a 400. reuseProviderMetadata is false once message.error is set, so the reasoning part is emitted without its signature. Reasoning text is empty whenever thinking display is "omitted" -- the default on current Anthropic models -- so the part then carries neither text nor provider state and is filtered out as empty, leaving the turn's tool calls orphaned. Drop the tool parts alongside the reasoning so the replayed turn stays self-consistent. Scoped to same-model replay: a model switch lowers reasoning to plain text on purpose, so its tool calls remain valid and are kept. Fixes anomalyco#38620
Contributor
|
The following comment was made by an LLM, it may be inaccurate: Potential duplicate found: PR #38763 -
Related PR: PR #40148 -
|
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
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.
Issue for this PR
Closes #38620
Type of change
What does this PR do?
Replaying an errored assistant turn can send Anthropic a
tool_usewith no thinking blockin front of it, which the API rejects with a 400.
The chain:
reuseProviderMetadatais false oncemessage.erroris set, so the reasoningpart is built with
providerMetadata: undefined. It then hits the empty-part filter justbelow, which keeps a reasoning part only if it has text or provider state. Reasoning text
is empty whenever thinking display is
"omitted"— the default on current Anthropicmodels, where the signature lives in the metadata rather than the text. So the part has
neither, gets filtered out, and the turn's tool calls are left with nothing in front of
them.
This drops the tool parts too when that happens, so the replayed turn is at least
self-consistent. Both the tool calls and their trailing
Message.toolresults go, sincedropping only the calls would leave a
tool_resultwith no matchingtool_use— the sameerror the other way round.
It only applies to same-model replay. A model switch lowers reasoning to plain text on
purpose, so no reasoning part survives there either, but those tool calls are still fine
and must be kept. Without that guard the model-switch test fails.
Note this is the second of the two options in #38620. #38763 already implements the first
(reuse the metadata on error) and predates this. I started there too, but it needs the
existing OpenAI expectation to change:
itemId/reasoningEncryptedContentpoint atprovider-side state that a failed turn never committed, so replaying them isn't obviously
safe. Anthropic's signature is different — it signs content rather than pointing at server
state — but telling those apart means this layer knowing about specific providers. Both
options are defensible and it's your call; happy to close this if you prefer #38763.
How did you verify your code works?
Two tests in
packages/core/test/session-runner-message.test.ts. The first covers the bugand fails on
dev(the turn replays a lonetool-call). The second covers a failed turnwhose reasoning still has text, so tool calls are kept — that one passes either way and is
there to stop the fix from dropping tool calls on every errored turn. I wrote it after the
first version of this change broke the model-switch test.
From
packages/core:bun test test/session-runner-message.test.ts— 8 passbun test test/session-*.test.ts— 203 pass, same asdevbun typecheck— cleanprettier --check,oxlinton the changed files — clean, warning count unchangedI could not reproduce the actual 400 end to end, since that needs a live Anthropic key and
an interrupt landing mid-step. The evidence here is the invariant at the unit level.
Screenshots / recordings
Not a UI change.
Checklist
AI assistance: written with Claude Code, reviewed by me, checks above run locally.