feat(codex): support gpt-image-2 image tools - #4481
Closed
jaredshuai wants to merge 1 commit into
Closed
Conversation
Contributor
WalkthroughThis PR adds support for Changes
Sequence DiagramsequenceDiagram
participant Client
participant Router
participant ImageHandler
participant CodexAdapter
participant CodexBackend
participant ResponseHandler
participant ClientResponse
Client->>Router: POST /v1/images/generations
Router->>ImageHandler: Route to image handler
ImageHandler->>CodexAdapter: ConvertImageRequest (Codex Responses format)
CodexAdapter->>CodexBackend: POST /backend-api/codex/responses<br/>(with image_generation tool)
CodexBackend-->>ResponseHandler: HTTP SSE stream response
ResponseHandler->>ResponseHandler: Parse SSE events<br/>Aggregate output items<br/>Extract usage data
ResponseHandler->>ResponseHandler: Convert to OpenAI<br/>ImageResponse format
ResponseHandler-->>ClientResponse: JSON response with images
ClientResponse-->>Client: 200 OK with image data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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. 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: 2
🧹 Nitpick comments (7)
relay/channel/codex/error.go (2)
28-35: Consider bounding the upstream body read.
io.ReadAll(resp.Body)is unbounded — a misbehaving upstream could send a large/unbounded error body and force an equivalently large allocation just to extract a short error message (which you cap at 2048 chars anyway). Usingio.LimitReader(e.g., a few hundred KB) would protect the relay process while still capturing enough body forextractCodexErrorMessage/truncateErrorMessage.♻️ Proposed change
- var responseBody []byte - if resp != nil && resp.Body != nil { - body, err := io.ReadAll(resp.Body) - if err == nil { - responseBody = body - } - service.CloseResponseBodyGracefully(resp) - } + var responseBody []byte + if resp != nil && resp.Body != nil { + const maxErrorBodyRead = 256 << 10 // 256 KiB + body, err := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyRead)) + if err == nil { + responseBody = body + } + service.CloseResponseBodyGracefully(resp) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/codex/error.go` around lines 28 - 35, The current unbounded io.ReadAll on resp.Body can allocate arbitrarily large memory; change the read to use an io.LimitReader so we only read a bounded number of bytes (e.g., 256KB) into responseBody before calling extractCodexErrorMessage/truncateErrorMessage. Locate the block that sets responseBody (checking resp != nil && resp.Body != nil) and replace the io.ReadAll(resp.Body) invocation with io.ReadAll(io.LimitReader(resp.Body, 256*1024)), keeping the existing error handling and the call to service.CloseResponseBodyGracefully(resp).
52-86: Edge case:errorpath may return a large raw object.When the upstream payload has
erroras a non-empty object but noerror.message(or any sibling path matches), the loop falls through to the last"error"path and returnsresult.Raw— i.e., the entire error JSON.truncateErrorMessagewill cap this, but the resulting message can be cryptic (e.g., a JSON blob). Consider also probing common nested OpenAI/Codex shapes likeerror.error.message,errors.0.message, or simply skipping non-scalar matches for the bare"error"path so structured errors fall back to the raw body trim path. Optional, since truncation already prevents harm.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/codex/error.go` around lines 52 - 86, extractCodexErrorMessage may return a large structured JSON when it matches the bare "error" path; update the search logic in extractCodexErrorMessage to (1) prefer nested/common Codex/OpenAI shapes by adding paths like "error.error.message", "errors.0.message", "errors.#.message" (or similar common variants) before the plain "error" entry, and (2) treat the plain "error" match as valid only for scalar values—if result.Type is an object/array, skip it so the function falls back to other candidates or the trimmed raw body instead of returning a cryptic JSON blob.relay/channel/codex/responses.go (1)
47-48: StaleContent-Lengthheader may remain after aggregating SSE → JSON.
cloneResponseWithContentTypedeletesTransfer-Encodingbut preserves all other headers. When upstream is SSE (noContent-Length), this is fine. However, if upstream ever sends aContent-Length(e.g., a JSON error returned via this path), the cloned response carries the original length while the body may differ in size. Consider also deletingContent-Lengthwhen the body is reconstructed.♻️ Suggested refactor
func cloneResponseWithContentType(resp *http.Response, contentType string) *http.Response { if resp == nil { return nil } cloned := *resp cloned.Header = resp.Header.Clone() cloned.Header.Set("Content-Type", contentType) cloned.Header.Del("Transfer-Encoding") + cloned.Header.Del("Content-Length") return &cloned }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/codex/responses.go` around lines 47 - 48, The cloned response can carry a stale Content-Length after converting SSE to aggregated JSON; update cloneResponseWithContentType (or immediately after creating jsonResp before calling service.IOCopyBytesGracefully) to also remove the "Content-Length" header (in addition to "Transfer-Encoding") so the response length isn't misleading when the body is reconstructed/changed; ensure the header deletion targets the exact "Content-Length" key on the http header map used by cloneResponseWithContentType/jsonResp.relay/channel/codex/image.go (2)
282-282:input_fidelityonly sourced from multipart form.
setStringToolOption(tool, "input_fidelity", firstFormValue(form, "input_fidelity"))reads only from the multipart form. JSON-body image edit requests (when client sends JSON instead of multipart) won't propagateinput_fidelity. Ifdto.ImageRequesthas (or could expose) this field, consider sourcing from there as a fallback to keep parity between multipart and JSON edit paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/codex/image.go` at line 282, The call to setStringToolOption(tool, "input_fidelity", firstFormValue(form, "input_fidelity")) only reads input_fidelity from the multipart form and thus misses JSON-body edit requests; update the code that builds the tool options (the spot using setStringToolOption and firstFormValue) to fallback to dto.ImageRequest.InputFidelity (or the appropriate field on dto.ImageRequest) when firstFormValue(form, "input_fidelity") is empty/nil so both multipart and JSON edit paths propagate input_fidelity; ensure you reference dto.ImageRequest where available and keep precedence: form value first, then dto.ImageRequest, then leave unset.
215-225: Octet-stream fallback may emitdata:application/octet-stream;base64,...if detection fails.If a client uploads a file with
Content-Type: application/octet-streamandhttp.DetectContentTypedoesn't recognize it as an image (e.g., for less common formats), the resulting data URL usesapplication/octet-stream. Codex may reject this. Consider returning an explicit error, or defaulting toimage/pngwhen the content is small/unknown but the field is required to be an image (since this is in theimage[]form field).♻️ Suggested refactor
- if mediaType == "" { - mediaType = "application/octet-stream" - } + if mediaType == "" || strings.EqualFold(mediaType, "application/octet-stream") { + return "", fmt.Errorf("unable to determine image media type for upload %q", fileHeader.Filename) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/codex/image.go` around lines 215 - 225, The code currently falls back to "application/octet-stream" and returns a data URL even when http.DetectContentType fails to recognize an image, which can cause Codex to reject the upload; update the logic around mediaType in the function that builds the data URL so that if fileHeader.Header.Get("Content-Type") is "application/octet-stream" (or empty) and http.DetectContentType(data) does not start with "image/", you either return a clear error (e.g., ErrInvalidImage) instead of constructing a non-image data URL, or (if you must produce an image) default mediaType to "image/png" for small/required image fields; adjust the return path that currently uses mediaType and base64.StdEncoding.EncodeToString(data) accordingly and ensure callers of this function handle the new error or PNG default.controller/channel-test.go (1)
46-50: Consider using theCodexImageModelconstant.
relay/channel/codexalready exposesCodexImageModel(used inadaptor.goandimage.go). Hard-coding"gpt-image-2"here creates a second source of truth that can drift if the model identifier changes.♻️ Suggested refactor
+ codexchannel "github.com/QuantumNous/new-api/relay/channel/codex" @@ func isCodexImageGenerationTestModel(channel *model.Channel, modelName string) bool { return channel != nil && channel.Type == constant.ChannelTypeCodex && - strings.EqualFold(strings.TrimSpace(modelName), "gpt-image-2") + strings.EqualFold(strings.TrimSpace(modelName), codexchannel.CodexImageModel) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel-test.go` around lines 46 - 50, Replace the hard-coded model string in isCodexImageGenerationTestModel with the shared constant to avoid duplicate sources of truth: import (or reference) the CodexImageModel constant from the codex package and use it instead of the literal "gpt-image-2" in the equality check inside isCodexImageGenerationTestModel so the function compares strings.EqualFold(strings.TrimSpace(modelName), codex.CodexImageModel).relay/channel/codex/adaptor.go (1)
153-178: Possible inconsistency: raw passthrough path defaultsinstructionsto""when no system prompt is set, but the existing logic only does this in a final step.In the non-raw path (line 266-268), the empty
instructionsdefault applies after the system-prompt branch. In the raw path, theelseat line 173-178 only runs when the outer condition (info != nil && info.ChannelMeta != nil && info.ChannelSetting.SystemPrompt != "") is false — but there is no fallthrough to also set the empty default ifinfo.ChannelSetting.SystemPrompt != ""andlen(instructions) == 0was already handled inside the first branch. Actually verifying: line 156-160 already handles this case (when instructions doesn't exist, set to systemPrompt). So the logic is correct, but the structure mirrors the non-raw path inconsistently. No functional issue — just noting that the two paths express the same intent differently, which makes them harder to keep in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/codex/adaptor.go` around lines 153 - 178, Refactor the conditional that sets "instructions" so the raw passthrough branch mirrors the non-raw path: consolidate the logic around gjson.Get(out, "instructions"), ChannelSetting.SystemPrompt and ChannelSetting.SystemPromptOverride (referencing variables systemPrompt and instructions) so that when no system prompt exists you explicitly set "instructions" to "" and when a system prompt exists you either set it or prepend/append existing instructions per SystemPromptOverride; this keeps the handling identical between the two paths and avoids divergent branching structures.
🤖 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/codex/image.go`:
- Around line 524-554: The buildImageAPIResponse function currently sets
dto.ImageData.Url to a data:...;base64,... string (via mimeTypeFromOutputFormat
and result.Result) when responseFormat == "url", which returns an embedded
base64 data URL rather than an OpenAI-hosted URL; update the codebase by adding
a clear comment immediately above buildImageAPIResponse (or above
dto.ImageData.Url field) stating that response_format="url" produces an embedded
data URL (not a hosted URL), can be very large for high-res outputs, and may
break client assumptions (e.g., attempting to fetch the URL or logging size
limits); if desired by product, optionally implement a different code path in
buildImageAPIResponse to return hosted URLs instead, otherwise document the
semantics so callers use B64Json when appropriate.
In `@relay/channel/codex/responses.go`:
- Around line 365-379: recordResponsesBuiltInToolUsageFromGJSON is dead code
because Codex Response objects don't include a "tools" field so the array check
always fails; either remove this function and any unused references to it (and
rely on recordResponsesBuiltInToolCall which already updates
ResponsesUsageInfo.BuiltInTools.CallCount), or keep it but add a clear comment
above recordResponsesBuiltInToolUsageFromGJSON explaining it's
defensive/placeholder for future API changes and why the current API doesn't
populate "tools" (referencing ResponsesUsageInfo.BuiltInTools and
recordResponsesBuiltInToolCall for context).
---
Nitpick comments:
In `@controller/channel-test.go`:
- Around line 46-50: Replace the hard-coded model string in
isCodexImageGenerationTestModel with the shared constant to avoid duplicate
sources of truth: import (or reference) the CodexImageModel constant from the
codex package and use it instead of the literal "gpt-image-2" in the equality
check inside isCodexImageGenerationTestModel so the function compares
strings.EqualFold(strings.TrimSpace(modelName), codex.CodexImageModel).
In `@relay/channel/codex/adaptor.go`:
- Around line 153-178: Refactor the conditional that sets "instructions" so the
raw passthrough branch mirrors the non-raw path: consolidate the logic around
gjson.Get(out, "instructions"), ChannelSetting.SystemPrompt and
ChannelSetting.SystemPromptOverride (referencing variables systemPrompt and
instructions) so that when no system prompt exists you explicitly set
"instructions" to "" and when a system prompt exists you either set it or
prepend/append existing instructions per SystemPromptOverride; this keeps the
handling identical between the two paths and avoids divergent branching
structures.
In `@relay/channel/codex/error.go`:
- Around line 28-35: The current unbounded io.ReadAll on resp.Body can allocate
arbitrarily large memory; change the read to use an io.LimitReader so we only
read a bounded number of bytes (e.g., 256KB) into responseBody before calling
extractCodexErrorMessage/truncateErrorMessage. Locate the block that sets
responseBody (checking resp != nil && resp.Body != nil) and replace the
io.ReadAll(resp.Body) invocation with io.ReadAll(io.LimitReader(resp.Body,
256*1024)), keeping the existing error handling and the call to
service.CloseResponseBodyGracefully(resp).
- Around line 52-86: extractCodexErrorMessage may return a large structured JSON
when it matches the bare "error" path; update the search logic in
extractCodexErrorMessage to (1) prefer nested/common Codex/OpenAI shapes by
adding paths like "error.error.message", "errors.0.message", "errors.#.message"
(or similar common variants) before the plain "error" entry, and (2) treat the
plain "error" match as valid only for scalar values—if result.Type is an
object/array, skip it so the function falls back to other candidates or the
trimmed raw body instead of returning a cryptic JSON blob.
In `@relay/channel/codex/image.go`:
- Line 282: The call to setStringToolOption(tool, "input_fidelity",
firstFormValue(form, "input_fidelity")) only reads input_fidelity from the
multipart form and thus misses JSON-body edit requests; update the code that
builds the tool options (the spot using setStringToolOption and firstFormValue)
to fallback to dto.ImageRequest.InputFidelity (or the appropriate field on
dto.ImageRequest) when firstFormValue(form, "input_fidelity") is empty/nil so
both multipart and JSON edit paths propagate input_fidelity; ensure you
reference dto.ImageRequest where available and keep precedence: form value
first, then dto.ImageRequest, then leave unset.
- Around line 215-225: The code currently falls back to
"application/octet-stream" and returns a data URL even when
http.DetectContentType fails to recognize an image, which can cause Codex to
reject the upload; update the logic around mediaType in the function that builds
the data URL so that if fileHeader.Header.Get("Content-Type") is
"application/octet-stream" (or empty) and http.DetectContentType(data) does not
start with "image/", you either return a clear error (e.g., ErrInvalidImage)
instead of constructing a non-image data URL, or (if you must produce an image)
default mediaType to "image/png" for small/required image fields; adjust the
return path that currently uses mediaType and
base64.StdEncoding.EncodeToString(data) accordingly and ensure callers of this
function handle the new error or PNG default.
In `@relay/channel/codex/responses.go`:
- Around line 47-48: The cloned response can carry a stale Content-Length after
converting SSE to aggregated JSON; update cloneResponseWithContentType (or
immediately after creating jsonResp before calling
service.IOCopyBytesGracefully) to also remove the "Content-Length" header (in
addition to "Transfer-Encoding") so the response length isn't misleading when
the body is reconstructed/changed; ensure the header deletion targets the exact
"Content-Length" key on the http header map used by
cloneResponseWithContentType/jsonResp.
🪄 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: f2a4a3e5-ddfa-4a95-b827-373567b2b6d0
📒 Files selected for processing (15)
common/model.gocontroller/channel-test.godto/openai_image.godto/openai_response.gorelay/channel/codex/adaptor.gorelay/channel/codex/adaptor_test.gorelay/channel/codex/constants.gorelay/channel/codex/error.gorelay/channel/codex/image.gorelay/channel/codex/responses.gorelay/constant/relay_mode.gorelay/constant/relay_mode_test.gorelay/image_handler.gorelay/responses_handler.gorouter/relay-router.go
Comment on lines
+524
to
+554
| func buildImageAPIResponse(results []imageCallResult, createdAt int64, usage *dto.Usage, firstMeta imageCallResult, responseFormat string) ([]byte, error) { | ||
| responseFormat = strings.ToLower(strings.TrimSpace(responseFormat)) | ||
| if responseFormat == "" { | ||
| responseFormat = "b64_json" | ||
| } | ||
|
|
||
| imageResponse := dto.ImageResponse{ | ||
| Created: createdAt, | ||
| Data: make([]dto.ImageData, 0, len(results)), | ||
| Background: firstMeta.Background, | ||
| OutputFormat: firstMeta.OutputFormat, | ||
| Quality: firstMeta.Quality, | ||
| Size: firstMeta.Size, | ||
| } | ||
| if usage != nil && usage.TotalTokens > 0 { | ||
| imageResponse.Usage = usage | ||
| } | ||
|
|
||
| for _, result := range results { | ||
| item := dto.ImageData{ | ||
| RevisedPrompt: result.RevisedPrompt, | ||
| } | ||
| if responseFormat == "url" { | ||
| item.Url = "data:" + mimeTypeFromOutputFormat(result.OutputFormat) + ";base64," + result.Result | ||
| } else { | ||
| item.B64Json = result.Result | ||
| } | ||
| imageResponse.Data = append(imageResponse.Data, item) | ||
| } | ||
| return common.Marshal(imageResponse) | ||
| } |
Contributor
There was a problem hiding this comment.
response_format=url returns a base64 data: URL, not a hosted URL.
When the client requests response_format=url, the result is a data:image/<fmt>;base64,... URL embedding the full image. This differs from the OpenAI-hosted URL semantics clients may expect, and can become very large (multi-MB) for high-resolution outputs. Consider documenting this in the PR description or in a comment near buildImageAPIResponse, since some clients may treat Url and B64Json quite differently (e.g., trying to fetch the URL, logging, size limits).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/codex/image.go` around lines 524 - 554, The
buildImageAPIResponse function currently sets dto.ImageData.Url to a
data:...;base64,... string (via mimeTypeFromOutputFormat and result.Result) when
responseFormat == "url", which returns an embedded base64 data URL rather than
an OpenAI-hosted URL; update the codebase by adding a clear comment immediately
above buildImageAPIResponse (or above dto.ImageData.Url field) stating that
response_format="url" produces an embedded data URL (not a hosted URL), can be
very large for high-res outputs, and may break client assumptions (e.g.,
attempting to fetch the URL or logging size limits); if desired by product,
optionally implement a different code path in buildImageAPIResponse to return
hosted URLs instead, otherwise document the semantics so callers use B64Json
when appropriate.
Comment on lines
+365
to
+379
| func recordResponsesBuiltInToolUsageFromGJSON(info *relaycommon.RelayInfo, response gjson.Result) { | ||
| if info == nil || info.ResponsesUsageInfo == nil || info.ResponsesUsageInfo.BuiltInTools == nil { | ||
| return | ||
| } | ||
| tools := response.Get("tools") | ||
| if !tools.IsArray() { | ||
| return | ||
| } | ||
| for _, tool := range tools.Array() { | ||
| buildToolInfo, ok := info.ResponsesUsageInfo.BuiltInTools[tool.Get("type").String()] | ||
| if ok && buildToolInfo != nil { | ||
| buildToolInfo.CallCount++ | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for usages and existing patterns of CallCount / BuiltInTools to confirm intent
rg -nP --type=go -C3 '\bBuiltInTools\b'
echo "---"
rg -nP --type=go -C5 '\bCallCount\b'
echo "---"
# Find similar handlers for comparison (e.g., openai responses handler counting)
rg -nP --type=go -C5 'recordResponses.*Tool'Repository: QuantumNous/new-api
Length of output: 25089
🏁 Script executed:
# Look for tests and documentation about Codex response.tools format
rg -nP --type=go "response\.completed" relay/channel/codex/ -A 5
echo "---"
# Search for any Codex API reference or comments
rg -nP --type=go -i "codex.*tool" relay/channel/codex/responses.go -B 2 -A 2
echo "---"
# Look at how recordResponsesBuiltInToolUsageFromGJSON is called and what responses it receives
rg -nP --type=go "recordResponsesBuiltInToolUsageFromGJSON" -B 10 -A 2Repository: QuantumNous/new-api
Length of output: 10890
🏁 Script executed:
# Search for test data or samples that show response.tools field
rg -nP --type=go "\.tools" relay/channel/codex/ -B 3 -A 3
echo "---"
# Look at test fixtures to see actual response structure
rg -nP "response\.completed" relay/channel/codex/adaptor_test.go -A 5 | head -100
echo "---"
# Check if response.tools is ever populated or documented
rg -nP "tools.*array\|tools.*object" relay/channel/codex/ -iRepository: QuantumNous/new-api
Length of output: 2663
🏁 Script executed:
# Check the implementation and structure of response object more carefully
cat -n relay/channel/codex/responses.go | sed -n '365,379p'
echo "---"
# Look for where tools might be added to response
rg -nP "response.*tools\|\"tools\"" relay/channel/codex/ --type=go
echo "---"
# Check if response.tools appears in any test data or is ever expected
rg -nP '\"tools\"' relay/channel/ --type=goRepository: QuantumNous/new-api
Length of output: 1161
🏁 Script executed:
# Look at the adaptor_test.go context around line 83 to see what tools field represents
cat -n relay/channel/codex/adaptor_test.go | sed -n '75,95p'
echo "---"
# Check adaptor.go to understand the relationship between request tools and response tools
cat -n relay/channel/codex/adaptor.go | sed -n '135,150p'
echo "---"
# Look for any comments or documentation about response.tools in Codex integration
rg -nP -B 5 "response\.Get.*tools" relay/channel/codex/Repository: QuantumNous/new-api
Length of output: 2022
🏁 Script executed:
# Find normalizeCodexResponsesTools to understand what response.tools contains
rg -nP "normalizeCodexResponsesTools" relay/channel/codex/ -A 20
echo "---"
# Search for any documentation or comments about Codex response structure
rg -nP "response\.completed" relay/channel/codex/ -B 2 -A 8 | head -80
echo "---"
# Check if there's any actual response sample with tools field from Codex
git log --all --full-history --source -- relay/channel/codex/responses.go | head -20Repository: QuantumNous/new-api
Length of output: 10743
🏁 Script executed:
# Let's verify: in actual test data, does response.completed ever have a "tools" field?
rg -nP '\"response\.completed\"' relay/channel/codex/adaptor_test.go -A 1 | grep -E '"response|"tools"'
echo "---"
# Check if there are any real examples in other tests
rg -nP 'response\.completed' relay/channel/ --type=go -A 2 | grep -E 'tools|image_gen|tool_usage' | head -20
echo "---"
# Let's examine what happens when tools field doesn't exist
python3 << 'EOF'
import json
# Simulate the behavior with missing tools field
response_text = '{"id":"resp_1","object":"response","created_at":1,"output":[],"usage":{"input_tokens":2,"output_tokens":3}}'
response = json.loads(response_text)
tools = response.get("tools")
print(f"tools field exists: {tools is not None}")
print(f"tools value: {tools}")
print(f"is array: {isinstance(tools, list)}")
# The code does: if !tools.IsArray() return
# In Go, this would also return early for None
EOFRepository: QuantumNous/new-api
Length of output: 2486
recordResponsesBuiltInToolUsageFromGJSON has dead code that suggests API misunderstanding.
This function attempts to iterate response.tools and increment CallCount, but Codex's response.completed objects never include a tools field. The array check !tools.IsArray() returns true immediately, causing the function to exit without counting anything.
Tool invocations are correctly tracked elsewhere via recordResponsesBuiltInToolCall on stream events (image_generation_call, web_search_call), which increments the same CallCount counter. The unused loop in this function suggests a misalignment with the actual Codex Responses API format.
Consider either:
- Removing this function entirely if it serves no purpose, or
- Adding a comment clarifying why it exists (defensive coding for future API changes).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/codex/responses.go` around lines 365 - 379,
recordResponsesBuiltInToolUsageFromGJSON is dead code because Codex Response
objects don't include a "tools" field so the array check always fails; either
remove this function and any unused references to it (and rely on
recordResponsesBuiltInToolCall which already updates
ResponsesUsageInfo.BuiltInTools.CallCount), or keep it but add a clear comment
above recordResponsesBuiltInToolUsageFromGJSON explaining it's
defensive/placeholder for future API changes and why the current API doesn't
populate "tools" (referencing ResponsesUsageInfo.BuiltInTools and
recordResponsesBuiltInToolCall for context).
11 tasks
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
为 Codex 渠道增加
gpt-image-2图片生成与编辑支持，并补齐 Codex responses 路径所需的兼容处理。主要变更：
/v1/responses请求中，将字符串input转为 Codex 可接受的 user message list。stream=true，并在下游非流式场景内部聚合 Codex SSE 为标准 Responses JSON。/v1/images/generations和/v1/images/edits转换，将 OpenAI image 请求转为 Codex/backend-api/codex/responses+image_generationtool。/backend-api/codex/responses//responses/compactrelay route。实现范围限定在 Codex channel。
gpt-image-2仍需要通过正常渠道模型/abilities 配置启用，不绕过现有渠道选择、分组、倍率和计费逻辑。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
本地测试通过：
手动验证过的场景：
/v1/responses非流式请求返回 200。/v1/responses流式请求返回text/event-stream。/v1/images/generations使用gpt-image-2返回 200。/v1/images/edits使用 multipart 图片上传返回 200。invalid character 'e' looking for beginning of value。Summary by CodeRabbit
New Features
Bug Fixes