Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
longzhihun wants to merge 1 commit into
base: main
Choose a base branch
from
+38 −1
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions litellm/litellm_core_utils/prompt_templates/factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3602,6 +3602,8 @@ def _validate_format(mime_type: str, image_format: str) -> str:
#########################################################
# Check if image_format is an image or video
#########################################################
format_aliases = {"jpg": "jpeg", "mpg": "mpeg"}
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 21, 2026

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).

image_format = format_aliases.get(image_format, image_format)
Comment on lines +3605 to +3606
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 21, 2026

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)

if image_format not in supported_image_and_video_formats:
raise ValueError(
f"Unsupported image format: {image_format}. Supported formats: {supported_image_and_video_formats}"
Expand Down
37 changes: 36 additions & 1 deletion ...m/litellm_core_utils/prompt_templates/test_litellm_core_utils_prompt_templates_factory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,17 @@ def test_bedrock_validate_format_image_or_video():
"webm",
"flv",
"mpeg",
"mpg",
"wmv",
"3gp",
]
for format in valid_video_formats:
result = BedrockImageProcessor._validate_format(f"video/{format}", format)
assert result == format, f"Expected {format}, got {result}"

# 'mpg' is aliased to 'mpeg'
result = BedrockImageProcessor._validate_format("video/mpg", "mpg")
assert result == "mpeg", f"Expected 'mpeg', got '{result}'"

# Test valid document formats
valid_document_formats = {
"application/pdf": "pdf",
Expand All @@ -155,6 +158,38 @@ def test_bedrock_validate_format_image_or_video():
assert result == expected, f"Expected {expected}, got {result}"


def test_bedrock_validate_format_resolves_common_aliases():
"""
Test that _validate_format resolves common format aliases like
'jpg' -> 'jpeg' and 'mpg' -> 'mpeg' instead of raising ValueError.

These aliases are standard equivalents (JPEG files commonly use .jpg
extension, MPEG videos commonly use .mpg extension) but were previously
rejected because only the canonical names appeared in the supported
formats list.

Regression test for: https://github.com/BerriAI/litellm/issues/XXXXX
"""
Comment on lines +170 to +172
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 21, 2026

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.

# 'jpg' should be resolved to 'jpeg'
result_jpg = BedrockImageProcessor._validate_format("image/jpg", "jpg")
assert result_jpg == "jpeg", f"Expected 'jpeg', got '{result_jpg}'"

# 'mpg' should be resolved to 'mpeg'
result_mpg = BedrockImageProcessor._validate_format("video/mpg", "mpg")
assert result_mpg == "mpeg", f"Expected 'mpeg', got '{result_mpg}'"

# Canonical names should still work unchanged
result_jpeg = BedrockImageProcessor._validate_format("image/jpeg", "jpeg")
assert result_jpeg == "jpeg", f"Expected 'jpeg', got '{result_jpeg}'"

result_mpeg = BedrockImageProcessor._validate_format("video/mpeg", "mpeg")
assert result_mpeg == "mpeg", f"Expected 'mpeg', got '{result_mpeg}'"

# Unsupported formats should still raise ValueError
with pytest.raises(ValueError, match="Unsupported image format"):
BedrockImageProcessor._validate_format("image/bmp", "bmp")


def test_bedrock_get_document_format_fallback_mimes():
"""
Test the _get_document_format method with fallback MIME types for DOCX and XLSX.
Expand Down
Loading