fix(openai): support streaming image relay and image edit for images API #4608
Open
gaoren002 wants to merge 2 commits intoQuantumNous:mainfrom
Open
fix(openai): support streaming image relay and image edit for images API #4608gaoren002 wants to merge 2 commits intoQuantumNous:mainfrom
gaoren002 wants to merge 2 commits intoQuantumNous:mainfrom
Conversation
Contributor
WalkthroughImage streaming for OpenAI Images endpoints is enabled: ImageRequest.IsStream now reflects the incoming ChangesOpenAI Images Streaming Support
Sequence DiagramsequenceDiagram
participant Client as Client
participant NewAPI as NewAPI
participant Upstream as Upstream
participant SSEHandler as OpenaiImageStreamHandler
Client->>NewAPI: POST /v1/images/edits (stream=true, multipart)
NewAPI->>NewAPI: Parse request (GetAndValidOpenAIImageRequest -> stream=true)
NewAPI->>NewAPI: info.IsStream() == true
NewAPI->>Upstream: Forward request to provider
Upstream-->>NewAPI: SSE response (text/event-stream)
NewAPI->>SSEHandler: OpenaiImageStreamHandler(resp)
SSEHandler->>Client: Set Content-Type: text/event-stream
loop For each SSE line
Upstream-->>SSEHandler: data: {...partial_image...} / usage
SSEHandler->>Client: Forward line (write + flush)
SSEHandler->>SSEHandler: Parse/normalize usage from data (store last valid)
end
Upstream-->>SSEHandler: [DONE] or EOF
SSEHandler->>SSEHandler: Apply usage post-processing using last data
SSEHandler-->>NewAPI: Return usage
NewAPI-->>Client: Complete response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/channel/openai/image_stream_test.go`:
- Around line 16-21: The test TestOpenaiImageStreamHandlerForwardsSSEAndUsage
sets global Gin mode via gin.SetMode(gin.TestMode) but doesn't restore the
previous mode; update the test to capture the current mode (e.g., prevMode :=
gin.Mode()), call gin.SetMode(gin.TestMode) as done now, and add a t.Cleanup
closure that restores gin.SetMode(prevMode) alongside the existing
constant.StreamingTimeout restore so the global Gin mode is reverted after the
test.
🪄 Autofix (Beta)
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
Run ID: 3b13492d-e5c4-4394-8191-5886936a74d5
📒 Files selected for processing (9)
dto/openai_image.godto/openai_image_test.gorelay/channel/openai/adaptor.gorelay/channel/openai/image_edit_test.gorelay/channel/openai/image_stream_test.gorelay/channel/openai/relay-openai.gorelay/helper/openai_image_request_test.gorelay/helper/stream_scanner.gorelay/helper/valid_request.go
gaoren002
force-pushed
the
pr/openai-image-stream-keepalive-20260504130050
branch
2 times, most recently
from
2d27fba to
c98a5b4
Compare
Contributor
Author
|
本次修复重点考虑了计费逻辑，应该能较完满解决问题。 |
gaoren002
force-pushed
the
pr/openai-image-stream-keepalive-20260504130050
branch
from
c98a5b4 to
55f3767
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
relay/helper/openai_image_request_test.go (1)
18-52: ⚡ Quick winLGTM — the happy-path multipart stream test is well-structured
The body-reusability assertion at lines 44–46 correctly verifies that
ParseMultipartFormReusablerestoresc.Request.Bodyafter first read. The subsequentParseMultipartFormReusable(c)call at line 48 correctly relies onc.Request.MultipartFormhaving been pre-populated byvalid_request.go(line 155), which prevents re-reading the already-drained body.Optional: The error branch at
valid_request.golines 164–166 (malformedstreamvalue → return error) has no test coverage. Consider adding:func TestGetAndValidOpenAIImageRequestMultipartStreamInvalidValue(t *testing.T) { gin.SetMode(gin.TestMode) var body bytes.Buffer writer := multipart.NewWriter(&body) require.NoError(t, writer.WriteField("model", "gpt-image-1")) require.NoError(t, writer.WriteField("stream", "notabool")) require.NoError(t, writer.Close()) recorder := httptest.NewRecorder() c, _ := gin.CreateTestContext(recorder) c.Request = httptest.NewRequest(http.MethodPost, "/v1/images/edits", &body) c.Request.Header.Set("Content-Type", writer.FormDataContentType()) _, err := GetAndValidOpenAIImageRequest(c, relayconstant.RelayModeImagesEdits) require.Error(t, err) require.Contains(t, err.Error(), "invalid stream value") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/helper/openai_image_request_test.go` around lines 18 - 52, Add a negative test that covers the malformed "stream" value path in GetAndValidOpenAIImageRequest: create a multipart request with model "gpt-image-1" and stream set to "notabool", set Content-Type to the writer's FormDataContentType, call GetAndValidOpenAIImageRequest(c, relayconstant.RelayModeImagesEdits) and assert it returns an error containing "invalid stream value" (e.g., name the test TestGetAndValidOpenAIImageRequestMultipartStreamInvalidValue and mirror the existing multipart setup/recorder/gin context used in TestGetAndValidOpenAIImageRequestMultipartStream).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/channel/openai/relay-openai.go`:
- Around line 604-620: OpenaiImageStreamHandler currently forces SSE headers for
every response; instead guard by checking resp.StatusCode and
resp.Header["Content-Type"] (or resp.Header.Get("Content-Type")) for an SSE
content type and a successful status before calling
helper.SetEventStreamHeaders; if the response is non-200 or not SSE (e.g.,
OpenAI JSON error), do NOT set SSE headers—read the body, return the original
status and a types.NewOpenAIError (including the response body/error text) so
the client gets the real HTTP error, and keep defer
service.CloseResponseBodyGracefully(resp) and existing handling of
info.StreamStatus unchanged.
---
Nitpick comments:
In `@relay/helper/openai_image_request_test.go`:
- Around line 18-52: Add a negative test that covers the malformed "stream"
value path in GetAndValidOpenAIImageRequest: create a multipart request with
model "gpt-image-1" and stream set to "notabool", set Content-Type to the
writer's FormDataContentType, call GetAndValidOpenAIImageRequest(c,
relayconstant.RelayModeImagesEdits) and assert it returns an error containing
"invalid stream value" (e.g., name the test
TestGetAndValidOpenAIImageRequestMultipartStreamInvalidValue and mirror the
existing multipart setup/recorder/gin context used in
TestGetAndValidOpenAIImageRequestMultipartStream).
🪄 Autofix (Beta)
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
Run ID: 8859bf41-d1cd-493f-921d-35f7672b79b3
📒 Files selected for processing (6)
relay/channel/openai/image_edit_test.gorelay/channel/openai/image_stream_test.gorelay/channel/openai/relay-openai.gorelay/helper/openai_image_request_test.gorelay/helper/stream_scanner.gorelay/helper/valid_request.go
✅ Files skipped from review due to trivial changes (1)
- relay/channel/openai/image_edit_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- relay/helper/stream_scanner.go
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
relay/channel/openai/image_stream_test.go (1)
19-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestore Gin mode in test cleanup to avoid global-state leakage.
At Line 19, the test mutates global Gin mode but only restores
constant.StreamingTimeout. Please also restore the previous Gin mode int.Cleanup.Suggested fix
func TestOpenaiImageStreamHandlerForwardsSSEAndUsage(t *testing.T) { - gin.SetMode(gin.TestMode) + oldMode := gin.Mode() + gin.SetMode(gin.TestMode) + t.Cleanup(func() { gin.SetMode(oldMode) }) oldTimeout := constant.StreamingTimeout constant.StreamingTimeout = 30 t.Cleanup(func() { constant.StreamingTimeout = oldTimeout })#!/bin/bash # Verify gin mode mutation is paired with cleanup restore in this test file. rg -n -C2 'gin\.SetMode\(gin\.TestMode\)|gin\.Mode\(\)|t\.Cleanup\(func\(\)\s*\{\s*gin\.SetMode' relay/channel/openai/image_stream_test.go🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/openai/image_stream_test.go` around lines 19 - 23, The test sets global Gin mode via gin.SetMode(gin.TestMode) but only restores constant.StreamingTimeout in t.Cleanup, causing global-state leakage; update the cleanup to capture the original Gin mode with gin.Mode() before calling gin.SetMode and restore it inside the existing t.Cleanup alongside restoring constant.StreamingTimeout so both constant.StreamingTimeout and Gin's mode (captured via gin.Mode()) are reset after the test.relay/channel/openai/relay-openai.go (1)
616-617:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve upstream non-SSE/error responses before forcing SSE output.
At Line 616,
helper.SetEventStreamHeaders(c)is unconditional. If upstream returns non-2xx or JSON error, this path can rewrite the response contract and mask real error semantics.Suggested fix
func OpenaiImageStreamHandler(c *gin.Context, info *relaycommon.RelayInfo, resp *http.Response) (*dto.Usage, *types.NewAPIError) { if resp == nil || resp.Body == nil { logger.LogError(c, "invalid image stream response") return nil, types.NewOpenAIError(fmt.Errorf("invalid response"), types.ErrorCodeBadResponse, http.StatusInternalServerError) } - defer service.CloseResponseBodyGracefully(resp) - - usage := &dto.Usage{} - var lastStreamData []byte - - helper.SetEventStreamHeaders(c) + contentType := strings.ToLower(resp.Header.Get("Content-Type")) + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices || + !strings.Contains(contentType, "text/event-stream") { + // keep non-stream/error behavior intact + return OpenaiHandlerWithUsage(c, info, resp) + } + defer service.CloseResponseBodyGracefully(resp) + + usage := &dto.Usage{} + var lastStreamData []byte + + helper.SetEventStreamHeaders(c)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/openai/relay-openai.go` around lines 616 - 617, helper.SetEventStreamHeaders(c) is being called unconditionally which forces SSE headers even when the upstream response is non-2xx or contains a JSON error; change the flow so that before calling helper.SetEventStreamHeaders(c) you check the upstream status/response (use info.StreamStatus and any upstream response body/status) and only set SSE headers when the upstream returns a successful stream (e.g., 2xx and not an error JSON). In practice, move or gate the helper.SetEventStreamHeaders(c) call behind a conditional that verifies info.StreamStatus indicates a streaming success and avoid mutating headers when upstream returned an error status or non-SSE payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@relay/channel/openai/image_stream_test.go`:
- Around line 19-23: The test sets global Gin mode via gin.SetMode(gin.TestMode)
but only restores constant.StreamingTimeout in t.Cleanup, causing global-state
leakage; update the cleanup to capture the original Gin mode with gin.Mode()
before calling gin.SetMode and restore it inside the existing t.Cleanup
alongside restoring constant.StreamingTimeout so both constant.StreamingTimeout
and Gin's mode (captured via gin.Mode()) are reset after the test.
In `@relay/channel/openai/relay-openai.go`:
- Around line 616-617: helper.SetEventStreamHeaders(c) is being called
unconditionally which forces SSE headers even when the upstream response is
non-2xx or contains a JSON error; change the flow so that before calling
helper.SetEventStreamHeaders(c) you check the upstream status/response (use
info.StreamStatus and any upstream response body/status) and only set SSE
headers when the upstream returns a successful stream (e.g., 2xx and not an
error JSON). In practice, move or gate the helper.SetEventStreamHeaders(c) call
behind a conditional that verifies info.StreamStatus indicates a streaming
success and avoid mutating headers when upstream returned an error status or
non-SSE payload.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fcf2b9f7-9e88-48f3-9a4d-0169ef31bc39
📒 Files selected for processing (7)
dto/openai_image_test.gorelay/channel/openai/image_edit_test.gorelay/channel/openai/image_stream_test.gorelay/channel/openai/relay-openai.gorelay/helper/openai_image_request_test.gorelay/helper/stream_scanner.gorelay/helper/valid_request.go
✅ Files skipped from review due to trivial changes (3)
- dto/openai_image_test.go
- relay/channel/openai/image_edit_test.go
- relay/helper/valid_request.go
🚧 Files skipped from review as they are similar to previous changes (2)
- relay/helper/openai_image_request_test.go
- relay/helper/stream_scanner.go
gaoren002
force-pushed
the
pr/openai-image-stream-keepalive-20260504130050
branch
from
55f3767 to
cfc9a34
Compare
gaoren002
force-pushed
the
pr/openai-image-stream-keepalive-20260504130050
branch
from
cfc9a34 to
f706730
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
relay/channel/openai/image_stream_test.go (1)
51-61: ⚡ Quick win
data: [DONE]forwarding is not asserted.The test verifies that SSE event lines and the usage payload are forwarded, but it never checks that the stream-termination sentinel (
data: [DONE]) is present in the recorded body. The handler forwards all lines unconditionally, so this is a coverage gap rather than a production defect — but a targeted regression could silently remove[DONE]forwarding without tripping any existing assertion.✅ Proposed addition
require.Contains(t, recorder.Body.String(), `data: {"usage":{"input_tokens":3,"output_tokens":4,"total_tokens":7,"input_tokens_details":{"image_tokens":2,"text_tokens":1}}}`) +require.Contains(t, recorder.Body.String(), `data: [DONE]`) require.Equal(t, "text/event-stream", recorder.Header().Get("Content-Type"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/openai/image_stream_test.go` around lines 51 - 61, The test for OpenaiImageStreamHandler misses asserting that the SSE stream terminator "data: [DONE]" is forwarded; update the test in relay/channel/openai/image_stream_test.go to also assert that recorder.Body.String() (or equivalent recorded output) contains the literal `data: [DONE]`, ensuring the handler's forwarding of the stream-termination sentinel is covered along with the existing checks for partial_image events and usage payloads.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@relay/channel/openai/image_stream_test.go`:
- Around line 51-61: The test for OpenaiImageStreamHandler misses asserting that
the SSE stream terminator "data: [DONE]" is forwarded; update the test in
relay/channel/openai/image_stream_test.go to also assert that
recorder.Body.String() (or equivalent recorded output) contains the literal
`data: [DONE]`, ensuring the handler's forwarding of the stream-termination
sentinel is covered along with the existing checks for partial_image events and
usage payloads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d47a1817-cb81-45ea-86f1-80384ff32ed3
📒 Files selected for processing (7)
dto/openai_image_test.gorelay/channel/openai/image_edit_test.gorelay/channel/openai/image_stream_test.gorelay/channel/openai/relay-openai.gorelay/helper/openai_image_request_test.gorelay/helper/stream_scanner.gorelay/helper/valid_request.go
✅ Files skipped from review due to trivial changes (2)
- relay/channel/openai/image_edit_test.go
- relay/helper/stream_scanner.go
🚧 Files skipped from review as they are similar to previous changes (4)
- relay/helper/openai_image_request_test.go
- dto/openai_image_test.go
- relay/helper/valid_request.go
- relay/channel/openai/relay-openai.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/channel/openai/relay-openai.go`:
- Around line 594-599: The normalization block copies several token subfields
from usage.InputTokensDetails to usage.PromptTokensDetails but omits
CachedCreationTokens, so add copying of that field as well: when
usage.InputTokensDetails != nil assign
usage.PromptTokensDetails.CachedCreationTokens =
usage.InputTokensDetails.CachedCreationTokens alongside the existing assignments
(i.e., update the block that references usage.InputTokensDetails and
usage.PromptTokensDetails to include CachedCreationTokens).
🪄 Autofix (Beta)
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
Run ID: dc2ddfb7-4a53-4df2-b3fc-fde827b24322
📒 Files selected for processing (8)
dto/openai_image_test.gorelay/channel/openai/adaptor.gorelay/channel/openai/image_edit_test.gorelay/channel/openai/image_stream_test.gorelay/channel/openai/relay-openai.gorelay/helper/openai_image_request_test.gorelay/helper/stream_scanner.gorelay/helper/valid_request.go
✅ Files skipped from review due to trivial changes (3)
- relay/channel/openai/image_stream_test.go
- relay/helper/valid_request.go
- dto/openai_image_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- relay/channel/openai/image_edit_test.go
- relay/helper/stream_scanner.go
- relay/helper/openai_image_request_test.go
Comment on lines
+594
to
+599
| if usage.InputTokensDetails != nil { | ||
| usage.PromptTokensDetails.CachedTokens = usage.InputTokensDetails.CachedTokens | ||
| usage.PromptTokensDetails.ImageTokens = usage.InputTokensDetails.ImageTokens | ||
| usage.PromptTokensDetails.TextTokens = usage.InputTokensDetails.TextTokens | ||
| usage.PromptTokensDetails.AudioTokens = usage.InputTokensDetails.AudioTokens | ||
| } |
Contributor
There was a problem hiding this comment.
Copy cached_creation_tokens during usage normalization.
dto.InputTokenDetails includes CachedCreationTokens, but this mapper drops it while copying the sibling fields. If an upstream image event only reports input_tokens_details.cached_creation_tokens, downstream billing/cache accounting will see 0 after normalization.
Suggested fix
if usage.InputTokensDetails != nil {
usage.PromptTokensDetails.CachedTokens = usage.InputTokensDetails.CachedTokens
+ usage.PromptTokensDetails.CachedCreationTokens = usage.InputTokensDetails.CachedCreationTokens
usage.PromptTokensDetails.ImageTokens = usage.InputTokensDetails.ImageTokens
usage.PromptTokensDetails.TextTokens = usage.InputTokensDetails.TextTokens
usage.PromptTokensDetails.AudioTokens = usage.InputTokensDetails.AudioTokens
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/openai/relay-openai.go` around lines 594 - 599, The
normalization block copies several token subfields from usage.InputTokensDetails
to usage.PromptTokensDetails but omits CachedCreationTokens, so add copying of
that field as well: when usage.InputTokensDetails != nil assign
usage.PromptTokensDetails.CachedCreationTokens =
usage.InputTokensDetails.CachedCreationTokens alongside the existing assignments
(i.e., update the block that references usage.InputTokensDetails and
usage.PromptTokensDetails to include CachedCreationTokens).
Contributor
Author
|
@coderabbitai resume |
Contributor
✅ Actions performedReviews resumed. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@relay/channel/openai/image_edit_test.go`:
- Around line 75-118: The test
TestConvertImageEditRequestParsesReusableMultipartWhenFormIsMissing needs to
exercise the "stream" path as well: include the "stream" field in the multipart
body (e.g., add writer.WriteField("stream", "true")) and set the
dto.ImageRequest.Stream flag (e.g., request.Stream = true) before calling
(&Adaptor{}).ConvertImageRequest so the fallback reusable-multipart parsing
covers the stream branch.
🪄 Autofix (Beta)
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
Run ID: 41d4fe8d-d518-4a7c-99e8-6a600afa91c6
📒 Files selected for processing (8)
dto/openai_image_test.gorelay/channel/openai/adaptor.gorelay/channel/openai/image_edit_test.gorelay/channel/openai/image_stream_test.gorelay/channel/openai/relay-openai.gorelay/helper/openai_image_request_test.gorelay/helper/stream_scanner.gorelay/helper/valid_request.go
✅ Files skipped from review due to trivial changes (2)
- dto/openai_image_test.go
- relay/helper/openai_image_request_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- relay/channel/openai/adaptor.go
- relay/channel/openai/image_stream_test.go
- relay/channel/openai/relay-openai.go
- relay/helper/valid_request.go
Comment on lines
+75
to
+118
| // TestConvertImageEditRequestParsesReusableMultipartWhenFormIsMissing verifies fallback parsing. | ||
| func TestConvertImageEditRequestParsesReusableMultipartWhenFormIsMissing(t *testing.T) { | ||
| gin.SetMode(gin.TestMode) | ||
|
|
||
| var body bytes.Buffer | ||
| writer := multipart.NewWriter(&body) | ||
| require.NoError(t, writer.WriteField("model", "gpt-image-1")) | ||
| require.NoError(t, writer.WriteField("prompt", "edit without pre-parsed form")) | ||
| part, err := writer.CreateFormFile("image", "input.png") | ||
| require.NoError(t, err) | ||
| _, err = part.Write([]byte("fake image")) | ||
| require.NoError(t, err) | ||
| require.NoError(t, writer.Close()) | ||
|
|
||
| recorder := httptest.NewRecorder() | ||
| c, _ := gin.CreateTestContext(recorder) | ||
| c.Request = httptest.NewRequest(http.MethodPost, "/v1/images/edits", &body) | ||
| c.Request.Header.Set("Content-Type", writer.FormDataContentType()) | ||
|
|
||
| storage, err := common.GetBodyStorage(c) | ||
| require.NoError(t, err) | ||
| c.Request.Body = io.NopCloser(storage) | ||
| c.Request.MultipartForm = nil | ||
| c.Request.PostForm = nil | ||
|
|
||
| info := &relaycommon.RelayInfo{ | ||
| RelayMode: relayconstant.RelayModeImagesEdits, | ||
| } | ||
| request := dto.ImageRequest{ | ||
| Model: "gpt-image-1", | ||
| Prompt: "edit without pre-parsed form", | ||
| } | ||
|
|
||
| converted, err := (&Adaptor{}).ConvertImageRequest(c, info, request) | ||
| require.NoError(t, err) | ||
|
|
||
| convertedBody, ok := converted.(*bytes.Buffer) | ||
| require.True(t, ok) | ||
| replayedRequest := httptest.NewRequest(http.MethodPost, "/v1/images/edits", bytes.NewReader(convertedBody.Bytes())) | ||
| replayedRequest.Header.Set("Content-Type", c.Request.Header.Get("Content-Type")) | ||
| require.NoError(t, replayedRequest.ParseMultipartForm(32<<20)) | ||
| require.Equal(t, "edit without pre-parsed form", replayedRequest.PostForm.Get("prompt")) | ||
| require.Len(t, replayedRequest.MultipartForm.File["image"], 1) | ||
| } |
Contributor
There was a problem hiding this comment.
Cover stream in the fallback-path test too.
This test verifies reusable multipart parsing when MultipartForm is missing, but it never exercises the stream field. That leaves the main regression from this PR untested in the fallback path.
Suggested tweak
writer := multipart.NewWriter(&body)
require.NoError(t, writer.WriteField("model", "gpt-image-1"))
require.NoError(t, writer.WriteField("prompt", "edit without pre-parsed form"))
+ require.NoError(t, writer.WriteField("stream", "true"))
part, err := writer.CreateFormFile("image", "input.png")
@@
require.NoError(t, replayedRequest.ParseMultipartForm(32<<20))
require.Equal(t, "edit without pre-parsed form", replayedRequest.PostForm.Get("prompt"))
+ require.Equal(t, "true", replayedRequest.PostForm.Get("stream"))
require.Len(t, replayedRequest.MultipartForm.File["image"], 1)📝 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.
Suggested change
| // TestConvertImageEditRequestParsesReusableMultipartWhenFormIsMissing verifies fallback parsing. | |
| func TestConvertImageEditRequestParsesReusableMultipartWhenFormIsMissing(t *testing.T) { | |
| gin.SetMode(gin.TestMode) | |
| var body bytes.Buffer | |
| writer := multipart.NewWriter(&body) | |
| require.NoError(t, writer.WriteField("model", "gpt-image-1")) | |
| require.NoError(t, writer.WriteField("prompt", "edit without pre-parsed form")) | |
| part, err := writer.CreateFormFile("image", "input.png") | |
| require.NoError(t, err) | |
| _, err = part.Write([]byte("fake image")) | |
| require.NoError(t, err) | |
| require.NoError(t, writer.Close()) | |
| recorder := httptest.NewRecorder() | |
| c, _ := gin.CreateTestContext(recorder) | |
| c.Request = httptest.NewRequest(http.MethodPost, "/v1/images/edits", &body) | |
| c.Request.Header.Set("Content-Type", writer.FormDataContentType()) | |
| storage, err := common.GetBodyStorage(c) | |
| require.NoError(t, err) | |
| c.Request.Body = io.NopCloser(storage) | |
| c.Request.MultipartForm = nil | |
| c.Request.PostForm = nil | |
| info := &relaycommon.RelayInfo{ | |
| RelayMode: relayconstant.RelayModeImagesEdits, | |
| } | |
| request := dto.ImageRequest{ | |
| Model: "gpt-image-1", | |
| Prompt: "edit without pre-parsed form", | |
| } | |
| converted, err := (&Adaptor{}).ConvertImageRequest(c, info, request) | |
| require.NoError(t, err) | |
| convertedBody, ok := converted.(*bytes.Buffer) | |
| require.True(t, ok) | |
| replayedRequest := httptest.NewRequest(http.MethodPost, "/v1/images/edits", bytes.NewReader(convertedBody.Bytes())) | |
| replayedRequest.Header.Set("Content-Type", c.Request.Header.Get("Content-Type")) | |
| require.NoError(t, replayedRequest.ParseMultipartForm(32<<20)) | |
| require.Equal(t, "edit without pre-parsed form", replayedRequest.PostForm.Get("prompt")) | |
| require.Len(t, replayedRequest.MultipartForm.File["image"], 1) | |
| } | |
| // TestConvertImageEditRequestParsesReusableMultipartWhenFormIsMissing verifies fallback parsing. | |
| func TestConvertImageEditRequestParsesReusableMultipartWhenFormIsMissing(t *testing.T) { | |
| gin.SetMode(gin.TestMode) | |
| var body bytes.Buffer | |
| writer := multipart.NewWriter(&body) | |
| require.NoError(t, writer.WriteField("model", "gpt-image-1")) | |
| require.NoError(t, writer.WriteField("prompt", "edit without pre-parsed form")) | |
| require.NoError(t, writer.WriteField("stream", "true")) | |
| part, err := writer.CreateFormFile("image", "input.png") | |
| require.NoError(t, err) | |
| _, err = part.Write([]byte("fake image")) | |
| require.NoError(t, err) | |
| require.NoError(t, writer.Close()) | |
| recorder := httptest.NewRecorder() | |
| c, _ := gin.CreateTestContext(recorder) | |
| c.Request = httptest.NewRequest(http.MethodPost, "/v1/images/edits", &body) | |
| c.Request.Header.Set("Content-Type", writer.FormDataContentType()) | |
| storage, err := common.GetBodyStorage(c) | |
| require.NoError(t, err) | |
| c.Request.Body = io.NopCloser(storage) | |
| c.Request.MultipartForm = nil | |
| c.Request.PostForm = nil | |
| info := &relaycommon.RelayInfo{ | |
| RelayMode: relayconstant.RelayModeImagesEdits, | |
| } | |
| request := dto.ImageRequest{ | |
| Model: "gpt-image-1", | |
| Prompt: "edit without pre-parsed form", | |
| } | |
| converted, err := (&Adaptor{}).ConvertImageRequest(c, info, request) | |
| require.NoError(t, err) | |
| convertedBody, ok := converted.(*bytes.Buffer) | |
| require.True(t, ok) | |
| replayedRequest := httptest.NewRequest(http.MethodPost, "/v1/images/edits", bytes.NewReader(convertedBody.Bytes())) | |
| replayedRequest.Header.Set("Content-Type", c.Request.Header.Get("Content-Type")) | |
| require.NoError(t, replayedRequest.ParseMultipartForm(32<<20)) | |
| require.Equal(t, "edit without pre-parsed form", replayedRequest.PostForm.Get("prompt")) | |
| require.Equal(t, "true", replayedRequest.PostForm.Get("stream")) | |
| require.Len(t, replayedRequest.MultipartForm.File["image"], 1) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/openai/image_edit_test.go` around lines 75 - 118, The test
TestConvertImageEditRequestParsesReusableMultipartWhenFormIsMissing needs to
exercise the "stream" path as well: include the "stream" field in the multipart
body (e.g., add writer.WriteField("stream", "true")) and set the
dto.ImageRequest.Stream flag (e.g., request.Stream = true) before calling
(&Adaptor{}).ConvertImageRequest so the fallback reusable-multipart parsing
covers the stream branch.
Collaborator
|
需要考虑一下增加stream的版本和没有增加stream之间的套娃，也就是当前端按流处理但是返回的端实际上是按非流处理的情况。 |
Contributor
Author
不知道这里算不算 relay/channel/openai/relay-openai.go:615 会检查 2xx + text/event-stream，否则回退 OpenaiHandlerWithUsage，不会强行包装成 SSE。 |
Contributor
Author
Contributor
Author
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Important
📝 变更描述 / Description
本 PR 修复 OpenAI 图片生成/编辑在
stream=true场景下没有按流式响应处理的问题。此前
ImageRequest.IsStream()固定返回false，即使客户端传入stream=true，图片接口仍会走非流式响应路径。对于耗时较长的图片生成/编辑请求，下游在上游完成前收不到任何响应数据，容易被 CDN / Cloudflare 等入口超时断开。本次改动让图片请求正确识别
stream参数，并在 OpenAI 图片生成与图片编辑接口中增加 SSE 转发处理：上游返回的event:/data:行会原样写回客户端，兼容image_generation.partial_image、完成事件和[DONE]，同时从流式数据中提取 usage，保持计费链路可用。另外，图片编辑接口使用
multipart/form-data。校验阶段如果直接调用c.MultipartForm()，会消费原始请求体，后续转发到 OpenAI 时可能导致 multipart 内容不可复用。本 PR 改为使用可复用的 multipart 解析方式，并回填c.Request.MultipartForm/c.Request.PostForm，确保校验后仍能完整重建并转发图片、mask 和普通表单字段。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
102 Processing为长 JSON 生成请求保活；本 PR 改为支持 OpenAI Images 原生 SSE 流式语义。✅ 提交前检查项 / Checklist
main。📸 运行证明 / Proof of Work
本地测试命令：
go test ./model ./controller ./relay/helper ./relay/channel/openai ./dto测试结果：
PR 分支：
提交：
Summary by CodeRabbit
New Features
Bug Fixes
Tests