Repository navigation
fix(bedrock): keep filtering anthropic-beta headers on the Claude platform messages path - #42260
Closed
yuneng-berri wants to merge 1 commit into
Closed
yuneng-berri wants to merge 1 commit into
yuneng-berri wants to merge 1 commit into
Conversation
…tform messages path PR #42019's successor #42152 gave AnthropicMessagesConfig an override that returns False when the resolved provider is "anthropic", so betas reach api.anthropic.com verbatim. BedrockClaudePlatformMessagesConfig overrides neither custom_llm_provider nor the filter, and nothing earlier in its MRO supplies a provider, so it inherited "anthropic" and stopped filtering - forwarding betas Bedrock does not accept, which the base class documents as the exact case cross-provider paths need filtering for. Opt the Claude platform config back into the provider-scoped filter and cover it, since no test exercised Bedrock beta filtering before. The two passthrough tests whose contrast assertion caught this now compare against the Bedrock config, because a native-Anthropic False is intended.
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
|
Merged
7 tasks done
Contributor
|
Superseded by #42275, merged: filtering with the bedrock beta map 400s beta-gated requests on the Claude Platform route, which accepts every Anthropic beta |
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.
Summary
BedrockClaudePlatformMessagesConfigsilently stopped filteringanthropic-betaheaders, so betas Bedrock does not accept are now forwarded verbatim to it.#42152 added an override to
AnthropicMessagesConfig:That is correct for its own case — when the upstream really is
api.anthropic.com, betas should pass through untouched. ButAnthropicMessagesConfig.custom_llm_provideris a hardcoded property returning"anthropic", and_resolved_providerjust reads it, so the method returnsFalsefor every subclass that does not override the provider.BedrockClaudePlatformMessagesConfigoverrides neithercustom_llm_providernor the filter, and nothing earlier in its MRO supplies a provider:so it inherits
"anthropic"and reportsfilter_betas=False. (Thecustom_llm_provider -> "bedrock"property inlitellm/llms/bedrock/claude_platform/transformation.pybelongs to the chat/converse config and is not in this MRO.)This is the exact case the base class documents as needing the filter, in
litellm/llms/base_llm/anthropic_messages/transformation.py:Before #42152 the base returned
True, so Bedrock filtered. The filter runs atlitellm/llms/custom_httpx/llm_http_handler.py:2281.Sweep of every
AnthropicMessagesconfig — Bedrock is the only unintendedFalse:AnthropicMessagesConfigGithubCopilotAnthropicMessagesConfigOpenAILikeAnthropicMessagesConfigAzureAnthropicMessagesConfigMinimaxMessagesConfigDeepSeekAnthropicMessagesConfigTencentAnthropicMessagesConfigBedrockClaudePlatformMessagesConfigChanges
BedrockClaudePlatformMessagesConfigopts back into the provider-scoped filter.Falseis intended behaviour.I deliberately did not fix this by giving the config
custom_llm_provider -> "bedrock". That would be the deeper fix, but_resolved_provideralso feeds six model-capability lookups inAnthropicMessagesConfig(reasoning-effort translation,maybe_drop_disabled_thinking, adaptive-effort and thinking-temperature handling), so changing it would alter Bedrock thinking/reasoning behaviour in a regression fix. Worth doing separately, with its own testing.Testing
Red/green against the new test, source fix removed then restored:
Filtering demonstrably active again — an unsupported beta is dropped and a renamed one translated:
tests/unit/llms/{bedrock,github_copilot,openai_like}+test_llm_http_handler.py: 618 passedtests/unitas CI runs it (-n 4 --dist=loadscope, 315 files): 6395 passedscripts/pre_commit_lint.sh: PASSHow this was found
Triaging the
#circleci-triagebacklog. The two passthrough tests fail deterministically onmainand CI had not yet run theunitjob since #42152 merged, so the next scheduled run would have gone red on them. They read like stale assertions, but in each one the test's own subject still passes — the failing line is the contrast guard on the default, which is what surfaced the Bedrock leak.