fix(anthropic): self-heal on missing thinking-signature errors from Bedrock/Vertex - #33719
Conversation
…edrock/Vertex Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
Greptile SummaryThis PR widens the
Confidence Score: 4/5Safe to merge; the change is a one-line detector relaxation gated behind an existing HTTP 400 check, with a bounded single-retry recovery that is a no-op when thinking blocks are absent. The logic change is correct and the trade-off is intentional. The only open question is whether a 400 error body that coincidentally contains both "thinking" and "signature" for an unrelated reason could trigger a spurious strip-and-retry — a real but low-probability scenario whose worst outcome is one extra HTTP call before the original error surfaces. The single-line change in
|
| Filename | Overview |
|---|---|
| litellm/llms/anthropic/common_utils.py | Loosened is_anthropic_invalid_thinking_signature_error to require only "thinking" + "signature" instead of all four keywords; correctly fixes Bedrock/Vertex validation-error formats, with a deliberate trade-off of a slightly wider match. |
| tests/test_litellm/llms/anthropic/test_anthropic_common_utils.py | Added two positive unit tests for Bedrock and Vertex error formats, and three additional negative tests; all are pure mock/unit tests with no real network calls. |
Reviews (1): Last reviewed commit: "fix(anthropic): self-heal on missing thi..." | Re-trigger Greptile
| return False | ||
| lower = error_text.lower() | ||
| return "invalid" in lower and "signature" in lower and "thinking" in lower and "block" in lower | ||
| return "thinking" in lower and "signature" in lower |
There was a problem hiding this comment.
Wider match could trigger spurious strip-and-retry on unrelated 400s
Any 400 response body that contains both "thinking" and "signature" will now trigger the self-heal path — stripping thinking blocks from the request and issuing one extra HTTP call — even if the root cause is unrelated (e.g. a hypothetical error like "signature validation for thinking_budget parameter invalid"). The recovery is bounded to one retry and is otherwise safe, but it can mask the real error for a full round-trip. Adding a negative test for a message that contains both words in an unrelated context would make the intended boundary explicit and guard against accidental scope creep in future edits.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
e59add1
into
litellm_internal_staging
Relevant issues
Resolves LIT-4514
Fixes #26005
Supersedes #26152, which only dropped the
"block"keyword but still required"invalid", so it does not cover the Bedrock error body (see below)Linear ticket
Pre-Submission checklist
Screenshots / Proof of Fix
Root cause:
is_anthropic_invalid_thinking_signature_errorgates a safe self-heal on the/v1/messagespath (strip thinking blocks, one bounded retry; the retry re-signs the request so Bedrock works too). It only returnedTruewhen the error contained all of"invalid","signature","thinking"and"block". When history from another provider (e.g. Fireworks) carries a thinking block with a missing/emptysignatureinto Anthropic/Bedrock, the provider raises a Pydantic-style validation error that has neither"invalid"nor"block", so the self-heal never fired and the request 400'dBedrock raw body the user hit:
{"message":"messages.2.content.0.thinking.signature.str: Input should be a valid string"}Detector behavior on that exact Bedrock body, before vs after, run against the real function at each commit:
End-to-end self-heal against the real Anthropic API (
claude-opus-4-8, live call costing real $, captured at d2fbb96), sending an assistant turn whose thinking block has an empty signature followed by a new user question:Note on scope of the live run: the exact Bedrock body could not be reproduced against a live endpoint because the available Bedrock test credentials lack
bedrock:InvokeModel(403 before request validation). The Anthropic direct run above exercises the full strip-and-retry recovery pipeline that this detector gates; the before/after snippet proves the detector now matches the Bedrock/Vertex format that previously slipped throughType
🐛 Bug Fix
Changes
is_anthropic_invalid_thinking_signature_errornow matches when the lowercased error contains both"thinking"and"signature", dropping the"invalid"and"block"requirements. Both known non-Anthropic formats include those two tokens, and the gated recovery (strip thinking blocks, retry once) is safe and bounded, so a looser match is preferable to silently 400'ing valid conversationsKnown formats now covered: the Bedrock
{"message": "...thinking.signature.str: Input should be a valid string"}body, the Vertex AImessages.N.content.M.thinking.signature.str: Input should be a valid stringform, and the classic Anthropicmessages.N.content.M: Invalid \signature` in `thinking` block`Regression tests were added in
TestAnthropicThinkingSignatureSelfHealcovering the Bedrock and Vertex positives plus negatives that fail if either the"invalid"or"block"requirement is reintroduced or the return is flippedFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/4d342e8d4f9f4502a7cb9533b8a018a7
Requested by: @krrish-berri-2