| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
The following comment was made by an LLM, it may be inaccurate: Based on my search, I found several related PRs addressing similar issues. Here are the most relevant ones: Directly Related (mentioned in the PR description):
Related to thinking block and reasoning content filtering:
The PR's description already acknowledges the relationship with #14393 and #12131, indicating these are complementary approaches to the same underlying issue. |
Sorry, something went wrong.
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
Sorry, something went wrong.
CI noteThe e2e (windows) failure is unrelated to this change — it's a runner infrastructure issue (ECONNRESET / timeouts). The same failure appears on recently merged PRs (#21827, #21803, #21796) as well. All code-related checks pass:
Happy to rebase or address any feedback if needed! |
Sorry, something went wrong.
|
Rebased on latest dev and updated the PR:
|
Sorry, something went wrong.
The first one shows up when the current model doesn't match the model stored on the message, which can happen pretty easily with plugin-based routing. In that case, toModelMessages drops providerMetadata for every part, including reasoning parts. For Anthropic, that's where the thinking signature lives, so once that metadata is gone the next request is already broken. The fix here is just to keep metadata on reasoning parts even when the model changes. Text and tool parts still keep the existing behavior. I was comfortable narrowing it that way because the metadata is already provider-scoped (anthropic: { signature }). No you can't do this because a lot of models use anthropic messages api, for example minimax, if I started a session w/ minimax and then used anthropic models it would error, if this is due to "plugin based routing" I think ur plugin is bugged or we arent providing correct hooks for u |
Sorry, something went wrong.
|
You're right — I went back and checked my sessions and differentModel was never actually true (same model throughout). This was a code-reading finding I didn't validate against real data. Dropping that commit and keeping just the empty text replacement fix. |
Sorry, something went wrong.
…ock positions normalizeMessages removes empty text parts, which shifts thinking block positions and invalidates signatures. Simple preservation does not work because the AI SDK has a second filter and the API rejects empty text. In assistant messages with signed reasoning, replace empty text with a placeholder instead of removing it. This preserves array positions through all filtering layers.
|
Closing — the same issue was resolved upstream via #21370 (space substitution at message-v2.ts layer). Thanks for the review! |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Issue for this PR
Closes #13286
Related: #18078, #16748, #21370, #16750, #18254
Type of change
What does this PR do?
Extended thinking sessions fail with "thinking blocks cannot be modified" when normalizeMessages removes empty text parts between reasoning blocks — shifting positions and breaking signatures.
While tracing that, I ran into two more constraints that altendky also documented in #16750:
I confirmed the API behavior with direct calls:
So preserving the empty string wasn't enough even in the cases where I stopped normalizeMessages from removing it. What ended up working consistently was replacing those empty interstitial text parts with "...": it keeps the array shape the same, it survives the downstream filter, and the API accepts it. Unsigned empty text still gets removed like before.
I also checked real session data from a broken run and found three truly empty interstitial text parts inside a single assistant message, so this wasn't just a synthetic repro.
Relation to other work
I compared notes with #21370 and #16750 while working through this since they're in the same part of the pipeline. The extra piece I ran into here was the combination of downstream empty-text filtering plus Anthropic's non-empty text requirement, so this PR handles that path too.
How did you verify your code works?
Tests in transform.test.ts, each fails against unpatched code and passes with the fix:
Full suite locally: 1937 pass, 0 fail.
Checklist