feat: zhipu 4v image generations - #2356
Conversation
WalkthroughAdds zhipu_4v image-generation support: extends ImageRequest DTO with three json.RawMessage fields, implements adaptor routing and URL handling for the images/generations endpoint, and adds a handler that converts zhipu image responses into an OpenAI-compatible payload. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Adaptor as zhipu_4v Adaptor
participant Zhipu as Zhipu API
participant Handler as zhipu4vImageHandler
participant ClientResp as OpenAI-format Response
Client->>Adaptor: POST ImageRequest
Adaptor->>Adaptor: ConvertImageRequest()
Adaptor->>Zhipu: POST /api/paas/v4/images/generations (zhipu format)
Zhipu-->>Handler: HTTP response (data[], created, errors?)
Handler->>Handler: Validate response, pick timestamp
Handler->>Handler: For each item: use B64Json/B64Image or fetch URL → base64
Handler->>Handler: Build OpenAI-style payload (created, data[])
Handler-->>ClientResp: JSON payload
ClientResp-->>Client: HTTP 200 with OpenAI-compatible body
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
dto/openai_image.go (1)
30-34: AlignUserIdnaming with Go conventions / other structsFunctionally this is fine and integrates cleanly with the existing custom JSON (un)marshal logic. The only nit is naming:
UserIdhere vsUserIDinzhipuImageRequest(and typical Go style). Renaming this field toUserID(keeping thejson:"user_id"tag) would avoid confusion and keep things consistent across DTOs while not affecting JSON compatibility.relay/channel/zhipu_4v/adaptor.go (2)
39-52: Clarifywatermark_enabledparsing and fallback behaviorThe fallback from
WatermarkEnabled→Watermarkis reasonable, but note the exact semantics:
- If
watermark_enabledis invalid JSON for a bool (e.g."foo"ornull),parseOptionalBoolreturns an error,enabledstaysnil, and you fall through to theWatermarkfield when it’s set.- That means malformed
watermark_enabledis silently ignored instead of surfacing a client error.If that’s intentional (best-effort compatibility), this is fine. If you’d rather fail fast on bad types, you may want to treat
err != nilas a hard error instead of quietly falling back.
39-56: Confirm whether zhipu image API needs additional fields (e.g.n,image)Right now
zhipuImageRequestonly receivesModel,Prompt,Quality,Size,WatermarkEnabled, andUserID. Fields likeNorImagefromdto.ImageRequestaren’t forwarded at all.If zhipu’s image generation API truly ignores those concepts, this is fine. If it supports multiple outputs or an input image, it might be worth wiring them through now to avoid a follow-up change.
relay/channel/zhipu_4v/image.go (2)
27-35: Propagate providerusagewhen available
zhipuImageResponsedefines aUsage *dto.Usage, butzhipu4vImageHandleralways returns&dto.Usage{}regardless of what the provider sends:// ... service.IOCopyBytesGracefully(c, resp, jsonResp) return &dto.Usage{}, nilIf upstream is already giving you a well-shaped
Usage, it’s better to pass it through so accounting/metrics remain accurate. For example:service.IOCopyBytesGracefully(c, resp, jsonResp) - - return &dto.Usage{}, nil + if zhipuResp.Usage != nil { + return zhipuResp.Usage, nil + } + return &dto.Usage{}, nilThis keeps the current behavior as a fallback when
Usageis missing.Also applies to: 120-128
130-150: Optional: treat JSONnullas “unset” in optional parsers
parseOptionalBool/parseOptionalStringtreat onlylen(raw) == 0as “absent”; a literalnullcausescommon.Unmarshalto fail. Combined with the adaptor swallowing errors for these helpers,nullbehaves differently from “missing” in a slightly surprising way.Not a blocker, but if zhipu (or clients) ever send
nullexplicitly, you may want to normalize it to “unset” instead of an error, e.g.:if len(raw) == 0 || string(raw) == "null" { return nil, nil // or "" for the string case }This keeps the helpers’ behavior closer to typical JSON optional-field semantics.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
dto/openai_image.go(1 hunks)relay/channel/zhipu_4v/adaptor.go(3 hunks)relay/channel/zhipu_4v/image.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
relay/channel/zhipu_4v/adaptor.go (1)
relay/constant/relay_mode.go (1)
RelayModeImagesGenerations(14-14)
🔇 Additional comments (2)
relay/channel/zhipu_4v/adaptor.go (2)
83-84: New images/generations URL path looks consistentThe new
RelayModeImagesGenerationsbranch follows the same pattern as embeddings/chat (base URL +/api/paas/v4/...). There’s no special-plan override for images yet, but that matches how this code was previously structured.
136-138: Correctly routes image generations to the zhipu image handlerBranching on
RelayModeImagesGenerationsand delegating tozhipu4vImageHandlerbefore falling back to the OpenAI adaptor is the right integration point for the new flow. No issues here.
| for _, data := range zhipuResp.Data { | ||
| url := data.Url | ||
| if url == "" { | ||
| url = data.ImageUrl | ||
| } | ||
| if url == "" { | ||
| logger.LogWarn(c, "zhipu_image_missing_url") | ||
| continue | ||
| } | ||
|
|
||
| var b64 string | ||
| switch { | ||
| case data.B64Json != "": | ||
| b64 = data.B64Json | ||
| case data.B64Image != "": | ||
| b64 = data.B64Image | ||
| default: | ||
| _, downloaded, err := service.GetImageFromUrl(url) | ||
| if err != nil { | ||
| logger.LogError(c, "zhipu_image_get_b64_failed: "+err.Error()) | ||
| continue | ||
| } | ||
| b64 = downloaded | ||
| } | ||
|
|
||
| if b64 == "" { | ||
| logger.LogWarn(c, "zhipu_image_empty_b64") | ||
| continue | ||
| } | ||
|
|
||
| imageData := openAIImageData{ | ||
| B64Json: b64, | ||
| } | ||
| payload.Data = append(payload.Data, imageData) | ||
| } |
There was a problem hiding this comment.
Images with only base64 but no URL are currently dropped
In the data loop, you require a non-empty URL before checking B64Json / B64Image:
url := data.Url
if url == "" {
url = data.ImageUrl
}
if url == "" {
logger.LogWarn(c, "zhipu_image_missing_url")
continue
}
var b64 string
switch {
case data.B64Json != "":
b64 = data.B64Json
case data.B64Image != "":
b64 = data.B64Image
// ...If zhipu returns entries that only contain base64 fields (no URL), those images will be skipped even though you already have usable data.
Reordering the logic so you prefer existing base64 fields and only fall back to fetching via URL when both are empty would avoid this data loss. For example:
- for _, data := range zhipuResp.Data {
- url := data.Url
- if url == "" {
- url = data.ImageUrl
- }
- if url == "" {
- logger.LogWarn(c, "zhipu_image_missing_url")
- continue
- }
-
- var b64 string
- switch {
- case data.B64Json != "":
- b64 = data.B64Json
- case data.B64Image != "":
- b64 = data.B64Image
- default:
- _, downloaded, err := service.GetImageFromUrl(url)
- if err != nil {
- logger.LogError(c, "zhipu_image_get_b64_failed: "+err.Error())
- continue
- }
- b64 = downloaded
- }
+ for _, data := range zhipuResp.Data {
+ var b64 string
+ switch {
+ case data.B64Json != "":
+ b64 = data.B64Json
+ case data.B64Image != "":
+ b64 = data.B64Image
+ default:
+ url := data.Url
+ if url == "" {
+ url = data.ImageUrl
+ }
+ if url == "" {
+ logger.LogWarn(c, "zhipu_image_missing_url")
+ continue
+ }
+
+ _, downloaded, err := service.GetImageFromUrl(url)
+ if err != nil {
+ logger.LogError(c, "zhipu_image_get_b64_failed: "+err.Error())
+ continue
+ }
+ b64 = downloaded
+ }The rest of the block (b64 == "" check and appending to payload.Data) can remain unchanged.
🤖 Prompt for AI Agents
In relay/channel/zhipu_4v/image.go around lines 84 to 118, the loop currently
rejects entries with no URL before checking B64Json/B64Image, causing images
that only contain base64 to be dropped; change the logic to first check
data.B64Json and data.B64Image and use those if present, and only when both
base64 fields are empty then resolve url (data.Url or data.ImageUrl) and call
service.GetImageFromUrl; log the "missing url" or "get b64 failed"
warnings/errors only when neither base64 is present and fetching by URL fails or
is absent, then proceed to the existing b64=="" check and append to
payload.Data.
981d0ed to
2e37347
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
relay/channel/zhipu_4v/image.go (1)
83-117: Reorder data loop to use base64 fields even when URL is missingRight now you require a non-empty URL before looking at
B64Json/B64Image, so entries that only contain base64 data are dropped as"zhipu_image_missing_url", even though they’re perfectly usable.Refactor the loop to prefer base64 fields and only fall back to URL download when both are empty. For example:
- for _, data := range zhipuResp.Data { - url := data.Url - if url == "" { - url = data.ImageUrl - } - if url == "" { - logger.LogWarn(c, "zhipu_image_missing_url") - continue - } - - var b64 string - switch { - case data.B64Json != "": - b64 = data.B64Json - case data.B64Image != "": - b64 = data.B64Image - default: - _, downloaded, err := service.GetImageFromUrl(url) - if err != nil { - logger.LogError(c, "zhipu_image_get_b64_failed: "+err.Error()) - continue - } - b64 = downloaded - } + for _, data := range zhipuResp.Data { + var b64 string + switch { + case data.B64Json != "": + b64 = data.B64Json + case data.B64Image != "": + b64 = data.B64Image + default: + url := data.Url + if url == "" { + url = data.ImageUrl + } + if url == "" { + logger.LogWarn(c, "zhipu_image_missing_url") + continue + } + + _, downloaded, err := service.GetImageFromUrl(url) + if err != nil { + logger.LogError(c, "zhipu_image_get_b64_failed: "+err.Error()) + continue + } + b64 = downloaded + }The existing
b64 == ""check and append logic can stay as-is.
🧹 Nitpick comments (3)
dto/openai_image.go (1)
30-34: New Zhipu-specific fields integrate cleanly; minor naming nit onlyThe added
WatermarkEnabled,UserId, andImagefields fit the existing RawMessage pattern and work with the current custom (un)marshal logic. If you care about Go naming consistency, you might consider renamingUserId→UserID(JSON tag can remainuser_id), but that’s purely cosmetic.relay/channel/zhipu_4v/adaptor.go (1)
65-67: ImagesGenerations URL wiring looks correct; consider special-plan override laterRouting
RelayModeImagesGenerationsto"%s/api/paas/v4/images/generations"matches the other Zhipu v4 paths. If you ever introduce a special-plan base for images (analogous toOpenAIBaseURLfor chat/embeddings), this is the place to hook it in.relay/channel/zhipu_4v/image.go (1)
119-127: Propagate upstreamusageinstead of always returning an empty structYou already parse
zhipuResp.Usagebut then discard it and return&dto.Usage{}. If upstream provides useful usage info (for logging or billing), it’s better to pass it through:- jsonResp, err := common.Marshal(payload) - if err != nil { - return nil, types.NewError(err, types.ErrorCodeBadResponseBody) - } - - service.IOCopyBytesGracefully(c, resp, jsonResp) - - return &dto.Usage{}, nil + jsonResp, err := common.Marshal(payload) + if err != nil { + return nil, types.NewError(err, types.ErrorCodeBadResponseBody) + } + + service.IOCopyBytesGracefully(c, resp, jsonResp) + + if zhipuResp.Usage != nil { + return zhipuResp.Usage, nil + } + return &dto.Usage{}, nilThis keeps existing behavior as a fallback while preserving real usage data when present.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
dto/openai_image.go(1 hunks)relay/channel/zhipu_4v/adaptor.go(3 hunks)relay/channel/zhipu_4v/image.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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, data: URLs (containing base64 encoded video data) are prevented from being stored in task.FailReason by checking if the URL starts with "data:" before assignment. This same pattern should be applied consistently across the codebase.
Applied to files:
relay/channel/zhipu_4v/image.go
🧬 Code graph analysis (1)
relay/channel/zhipu_4v/adaptor.go (1)
relay/constant/relay_mode.go (1)
RelayModeImagesGenerations(14-14)
🔇 Additional comments (2)
relay/channel/zhipu_4v/adaptor.go (2)
118-120: ImagesGenerations correctly bypasses generic OpenAI adapterBranching to
zhipu4vImageHandlerforRelayModeImagesGenerationsensures the Zhipu-specific image payload is normalized before returning. The rest of the formats still go through the generic OpenAI adapter, which keeps behavior consistent.
38-40: Passing ImageRequest through as-is is acceptable for nowReturning the
dto.ImageRequestdirectly keeps the adapter simple and lets the HTTP layer marshal it to Zhipu's JSON. This is fine as long as Zhipu's API accepts the current OpenAI-shaped payload (including the new raw fields).
…mage feat: zhipu 4v image generations
…es-multi-tool-continuation Preserve multi-tool context in OpenAI messages continuation
2 participants
#2342
Summary by CodeRabbit
New Features
Bug Fixes / Reliability
✏️ Tip: You can customize this high-level summary in your review settings.