Skip to content

fix(bedrock): handle common image format aliases in BedrockImageProce…#24269

Open
longzhihun wants to merge 1 commit intoBerriAI:mainfrom
longzhihun:fix/bedrock-image-format-aliases
Open

fix(bedrock): handle common image format aliases in BedrockImageProce…#24269
longzhihun wants to merge 1 commit intoBerriAI:mainfrom
longzhihun:fix/bedrock-image-format-aliases

Conversation

@longzhihun
Copy link

Summary

  • Bug: BedrockImageProcessor._validate_format() rejects jpg and mpg formats with ValueError: Unsupported image format, even though they are standard aliases for jpeg and mpeg respectively.
  • Fix: Add a format_aliases dictionary to normalize common format aliases before validation. This maps jpgjpeg and mpgmpeg.
  • Test: Added test_bedrock_validate_format_resolves_common_aliases to verify alias resolution, canonical format passthrough, and unsupported format rejection. Updated existing test_bedrock_validate_format_image_or_video to reflect mpgmpeg normalization.

Motivation

When users send .jpg images (the most common JPEG file extension) through Bedrock-backed models via LiteLLM, the request fails because the format validator only recognizes jpeg as the canonical name. The JPEG standard uses both .jpg and .jpeg extensions interchangeably, and similarly MPEG videos use both .mpg and .mpeg. This fix ensures both common extensions are accepted.

Changes

  • litellm/litellm_core_utils/prompt_templates/factory.py: Added format alias resolution in _validate_format()
  • tests/test_litellm/.../test_litellm_core_utils_prompt_templates_factory.py: Added new test + updated existing test

Test plan

  • Added test_bedrock_validate_format_resolves_common_aliases covering:
    • jpgjpeg alias resolution
    • mpgmpeg alias resolution
    • Canonical formats (jpeg, mpeg) still work unchanged
    • Unsupported formats still raise ValueError
  • Updated test_bedrock_validate_format_image_or_video to expect mpeg when input is mpg
  • All tests pass locally

Type

🐛 Bug Fix

…ssor

Feishu (Lark) and other platforms may send images with MIME type
`image/jpg` instead of `image/jpeg`. The Bedrock converse API
validator rejects `jpg` even though it is the same format as `jpeg`.

Add a format alias mapping to normalize common equivalent extensions
before validation (`jpg` → `jpeg`, `mpg` → `mpeg`), preventing
unnecessary 500 errors for valid media types.

Made-with: Cursor
@vercel
Copy link

vercel bot commented Mar 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Mar 21, 2026 5:18am

Request Review

@codspeed-hq
Copy link
Contributor

codspeed-hq bot commented Mar 21, 2026

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing longzhihun:fix/bedrock-image-format-aliases (53841f8) with main (d8e4fc4)

Open in CodSpeed

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 21, 2026

Greptile Summary

This PR fixes a real bug where .jpg images were rejected by BedrockImageProcessor._validate_format() because "jpg" is absent from AmazonConverseConfig.get_supported_image_types() (which only lists "jpeg"). The fix for jpg→jpeg is correct and well-tested. However, the companion mpg→mpeg alias is problematic: "mpg" is already present in get_supported_video_types(), meaning it was already validated and returned unchanged prior to this PR. Adding the alias silently changes the return value from "mpg" to "mpeg" for a code path that was not broken, constituting an undocumented behavior change.

Key changes:

  • factory.py: Two-entry format_aliases dict (jpg→jpeg, mpg→mpeg) added inside _validate_format() before the supported-format check.
  • Tests: New test_bedrock_validate_format_resolves_common_aliases added; existing test_bedrock_validate_format_image_or_video updated to expect "mpeg" for "mpg" input.

