fix: 对齐 Replicate 原生图片参数 - #6701
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplicate image requests now support native ChangesReplicate input validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
relay/channel/replicate/adaptor.go (5)
334-336: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDecode each poll response into a fresh
PredictionResponse.
common.Unmarshalmerges into the existingpredictionvalue. JSON decoding does not clear fields that the new payload omits. If one poll response omitsoutputorerror, the value from the previous response persists andevaluatePredictionacts on stale data.♻️ Proposed fix
- if err := common.Unmarshal(responseBody, &prediction); err != nil { + var polled PredictionResponse + if err := common.Unmarshal(responseBody, &polled); err != nil { return PredictionResponse{}, fmt.Errorf("replicate adaptor: failed to decode polling response: %w", err) } + prediction = polled🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/replicate/adaptor.go` around lines 334 - 336, Update the polling decode flow around common.Unmarshal to decode each response into a newly initialized PredictionResponse rather than reusing prediction across iterations. Ensure evaluatePrediction receives only fields present in the current poll response while preserving the existing decode error handling.
33-37: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider a backoff for long-running predictions.
predictionPollIntervalis a fixed 1 second andpredictionPollTimeoutis 20 minutes. A single slow prediction can then issue up to about 1200 upstream GET requests. Replicate rate limits the predictions API per account, so many concurrent long jobs can exhaust that budget. An incremental backoff (for example 1s, growing to 5s) keeps latency low for fast jobs and reduces upstream load for slow jobs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/replicate/adaptor.go` around lines 33 - 37, Update the prediction polling logic that uses predictionPollInterval to apply incremental backoff between status requests, starting at 1 second and capping around 5 seconds while predictionPollTimeout remains enforced. Preserve fast polling for short-lived predictions and reduce request frequency for long-running ones.
319-333: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winA single transient failure aborts a running prediction.
Any network error from
client.Do, or any non-200 status, ends the loop immediately. The prediction continues upstream, but the user receives an error and is not billed for a result they may still be charged for on the Replicate side. Over a 20 minute window, one connection reset or one 429/5xx from Replicate is likely.Tolerate transient failures: retry on network errors and on 429/5xx up to a small consecutive-failure budget, and fail only on 4xx that are not 429.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/replicate/adaptor.go` around lines 319 - 333, Update the polling loop around client.Do and the response status handling to tolerate transient failures: retry network errors and HTTP 429/5xx responses up to a small consecutive-failure budget, resetting that budget after a successful poll. Preserve response-size validation, fail immediately for non-429 4xx statuses, and return an error only when the transient retry budget is exhausted.
421-435: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueDownloads run sequentially.
Each output URL is downloaded one after another. For a request that returns several images, the added latency is the sum of all downloads. A bounded parallel download, for example with
errgroupand a small concurrency limit, keeps the ordering ofresultsand reduces the total time.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/replicate/adaptor.go` around lines 421 - 435, Update downloadImagesToBase64 to download non-empty output URLs concurrently with a small bounded worker limit, while preserving each URL’s original position in results and returning download errors with the existing context. Ensure synchronization protects shared result and error state, and avoid launching unnecessary work after cancellation.
383-391: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInclude the raw payload in the fallback error message.
If the error payload is a JSON object with no
message,detail, orcode, the returned text is the constant"replicate adaptor: prediction error". The upstream detail is then lost for diagnosis. Append the trimmed raw payload, bounded to a safe length.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/replicate/adaptor.go` around lines 383 - 391, Update the fallback return in the prediction-error parsing logic to append the trimmed raw payload, preserving the existing prefix and limiting the payload to a safe maximum length. Keep the parsed Message, Detail, and Code priority unchanged, and use the existing trimmed payload represented by trimmed.relay/channel/replicate/adaptor_test.go (1)
104-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd polling cases for the pending and failure paths.
This test covers only a single poll that returns
succeeded. The new polling code has several untested branches that carry real risk:
- A first response with status
processingfollowed by a second response with statussucceeded. This proves that the loop repeats and that the final output replaces the earlier one.- A poll response with a non-200 status. This proves the error text includes the status.
- A poll response with status
failedand an error payload. This proves the provider error reaches the caller.The
processingcase is the most valuable, because the currentevaluatePredictionreturns early when a non-terminal response already carries output.Based on the coding guideline "Backend tests must protect real behavior, API contracts, billing/accounting invariants, compatibility, or regression paths."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/channel/replicate/adaptor_test.go` around lines 104 - 136, Add polling test cases alongside TestWaitForPredictionPollsThroughConfiguredBaseURL for: an initial processing response followed by succeeded, asserting the loop repeats and final output replaces earlier output; a non-200 response, asserting the returned error includes the HTTP status; and a failed prediction with an error payload, asserting the provider error reaches the caller. Reuse the existing server, request setup, and waitForPrediction symbols, with the processing case specifically covering responses that include intermediate output.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@relay/channel/replicate/adaptor.go`:
- Around line 98-113: The image count can be billed higher than the number
Replicate produces when outer n exceeds one and native input lacks a count. In
relay/channel/replicate/adaptor.go, update the native-input handling in the
adaptor to reconcile the outer count into inputPayload using the target model
family’s correct count key when neither number_of_images nor num_outputs exists,
or reject the unreconciled case; in relay/helper/valid_request.go, add post-loop
validation for imageRequest.N greater than one with a native input and empty
declarations, returning an explicit error. Add the corresponding n >
1/countless-native-input table case in
relay/helper/openai_image_request_test.go.
- Around line 363-364: Update the non-terminal status handling in the prediction
completion logic so the "starting" and "processing" cases always return pending
regardless of output URLs. Leave output inspection to the terminal-status
branch, including the existing ""/"succeeded" handling.
In `@relay/helper/valid_request.go`:
- Around line 177-187: Scope the number_of_images/num_outputs handling in
normalizeImageRequestCount to Replicate image requests only, or move this legacy
branch after normal OpenAI fields are rejected. Ensure non-Replicate image
requests can pass these provider-specific extras without validation failure,
while preserving Replicate count parsing and declarations.
In `@service/download.go`:
- Around line 59-70: Update the worker request construction in the EnableWorker
branch to prevent upstream provider credentials from crossing the worker
boundary. Exclude the Authorization header from workerHeaders, or restrict
forwarding to an explicit allowlist of safe headers, while preserving forwarding
for permitted headers before calling DoWorkerRequest.
- Line 58: Update the worker download log in the surrounding download flow to
pass originUrl through common.MaskSensitiveInfo before formatting it with the
existing reason details. Preserve the current log message structure while
ensuring signed query parameters and other sensitive URL data are never logged
raw.
- Around line 82-83: Update the header handling in DoDownloadRequest to copy
supplied headers into the request’s existing initialized Header map rather than
assigning headers.Clone() directly. Preserve all provided header values while
keeping the map non-nil when the caller passes nil.
---
Nitpick comments:
In `@relay/channel/replicate/adaptor_test.go`:
- Around line 104-136: Add polling test cases alongside
TestWaitForPredictionPollsThroughConfiguredBaseURL for: an initial processing
response followed by succeeded, asserting the loop repeats and final output
replaces earlier output; a non-200 response, asserting the returned error
includes the HTTP status; and a failed prediction with an error payload,
asserting the provider error reaches the caller. Reuse the existing server,
request setup, and waitForPrediction symbols, with the processing case
specifically covering responses that include intermediate output.
In `@relay/channel/replicate/adaptor.go`:
- Around line 334-336: Update the polling decode flow around common.Unmarshal to
decode each response into a newly initialized PredictionResponse rather than
reusing prediction across iterations. Ensure evaluatePrediction receives only
fields present in the current poll response while preserving the existing decode
error handling.
- Around line 33-37: Update the prediction polling logic that uses
predictionPollInterval to apply incremental backoff between status requests,
starting at 1 second and capping around 5 seconds while predictionPollTimeout
remains enforced. Preserve fast polling for short-lived predictions and reduce
request frequency for long-running ones.
- Around line 319-333: Update the polling loop around client.Do and the response
status handling to tolerate transient failures: retry network errors and HTTP
429/5xx responses up to a small consecutive-failure budget, resetting that
budget after a successful poll. Preserve response-size validation, fail
immediately for non-429 4xx statuses, and return an error only when the
transient retry budget is exhausted.
- Around line 421-435: Update downloadImagesToBase64 to download non-empty
output URLs concurrently with a small bounded worker limit, while preserving
each URL’s original position in results and returning download errors with the
existing context. Ensure synchronization protects shared result and error state,
and avoid launching unnecessary work after cancellation.
- Around line 383-391: Update the fallback return in the prediction-error
parsing logic to append the trimmed raw payload, preserving the existing prefix
and limiting the payload to a safe maximum length. Keep the parsed Message,
Detail, and Code priority unchanged, and use the existing trimmed payload
represented by trimmed.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f1645f7d-dc5a-4bfa-8cac-cbf13d1f178d
📒 Files selected for processing (7)
relay/channel/replicate/adaptor.gorelay/channel/replicate/adaptor_test.gorelay/channel/replicate/dto.gorelay/helper/openai_image_request_test.gorelay/helper/valid_request.goservice/download.goservice/image.go
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@relay/channel/replicate/adaptor_test.go`:
- Around line 147-151: Add a deterministic test case alongside the existing
processing prediction case, using Status "starting" and the same successful
polling assertions. Ensure the test exercises the polling path in the relevant
adaptor test so both starting and processing predictions continue polling until
completion.
In `@service/download_test.go`:
- Around line 70-71: Update the worker bypass test setup around
system_setting.WorkerUrl and WorkerValidKey to explicitly configure every
setting required by EnableWorker(), assert that system_setting.EnableWorker()
returns true before exercising the bypass, and save and restore any additional
mutable setting state used by the fixture.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2361066b-f99d-4863-aef6-9a8b25a4a604
📒 Files selected for processing (6)
relay/channel/replicate/adaptor.gorelay/channel/replicate/adaptor_test.gorelay/helper/openai_image_request_test.gorelay/helper/valid_request.goservice/download.goservice/download_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- relay/helper/valid_request.go
- relay/channel/replicate/adaptor.go
|
@Calcium-Ion 麻烦审核下 |
|
感谢您的贡献,请问可以把功能上无关的代码先移除,或者抽离成新的pr吗,这样更方便维护者review |
好的 我修改下 |
0bd021b to
359ce49
Compare
|
感谢提醒,已按建议重新整理并强制更新分支。当前 PR 只保留 Replicate 原生 input 参数透传、Flux 旧调用兼容,以及 n / number_of_images / num_outputs 的数量与计费一致性校验;前端测试迁移、长任务轮询、下载鉴权和通用 service 修改均已移除。变更范围已从 41 个文件收缩到 4 个 Replicate 参数相关文件,并已在最新 main 上完成全量 vet、build 和 make test。烦请您有空时再帮忙 review。 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@relay/channel/replicate/adaptor_test.go`:
- Around line 70-89: Extend TestConvertImageRequestNativeInputCanProvidePrompt
with a native input payload that omits prompt while setting the outer
dto.ImageRequest.Prompt; assert ConvertImageRequest succeeds and the converted
payload’s input map contains the outer prompt. Preserve the existing
native-prompt case and verify the fallback injection behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 827a9dcd-c211-4ce5-969d-8f9fffcbb252
📒 Files selected for processing (3)
relay/channel/replicate/adaptor.gorelay/channel/replicate/adaptor_test.gorelay/helper/openai_image_request_test.go
359ce49 to
b9f685e
Compare
|
@Calcium-Ion 已经处理了,不过那个前端的问题是main分支就有问题,得先把#6569 这个处理了 可能才不会报错了 |
|
@Calcium-Ion 大佬 这个麻烦在审核下,这个影响还有点大,就是我们调用replicate网络有时可以有时不可以,我们排查发现是replicate对应的是cloudflare节点国内访问不稳定。所以部署了一套海外的newapi作为中转,保持参数可以透传过去,能跟直连repliate效果差不多,需要这个pr的修改 |
|
@seefs001 你好,这个 PR 已按维护者建议从 41 个文件收敛到 4 个 Replicate 参数相关文件 主要解决 GPT Image 2 等非 Flux 模型的原生 input 和多参考图透传问题,麻烦您有空时帮忙 review,感谢! |
Important
📝 变更描述 / Description
NewAPI 的 Replicate 图片适配默认按 Flux 参数组装请求。客户端为其他 Replicate 模型提供原生
input时,旧逻辑仍会注入或转换num_outputs、image_prompt、prompt_upsampling等 Flux 参数,可能造成参数冲突或语义不一致。本 PR 仅处理 Replicate 参数对齐:
input时,将其作为上游参数的权威来源;仅在其中缺少prompt时补充外层提示词,不再注入 Flux 专用参数。input时,继续使用现有 Flux 参数转换,保持旧调用兼容。n、number_of_images和num_outputs,覆盖原生input、extra_fields及兼容扩展字段;拒绝超限值和相互冲突的数量,避免实际生成数量与计费数量不一致。例如
openai/gpt-image-2使用number_of_images、input_images、quality等原生字段时,不会再被追加 Flux 的num_outputs、image_prompt或prompt_upsampling。根据维护者建议,本次已移除前端测试迁移、长任务轮询、下载鉴权及通用
service修改;这些内容不在本 PR 范围内。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
已通过:
go test ./relay/channel/replicate ./relay/helper -count=1go test ./relay/... -count=1go test -race ./relay/channel/replicate -count=1go test -race ./relay/helper -run 'TestGetAndValidOpenAIImageRequest' -count=1GOWORK=off go vet ./...GOWORK=off go build ./...cd relaykit && GOWORK=off go vet ./...cd relaykit && GOWORK=off go build ./...make testgit diff --check未执行会产生费用的 Replicate 真机生成;参数转换、旧 Flux 兼容、数量归一化、冲突与边界校验均由确定性回归测试覆盖。
Summary by CodeRabbit
New Features
inputobject, including prompts supplied within that object.number_of_imagesornum_outputs.Bug Fixes