Skip to content

feat(Bedrock): add image support to Bedrock#1874

Merged
jxnl merged 1 commit into567-labs:mainfrom
geekbass:geekbass/add-image-support-bedrock
Oct 29, 2025
Merged

feat(Bedrock): add image support to Bedrock#1874
jxnl merged 1 commit into567-labs:mainfrom
geekbass:geekbass/add-image-support-bedrock

Conversation

@geekbass
Copy link
Copy Markdown
Contributor

@geekbass geekbass commented Oct 29, 2025

Important

Adds image support to Bedrock by converting OpenAI-style image parts to Bedrock format and updates related utilities and tests.

  • Behavior:
    • Adds _normalize_bedrock_image_format() to map image types to Bedrock's format in utils.py.
    • Adds _openai_image_part_to_bedrock() to convert OpenAI-style image parts to Bedrock format in utils.py.
    • Updates _prepare_bedrock_converse_kwargs_internal() to handle image content conversion in utils.py.
  • Tests:
    • Adds test_bedrock_native_passthrough.py to test Bedrock-native text and image passthrough.
    • Adds test_normalize.py to test image format normalization.
    • Adds test_openai_image_conversion.py to test OpenAI image part conversion.
    • Adds test_prepare_kwargs.py to test preparation of Bedrock API call arguments with images.
    • Adds fixtures in conftest.py for image data used in tests.

This description was created by Ellipsis for f108825. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Copy Markdown
Contributor

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to f108825 in 1 minute and 7 seconds. Click for details.
  • Reviewed 376 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. instructor/providers/bedrock/utils.py:135
  • Draft comment:
    Defaulting to 'jpeg' for unsupported mime types is intentional; consider logging a warning for unexpected mime types for easier debugging in the future.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 85% None
2. instructor/providers/bedrock/utils.py:170
  • Draft comment:
    Error handling for data URLs is clear; optionally, include the invalid input value in the error message for enhanced troubleshooting.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 85% None
3. instructor/providers/bedrock/utils.py:181
  • Draft comment:
    Good use of specific exception handling for HTTP requests; consider implementing a retry mechanism or centralizing HTTP calls if image fetching failures become common in production.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 85% None
4. tests/llm/test_bedrock/test_openai_image_conversion.py:20
  • Draft comment:
    This test depends on an external URL, which may lead to flaky tests. Consider mocking the HTTP request to simulate image fetching reliably.
  • Reason this comment was not posted:
    Confidence changes required: 66% <= threshold 85% None
5. tests/llm/test_bedrock/test_openai_image_conversion.py:30
  • Draft comment:
    Consider adding tests for unsupported content types in _to_bedrock_content_items to further validate error handling robustness.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 85% None

Workflow ID: wflow_D1FCx1OBWCObmP6U

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@geekbass geekbass closed this Oct 29, 2025
@geekbass geekbass reopened this Oct 29, 2025
@geekbass
Copy link
Copy Markdown
Contributor Author

Sorry I'm making this a mess but I'm not getting the PR template.

@jxnl jxnl merged commit d50f388 into 567-labs:main Oct 29, 2025
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