Repository navigation
feat(providers): convert OpenAI tool result media like user files - #74
ethanndickson wants to merge 8 commits into
Conversation
95c3a00 to
4cbcb15
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
4cbcb15 to
837d14b
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e909cdae2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The Responses API accepts an array of input_text, input_image and input_file items as function_call_output. Send media tool results in that array instead of a synthetic user message after the output. The media uses the same conversion as a user file part, so PDFs now reach the model too. A parallel batch no longer has a user message between its outputs. Media that cannot be converted becomes a text output that says the content cannot be displayed, plus a warning.
Move the Chat Completions conversion of a user file part into openai.FilePartToChatContentPart. The openai and openaicompat user message paths now call it instead of keeping two copies of the same switch. Behavior does not change. The converter also lets tool-result media use the same rules as a user file part.
toolResultMediaUserPart only knew images and audio, so a PDF from a tool reached Chat Completions models as a placeholder and a warning. It now decodes the media and converts it with FilePartToChatContentPart, the same rules as a user file. A PDF becomes a file part that keeps the tool's filename, in the deferred user message. openai, openaicompat, openrouter and vercel all get this through ToolMessages. When the media cannot be converted and the tool also returned text, the tool message now keeps the text and adds the "cannot be displayed" notice after a blank line. Before, the text went out alone, so the model could assume it saw media that was never sent. Media data that is not valid base64 now gets a warning and the placeholder instead of going out as a broken data URL. The prompttest corpus gains a PDF case and an unsupported-with-text case.
The user file rules send text/* as a file part with bare base64 file_data and no filename. On the Chat tool path that part reached openai, openaicompat, openrouter and vercel, and a provider that rejects it fails the request. The tool result stays in the chat, so every later request fails too. Text tool media now gets the "cannot be displayed" placeholder and the unsupported warning again, as before this stack. PDF, image and audio still use the shared converter. The user path and FilePartToChatContentPart do not change.
The Chat Completions converter sent text/* files as a file part with bare base64 file_data. The Responses, OpenRouter and Vercel converters already refuse them, so user text files now get the same unsupported warning, and tool-result text media no longer needs its own exception.
The function_call_output array now holds the media item and then the accompanying text, the same order as the Anthropic tool_result content.
… its text The notice takes the place of the media, so it now comes first, the same order as media then text in the Anthropic tool_result content. Media without a media type is named "file" in the notice.
… trip Tool-result media data is already base64, but the Chat and Responses converters decoded it with FilePart() and then encoded it again. For a 4 MiB image in a Chat request this took 4.4 ms and 21 MB; it now takes 0.5 ms and 5.6 MB, the same as a user image. Both converters now take a providers/internal/filepart.File and send its base64 unchanged. A tool-result PDF is no longer read as an OpenAI file ID; only user file parts can carry one. Invalid base64 goes to the API unchanged, the same as upstream.
7e909cd to
91401a1
Compare
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
A PDF tool result never reached the model on the OpenAI paths, as the tool media conversion only knew about images (and audio, on Chat Completions). On Responses, a PDF went out as the placeholder "...see the following user message." with no message after it. On Chat Completions (openai, openaicompat, openrouter and vercel), it became a note saying the content can't be displayed.
For Responses,
function_call_output.outputaccepts an array ofinput_text,input_imageandinput_file, so we'll now put tool media there, converted the same way as a user file part, followed by the tool's text. That's the same order as the Anthropictool_result. Before, images went in a synthetic user message after each output, which also landed between the outputs of a parallel tool batch.Chat Completions tool messages can only hold text, so the media still goes in the user message after the run of tool messages, but that message now uses the user file part converter. I've exported it as
openai.FilePartToChatContentPart, so openaicompat can drop its copy. A PDF becomes afilepart with the tool's filename. I've also dropped thetext/*case from that converter, which sent text files as afilepart with bare base64 infile_data. User text files on Chat Completions now get the same unsupported warning that Responses, openrouter and vercel already give, and text tool media gets the note without a special case. This is a change from upstream for user files on openai and openaicompat. Where we can't convert the media, the tool message now keeps the tool's text and adds the note. Upstream sends the text alone, so the model could assume it saw a file that was never sent.I've checked a PDF and a PNG tool result on Responses, openai Chat and openaicompat Chat through our AI gateway, and since then through chatd on a local coderd, where the model read every file on all three, including from parallel tool calls and with Responses reasoning replayed in the tool loop. I haven't been able to try Bedrock Mantle, which coder/coder uses for non-Claude Bedrock models over Responses, so I'm not sure it accepts the array output. If it doesn't, image tool results there would break where they worked before.
Part of CODAGT-1430