Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
|
||
|
|
||
| VideoFormat = Literal["flv", "mkv", "mov", "mpeg", "mpg", "mp4", "three_gp", "webm", "wmv"] | ||
| VideoFormat = Literal["flv", "mkv", "mov", "mpeg", "mpg", "mp4", "3gp", "three_gp", "webm", "wmv"] |
There was a problem hiding this comment.
Issue: VideoFormat now exposes both "3gp" and "three_gp", giving callers two ways to express the same format. three_gp is really the Bedrock-internal enum value, while 3gp is the user-facing extension. Exposing both publicly is a bit ambiguous.
Suggestion: Consider documenting that 3gp is the canonical input and three_gp is the Bedrock wire value (the alias map handles translation), or keep only 3gp in the public type and translate internally. Not blocking, but worth a one-line docstring note so users know which to use.
|
Assessment: Comment (approve-leaning) Well-scoped, correct fix for two deterministic provider-format bugs, with passing tests and clean lint/format. I confirmed the webp Review themes
Nice, focused bugfix that keeps the broader #2204 scope appropriately separate. |
|
Hi, could you resolve the conflict so we could have a look? |
|
@poshinchen 抱歉这条挂了两个月的冲突。今天核对 current main 准备解冲突时发现两类修复上游都已自行落地:bedrock 的 3gp/3g2/3gpp 别名映射(_BEDROCK_VIDEO_FORMAT_ALIASES,且 tests 2873 起有对应参数化用例)和 anthropic 图片媒体类型表(_IMAGE_MEDIA_TYPES,webp 覆盖,且不支持的格式显式 TypeError)。这条 PR 没有剩余增量了,主动关闭。媒体字面量放宽(jpg 别名)那条上游选择了严格报错的设计,我尊重这个取舍。 |
Fixes part of #2204.
Summary
webp, instead of relying only on platformmimetypes3gpas a video input format alias and map it to Bedrock'sthree_gpenum value when formatting requestsI kept this PR scoped to the two deterministic provider-format bugs. The broader type/guardContent expansion mentioned in the issue should be handled separately.
To verify
python -m pytest tests/strands/models/test_anthropic.py::test_format_request_with_image tests/strands/models/test_anthropic.py::test_format_request_with_webp_image_uses_explicit_media_type tests/strands/models/test_bedrock.py::test_format_request_video_s3_location tests/strands/models/test_bedrock.py::test_format_request_video_3gp_maps_bedrock_enum -qpython -m ruff check src/strands/models/anthropic.py src/strands/models/bedrock.py src/strands/types/media.py tests/strands/models/test_anthropic.py tests/strands/models/test_bedrock.pypython -m ruff format --check src/strands/models/anthropic.py src/strands/models/bedrock.py src/strands/types/media.py tests/strands/models/test_anthropic.py tests/strands/models/test_bedrock.py