Repository navigation
fix(gemini): gate video injection on the declared modality - #11019
Open
JosephTian876 wants to merge 6 commits into
Open
JosephTian876 wants to merge 6 commits into
JosephTian876 wants to merge 6 commits into
Conversation
Add ProviderRequest.video_urls so providers that declare the video modality receive the video itself instead of only a text placeholder. Fixes two failure modes that a video block would otherwise introduce: - gemini_source._prepare_conversation dispatched user content with a catch-all else that read item["audio_url"]. A video_url block raised KeyError outside the try that guards the API call, and because the block was persisted the error repeated on every later request. The branch is now exhaustive and unknown block types degrade to text. - assemble_context output is replayed from history on every request, so video blocks are now flagged _no_save and dropped when the message is written back. Video is opt-in: an empty modalities list keeps its existing "supports everything" meaning for text/image/audio but not for video, so existing configurations are unaffected.
Degrade `video_url` for an unconfigured provider on the fallback path. `sanitize_contexts_by_modalities` treats empty or missing `modalities` as "supports everything" for backward compatibility, but video is strictly opt-in. Since that function also sanitizes an already-assembled context when a request falls back to another provider, an unconfigured fallback could receive a `video_url` block that the primary provider produced, and fail the whole request instead of getting `[Video]`. Handle a `VideoURLPart` passed through `extra_user_content_parts` the same way as `video_urls`: resolve it to a portable data URI and flag it transient. Without that the block was persisted verbatim, so a data-URI video stayed in the conversation history as a multi-megabyte payload and a local path was replayed after its file was gone. A message left with nothing but transient blocks now keeps a text placeholder so it cannot persist as empty content. The unconfigured provider keeps receiving its original context objects when there is no video block to degrade, so the sanitizer is not entered and nothing else about that path changes.
Providers without native video input fell through to the catch-all else: raise ValueError(...) in their extra_user_content_parts loop, so a VideoURLPart raised instead of degrading. Video is opt-in and those APIs cannot accept it, so emit a [Video] text placeholder like every other unsupported modality and never persist the payload.
AstrBotDevs#10934 moved the main agent into the `astrbot/core/astr_main_agent` package, so the single-file module this feature originally edited no longer exists and the ported lines now live in `media.py` and `request.py`. Pin both ports: - `append_video_attachment` must collect the resolved path into `req.video_urls`, otherwise providers that declare the `video` modality never receive the video and the feature regresses silently. - the single-file `astr_main_agent.py` must never come back next to the package. Python resolves the package first, so a resurrected file would be imported by nobody and edits landing in it would silently do nothing.
`text_chat(prompt=...)` assembles its user message with `assemble_context`, whose audio slot is the only media channel some callers expose. A video handed over that way was resolved with `media_type="audio"`, which re-encodes the payload to WAV: a video with a soundtrack reached Gemini as one audio part and lost its picture, and a video without one failed the conversion and was dropped without a replacement. Recognise the media family of an audio reference before converting it and route videos to the `video_url` channel this path already has, so `_prepare_conversation` builds an inline data part from the original container. A video that cannot be resolved now leaves a `[Video]` placeholder instead of disappearing. Audio references keep their existing WAV path.
`text_chat(prompt=...)` builds its user message in the provider's own `assemble_context`, so it never passes the agent runner's modality gate. The video that 750502dc0 routes out of the audio slot therefore reached providers that never declared the `video` modality and failed the request with an API error instead of degrading. `resolve_video_part` now returns a `[Video]` text block unless the provider declares `video` in `modalities`, the same opt-in rule the runner applies to `ProviderRequest.video_urls` and the same placeholder it substitutes. The decision is made before the media is resolved, so an unsupported reference costs neither a base64 encode nor an ffmpeg run. Tests cover both media channels this path exposes, including an unconfigured capability list.
Contributor
|
Sorry @JosephTian876, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 1 day and 12 hours by commenting |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This branch has not been deployed
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.
fix(gemini): gate video injection on the declared modality
Background
ProviderGoogleGenAIdelivers a video as an inline part as soon as it recognises one.The first change in this series extended that recognition to a video that arrives
through the legacy audio slot (
data:video/mp4;…or a saved.mp4), but the deliverystayed unconditional, while every other route into the provider treats video as
opt-in through the provider's
modalitieslist.A Gemini model that cannot accept inline video therefore received one and failed the
whole request with an API error, instead of receiving
[Video]text the way the agentpath does.
This change is a follow-up addition to that same media-family fix: it keeps the
routing the first change introduced and adds the missing capability check on the one
path that builds its user message inside the provider.
Changes
astrbot/core/provider/sources/gemini_source.pyresolve_video_partnow decides before it resolves anything. Two additions do that.The flag, read once at the top of
assemble_context(gemini_source.py:1156-1163):The early return, at the head of
resolve_video_part(gemini_source.py:1211-1235):resolve_video_partis the only place on this path that produces a video block, andall three video entries go through it: the audio slot (
gemini_source.py:1186), aVideoURLPartextra part (:1269), and thevideo_urlsargument (:1292). Gatingthere covers the legacy path completely and applies the same rule as the runner:
videohas to be declared, so an empty or missingmodalitieslist means "notdeclared" and degrades.
The placeholder is decided before the media is read, so an unsupported reference costs
neither a base64 encode nor an ffmpeg run.
[Video]is the same text the runner's gatesubstitutes (
tool_loop_agent_runner.py:388-390) and the same text theunconfigured-provider sanitizer uses (
modalities.py:97and:165).The comment above the check records why this duplication exists: the method is reached
through
text_chat(prompt=…), which bypasses the runner, so the rule has to be appliedhere as well.
Why
The legacy shape is
Context.llm_generate, which calls the provider directly:astrbot/core/star/context.py:217text_chat(prompt=…)assembles a user messagegemini_source.py:972→:993-994assemble_contextresolves the audio slotgemini_source.py:1146→resolve_audio_part:1178media_utils.py:408, called atgemini_source.py:1185video_urlblock is producedgemini_source.py:1210→:1232-1235gemini_source.py:1021gemini_source.py:386→process_video_url:417, dispatch:458No modality check sits anywhere between step 5 and step 7:
sanitize_contexts_by_modalities(astrbot/core/provider/modalities.py:66) hasexactly two callers:
tool_loop_agent_runner.py:693andastrbot/core/agent/context/compressor.py:268. Both belong to the agent runner,which this path never enters.
_assemble_request_context_for_provider(
tool_loop_agent_runner.py:346-392), only filtersrequest.video_urlsbeforecalling
ProviderRequest.assemble_context(entities.py:303) — a different functionfrom the provider's
assemble_context. The runner also never passesprompt=(
tool_loop_agent_runner.py:529-539), so the branch that produces the block is notreachable from it in the first place.
gemini_source.pyreads no input modality list. Everymodalitiesoccurrence there(
:233,:243,:376,:728,:799) is a Gemini response modality(
TEXT/IMAGE).text_chatretries anAPIErrorby rotating API keys only (:1039), with nofallback provider and no context sanitization.
A video produced by the legacy path was therefore injected as-is under every provider
configuration. Applying the check at the single block-producing function is also what
keeps the change small: it needs no new parameter on
assemble_context, no change tothe modality policy, and no change to the runner.
Tests
tests/unit/test_gemini_legacy_media_kind.pynow has 9 tests and 21 cases (was 7 and18):
_provider()declares every modality by default, so the media-family tests keeptesting the routing of a provider that opted into video;
test_video_in_the_audio_slot_degrades_without_the_video_modality— two cases (anempty
modalitieslist, and a list withoutvideo): the video is replaced by{"type": "text", "text": "[Video]"}, novideo_urlblock is produced, and themedia resolver is never awaited, which also proves the payload cannot be re-encoded
to WAV on the way out;
test_video_url_argument_degrades_without_the_video_modality— the separatevideo_urlschannel follows the same rule.python -m pytest tests/unit/test_gemini_legacy_media_kind.py \ tests/test_video_input_modality.py \ -q -n 4 -p no:cacheprovider # 44 passed, 4 warnings (41 before this change)Gemini and media regression surface — 169 passed:
python -m pytest tests/test_gemini_source.py tests/test_media_utils.py \ tests/test_agent_runner_media_resolver.py \ tests/test_model_image_preparation.py -q -n 4 -p no:cacheproviderFull unit suite:
python -m pytest tests/unit -q -p no:cacheprovider # 4 failed, 2193 passed, 23 skipped2193 = 2190 + 3new cases. The 4 failures are the parametrizations oftests/unit/test_wakeup_pipeline.py::test_wakeup_uses_current_context_and_sends_final_reply(state leakage between unit tests); the same 4 fail on the preceding change in this
series (
4 failed, 2190 passed) and are unrelated to this change.Compatibility
videokeep receiving an inline video on every entrypoint; nothing changes for them.
videonow get[Video]text instead of a requestthat fails at the API. This matches what the agent runner already does for
ProviderRequest.video_urlsand what the unconfigured-provider sanitizer alreadydoes for a replayed history.
modalitiescheckbox list — the sameswitch the runner path has always required.
resolve_video_part.text_chat,text_chat_stream, andassemble_contextkeeptheir signatures, and no dashboard, i18n, or generated asset is touched.
Privacy and delivery
returned before resolution, so an unsupported reference is never downloaded, read,
or base64-encoded, and no temporary file is created for it.
log line; the
[Video]placeholder carries no path.the tests use synthetic bytes and temporary files.
no configuration change: an existing provider behaves exactly as before until an
operator ticks
video, and behaviour for audio and images is byte-for-byteunchanged.
Suggested merge order
This is the second half of one change: the first half makes the legacy path recognise
a video in the audio slot and deliver it, and this half makes that delivery respect the
declared modality. Reviewing them in order keeps each diff small and each step
independently testable.
VideoURLPart, thevideomodality,the
video_urlstransport, andprocess_video_urlit to that channel
Base — this pull request is stacked. Open it against the branch that carries the
media-family fix (
fix/video-not-audio-in-legacy-path-k2), not againstmaster. Thatbranch is reachable here, so the diff shows only the gate — 1 commit, 2 files, +71 / −3
— instead of repeating the previous change for review. With
masteras the base thepull request would expose both commits and could not be reviewed on its own, and it
would not even apply before the media-family fix. Merge order: the native video input
modality change, then the media-family fix, then this change; retarget to
masterafter those two have landed.
If the series is landed as a single PR, this change is the second commit of the
media-family fix and needs no separate review cycle; if it is landed as separate PRs,
it has to follow the two above, because the function it guards does not exist before
them.
The OpenAI and Anthropic sources are unaffected by this change: their legacy
audio_urlsbranches still do not recognise video at all, and they remain a follow-up.Summary by Sourcery
Support video attachments end to end while requiring explicit provider video capability and safely degrading unsupported inputs.
New Features:
Bug Fixes:
[Video]placeholder rather than sending unsupported payloads or failing requests.Enhancements:
Tests: