Skip to content

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

Merged
krrish-berri-2 merged 3 commits into
BerriAI:litellm_shin_staging_05_22_2026from
oss-agent-shin:fix/lit-3274-lint
May 22, 2026
Merged

style: apply black formatting to fix lint CI (LIT-3274)#28639
krrish-berri-2 merged 3 commits into
BerriAI:litellm_shin_staging_05_22_2026from
oss-agent-shin:fix/lit-3274-lint

Conversation

@oss-agent-shin

@oss-agent-shin oss-agent-shin commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the lint CI failure on #28551 by applying black formatting to the two files changed in that PR.

What changed

  • Added missing blank line after import in litellm/llms/bedrock/base_aws_llm.py
  • Reformatted multi-line assert statements in tests/test_litellm/llms/bedrock/test_base_aws_llm.py to Black style

Related

Branched from krrishdholakia1/lit-3274-v1messages-with-bedrock-inference-profile-arns-returns — this PR contains only formatting fixes, zero logic changes.

Closes #28551 (or can be merged into it)

Fixes: LIT-3274

krrish-berri-2 and others added 3 commits May 21, 2026 21:28
…k_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
…fixes

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.
@codspeed-hq

codspeed-hq Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing oss-agent-shin:fix/lit-3274-lint (b3c3c4b) with main (79b4578)

Open in CodSpeed

@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!

@krrish-berri-2 krrish-berri-2 changed the base branch from main to litellm_internal_staging May 22, 2026 18:41
@krrish-berri-2 krrish-berri-2 changed the base branch from litellm_internal_staging to litellm_shin_staging_05_22_2026 May 22, 2026 18:41
@greptile-apps

greptile-apps Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR is described as a pure formatting fix (Black style) on top of the parent feature branch krrishdholakia1/lit-3274-v1messages-with-bedrock-inference-profile-arns-returns, but the diff from main includes the full LIT-3274 logic change: routing-prefix stripping and ARN URL-encoding in get_bedrock_model_id for the invoke path.

  • litellm/llms/bedrock/base_aws_llm.py — adds a 24-line block that strips LiteLLM routing prefixes via strip_bedrock_routing_prefix and URL-encodes bare ARNs before returning, preventing a malformed-URL / ChecksumMismatch error on the invoke path.
  • tests/test_litellm/llms/bedrock/test_base_aws_llm.py — adds TestGetBedrockModelIdArnHandling with six unit tests covering bare ARNs, compound bedrock/invoke/ prefixes, URL construction, and regression checks for non-ARN models; all tests are pure unit tests with no real network calls.

Confidence Score: 4/5

The logic change is narrow and well-covered by the new unit tests; the main risk is the inline import and a test ARN that uses a real-looking AWS account ID.

The routing-prefix stripping and ARN encoding logic is straightforward, the new tests exercise the key scenarios including compound prefixes, and get_complete_url was confirmed to be a pure URL-construction method with no network calls. The two findings are stylistic: an unexplained deferred import and a test fixture that embeds what may be a real AWS account ID.

The inline import on line 464 of litellm/llms/bedrock/base_aws_llm.py and the ARN constant in tests/test_litellm/llms/bedrock/test_base_aws_llm.py deserve a quick look.

Important Files Changed

Filename Overview
litellm/llms/bedrock/base_aws_llm.py Adds routing-prefix stripping and ARN URL-encoding to the invoke path inside get_bedrock_model_id; includes an inline function-body import and an early return for ARN models
tests/test_litellm/llms/bedrock/test_base_aws_llm.py Adds TestGetBedrockModelIdArnHandling covering bare ARNs, compound prefixes, and URL construction; tests appear to be pure unit tests without real network calls

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

@krrish-berri-2 krrish-berri-2 merged commit 0544881 into BerriAI:litellm_shin_staging_05_22_2026 May 22, 2026
43 of 47 checks passed
Comment on lines +464 to +466
from litellm.llms.bedrock.common_utils import strip_bedrock_routing_prefix

model_id = strip_bedrock_routing_prefix(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 The import of strip_bedrock_routing_prefix is placed inside the function body instead of at the module level. Deferred imports are typically used to break circular dependencies, but there's no comment explaining why that's necessary here. If this location truly is required to avoid a circular import, a short inline comment would clarify intent. Otherwise, moving it to the top of the file keeps the import section clear and avoids the per-call overhead of looking up an already-cached module entry.

Suggested change
from litellm.llms.bedrock.common_utils import strip_bedrock_routing_prefix
model_id = strip_bedrock_routing_prefix(model_id)
# Deferred to avoid circular import with common_utils.
from litellm.llms.bedrock.common_utils import strip_bedrock_routing_prefix
model_id = strip_bedrock_routing_prefix(model_id)

Comment on lines +2120 to +2122
# the Bedrock API receives a malformed URL, returns a JSON error body, and
# botocore's EventStreamBuffer raises ChecksumMismatch instead of the real
# error. 0x223a7b22 == ':{\"' — the start of a JSON object.

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 class-level ARN constant contains 086734376398, a 12-digit number that looks like a real AWS account ID. Test fixtures should use an obviously synthetic ID (e.g. 123456789012, Amazon's documented placeholder) to avoid embedding what may be a real account ID in a public repository's history.

krrish-berri-2 added a commit that referenced this pull request May 22, 2026
)

* 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: oss-agent-shin <ext-agent-shin@berri.ai>
Co-authored-by: LiteLLM Bot <bot@berri.ai>
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.

2 participants