fix(intercept/responses): record all tool call types in responses interceptor#273
fix(intercept/responses): record all tool call types in responses interceptor#273dannykopping wants to merge 7 commits intomainfrom
Conversation
…erceptor Previously, the responses interceptor only recorded function_call and custom_tool_call types. This left tool calls like web_search_call, computer_call, shell_call, etc. unrecorded, meaning interceptions that did significant work via built-in tools showed no tool usage. Now all tool call types are recorded: - web_search_call - computer_call - local_shell_call - shell_call - apply_patch_call - code_interpreter_call - mcp_call - file_search_call - image_generation_call For these types, the tool name falls back to the type when no explicit name is set, and the call ID falls back to the item ID when call_id is empty (web_search_call, file_search_call, etc. use id instead of call_id). Adds a web_search blocking fixture and comprehensive tests covering all new tool types. Closes: #121
- Agentic tools (computer_call, local_shell_call, shell_call, apply_patch_call) use call_id for correlation. - Hosted tools (web_search_call, file_search_call, code_interpreter_call, image_generation_call, mcp_call) use id only — referenced in OpenAI API docs. - Use constant.ValueOf[...] for all types with SDK constants; define local computerCall const for the missing one. - Remove the nolint:goconst directive.
The web_search_preview tool type is deprecated in favor of web_search.
Add blocking_web_search case to TestResponsesOutputMatchesUpstream that exercises the hosted-tool recording path (id, not call_id). Also fix the fixture's missing -- request -- txtar header.
| if toolName == "" { | ||
| toolName = item.Type | ||
| } | ||
| callID = item.ID |
There was a problem hiding this comment.
I don't like callID being assigned here to item.ID. Those are different types.
In theory it would be possible to decipher value type (item.callID / item.ID) based on function call type but I think in this form it would cause too much confusion.
Separate field would be clearer, eg ToolCallItemID.
There was a problem hiding this comment.
All we're doing is storing these values. At the end of the day they're used to track particular instances of tool calls. Whether it's a call ID or a regular ID is kinda irrelevant; I think expanding the API (and therefore the DB) to accommodate this extra detail is unnecessary and an implementation detail of OpenAI specifically.
There was a problem hiding this comment.
Yes, for just storing some ID for identification purpose it is fine to do it this way, but If there is some other data source that correctly records fields it becomes hard / unclear why sometimes tool call ID doesn't match.
Maybe I'm too paranoid but I think we should be careful about data consistency in recordings.
There was a problem hiding this comment.
Turns out tool calls actually have two IDs: id and call_id 😆
Right now we're throwing id on the floor
So maybe it is actually just better to capture both separately and avoid the correctness issues you're raising
I'll expand the scope of the ticket and get to this next week
Thanks 👍
Disclaimer: implemented by a Coder Agent using Claude Opus 4.6
Summary
Previously, the responses interceptor only recorded
function_callandcustom_tool_calltypes. This left tool calls likeweb_search_call,computer_call,shell_call, etc. unrecorded — meaning interceptions that did significant work via built-in tools showed no tool usage at all.Now all tool call types are recorded for maximum auditor visibility:
function_callcustom_tool_callweb_search_callcomputer_calllocal_shell_callshell_callapply_patch_callcode_interpreter_callmcp_callfile_search_callimage_generation_callApproach
For the new tool types:
typefield when no explicitnameis set (most built-in tools don't have a name, unlike function calls).idwhencall_idis empty (web_search_call,file_search_call,code_interpreter_call,image_generation_calluseidinstead ofcall_id).message,reasoning,*_output, etc.) are still skipped.Changes
intercept/responses/base.go— expandedrecordNonInjectedToolUsageswitch to handle all tool call types.intercept/responses/base_test.go— addedweb_search_call_with_no_nameandall_additional_tool_typestest cases.fixtures/openai/responses/blocking/web_search.txtar— new fixture for a web search response.fixtures/fixtures.go— embedded the new fixture.Closes #121
Linear: AIGOV-96