fix(proxy): stop folding search findings into the text on /v1/messages - #154
Merged
Merged
Conversation
A turn that mixed a locally executed web search with a client-owned tool call could not continue in the loop, so `buildMixedTurnPayload` folded the search findings into the assistant text alongside the outstanding calls. On /v1/messages those findings were already on the wire: the route emits `server_tool_use` and `web_search_tool_result` blocks for the same search. The client therefore received every result twice — once as a result block and once as prose reading as if the model had written it, including the "Cite the URL of any result you rely on" line, which is an instruction to the model rather than something the user asked to see. Replaying that transcript put both copies back on the wire to the upstream model. The fold exists because a mixed turn cannot be continued locally: the client owns the outstanding calls, and re-issuing the transcript with only server-tool results would leave them unanswered. It is still needed by routes with no other way to carry the findings, so it is now opted out of rather than removed: routes that render findings structurally pass `findingsAsStructuredBlocks`, and the text is left alone. /v1/messages sets the flag on both its streaming and non-streaming paths. /v1/chat/completions and /v1/responses keep folding — neither has a structured channel for the results, so the fold is their only way to surface them.
![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: 205db66b2b
ℹ️ 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".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #154 +/- ##
=======================================
Coverage 95.41% 95.41%
=======================================
Files 36 36
Lines 6349 6351 +2
Branches 1813 1816 +3
=======================================
+ Hits 6058 6060 +2
Misses 291 291
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
The opt-out added in the previous commit was only consulted by `buildMixedTurnPayload`. A streaming /v1/messages request whose *first* upstream turn mixed a local search with a client-owned call never reaches that helper: `createInlineServerToolStream` emits its own text delta from the search results before handing the outstanding calls back, so the findings were still sent as prose alongside the `web_search_tool_result` block on the most common streaming path. Apply the same flag there. The result event carries the findings once, so a text copy is the second.
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
1 participant
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.
What happened
A turn that mixed a locally executed web search with a client-owned tool call (e.g.
Bash) could not continue in the server-tool loop — the client owns the outstanding calls — sobuildMixedTurnPayloadfolded the search findings into the assistant text alongside those calls.On
/v1/messagesthose findings were already on the wire: the route emitsserver_tool_useandweb_search_tool_resultblocks for the same search. The client got every result twice — once as a structured block, once as prose reading as if the model had written it, including this line, which is an instruction to the model rather than something the user asked to see:Reproduced against one upstream call, streaming and non-streaming. Client-facing blocks were:
Replaying that transcript put both copies back on the wire to the upstream model, so the duplication also accumulated across turns.
Why the fold existed
A mixed turn genuinely cannot be continued locally: re-issuing the transcript with only server-tool results would leave the client's calls unanswered, which upstream rejects as an invalid tool-call transcript. Handing the outstanding calls back with the findings attached is what keeps the transcript valid — so the fold is removed only where the results have another channel, not deleted outright.
The change
Routes that render findings structurally now pass
findingsAsStructuredBlocks, andbuildMixedTurnPayloadleaves the text alone./v1/messagessets it on both the streaming and non-streaming paths./v1/chat/completionsand/v1/responseskeep folding — neither has a structured channel for the results, so the fold is their only way to surface them. (/v1/responsesemits onlyaction.queryviaweb_search_call, never the results.)Tests
Two tests, both verified to fail/pass as expected:
does not fold findings into the text when a structured block carries them— asserts the result block is present, the prose does not repeat the URL, and the model-only instruction never reaches the user. Fails without the fix.keeps folding findings for routes without a structured channel— guards/v1/chat/completionsso the opt-out is not applied more broadly. Passes with and without the fix.Verification
lint,format:check,typecheck,test:coverage(95.24% lines, above the 90% floor),build, andtest:patch-branches(100%) all pass.Note
Related: #152 fixes adjacent block-ordering for non-streaming turns. Its commit message notes the merged-string problem is unfixable at the renderer because
withIntermediateTurnscollapses every hop into onecontent— the same constraint that made this fold the wrong place to carry the results.