fix: accept URL-string image input on the Gemini Veo video route (NOL-252) - #34
Conversation
This comment has been minimized.
This comment has been minimized.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68f983ef82
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def _image_string_to_gemini_format(image: str) -> Dict[str, str]: | ||
| if image.startswith(("http://", "https://")): | ||
| base64_data, mime_type = fetch_image_as_base64(image) |
There was a problem hiding this comment.
When an authenticated proxy caller supplies input_reference as a URL, this new branch performs a server-side GET without the repository's SSRF validation, allowing requests to loopback, cloud metadata, or other private-network services (including through redirects). Use litellm.litellm_core_utils.url_utils.safe_get, which validates every redirect hop, rather than routing the user-controlled URL directly through module_level_client.get.
Useful? React with 👍 / 👎.
|
|
||
| def _image_string_to_gemini_format(image: str) -> Dict[str, str]: | ||
| if image.startswith(("http://", "https://")): | ||
| base64_data, mime_type = fetch_image_as_base64(image) |
There was a problem hiding this comment.
When the signed image URL returns a generic Content-Type such as application/octet-stream, or omits the header for a non-JPEG image, this new input_reference path forwards the generic type or labels the content as JPEG even when the downloaded bytes are PNG/WebP. The resulting instances[0].image.mimeType can therefore be unsupported or disagree with the payload and cause Veo to reject an otherwise valid reference image; detect the image type from response.content as the file-like path already does rather than trusting the response header.
Useful? React with 👍 / 👎.
|
|
||
| def _image_string_to_gemini_format(image: str) -> Dict[str, str]: | ||
| if image.startswith(("http://", "https://")): | ||
| base64_data, mime_type = fetch_image_as_base64(image) |
There was a problem hiding this comment.
For avideo_generation and the proxy video route, async_video_generation_handler invokes this synchronous transform directly on the event-loop thread before reaching its asynchronous provider POST. A slow or unresponsive input_reference URL therefore blocks the shared event loop inside module_level_client.get, delaying unrelated proxy requests for the duration of the download; prefetch the image with an async client or offload the synchronous fetch to a worker thread.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The blocking fetch is a pre-existing property of the shared sync transform_video_create_request path (the image_url/image_urls branches already download the same way); making it non-blocking requires offloading the transform in llm_http_handler.async_video_generation_handler for all video providers, which is out of scope for this fix.
…-252)
nolgia-api mirrors the start frame into `input_reference` on every
image-to-video submission (a signed HTTPS URL). `map_openai_params` maps
`input_reference` -> `image`, and `transform_video_create_request` handed
that string straight to `_convert_image_to_gemini_format`, which calls
`.read()` on it:
File "litellm/llms/gemini/videos/transformation.py", line 67
image_bytes = image_file.read()
AttributeError: 'str' object has no attribute 'read'
The request died inside the proxy before any upstream call, surfaced as
APIConnectionError -> HTTP 400, which the platform re-emitted to callers
as a generic 422. Every Veo 3.1 reference-image run was rejected this way
and fell back to Kling; the failure was misread as Veo refusing reference
images, when Google was never contacted at all.
The sibling `image_url` branch in this same transform already downloads a
URL and inlines it as base64, so an http(s) string image is now deferred
to it instead of the file-object encoder. A string that is not an http(s)
URL raises a clear ValueError rather than an opaque AttributeError,
mirroring the Vertex video config, which already guards this case. Dict
and file-like inputs are untouched.
Tests cover the production payload shape: `input_reference` as a signed
URL, the start frame mirrored into both `input_reference` and `image_url`
(downloaded once) alongside three `image_urls` reference images, and the
non-URL string error path. All mocked - no live provider calls.
ca668ea to
85e29e3
Compare
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Base moved four commits ahead (MiniMax video/image providers #31, the blind-catch narrowing #33, and the two Gemini Veo route fixes #34/#35). Everything auto-merged except tests/test_litellm/interactions/test_openapi_compliance.py, where both sides had independently loosened the same Content-discriminator assertion after Google's spec dropped the keyword: base via #31, upstream via BerriAI#35161. Kept upstream's version, which is a strict superset (it accepts a discriminator mapping *or* a per-variant `type` const/1-item enum, asserts the values are distinct, and pins TextContent to "text") and matches the `_declared_type_value` helper already in the file. Budget ceilings: LIT002 (27511 -> 27678) and TRY004 (98 -> 100) were the only two rules over limit on the merged tree with a count above the base, so the gates would have failed. The merge adds no net-new violations: every file's LIT002 count in the merged tree equals one of its two parents, so the overage is purely the union of ceilings both sides had ratcheted down independently since the branch point. Raised those two to the merged tree's actual counts; every other ratchet is untouched. Also regenerated model_prices_and_context_window.schema.json, which the new upstream sync check flagged as stale (missing `output_cost_per_audio`) already before this merge. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
1 participant
TLDR
Problem this solves:
input_reference.GeminiVideoConfig.map_openai_paramsmapsinput_referencetoimage, andtransform_video_create_requestpassed that string to_convert_image_to_gemini_format, which expects a file-like object and calls.read()on itHow it solves it:
http(s)stringimageis now handed to theimage_urlbranch that already lives in the same transform, which downloads it and inlines it asbytesBase64Encodedhttp(s)URL raises aValueErrornaming the problem instead of an opaqueAttributeError; the Vertex video config already guards this same caseimageinputs are untouchedThe traceback below is verbatim from the logged upstream response body in production:
That surfaced as
APIConnectionError, then HTTP 400 from the proxy, which the platform re-emitted to its own callers as a generic 422. Kling was unaffected because the fal transform accepts a URL verbatimRelevant issues
Linear ticket
Resolves NOL-252
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
A real-provider run is deliberately not included. The organization is under a hard spend freeze on generation calls, so this was proven against a local proxy pointed at a stub upstream on
127.0.0.1:9099that records the request body Veo would have received. The runbook for a maintainer to repeat it against real Veo is in the QA runbook section belowProxy started from this branch with a config whose only model is
veo-3.1-fastmapped togemini/veo-3.1-fast-generate-previewwithapi_base: http://127.0.0.1:9099Output on this branch:
The body the proxy sent upstream, as recorded by the stub:
The start frame arrives inline as base64 with a sniffed MIME type, and the two reference images land on the instance as
referenceImages. Onlitellm_internal_stagingthe same request never reaches the upstream at all; it raises theAttributeErrorshown above during transformationOne thing this makes visible:
generate_audio: truewent in and does not appear inparameters.GeminiVideoGenerationParametersdeclares no such field and ignores extras, so it is silently dropped.personGenerationis likewise never populated, though the field's own docstring says image-bearing Veo 3.x runs acceptallow_adultonly. Both are out of scope here because they change request semantics rather than fixing a crash, and both are tracked separatelyType
Bug Fix
Changes
litellm/llms/gemini/videos/transformation.pygains a string branch in theimagehandling oftransform_video_create_request. There is no new helper and no new dict construction, so the type-discipline budget is unchangedtests/test_litellm/llms/gemini/videos/test_gemini_video_transformation.pygains three regression tests, all with mocked HTTP and no live provider calls. The first sendsinput_referenceas a signed URL throughmap_openai_paramsinto the transform and asserts it lands as inline base64. The second reproduces the exact production payload, where the start frame is mirrored into bothinput_referenceandimage_urlalongside threeimage_urlsreferences, and asserts the start frame is downloaded exactly once while the references becomereferenceImages. The third asserts the non-URL string path raisesValueErrorThe tests were verified to be load-bearing: with the source change reverted, all three fail with the same
AttributeErrorattransformation.py:67seen in productionQA runbook
To repeat the proof against real Veo rather than the stub, which does cost money:
GEMINI_API_KEYin.envand drop theapi_baseoverride so the route resolves tohttps://generativelanguage.googleapis.compython litellm/proxy/proxy_cli.py --config litellm/proxy/dev_config.yaml --detailed_debug --reload --use_v2_migration_resolver 2>&1 | tee litellm.loginput_referenceset to any publicly reachable image URL, droppingimage_urlsfor the cheapest possible checkvideo_id andstatus: processing, then pollGET /v1/videos/{id}untilcompletedlitellm.logthat the outbound instance carriesimage.bytesBase64Encodedand that noAttributeErrorappearsNote that step 4 is the first genuine Google response this route will ever have received, so a provider-side rejection there would be new information rather than a regression from this PR
Final Attestation