feat(protocols): implement T3 web_search non-preview tool + WebSearchCall.results - #1304
Conversation
… field
What: add `ResponseTool::WebSearch(WebSearchTool)` variant with typed
`filters { allowed_domains }`, `search_context_size` enum (low|medium|
high), and `user_location { city, country, region, timezone, type }`
sub-shapes. Wire the `results: Option<Vec<WebSearchResult>>` field on
`ResponseOutputItem::WebSearchCall` so callers requesting
`web_search_call.results` via the top-level `include[]` array receive a
typed array. Accept the versioned alias `web_search_2025_08_26` on
deserialization.
Why: OpenAI Responses spec §tools line 439 defines `web_search` as a
distinct tool from the existing `web_search_preview` — non-preview adds
`filters.allowed_domains` and constrains `search_context_size` to a
typed enum. P4 (merged in #1274) already landed the matching `IncludeField`
variants for `web_search_call.results` and `web_search_call.action.sources`;
T3 now wires the actual output struct so the wire round-trip is complete.
How: new tagged variant on `ResponseTool` using `#[serde(rename =
"web_search", alias = "web_search_2025_08_26")]` so canonical
serialization emits `"web_search"` while still accepting the dated tag.
The `WebSearchCall` output struct gains `results` gated by
`#[serde(default, skip_serializing_if = "Option::is_none")]` — absent
results serialize byte-identically to `{id, action, status, type}`
per spec, populated results ride alongside. Mirrors the
`FileSearchResult` shape precedent for the `results` entry fields.
Construction sites in the MCP response transformer and exhaustive matches
in the model_gateway harmony builder and OpenAI responses utils are
updated to compile. Two serde round-trip tests pin both the tool
declaration and the output struct (populated + absent cases).
Refs: T3
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
github-actions
Bot
added
grpc
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a non-preview Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| .map(|tool| match tool { | ||
| ResponseTool::Function(_) => "function", | ||
| ResponseTool::WebSearchPreview(_) => "web_search_preview", | ||
| ResponseTool::WebSearch(_) => "web_search", |
There was a problem hiding this comment.
🟡 Nit: "web_search" is added to the tool_types collection here, but the BUILTIN_TOOLS constant (line 63) and ToolLike::is_builtin() impl for ResponseTool (line 110) in this same file were not updated to include the new variant.
This means has_custom_tools(&tool_types) at line 441 will return true when web_search is the only tool in the request (since "web_search" is not in BUILTIN_TOOLS), incorrectly triggering the developer-message injection path.
Similarly, collect_builtin_routing and extract_builtin_types in mcp_utils.rs, and ensure_mcp_connection in grpc/common/responses/utils.rs, don't recognize ResponseTool::WebSearch as a builtin — though that full fix also needs a BuiltinToolType::WebSearch enum variant in crates/mcp/src/core/config.rs. If MCP routing for the non-preview variant is intentionally deferred, at minimum BUILTIN_TOOLS and is_builtin() in this file should be updated.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
model_gateway/src/routers/grpc/harmony/builder.rs (1)
423-433:⚠️ Potential issue | 🟠 Major
web_searchis mapped but still classified as custom in Harmony.Line 432 adds
"web_search"totool_types, butBUILTIN_TOOLS(Line 63) still lacks"web_search". That makeshas_custom_tools()return true for a builtin-only request and changes system/developer message shaping.💡 Proposed fix
const BUILTIN_TOOLS: &[&str] = &[ + "web_search", "web_search_preview", "code_interpreter", "container", "file_search", ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model_gateway/src/routers/grpc/harmony/builder.rs` around lines 423 - 433, The mapping adds "web_search" to tool_types but BUILTIN_TOOLS (used by has_custom_tools()) does not include it, causing builtin-only requests to be treated as custom; update the BUILTIN_TOOLS set/array (the symbol named BUILTIN_TOOLS) to include "web_search" so has_custom_tools() returns false for requests containing only builtins, and ensure this aligns with the ResponseTool enum mapping in the tool_types construction in builder.rs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@model_gateway/src/routers/grpc/harmony/builder.rs`:
- Around line 423-433: The mapping adds "web_search" to tool_types but
BUILTIN_TOOLS (used by has_custom_tools()) does not include it, causing
builtin-only requests to be treated as custom; update the BUILTIN_TOOLS
set/array (the symbol named BUILTIN_TOOLS) to include "web_search" so
has_custom_tools() returns false for requests containing only builtins, and
ensure this aligns with the ResponseTool enum mapping in the tool_types
construction in builder.rs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 14b6c241-8269-41ec-bd64-4e34d803b2c9
📒 Files selected for processing (4)
crates/mcp/src/transform/transformer.rscrates/protocols/src/responses.rsmodel_gateway/src/routers/grpc/harmony/builder.rsmodel_gateway/src/routers/openai/responses/utils.rs
![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: 8ea4370a03
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…S + is_builtin) Claude nit on PR #1304 comment 3121138818: adding `"web_search"` to the tool_types collection in `extract_tool_types_from_response_tools` without also adding it to the `BUILTIN_TOOLS` slice and `ToolLike::is_builtin()` for `ResponseTool` meant `has_custom_tools` would misclassify web-search- only requests as custom. Follow the T1 file_search pattern: add `"web_search"` to the BUILTIN_TOOLS slice at L63 and add `ResponseTool::WebSearch(_)` to the `matches!` arm in `is_builtin()` at L108-112. Codex P1 comment 3121145843 (MCP routing for web_search) deliberately left for a follow-up task — the proper fix spans the `BuiltinToolType` enum in `crates/mcp/src/core/config.rs` and its ~30 callsites plus `grpc/common/responses/utils.rs::ensure_mcp_connection`, which is out of T3's protocol-only charter. Refs: T3 Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
1 participant
Summary
Add the OpenAI Responses API non-preview `web_search` tool variant and wire the `WebSearchCall.results` output field so the P4 `web_search_call.results` include-field (shipped in #1274) has a typed carrier. Spec reference: `openai-responses-api-spec.md` §tools line 439-440.
What changed
Why
Spec §tools line 439-440 distinguishes non-preview `web_search` from `web_search_preview` — non-preview adds `filters.allowed_domains` and pins `search_context_size` to a typed enum. P4 (merged in #1274) already landed the matching `IncludeField` variants for `web_search_call.results` and `web_search_call.action.sources`; T3 now wires the actual output carrier so the wire round-trip is complete and closes the gemini-bot flag raised on P4 that `WebSearchCall.results` had no typed home.
How
Scrutiny passes (lead review)
Notes & scope
Test plan
Refs: T3
Summary by CodeRabbit
New Features
Tests