Repository navigation
Conversation
With store false a hosted web search was dropped from later turns: item_reference returns 404 when nothing is stored, so the builder sent nothing and the model denied having searched. Rebuild the search from its result metadata as an inline web_search_call without an id. Without an id the API does no lookup, so the item cannot resolve to a stored item of another credential. The store true path is unchanged.
This comment has been minimized.
This comment has been minimized.
|
Chat: Review posted | View chat Review history
deep-review v0.13.0 | Round 8 | Last posted: Round 8, 24 findings (1 P1, 1 P2, 5 P3, 11 P4, 6 Nit), APPROVE. Review Finding inventoryFindings
Contested and acknowledgedCRF-27 (P4, responses_web_search_test.go:577) - StoredResultBeforeCall pins an unprobed wire shape
CRF-16 (P4, responses_language_model.go:1069) - hasVisibleResponsesAssistantContent hand-kept list
CRF-4 (P1, responses_language_model.go:929) - Pre-#62 find_in_page replays without url and pattern
CRF-5 (P2, responses_language_model.go:698) - store gate drops unreferenced searches
CRF-7 (P3, responses_language_model.go:706) - Inline web_search_call sent to non-OpenAI Responses backends
Round logRound 1Netero first pass: 2 P4, 1 Note. No P0, panel proceeds. Round 2Fix round (no new files created; Round 3Fix round: a410684 changes only Round 4Restructure round: PR title and description changed goal from unstored to unreferenced searches; 37e7dd2 removes the store gate. Churn guard PROCEED: CRF-17 addressed, CRF-16 contested. CRF-5 status changed from Deferred to Author fixed (37e7dd2); verified by Meruem, Ryosuke, Melody, Pariston, Mafuuu, Bisky, Razor, Netero. a52f08e reverted the CRF-2 struct reuse in response to CRF-14; Netero and Ryosuke accept. Netero: no new findings. Full panel: ging-go, ryosuke, melody, pariston, mafuuu, bisky, gon, leorio, wildcard razor; meruem resumed for CRF-16 dispute (closed 1/1). New: 1 P3, 2 P4, 1 Nit, 2 out of scope. CRF-20 set at P3 (Gon P3, Leorio P4; higher wins, since 37e7dd2 sends the item on stored requests where an id fails). Reviewed against 2e8e2cb..50bf6c5. Round 5Fix round: 67300d9 only. Churn guard PROCEED: CRF-18 to CRF-21 addressed. Netero: CRF-24 (P3). Panel: meruem (CRF-18 raiser), bisky (tests changed), gon, leorio (comments and commit text), mafuuu (CRF-21 raiser), ging-go, ryosuke, melody (files changed), wildcard zoro. Workspace agent disconnected mid-round; meruem, bisky and melody were resumed with message_agent after reconnect and wrote their files. CRF-18 to CRF-21 verified. Meruem and Bisky converge with Netero on CRF-24 and propose a class fix (derive reference adjacency instead of manual flag resets), which also removes OOS CRF-28 and inlinedSearches. CRF-25 Meruem P4 and Zoro P4. CRF-27 raised from Mafuuu Note to P4: keep-argument for Note is that it asks only for a probe; raised because the test and the proposed CRF-24 fix both pin a wire shape that may get a 400. CRF-29 and CRF-30 dropped: keep-argument is that commit text is visible in PR history; dropped because base history (#62, #67, #72) shows squash merges, so the PR description replaces it. CRF-24 kept at P3 (three reviewers P3): keep-argument is that base accepted the prompt and 67300d9 errors on every later turn; downgrade argument is that only caller-built prompts reach it. Reviewed against 2e8e2cb..67300d9. Round 6Fix round: cb92642 and 9bd1001 (patches to the existing branches; no title change). Churn guard PROCEED: CRF-24, CRF-25, CRF-26 addressed, CRF-27 contested. Netero: no findings; mutation runs confirm the new tests. Panel: mafuuu resumed for CRF-27 (closed 1/1), meruem, bisky, zoro, ryosuke (raisers of the fixed findings), ging-go, melody (files changed), gon (comments added), leorio (new commit messages), wildcard kite. CRF-24, CRF-25, CRF-26 and out-of-scope CRF-28 verified fixed by Netero, Meruem, Bisky, Zoro, Ryosuke, Melody, Kite. New: 1 P4 (CRF-31), 1 Nit (CRF-32). CRF-31 merges Gon P3 and Leorio P4 on the same comment and is set at P4: keep-argument for P3 is that the panel itself proposed Round 7Fix round: e06de3d (comment) and b4d4cde (test rename); no production logic changed. Churn guard PROCEED: CRF-31, CRF-32 addressed. Netero: no findings. Panel: gon, leorio (CRF-31/32 raisers; comment changed), ging-go (Go files), bisky (tests changed), wildcard kite. CRF-32 verified. CRF-31 partially fixed: Gon and Leorio both verify "referenceable" is now defined but the reason for counting parts (a reference after a part that appends nothing is unprobed) is still only in the review reply; re-raised on the existing thread, no new ID. Kite read it as fixed; Gon and Leorio raised it and cite the specific missing clause, so their evidence sets the floor. Reviewed against 2e8e2cb..b4d4cde. Round 8Fix round: e782df2 rewrites only the About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
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. |
This comment has been minimized.
This comment has been minimized.
![coder-agents-review[bot]](https://avatars.githubusercontent.com/in/3454270?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Reuse WebSearchAction for the inline item so new action fields replay, pass it to param.Override without an unreachable marshal error, and warn when a search's metadata has no replayable action. Tighten the comments and name the pre-#62 test cases by their shape.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The skip warning now says whether the search had no action or an action type the API cannot take, and names that type.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The fake Responses server required a reasoning item before every web_search_call. Live probes in coder/fantasy#78 show OpenAI accepts an inline web_search_call without an id and no reasoning item, so only searches with an id need one.
coder/fantasy#78 resends an unstored web search as an inline web_search_call without an id. That item carries no provider state, so the stale-replay check now flags a web_search_call only when it has an id, and the stateless case expects the inline item.
The head adds warnings for searches it drops and reuses WebSearchAction for the inline item; the replayed items are the same.
The fake Responses server required a reasoning item before every web_search_call. Live probes in coder/fantasy#78 show OpenAI accepts an inline web_search_call without an id and no reasoning item, so only searches with an id need one.
coder/fantasy#78 resends an unstored web search as an inline web_search_call without an id. That item carries no provider state, so the stale-replay check now flags a web_search_call only when it has an id, and the stateless case expects the inline item.
The head adds warnings for searches it drops and reuses WebSearchAction for the inline item; the replayed items are the same.
This comment has been minimized.
This comment has been minimized.
e7a80a4 to
b4d4cde
Compare
This comment has been minimized.
This comment has been minimized.
The fake Responses server required a reasoning item before every web_search_call. Live probes in coder/fantasy#78 show OpenAI accepts an inline web_search_call without an id and no reasoning item, so only searches with an id need one.
coder/fantasy#78 resends an unstored web search as an inline web_search_call without an id. That item carries no provider state, so the stale-replay check now flags a web_search_call only when it has an id, and the stateless case expects the inline item.
The head adds warnings for searches it drops and reuses WebSearchAction for the inline item; the replayed items are the same.
coder/fantasy#78 now sends a search inline whenever it does not go back as an item reference, not only when store is false. A stored turn after an unstored one, and stored searches with no reasoning before them, now keep their searches; the tests expect the inline item in those cases.
The head references a search only from the part right after its reasoning and replays each search once, so a search is never sent both inline and as a reference.
The head changes only comments and a test field name since 9bd1001.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The fake Responses server required a reasoning item before every web_search_call. Live probes in coder/fantasy#78 show OpenAI accepts an inline web_search_call without an id and no reasoning item, so only searches with an id need one.
coder/fantasy#78 resends an unstored web search as an inline web_search_call without an id. That item carries no provider state, so the stale-replay check now flags a web_search_call only when it has an id, and the stateless case expects the inline item.
The head adds warnings for searches it drops and reuses WebSearchAction for the inline item; the replayed items are the same.
coder/fantasy#78 now sends a search inline whenever it does not go back as an item reference, not only when store is false. A stored turn after an unstored one, and stored searches with no reasoning before them, now keep their searches; the tests expect the inline item in those cases.
The head references a search only from the part right after its reasoning and replays each search once, so a search is never sent both inline and as a reference.
The head changes only comments and a test field name since 9bd1001.
The fake Responses server required a reasoning item before every web_search_call. Live probes in coder/fantasy#78 show OpenAI accepts an inline web_search_call without an id and no reasoning item, so only searches with an id need one.
coder/fantasy#78 resends an unstored web search as an inline web_search_call without an id. That item carries no provider state, so the stale-replay check now flags a web_search_call only when it has an id, and the stateless case expects the inline item.
The head adds warnings for searches it drops and reuses WebSearchAction for the inline item; the replayed items are the same.
coder/fantasy#78 now sends a search inline whenever it does not go back as an item reference, not only when store is false. A stored turn after an unstored one, and stored searches with no reasoning before them, now keep their searches; the tests expect the inline item in those cases.
The head references a search only from the part right after its reasoning and replays each search once, so a search is never sent both inline and as a reference.
The head changes only comments and a test field name since 9bd1001.
Relates to CODAGT-1482
toResponsesPromptonly replays a hosted web search as anitem_reference, and only right after a reasoning item from a stored response. Every other search is dropped, and the model then says it made no tool calls. That covers every search withstore: false, every search from an unstored response when a later request is stored (a chat switching model configs, or a zero data retention org), and stored searches with no reasoning before them.We'll now send each search that doesn't get a reference inline, rebuilt from its tool result's
WebSearchCallMetadataas{"type":"web_search_call","status":...,"action":{...}}. I've used the result rather than the call input because only the result keeps the status and the deprecated singlequery, and searches saved before #62 still have their action there. Only the action fields the API uses are copied;action.sourcesare left out because the API ignores them on input. A search whose metadata has no action, or an action type the API doesn't know, is skipped with a warning, since an unknown type gets a 400 for the whole request.The item carries no
id. With anid, a stored request resolves the item server side to the stored one and requires its reasoning item before it. Without one there is no lookup, so a credential or provider switch can't make it resolve another org's stored item (the concern #77 raises for references). #77 decides which searches get a reference; whatever it leaves unreferenced falls back to the inline item here, so the two compose and either can rebase over the other.Evidence
Probes against the AI Gateway on gpt-6.1-sol, gpt-5-mini and gpt-4.1-mini, asking on turn 2 what the model did before its previous answer:
id,store: false: 200 every time, and the model listed the exact action in 9/9 runs across search,open_page,find_in_pageand multi-search shapes.store: false, turn 2 withstore: true(gpt-6.1-sol and gpt-5-mini, search andopen_page): inline items withoutid, alone or after the encrypted reasoning item, returned 200 and the model listed the exact action in 12/12 runs. Anitem_referenceto the same search returned 404 "Item … not found".store: true(gpt-5-mini and gpt-6.1-sol): a stored reasoning item, sent encrypted with itsidor as anitem_reference, followed by an inline item withoutidreturned 200 in 6/6 runs, and the model listed the exact action.item_referencewithstore: false: 404 on all three models.statusvaluesfailed,in_progress,searchingand a missingstatusare accepted, as are pre-feat: tag web search sources with the search that found them #62 shapes such as{"type":"find_in_page"}with no url.The model gets the action, not the pages it read: asked about uncited pages from the search, no inline run answered correctly. Zero data retention orgs weren't probed, since the gateway's org isn't one.
Validation
go test ./... -count=1,go vet ./...,gofmt -l .and golangci-lint v2.15.0 pass. gofumpt only flags two existing blank lines inopenai_test.go, which this PR doesn't touch.[ws, rs, ws]with encrypted reasoning both unstored and stored (the first search inline, the second referenced), a stored request replaying an unstored response (both inline), a result placed before its call (sent once, inline), a search after a computer call or a skipped part (inline), repeated parts and a call and result in different messages (sent once), a search-only message, pre-feat: tag web search sources with the search that found them #62searchandfind_in_pagemetadata, and skipped cases with their warning.agent.Streamandagent.Generate, with the turn 1 messages round-tripped through JSON: withstore: falseon both turns (gpt-5-mini), and withstore: falsethenstore: true(gpt-5-mini and gpt-6.1-sol). Turn 2 carried every search inline with noid, and the model listed each action with its inputs.