Gemini Veo3.1[AI Studio]增加图生视频支持 - #2315
Conversation
WalkthroughRefactored Gemini video generation adapter types and payloads: renamed Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
relay/channel/task/gemini/adaptor.go (1)
310-315: Data URL parsing may silently produce incorrect results.If a data URL doesn't contain
;base64,(e.g.,data:image/png,<raw-data>), the split produces only one part butimgretains the full data URL asBytesBase64Encoded, which is invalid.Consider adding validation:
if strings.HasPrefix(imageStr, "data:image/") { parts := strings.Split(imageStr, ";base64,") if len(parts) == 2 { img.MimeType = strings.TrimPrefix(parts[0], "data:") img.BytesBase64Encoded = parts[1] + } else { + // Invalid data URL format, return nil or error + return nil } + return img }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
relay/channel/task/gemini/adaptor.go(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-08T17:12:43.157Z
Learnt from: RedwindA
Repo: QuantumNous/new-api PR: 1537
File: relay/gemini_handler.go:330-342
Timestamp: 2025-08-08T17:12:43.157Z
Learning: In the new-api repository, the `GeminiEmbeddingHandler` function in `relay/gemini_handler.go` is designed specifically for native Gemini embedding requests and therefore does not require the `ConvertGeminiRequest` step that is used in the chat handler. The embedding requests are already in the native Gemini format and don't need conversion.
Applied to files:
relay/channel/task/gemini/adaptor.go
📚 Learning: 2025-08-26T09:59:00.337Z
Learnt from: Sh1n3zZ
Repo: QuantumNous/new-api PR: 1659
File: relay/relay_task.go:285-305
Timestamp: 2025-08-26T09:59:00.337Z
Learning: In controller/task_video.go, the redactVideoResponseBody function sanitizes video task responses by removing bytesBase64Encoded fields and truncating base64 strings to 256 characters to prevent large binary data from being stored in task.Data.
Applied to files:
relay/channel/task/gemini/adaptor.go
🧬 Code graph analysis (1)
relay/channel/task/gemini/adaptor.go (5)
relay/common/relay_utils.go (1)
ValidateBasicTaskRequest(204-228)constant/task.go (1)
TaskActionGenerate(14-14)model/task.go (2)
TaskStatusFailure(37-37)TaskStatusSuccess(38-38)setting/system_setting/system_setting_old.go (1)
ServerAddress(3-3)service/image.go (1)
GetImageFromUrl(69-117)
🔇 Additional comments (3)
relay/channel/task/gemini/adaptor.go (3)
27-52: LGTM!The type definitions are well-structured. Using pointers for optional
ImageandLastFramefields ensures proper JSON serialization withomitempty.
80-82: LGTM!Adding
RaiMediaFilteredCountandRaiMediaFilteredReasonsfields enables proper failure reporting when content is filtered by Gemini's RAI system.
106-108: The repository clone operation failed, which prevents me from executing shell scripts to verify the constant definitions and usage patterns in the codebase.To complete the verification, I need you to provide one of the following:
Run the shell script provided in the review comment on your local environment and share the output:
# Verify usage of TaskActionGenerate vs TaskActionTextGenerate for video tasks echo "=== Checking TaskActionGenerate usage ===" rg -n "TaskActionGenerate" --type go -C2 echo "" echo "=== Checking TaskActionTextGenerate usage ===" rg -n "TaskActionTextGenerate" --type go -C2Alternatively, provide context about:
- Where
TaskActionGenerateandTaskActionTextGenerateare defined (typically inconstant/task.gobased on the review comment)- How these constants are used in other task adaptor files
- Whether other video-related task handlers use
TaskActionGenerateorTaskActionTextGenerateOnce I have this information, I can complete the verification and provide the rewritten review comment.
| if len(req.Images) > 0 { | ||
| body.Instances[0].Image = a.convertImage(req.Images[0]) | ||
| } | ||
| if len(req.Images) > 1 { | ||
| body.Instances[0].LastFrame = a.convertImage(req.Images[1]) | ||
| } |
There was a problem hiding this comment.
Silent failure when image fetch fails.
convertImage silently returns an Image with the original URL string as BytesBase64Encoded when service.GetImageFromUrl fails. This will likely cause the upstream API to reject the request with a confusing error.
Consider returning an error from convertImage or at least logging the failure:
-func (a *TaskAdaptor) convertImage(imageStr string) *Image {
+func (a *TaskAdaptor) convertImage(imageStr string) (*Image, error) {
if strings.TrimSpace(imageStr) == "" {
- return nil
+ return nil, nil
}
- img := &Image{
- MimeType: "image/png",
- BytesBase64Encoded: imageStr,
- }
if strings.HasPrefix(imageStr, "data:image/") {
parts := strings.Split(imageStr, ";base64,")
if len(parts) == 2 {
- img.MimeType = strings.TrimPrefix(parts[0], "data:")
- img.BytesBase64Encoded = parts[1]
+ return &Image{
+ MimeType: strings.TrimPrefix(parts[0], "data:"),
+ BytesBase64Encoded: parts[1],
+ }, nil
}
+ return nil, fmt.Errorf("invalid data URL format")
} else if strings.HasPrefix(imageStr, "http") {
mimeType, data, err := service.GetImageFromUrl(imageStr)
- if err == nil {
- img.MimeType = mimeType
- img.BytesBase64Encoded = data
+ if err != nil {
+ return nil, fmt.Errorf("fetch image failed: %w", err)
}
+ return &Image{MimeType: mimeType, BytesBase64Encoded: data}, nil
}
- return img
+ // Assume raw base64
+ return &Image{MimeType: "image/png", BytesBase64Encoded: imageStr}, nil
}Then update BuildRequestBody to handle the error:
if len(req.Images) > 0 {
- body.Instances[0].Image = a.convertImage(req.Images[0])
+ img, err := a.convertImage(req.Images[0])
+ if err != nil {
+ return nil, errors.Wrap(err, "convert image failed")
+ }
+ body.Instances[0].Image = img
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In relay/channel/task/gemini/adaptor.go around lines 150-155, convertImage
currently swallows failures from service.GetImageFromUrl and returns an Image
with the original URL in BytesBase64Encoded causing upstream rejections; change
convertImage to return (Image, error) (or at minimum log the specific fetch
error), propagate and handle that error in BuildRequestBody so you either return
the error to the caller or skip/omit the image field when fetching fails, and
update the two call sites in BuildRequestBody to check the error and act
accordingly (return a wrapped error with context or skip setting
body.Instances[0].Image/LastFrame).
| if len(op.Response.GenerateVideoResponse.GeneratedSamples) == 0 { | ||
| ti.Status = model.TaskStatusFailure | ||
| ti.Reason = fmt.Sprintf("no generated video url found: %s", strings.Join(op.Response.GenerateVideoResponse.RaiMediaFilteredReasons, "; ")) | ||
| } else { | ||
| if uri := op.Response.GenerateVideoResponse.GeneratedSamples[0].Video.URI; uri != "" { | ||
| ti.RemoteUrl = uri | ||
| } | ||
| ti.Status = model.TaskStatusSuccess | ||
| } | ||
|
|
||
| ti.Progress = "100%" | ||
| taskID := encodeLocalTaskID(op.Name) | ||
| ti.TaskID = taskID | ||
| ti.Url = fmt.Sprintf("%s/v1/videos/%s/content", system_setting.ServerAddress, taskID) | ||
| return ti, nil |
There was a problem hiding this comment.
URL is set even when generation fails.
When no GeneratedSamples exist (RAI filtered), the status is set to FAILURE but ti.Url is still populated on line 273. This URL would point to non-existent content, potentially confusing clients.
Consider only setting the URL on success:
if len(op.Response.GenerateVideoResponse.GeneratedSamples) == 0 {
ti.Status = model.TaskStatusFailure
ti.Reason = fmt.Sprintf("no generated video url found: %s", strings.Join(op.Response.GenerateVideoResponse.RaiMediaFilteredReasons, "; "))
+ ti.Progress = "100%"
} else {
if uri := op.Response.GenerateVideoResponse.GeneratedSamples[0].Video.URI; uri != "" {
ti.RemoteUrl = uri
}
ti.Status = model.TaskStatusSuccess
+ ti.Progress = "100%"
+ taskID := encodeLocalTaskID(op.Name)
+ ti.TaskID = taskID
+ ti.Url = fmt.Sprintf("%s/v1/videos/%s/content", system_setting.ServerAddress, taskID)
}
- ti.Progress = "100%"
- taskID := encodeLocalTaskID(op.Name)
- ti.TaskID = taskID
- ti.Url = fmt.Sprintf("%s/v1/videos/%s/content", system_setting.ServerAddress, taskID)
return ti, nil📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if len(op.Response.GenerateVideoResponse.GeneratedSamples) == 0 { | |
| ti.Status = model.TaskStatusFailure | |
| ti.Reason = fmt.Sprintf("no generated video url found: %s", strings.Join(op.Response.GenerateVideoResponse.RaiMediaFilteredReasons, "; ")) | |
| } else { | |
| if uri := op.Response.GenerateVideoResponse.GeneratedSamples[0].Video.URI; uri != "" { | |
| ti.RemoteUrl = uri | |
| } | |
| ti.Status = model.TaskStatusSuccess | |
| } | |
| ti.Progress = "100%" | |
| taskID := encodeLocalTaskID(op.Name) | |
| ti.TaskID = taskID | |
| ti.Url = fmt.Sprintf("%s/v1/videos/%s/content", system_setting.ServerAddress, taskID) | |
| return ti, nil | |
| if len(op.Response.GenerateVideoResponse.GeneratedSamples) == 0 { | |
| ti.Status = model.TaskStatusFailure | |
| ti.Reason = fmt.Sprintf("no generated video url found: %s", strings.Join(op.Response.GenerateVideoResponse.RaiMediaFilteredReasons, "; ")) | |
| ti.Progress = "100%" | |
| } else { | |
| if uri := op.Response.GenerateVideoResponse.GeneratedSamples[0].Video.URI; uri != "" { | |
| ti.RemoteUrl = uri | |
| } | |
| ti.Status = model.TaskStatusSuccess | |
| ti.Progress = "100%" | |
| taskID := encodeLocalTaskID(op.Name) | |
| ti.TaskID = taskID | |
| ti.Url = fmt.Sprintf("%s/v1/videos/%s/content", system_setting.ServerAddress, taskID) | |
| } | |
| return ti, nil |
🤖 Prompt for AI Agents
In relay/channel/task/gemini/adaptor.go around lines 261 to 274, the code sets
ti.Url unconditionally even when generation failed; move the ti.Url assignment
so it is only set when the task status is success (i.e., inside the branch where
GeneratedSamples exist and after setting ti.TaskID), and ensure that on failure
ti.Url is left empty (or explicitly cleared) so clients won't receive a pointer
to non-existent content.
…-i2v Gemini Veo3.1[AI Studio]增加图生视频支持
请求示例
返回结果
Summary by CodeRabbit
Release Notes
Refactor
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.