feat: wire up video input modality for MiniMax providers - #9424
Open
octo-patch wants to merge 1 commit into
Open
feat: wire up video input modality for MiniMax providers#9424octo-patch wants to merge 1 commit into
octo-patch wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The helper
_context_contains_imageinopenai_sourcenow checks forvideo_url(andaudio_url) as well; consider renaming or generalizing it (and its callers) to something like_context_contains_multimodalto better reflect its behavior. - There are now two separate code paths that resolve video references to data URLs (
ProviderRequest.assemble_contextviaMediaResolverandProviderOpenAIOfficial._resolve_video_partviaresolve_media_ref_to_base64_data); consider consolidating this logic or sharing a common helper to avoid future divergence in preprocessing behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The helper `_context_contains_image` in `openai_source` now checks for `video_url` (and `audio_url`) as well; consider renaming or generalizing it (and its callers) to something like `_context_contains_multimodal` to better reflect its behavior.
- There are now two separate code paths that resolve video references to data URLs (`ProviderRequest.assemble_context` via `MediaResolver` and `ProviderOpenAIOfficial._resolve_video_part` via `resolve_media_ref_to_base64_data`); consider consolidating this logic or sharing a common helper to avoid future divergence in preprocessing behavior.
## Individual Comments
### Comment 1
<location path="astrbot/core/provider/sources/anthropic_source.py" line_range="295-304" />
<code_context>
+ elif part.get("type") == "video_url":
</code_context>
<issue_to_address>
**suggestion:** Dropping non-data video URLs entirely may be a surprising degradation for users.
For non-`data:` `video_url` parts, the code only logs a warning and skips the part, so the video attachment effectively disappears from the Anthropic request. This silent loss of user content can be surprising when callers send normal HTTP URLs. Please consider a more graceful degradation, e.g., inserting a `[Video Attachment]` placeholder (as with audio) or preserving the original `video_url` block so callers can see the unconverted modality is still present.
Suggested implementation:
```python
if url.startswith("data:"):
try:
header, base64_data = url.split(",", 1)
media_type = (
header.split(":", 1)[1]
)
# existing logic for decoding the data: URL and
# appending Anthropic-formatted video input continues here
else:
# For non-data URLs, gracefully degrade by inserting
# a placeholder text part instead of silently
# dropping the attachment.
# This mirrors the audio placeholder behavior.
logger.warning(
"Non-data video_url encountered; sending '[Video Attachment]' "
"placeholder text instead of video content."
)
anthropic_message["content"].append(
{
"type": "text",
"text": "[Video Attachment]",
}
)
```
I assumed:
1. `logger` is the same logger used elsewhere in this file; if it has a different name (e.g., `log`, `LOGGER`), update the `logger.warning(...)` call accordingly.
2. `anthropic_message["content"]` is the list being populated with parts in this loop; if you are collecting parts in a different list (e.g., `anthropic_parts` or `content_parts`), append the placeholder to that list instead.
3. The existing logic below `media_type = (...)` remains intact and continues to handle the `data:` URL case; ensure you keep all existing video decoding and Anthropic video-block creation code after that line.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+295
to
+304
| elif part.get("type") == "video_url": | ||
| # Convert OpenAI-style video_url format to Anthropic | ||
| # video format so models that accept video input | ||
| # actually receive the modality. | ||
| video_url_data = part.get("video_url", {}) | ||
| url = video_url_data.get("url", "") | ||
| if url.startswith("data:"): | ||
| try: | ||
| header, base64_data = url.split(",", 1) | ||
| media_type = ( |
Contributor
There was a problem hiding this comment.
suggestion: Dropping non-data video URLs entirely may be a surprising degradation for users.
For non-data: video_url parts, the code only logs a warning and skips the part, so the video attachment effectively disappears from the Anthropic request. This silent loss of user content can be surprising when callers send normal HTTP URLs. Please consider a more graceful degradation, e.g., inserting a [Video Attachment] placeholder (as with audio) or preserving the original video_url block so callers can see the unconverted modality is still present.
Suggested implementation:
if url.startswith("data:"):
try:
header, base64_data = url.split(",", 1)
media_type = (
header.split(":", 1)[1]
)
# existing logic for decoding the data: URL and
# appending Anthropic-formatted video input continues here
else:
# For non-data URLs, gracefully degrade by inserting
# a placeholder text part instead of silently
# dropping the attachment.
# This mirrors the audio placeholder behavior.
logger.warning(
"Non-data video_url encountered; sending '[Video Attachment]' "
"placeholder text instead of video content."
)
anthropic_message["content"].append(
{
"type": "text",
"text": "[Video Attachment]",
}
)I assumed:
loggeris the same logger used elsewhere in this file; if it has a different name (e.g.,log,LOGGER), update thelogger.warning(...)call accordingly.anthropic_message["content"]is the list being populated with parts in this loop; if you are collecting parts in a different list (e.g.,anthropic_partsorcontent_parts), append the placeholder to that list instead.- The existing logic below
media_type = (...)remains intact and continues to handle thedata:URL case; ensure you keep all existing video decoding and Anthropic video-block creation code after that line.
Mirror the existing image input pipeline so MiniMax providers can transmit the video input modality that MiniMax-M3 supports: - Populate ProviderRequest.video_urls from Video attachments so resolved video paths reach the provider instead of only a textual placeholder. - Resolve video_url context blocks into data URLs in the OpenAI source payload materialization. - Convert video_url blocks into Anthropic video blocks in the Anthropic source _prepare_payload so the MiniMax Token Plan provider transmits video. - Add a VideoURLPart content block type for extra user content parts. - Expose "video" as a model modality option and honor it in context assembly and modality sanitization. - Add tests covering assemble_context, modality sanitization, OpenAI payload materialization, and Anthropic payload conversion for video.
octo-patch
force-pushed
the
octo/20260728-input-capability-recvpdWH4lKOON
branch
from
July 28, 2026 06:18
68affdb to
4765763
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason: MiniMax-M3 supports video input, but AstrBot only emitted a textual placeholder for videos and never transmitted the video modality to MiniMax providers.
Changes:
ProviderRequest.video_urlsfromVideoattachments (mirroring the existing image flow) so resolved video paths reach the provider instead of only a text placeholder.video_urlcontext blocks into data URLs in the OpenAI source payload preparation, so MiniMax via the OpenAI-compatible endpoint transmits video.video_urlblocks into Anthropicvideoblocks in the Anthropic source_prepare_payload, so the MiniMax Token Plan provider transmits video.VideoURLPartcontent block type for extra user content parts, consistent withImageURLPart/AudioURLPart.videoas a model modality option and honor it during request context assembly and context sanitization by modalities.Checks:
ruff checkon all changed files.pytest tests/test_video_input_modality.py(5 passed).pytest tests/test_anthropic_kimi_code_provider.py tests/test_openai_source.py(90 passed).Summary by Sourcery
Wire video input through the provider pipeline so MiniMax and compatible providers receive actual video modality instead of text placeholders.
New Features:
Enhancements:
Tests: