Repository navigation
fix(anthropic): remap disabled thinking to between_tools on Sonnet 5.5 - #44299
pcbeingused333 wants to merge 7 commits into
Conversation
maybe_drop_disabled_thinking dropped thinking={type: disabled} for every
always-on-thinking model, including Sonnet 5.5, which silently left thinking
on at whatever effort the caller set. Sonnet 5.5 has a real off-equivalent,
between_tools, valid at low/medium/high effort. Remap to that instead of
dropping, and keep the existing drop-and-warn path for xhigh/max effort
(where between_tools 400s) and for every other always-on-thinking family,
whose own off-equivalent is not confirmed yet.
Fixes BerriAI#44298
|
| if AnthropicModelInfo._supports_between_tools_thinking( | ||
| model, custom_llm_provider | ||
| ) and not AnthropicModelInfo._effort_rejects_between_tools(optional_params): | ||
| litellm.verbose_logger.warning(REMAP_DISABLED_TO_BETWEEN_TOOLS_WARNING, model) | ||
| optional_params["thinking"] = {"type": "between_tools"} |
There was a problem hiding this comment.
With litellm.modify_params=True, a Sonnet 5.5 tool-result turn whose assistant history has no thinking_blocks loses thinking={"type":"disabled"} before this remap runs. Both chat and Converse remove the setting in their earlier history guard. The helper then returns without supplying between_tools, so the provider resumes adaptive thinking and the turn can take longer and cost more
Exempt disabled requests from those guards and add a regression test that sends a tool-result turn through the request transformation
There was a problem hiding this comment.
@greptileai Fixed in 601a409: both guards now skip disabled thinking, with regression tests through the chat and Converse request transformations
There was a problem hiding this comment.
Confirmed fixed in 601a4092d09a50202178786e806816ee7cbdeb55. Both request transformations now preserve the bare {"type": "disabled"} value through the missing-thinking-blocks guard, allowing the existing Sonnet 5.5 remap to {"type": "between_tools"}. The new chat and Converse regression tests exercise the tool-result turn with litellm.modify_params=True and assert the forwarded thinking mode. No further issue here.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merging this PR will not alter performance
|
….py cost-map schema test_aaamodel_prices_and_context_window_json_is_valid validates the cost map against its own copy of the schema, separate from model_prices_and_context_window.schema.json. Missed updating it in the previous commit.
|
can you address the greptile feedback? @pcbeingused333 |
…locks guard
With modify_params=True, a tool-result turn whose assistant history has no
thinking_blocks dropped thinking={'type': 'disabled'} before
maybe_drop_disabled_thinking could remap it to between_tools on Sonnet 5.5,
so the provider fell back to adaptive thinking. Disabled thinking needs no
thinking_blocks, so both the chat and Converse guards now skip it
…tools-thinking-fallback # Conflicts: # model_prices_and_context_window.schema.json # tests/unit/llms/anthropic/test_anthropic_common_utils.py
|
@krrish-berri-2 addressed in 601a409: both the chat and Converse request transformations now keep the bare The two red Postgres jobs ( |
…tools-thinking-fallback # Conflicts: # litellm/model_prices_and_context_window_backup.json # model_prices_and_context_window.json # tests/unit/llms/anthropic/chat/test_anthropic_chat_transformation.py
…e write maybe_drop_disabled_thinking mutates optional_params by design (its signature already carries mutable-ok); annotate the new store so the type-discipline gate's LIT011 ceiling doesn't grow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The bare dict narrowed from optional_params has unknown key/value types, so .get() added a reportUnknownMemberType error past the basedpyright ceiling. A membership check plus subscript reads the same value without the member access. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TLDR
Problem this solves:
thinking.type=disabledgets dropped entirely instead of mapped to the model's real off switchHow it solves it:
disabledtobetween_tools, Sonnet 5.5's documented off-equivalent, atlow/medium/higheffortxhigh/maxeffort (wherebetween_toolsitself 400s) and for every other always-on-thinking family, whose own off-equivalent is not confirmed yetlitellm.modify_params=True, stop the missing-thinking_blocksguard in the chat and Converse transformations from droppingdisabledon tool-result turns. That guard runs before the remap, so it used to removedisabledfirst and the provider resumed adaptive thinkingIntentional product change: a caller asking for
disabledthinking on Sonnet 5.5 atlow/medium/higheffort now getsbetween_toolsinstead of silent full adaptive thinking. This changes model behavior and cost for those requests, matching whatdisabledmeant on every Claude model before Sonnet 5.5 removed it.User Flow
Before: a developer running a latency-sensitive agent on Claude Sonnet 5.5 asks for thinking off and silently gets full adaptive thinking instead
litellm.completion(model="anthropic/claude-sonnet-5-5", messages=[...], thinking={"type": "disabled"})thinkingfield and forwards the request with none, logging a warning that thinking cannot be disabled for this modelAfter: the same call runs the model's real thinking-off mode
litellm.completion(...)call withthinking={"type": "disabled"}thinking={"type": "between_tools"}instead, logging that it made the substitutiondisabledgives on every earlier Claude modelRelevant issues
Fixes #44298
Pre-Submission checklist
uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
I do not have Anthropic API credentials in this environment, so I could not run this against a live proxy hitting the real API as the template asks. The commands below call the actual
AnthropicModelInfo.maybe_drop_disabled_thinkingfunction directly, the same function the request path calls, against the two commits below, and show the real log line and the realoptional_params["thinking"]value each one produces. No mocks: this is the production code path, just invoked without a network call after it since the model-info branch and the mutation it makes do not depend on one. A maintainer with API access can confirm the end-to-end behavior with:and checking that the request LiteLLM forwards carries
thinking: {"type": "between_tools"}and the response contains no up-front thinking block.Before (22bd49e)
python3 -c "from litellm.llms.anthropic.common_utils import AnthropicModelInfo; params = {'thinking': {'type': 'disabled'}, 'output_config': {'effort': 'medium'}}; AnthropicModelInfo.maybe_drop_disabled_thinking(model='claude-sonnet-5-5', optional_params=params, custom_llm_provider='anthropic'); print(params.get('thinking', '<absent>'))"LiteLLM:WARNING: Dropping thinking={'type': 'disabled'} for model=claude-sonnet-5-5: thinking is always on for this model and cannot be disabled (the alternative is a provider 400). The model will still think adaptively, its response can contain thinking blocks, and those thinking tokens are billed as output tokens.then<absent>After (2927cbd)
medium effort
LiteLLM:WARNING: Replacing thinking={'type': 'disabled'} with thinking={'type': 'between_tools'} for model=claude-sonnet-5-5: this model 400s on disabled, and between_tools is its lowest thinking setting. The model will not think before responding; progress notes it writes between tool calls still come back as thinking blocks.then{'type': 'between_tools'}xhigh effort
"effort": "xhigh"<absent>as Before, sincebetween_toolsitself 400s atxhigh/maxand adaptive thinking is the only off-equivalent at that effortType
🐛 Bug Fix
✅ Test
Caveats (if any)
Medium
supports_between_tools_thinkingis set only on Sonnet 5.5 cost-map entries. Opus 5.5, Fable 5(.1), and Mythos 5(.1) keep the drop-and-warn path because I have not found documentation confirmingbetween_tools(or an equivalent) works on themLow
modify_params=True, a tool-result turn on a model that acceptsdisablednow forwardsthinking={"type": "disabled"}instead of dropping it. Both mean thinking off, anddisabledneeds nothinking_blocksin historybetween_toolsalso rejectsdisplay,budget_tokens, andblock_bindingon thethinkingobject per the migration guide, but the only shape this function ever matches is the bare{"type": "disabled"}the caller sent, so there is nothing to stripFinal Attestation