Skip to content

style: apply black formatting to fix lint CI (LIT-3274) (#28639)#28641

Merged
krrish-berri-2 merged 1 commit into
litellm_internal_stagingfrom
litellm_shin_staging_05_22_2026
May 22, 2026
Merged

style: apply black formatting to fix lint CI (LIT-3274) (#28639)#28641
krrish-berri-2 merged 1 commit into
litellm_internal_stagingfrom
litellm_shin_staging_05_22_2026

Conversation

@krrish-berri-2

Copy link
Copy Markdown
Contributor
  • fix(bedrock): strip bedrock/ prefix and URL-encode ARNs in get_bedrock_model_id for invoke path

The invoke path (used by /v1/messages → Anthropic SDK / Claude Code) called get_bedrock_model_id() which, when falling back to the raw model string, did not strip the 'bedrock/' routing prefix and did not URL-encode ARNs.

For a model like:
bedrock/arn:aws:bedrock:us-east-1::inference-profile/global.anthropic...

the URL built was:
/model/bedrock/arn:aws:bedrock:…/invoke-with-response-stream ❌

Bedrock returned a JSON error body. LiteLLM's AWSEventStreamDecoder passed those bytes into botocore's EventStreamBuffer which expects binary event-stream framing. Checksum validation failed on the JSON prelude (0x223a7b22 == ':{"') producing a misleading botocore.eventstream.ChecksumMismatch instead of the actual Bedrock error.

Fix: strip 'bedrock/' (and 'invoke/') routing prefix from model string, then URL-encode if the result is an ARN — matching what the converse path already does in converse_handler.py.

Fixes: LIT-3274

  • fix(bedrock): use strip_bedrock_routing_prefix to handle compound prefixes

Address greptile review: the original fix used a loop with break, so bedrock/invoke/arn:... only stripped bedrock/ leaving invoke/arn:... which is not an ARN → fell through to .replace('invoke/','',1) → bare unencoded ARN → same malformed-URL bug.

strip_bedrock_routing_prefix() iterates without break, correctly stripping bedrock/ then invoke/ in sequence. Also adds test case for the compound-prefix scenario.

  • style: apply black formatting to fix lint CI (LIT-3274)

Relevant issues

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Screenshots / Proof of Fix

Type

🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test

Changes

* fix(bedrock): strip bedrock/ prefix and URL-encode ARNs in get_bedrock_model_id for invoke path

The invoke path (used by /v1/messages → Anthropic SDK / Claude Code) called
get_bedrock_model_id() which, when falling back to the raw model string, did
not strip the 'bedrock/' routing prefix and did not URL-encode ARNs.

For a model like:
  bedrock/arn:aws:bedrock:us-east-1:<ACCOUNT>:inference-profile/global.anthropic...

the URL built was:
  /model/bedrock/arn:aws:bedrock:…/invoke-with-response-stream  ❌

Bedrock returned a JSON error body.  LiteLLM's AWSEventStreamDecoder passed
those bytes into botocore's EventStreamBuffer which expects binary event-stream
framing.  Checksum validation failed on the JSON prelude (0x223a7b22 == ':{"')
producing a misleading botocore.eventstream.ChecksumMismatch instead of the
actual Bedrock error.

Fix: strip 'bedrock/' (and 'invoke/') routing prefix from model string, then
URL-encode if the result is an ARN — matching what the converse path already
does in converse_handler.py.

Fixes: LIT-3274

* fix(bedrock): use strip_bedrock_routing_prefix to handle compound prefixes

Address greptile review: the original fix used a loop with break, so
bedrock/invoke/arn:... only stripped bedrock/ leaving invoke/arn:...
which is not an ARN → fell through to .replace('invoke/','',1) →
bare unencoded ARN → same malformed-URL bug.

strip_bedrock_routing_prefix() iterates without break, correctly
stripping bedrock/ then invoke/ in sequence. Also adds test case
for the compound-prefix scenario.

* style: apply black formatting to fix lint CI (LIT-3274)

---------

Co-authored-by: Krrish Dholakia <krrish+github@berri.ai>
Co-authored-by: LiteLLM Bot <bot@berri.ai>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codecov

codecov Bot commented May 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a malformed-URL bug in the Bedrock invoke path (/v1/messages → Anthropic SDK) where get_bedrock_model_id was forwarding raw LiteLLM routing prefixes (e.g. bedrock/) and un-encoded ARNs directly into the URL, causing botocore's EventStreamBuffer to receive a JSON error body and surface a misleading ChecksumMismatch instead of the real Bedrock error.

  • Core fix (base_aws_llm.py): calls strip_bedrock_routing_prefix() (which iterates without break) to fully strip compound prefixes like bedrock/invoke/, then URL-encodes the cleaned ARN and returns early — matching the logic already present in the converse path.
  • Tests (test_base_aws_llm.py): adds five unit tests covering bare ARN, bedrock/ prefix, compound bedrock/invoke/ prefix, full URL assertion, and regression checks for plain model IDs; all tests are local with no real network calls.

Confidence Score: 4/5

The change fixes a real invoke-path bug for ARN models and is safe to merge; the only concern is a subtle side-effect on openai/nova non-ARN prefix models worth validating.

The ARN-stripping and URL-encoding logic is correct and well-tested. One behavioural edge case — non-ARN models with openai/, nova/, or nova-2/ prefixes no longer pass through the _get_model_id_from_model_with_spec URL-encoding step — is worth a quick sanity check, but is unlikely to affect real production traffic and does not break the primary use-case this PR targets.

litellm/llms/bedrock/base_aws_llm.py — verify the openai/ and nova/ prefix handling in get_bedrock_model_id still produces correct Bedrock API URLs after strip_bedrock_routing_prefix strips those prefixes upstream of the provider-specific branches.

Important Files Changed

Filename Overview
litellm/llms/bedrock/base_aws_llm.py Adds strip_bedrock_routing_prefix call and ARN URL-encoding with early return in get_bedrock_model_id; fix is logically correct for the stated bug but introduces a subtle side-effect for openai/, nova/, nova-2/ prefix models
tests/test_litellm/llms/bedrock/test_base_aws_llm.py Adds five unit tests for get_bedrock_model_id ARN handling; tests are mocked (no real network calls) and cover the compound-prefix scenario; contains a real AWS account ID in the test ARN constant

Reviews (1): Last reviewed commit: "style: apply black formatting to fix lin..." | Re-trigger Greptile

class TestGetBedrockModelIdArnHandling:
"""Unit tests for get_bedrock_model_id with inference-profile ARNs."""

ARN = "arn:aws:bedrock:us-east-1:086734376398:inference-profile/global.anthropic.claude-sonnet-4-5-20250929-v1:0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The test ARN includes a real AWS account ID (086734376398). For test code in a public repository it's better practice to use a placeholder account ID so that real account numbers are not unnecessarily embedded in the history. The canonical placeholder for AWS examples is 123456789012.

Suggested change
ARN = "arn:aws:bedrock:us-east-1:086734376398:inference-profile/global.anthropic.claude-sonnet-4-5-20250929-v1:0"
ARN = "arn:aws:bedrock:us-east-1:123456789012:inference-profile/global.anthropic.claude-sonnet-4-5-20250929-v1:0"

Comment on lines +466 to +470
model_id = strip_bedrock_routing_prefix(model_id)
# URL-encode ARNs so colons and slashes are safe in the URL path.
if model_id.startswith("arn:"):
model_id = BaseAWSLLM.encode_model_id(model_id=model_id)
return model_id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent bypass of provider-specific URL-encoding for openai/, nova/, nova-2/ prefixed non-ARN models

strip_bedrock_routing_prefix strips openai/, nova/, and nova-2/ prefixes before the model ID reaches the downstream elif provider == "openai" and "openai/" in model_id (and equivalent nova) branches. Those branches previously called _get_model_id_from_model_with_spec, which also runs encode_model_id on the cleaned ID. After this change, a model like openai/us.amazon.nova-lite-v1:0 is stripped to us.amazon.nova-lite-v1:0 (unencoded) because "openai/" in model_id is now False. The ARN path is unaffected, but if any Bedrock OpenAI-compatible or Nova model variant relied on the old encoding for its URL path the request URL will differ from the previous behavior.

@krrish-berri-2 krrish-berri-2 enabled auto-merge (squash) May 22, 2026 19:09
@krrish-berri-2 krrish-berri-2 disabled auto-merge May 22, 2026 19:09
@krrish-berri-2 krrish-berri-2 enabled auto-merge (squash) May 22, 2026 19:09
@krrish-berri-2 krrish-berri-2 merged commit a3c953e into litellm_internal_staging May 22, 2026
117 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants