[WRONG BRANCH] fix(responses): preserve forced hosted web_search choice - #255
[WRONG BRANCH] fix(responses): preserve forced hosted web_search choice#255luvs01 wants to merge 2 commits into
Conversation
|
✅ Deterministic PR hygiene checks passed. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe response parser now maps ChangesHosted web-search routing
Estimated code review effort: 1 (Trivial) | ~5 minutes Mergeability Score: ⚪ Minimal · up to This localized parser change preserves forced hosted web-search behavior without introducing a supported merge-blocking correctness, security, or availability risk. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/responses-parser.test.ts`:
- Around line 164-176: Add a focused test alongside the existing forced hosted
web_search test in the responses parser suite, using tool_choice type
"web_search_preview" and asserting parsed.options.toolChoice equals { name:
"web_search" }. Keep the same request setup and preserve the existing web_search
coverage.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d8bc499e-8d5d-4dab-a66d-1d6d033f1207
📒 Files selected for processing (2)
src/responses/parser.tstests/responses-parser.test.ts
| test("maps a forced hosted web_search choice to the synthetic routed tool", () => { | ||
| const parsed = parseRequest({ | ||
| model: "umans/umans-kimi-k2.7", | ||
| input: "search", | ||
| tools: [ | ||
| { type: "web_search" }, | ||
| { type: "function", name: "run_shell", parameters: { type: "object" } }, | ||
| ], | ||
| tool_choice: { type: "web_search" }, | ||
| }); | ||
|
|
||
| expect(parsed.options.toolChoice).toEqual({ name: "web_search" }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add coverage for web_search_preview.
The parser now handles both web_search and web_search_preview, but this test covers only web_search. Add a second case that expects { name: "web_search" } for tool_choice: { type: "web_search_preview" }.
As per path instructions, behavior changes in src/ should have focused regression coverage in tests/.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/responses-parser.test.ts` around lines 164 - 176, Add a focused test
alongside the existing forced hosted web_search test in the responses parser
suite, using tool_choice type "web_search_preview" and asserting
parsed.options.toolChoice equals { name: "web_search" }. Keep the same request
setup and preserve the existing web_search coverage.
Source: Path instructions
![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: bf9840c5c2
ℹ️ 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".
| if (t === "web_search" || t === "web_search_preview") { | ||
| return { name: WEB_SEARCH_TOOL_NAME }; |
There was a problem hiding this comment.
For a routed Responses request whose tool declaration and choice both use the supported web_search_preview type, this new branch sets toolChoice to { name: "web_search" }, but extractHostedWebSearch() recognizes only type === "web_search". Consequently _webSearch remains unset, planWebSearch() returns no plan, the synthetic tool is never injected, and adapter filtering removes every unrelated declared tool, so the model produces an ordinary answer without performing the forced search. Normalize or extract web_search_preview as a hosted search too, and add a routed-planning regression case for this branch.
AGENTS.md reference: src/AGENTS.md:L22-L25
Useful? React with 👍 / 👎.
@Wibias stepped down from developing opencodex, and repository permission was reduced to read access. Move him out of the current-maintainers table into a new Former maintainers section, drop him from the CODEOWNERS default-reviewer line and the four high-impact runtime paths, and record the change with the 2026-07-27 addition entry it closes. Nothing he authored is unwound: commits, merged pull requests, release-note attributions, and the code comments citing his reviews stay as they are.
bf9840c to
9c6859c
Compare
2 participants
Motivation
{ type: "web_search" }shape that the internal parser mapped to"auto", which widened the advertised tool set and allowed routed models to access unrelated client/local tools.web_searchdeclarations to the internal synthetic routed web-search tool so sidecar injection does not leak other declared tools.Description
mapToolChoiceinsrc/responses/parser.tsto maptype: "web_search"andtype: "web_search_preview"to the synthetic routed tool viaWEB_SEARCH_TOOL_NAMEinstead of falling through to"auto".tests/responses-parser.test.tsthat asserts a forced hostedweb_searchtool_choiceis parsed to the synthetic routedweb_searchtool even when other client tools are declared.Testing
bun test tests/responses-parser.test.ts, and the updatedResponses parsertests passed.bun run typecheck, which completed successfully.bun run privacy:scan, which completed successfully.Codex Task
Summary by CodeRabbit