Uh oh!
There was an error while loading. Please reload this page.
feat(read): attach short video files for video-capable models - #44989
Draft
mossein wants to merge 1 commit into
Draft
feat(read): attach short video files for video-capable models#44989mossein wants to merge 1 commit into
mossein wants to merge 1 commit into
Conversation
The read tool attached images and PDFs but rejected video as binary, even for models whose declared input modalities include video. The capability plumbing already existed downstream (mimeToModality maps video/*, capabilities.input.video, unsupportedParts gates on it) — only the tool refused to produce the attachment. - read: attach mp4/webm/mov up to 20 MB inline; larger clips fail with a trim-and-retry error instead of a truncated request - message-v2: video in tool results is provider-gated like other media — extracted into a user message (where unsupportedParts handles models without video) unless the provider is proven to carry inline video - media: isMedia now covers video so compaction and stripMedia replace clips with placeholders instead of silently retaining megabytes Video stays a flipbook of sampled frames server-side; audio tracks are not carried by this path. Co-Authored-By: ox-alpha
Contributor
The following comment was made by an LLM, it may be inaccurate: Duplicate PR AnalysisI found one potentially related PR that may be worth reviewing for overlap: Related PR:
The search results show that PR #44989 is quite specific in its implementation (attaching mp4/webm/mov ≤20 MB, provider-gated via modality support), and most searches return only the current PR itself, suggesting there are no direct duplicates actively in progress. Recommendation: Review #18005 to confirm there's no overlapping work, but the current PR appears to be filling a specific gap in the existing capability pipeline rather than duplicating broader feature work. |
Contributor
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Issue for this PR
Related to #10531 (video part only, this PR does not attempt audio)
Type of change
What does this PR do?
Right now the read tool rejects video files as binary, even when the model supports video input. The code for handling video already exists elsewhere in the codebase, it just never gets used because read refuses to attach the file. This PR fills that gap.
What changed:
One thing to know: tests here are unit level. I have not yet pushed a real mp4 through a live video-capable endpoint, so how OpenAI compatible providers serialize a video file part is the piece most worth reviewer attention.
Also, #18005 covers similar ground (and adds audio support, which this one does not). Happy to close this in favor of that PR or coordinate. This one is just the narrower slice.
How did you verify your code works?
Added two tests: one checking that an mp4 comes back as a proper video attachment, one checking oversized files fail with a clear error. The full read suite (41 tests) and session suite (412 tests) pass, typecheck is clean.
Screenshots / recordings
Not a UI change
Checklist