Skip to content

fix(codex): preserve image content in tool outputs #2349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DarkNoah wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix(codex): preserve image content in tool outputs #2349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
38cf661
fix(codex): preserve image content in tool outputs
DarkNoah Mar 25, 2026
37cfef3
fix(codex): enhance handling of image and file content in tool outputs
DarkNoah Mar 26, 2026
bcd8672
fix(codex): improve handling of null and object content in tool outputs
DarkNoah Mar 31, 2026
726381c
fix(codex): require file payload for user file parts
DarkNoah Apr 1, 2026
831e937
fix(codex): guard tool image output sources
DarkNoah Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 102 additions & 4 deletions internal/translator/codex/openai/chat-completions/codex_openai_request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,94 @@ func ConvertOpenAIRequestToCodex(modelName string, inputRawJSON []byte, stream b
case "tool":
// Handle tool response messages as top-level function_call_output objects
toolCallID := m.Get("tool_call_id").String()
content := m.Get("content").String()
content := m.Get("content")

// Create function_call_output object
funcOutput := []byte(`{}`)
funcOutput, _ = sjson.SetBytes(funcOutput, "type", "function_call_output")
funcOutput, _ = sjson.SetBytes(funcOutput, "call_id", toolCallID)
funcOutput, _ = sjson.SetBytes(funcOutput, "output", content)

// Handle content: can be string, array, or any other JSON value.
// Always set output to avoid dropping tool payloads for null/non-array content.
if content.Type == gjson.String {
funcOutput, _ = sjson.SetBytes(funcOutput, "output", content.String())
} else if content.IsArray() {
Comment on lines +133 to +135
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep tool output when content is not string or array

This branch now sets function_call_output.output only when tool content is a string or an array. If a caller sends tool content as an object, number, boolean, or null, the generated function_call_output has no output field at all, whereas the previous code always serialized content with .String(). In those inputs, tool results are dropped (or the request is rejected as malformed), so this should include an else fallback that preserves non-array payloads.

Useful? React with 👍 / 👎.

// Build output array from content items
outputArr := []byte(`[]`)
items := content.Array()
for j := 0; j < len(items); j++ {
it := items[j]
t := it.Get("type").String()
switch t {
case "text":
outputPart := []byte(`{}`)
outputPart, _ = sjson.SetBytes(outputPart, "type", "input_text")
outputPart, _ = sjson.SetBytes(outputPart, "text", it.Get("text").String())
outputArr, _ = sjson.SetRawBytes(outputArr, "-1", outputPart)
case "image_url":
// Handle image_url content in tool message
imageURL := it.Get("image_url.url").String()
fileID := it.Get("image_url.file_id").String()
if imageURL != "" || fileID != "" {
outputPart := []byte(`{}`)
outputPart, _ = sjson.SetBytes(outputPart, "type", "input_image")
if imageURL != "" {
outputPart, _ = sjson.SetBytes(outputPart, "image_url", imageURL)
}
if fileID != "" {
outputPart, _ = sjson.SetBytes(outputPart, "file_id", fileID)
}
if detail := it.Get("image_url.detail").String(); detail != "" {
outputPart, _ = sjson.SetBytes(outputPart, "detail", detail)
}
outputArr, _ = sjson.SetRawBytes(outputArr, "-1", outputPart)
} else {
outputPart := []byte(`{}`)
outputPart, _ = sjson.SetBytes(outputPart, "type", "input_text")
outputPart, _ = sjson.SetBytes(outputPart, "text", it.Raw)
outputArr, _ = sjson.SetRawBytes(outputArr, "-1", outputPart)
}
case "file":
// Handle file content in tool message
fileID := it.Get("file.file_id").String()
fileData := it.Get("file.file_data").String()
filename := it.Get("file.filename").String()
fileUrl := it.Get("file.file_url").String()
if fileID != "" || fileData != "" || filename != "" || fileUrl != "" {
outputPart := []byte(`{}`)
outputPart, _ = sjson.SetBytes(outputPart, "type", "input_file")
Comment on lines +177 to +179
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require file payload in tool output file parts

This guard now allows filename alone to create an input_file entry inside function_call_output.output. If a tool message contains {"type":"file","file":{"filename":"x.pdf"}} without file_id, file_data, or file_url, we emit an unresolved file part that is invalid for downstream Responses processing and can turn a previously tolerated malformed item into a request failure. Restrict emission to cases where at least one real file source is present.

Useful? React with 👍 / 👎.

if fileID != "" {
outputPart, _ = sjson.SetBytes(outputPart, "file_id", fileID)
}
if fileData != "" {
outputPart, _ = sjson.SetBytes(outputPart, "file_data", fileData)
}
if filename != "" {
outputPart, _ = sjson.SetBytes(outputPart, "filename", filename)
}
if fileUrl != "" {
outputPart, _ = sjson.SetBytes(outputPart, "file_url", fileUrl)
}
outputArr, _ = sjson.SetRawBytes(outputArr, "-1", outputPart)
}

default:
// Fallback: convert unknown types to string representation
outputPart := []byte(`{}`)
outputPart, _ = sjson.SetBytes(outputPart, "type", "input_text")
outputPart, _ = sjson.SetBytes(outputPart, "text", it.Raw)
outputArr, _ = sjson.SetRawBytes(outputArr, "-1", outputPart)
}
}
funcOutput, _ = sjson.SetRawBytes(funcOutput, "output", outputArr)
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fall back when tool content array has unknown part types

This branch always rewrites array-based tool content into outputArr and then assigns it to output, but the loop only maps text and image_url parts. Any other part type is silently dropped, and if none match the resulting function_call_output.output becomes [], losing the tool payload entirely. That is a regression from the previous behavior (which preserved the original content as a string) and will break tool calls that return other structured parts (for example file/audio/custom parts) because the model no longer receives that output.

Useful? React with 👍 / 👎.

} else {
fallbackOutput := content.Raw
if fallbackOutput == "" {
fallbackOutput = content.String()
}
funcOutput, _ = sjson.SetBytes(funcOutput, "output", fallbackOutput)
}

out, _ = sjson.SetRawBytes(out, "input.-1", funcOutput)
Comment on lines 128 to 212
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Throughout this switch block, errors returned by sjson.SetBytes and sjson.SetRawBytes are consistently ignored. While sjson is designed to be robust, silently discarding errors can lead to malformed JSON output if unexpected input data or paths are encountered. This can result in incorrect API requests being sent without any indication of a problem.

It is highly recommended to implement error handling for these operations, at least by logging the errors. For example, you could change funcOutput, _ = sjson.SetBytes(...) to funcOutput, err := sjson.SetBytes(...); if err != nil { log.Printf("sjson error: %v", err) } to ensure that any issues during JSON construction are visible and can be debugged. This improves the robustness and maintainability of the translation logic.


default:
Expand Down Expand Up @@ -177,16 +258,33 @@ func ConvertOpenAIRequestToCodex(modelName string, inputRawJSON []byte, stream b
if u := it.Get("image_url.url"); u.Exists() {
part, _ = sjson.SetBytes(part, "image_url", u.String())
}
if fid := it.Get("image_url.file_id").String(); fid != "" {
part, _ = sjson.SetBytes(part, "file_id", fid)
}
if detail := it.Get("image_url.detail").String(); detail != "" {
part, _ = sjson.SetBytes(part, "detail", detail)
}
msg, _ = sjson.SetRawBytes(msg, "content.-1", part)
}
case "file":
if role == "user" {
fileID := it.Get("file.file_id").String()
fileData := it.Get("file.file_data").String()
fileURL := it.Get("file.file_url").String()
filename := it.Get("file.filename").String()
if fileData != "" {

if fileData != "" || fileURL != "" {
part := []byte(`{}`)
part, _ = sjson.SetBytes(part, "type", "input_file")
part, _ = sjson.SetBytes(part, "file_data", fileData)
if fileID != "" {
part, _ = sjson.SetBytes(part, "file_id", fileID)
}
if fileData != "" {
part, _ = sjson.SetBytes(part, "file_data", fileData)
}
if fileURL != "" {
part, _ = sjson.SetBytes(part, "file_url", fileURL)
}
if filename != "" {
part, _ = sjson.SetBytes(part, "filename", filename)
}
Expand Down
Loading
Loading