feat: search_tool migrate to bring you own tool of Responses API#14274
feat: search_tool migrate to bring you own tool of Responses API#14274apanasenko-oai merged 3 commits intomainfrom
Conversation
![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: 81e294a3da
ℹ️ 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".
apanasenko-oai
force-pushed
the
anton_panasenko_search_tool_client
branch
2 times, most recently
from
d8f8238 to
9e81830
Compare
apanasenko-oai
force-pushed
the
anton_panasenko_search_tool_client
branch
from
9e81830 to
94eeb39
Compare
apanasenko-oai
force-pushed
the
anton_panasenko_search_tool_client
branch
2 times, most recently
from
8e768db to
a5d907a
Compare
apanasenko-oai
force-pushed
the
anton_panasenko_search_tool_client
branch
from
a5d907a to
f99b756
Compare
apanasenko-oai
force-pushed
the
anton_panasenko_search_tool_client
branch
5 times, most recently
from
aba8b4f to
763ab20
Compare
| } | ||
|
|
||
| #[test] | ||
| fn normalize_keeps_server_tool_search_output_without_matching_call() { |
apanasenko-oai
force-pushed
the
anton_panasenko_search_tool_client
branch
2 times, most recently
from
53c413b to
864bb31
Compare
| @@ -1839,6 +1860,59 @@ pub(crate) fn mcp_tool_to_openai_tool( | |||
| fully_qualified_name: String, | |||
| tool: rmcp::model::Tool, | |||
There was a problem hiding this comment.
nit, add defer to mcp_tool_to_openai_tool ?
There was a problem hiding this comment.
I did go with separate mcp_tool_to_deferred_openai_tool to avoid creating noise and updating every call site of mcp_tool_to_openai_tool.
| } | ||
|
|
||
| #[test] | ||
| fn handler_looks_up_namespaced_aliases_explicitly() { |
There was a problem hiding this comment.
maybe we should add an intergration test that calls a tool via namespace instead.
| } | ||
|
|
||
| pub(crate) async fn parse_mcp_tool_name(&self, tool_name: &str) -> Option<(String, String)> { | ||
| pub(crate) async fn parse_mcp_tool_name( |
There was a problem hiding this comment.
why do we need namespace here? Won't the function call come back with the correct "namespace" and "name" on it considering all the calculations are happening on mcp manager level?
There was a problem hiding this comment.
mcp operates on qualified names, for apps we split them on namespace and tool name, when we do lookup for mcp handler, we need to combine them back together.
pakrym-oai
left a comment
pakrym-oai
left a comment
There was a problem hiding this comment.
Mostly good but something isn't right with tool naming for code mode
apanasenko-oai
force-pushed
the
anton_panasenko_search_tool_client
branch
3 times, most recently
from
8fa039f to
c1e5374
Compare
apanasenko-oai
force-pushed
the
anton_panasenko_search_tool_client
branch
from
c1e5374 to
9f006ad
Compare
3 participants
Why
to support a new bring your own search tool in Responses API(https://developers.openai.com/api/docs/guides/tools-tool-search#client-executed-tool-search) we migrating our bm25 search tool to use official way to execute search on client and communicate additional tools to the model.
What
search_tool_bm25flow with client-executedtool_searchtool_search_callandtool_search_output