Issues found:

  • The mpg→mpeg alias changes the return value for "mpg" inputs even though "mpg" was already a supported and passthrough format; this is an unnecessary behavior change.
  • The format_aliases dict is allocated on every call to the @staticmethod — it should be a class-level constant.
  • A placeholder issue URL (https://github.com/BerriAI/litellm/issues/XXXXX) was left unfilled in the new test docstring.

Confidence Score: 3/5

  • The jpg→jpeg fix is correct, but the mpg→mpeg alias introduces an undocumented behavior change for a format that was already working.
  • The real bug (rejecting jpg) is properly fixed and tested. However, mpg was already in the supported list and was being returned as-is; the new alias silently changes the return value to mpeg, which is a backwards-incompatible behavior change for users relying on the mpg format string being preserved. This could affect downstream Bedrock API calls if the exact format string matters. The test is updated to reflect the new (changed) behavior rather than exposing a previous regression.
  • litellm/litellm_core_utils/prompt_templates/factory.py — specifically the mpg→mpeg alias entry and whether "mpg" should be removed from get_supported_video_types() to remain consistent.

Important Files Changed

Filename Overview
litellm/litellm_core_utils/prompt_templates/factory.py Adds jpg→jpeg and mpg→mpeg aliases in _validate_format. The jpg→jpeg alias correctly fixes a real bug. However, mpg was already in get_supported_video_types(), so the mpg→mpeg alias introduces a silent behavior change (return value changes from "mpg" to "mpeg") for an input that was already working.
tests/test_litellm/litellm_core_utils/prompt_templates/test_litellm_core_utils_prompt_templates_factory.py Adds a new test test_bedrock_validate_format_resolves_common_aliases covering alias resolution and updates test_bedrock_validate_format_image_or_video to expect mpeg for mpg input. Tests are pure unit tests with no network calls. Contains a placeholder issue URL (XXXXX) in a docstring.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["_validate_format(mime_type, image_format)"] --> B{Is document?\nmime_type starts with\n'application' or 'text'}
    B -- Yes --> C["_get_document_format()"]
    B -- No --> D["NEW: Apply format_aliases\n{'jpg':'jpeg', 'mpg':'mpeg'}"]
    D --> E{image_format in\nsupported_image_and_video_formats?}
    E -- No --> F["raise ValueError\nUnsupported image format"]
    E -- Yes --> G["return image_format\n(normalized)"]

    subgraph supported_image_and_video_formats
        H["Images: png, jpeg, gif, webp"]
        I["Videos: mp4, mov, mkv, webm,\nflv, mpeg, mpg, wmv, 3gp"]
    end

    style D fill:#ffe0b2,color:#000
    style I fill:#fff9c4,color:#000
    note1["⚠️ 'mpg' already in supported list\nbefore alias lookup"]
Loading

Last reviewed commit: "fix(bedrock): handle..."

@CLAassistant
Copy link

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.

Comment on lines +3605 to +3606
format_aliases = {"jpg": "jpeg", "mpg": "mpeg"}
image_format = format_aliases.get(image_format, image_format)
Copy link
Contributor

Choose a reason for hiding this comment

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

P1 mpg was already a supported format — unnecessary behavior change

get_supported_video_types() in converse_transformation.py already returns "mpg" in its list:

def get_supported_video_types(self) -> List[str]:
    return ["mp4", "mov", "mkv", "webm", "flv", "mpeg", "mpg", "wmv", "3gp"]

This means "mpg" was already passing the if image_format not in supported_image_and_video_formats check correctly and was returned unchanged as "mpg". By adding the mpg → mpeg alias, this PR silently changes the return value of _validate_format("video/mpg", "mpg") from "mpg" to "mpeg". Any caller that previously relied on "mpg" being passed through (e.g., as the format field in a BedrockVideoBlock) will now see "mpeg" instead.

The real bug — and the correct, minimal fix — only affects "jpg", which is genuinely absent from get_supported_image_types() (which returns ["png", "jpeg", "gif", "webp"]).

Consider either removing the "mpg" entry from the alias dict, or first removing "mpg" from get_supported_video_types() if the intent is to consolidate on "mpeg" as the canonical form:

Suggested change
format_aliases = {"jpg": "jpeg", "mpg": "mpeg"}
image_format = format_aliases.get(image_format, image_format)
format_aliases = {"jpg": "jpeg"}
image_format = format_aliases.get(image_format, image_format)

Comment on lines +170 to +172

Regression test for: https://github.com/BerriAI/litellm/issues/XXXXX
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

P2 Placeholder issue URL should be removed

The docstring references a placeholder GitHub issue URL (https://github.com/BerriAI/litellm/issues/XXXXX) that was never filled in. This will confuse future contributors trying to find the original context.

Suggested change
Regression test for: https://github.com/BerriAI/litellm/issues/XXXXX
"""
Regression test for the bug where `jpg` and `mpg` format aliases were
not accepted by _validate_format.

#########################################################
# Check if image_format is an image or video
#########################################################
format_aliases = {"jpg": "jpeg", "mpg": "mpeg"}
Copy link
Contributor

Choose a reason for hiding this comment

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

P2 format_aliases dict re-created on every invocation

_validate_format is called on every image/video request. Allocating a new dict object on each call is a minor but unnecessary overhead. Since the alias map is static, it should be defined as a class-level constant:

class BedrockImageProcessor:
    _FORMAT_ALIASES: Dict[str, str] = {"jpg": "jpeg"}
    ...

Then use self._FORMAT_ALIASES.get(image_format, image_format) (or BedrockImageProcessor._FORMAT_ALIASES.get(...) since it's a @staticmethod).

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