Skip to content

Allow full web search tool config#13675

Merged
rm-openai merged 7 commits intomainfrom
dev/rm/websearch
Mar 7, 2026
Merged

Allow full web search tool config#13675
rm-openai merged 7 commits intomainfrom
dev/rm/websearch

Conversation

@rm-openai
Copy link
Contributor

@rm-openai rm-openai commented Mar 6, 2026

Previously, we could only configure whether web search was on/off.

This PR enables sending along a web search config, which includes all the stuff responsesapi supports: filters, location, etc.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@rm-openai
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Mar 6, 2026
@rm-openai rm-openai requested review from jif-oai and sayan-oai March 6, 2026 03:37
@rm-openai rm-openai changed the title Fix web search config handling for partial user locations Allow full web search tool config Mar 6, 2026
@rm-openai rm-openai force-pushed the dev/rm/websearch branch 2 times, most recently from a3d7fe4 to 272e6b8 Compare March 6, 2026 19:28
}
}

fn serialize_web_search_filters<S>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this need a custom serializer?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we export this type as app_server protocol?

cc: @owenlin0

Copy link
Collaborator

@owenlin0 owenlin0 Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

took a look - luckily no, this is only used for codex harness -> responsesapi calls. opened a PR to clean this up while I am here: #14136

(None, None) => None,
(Some(base_location), None) => Some(base_location.clone()),
(None, Some(profile_location)) => Some(profile_location.clone()),
(Some(base_location), Some(profile_location)) => Some(WebSearchLocation {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we maybe add .merge() helpers to these. types?

Copy link
Collaborator

@pakrym-oai pakrym-oai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's make merging less verbose

@rm-openai rm-openai enabled auto-merge (squash) March 7, 2026 00:11
@rm-openai rm-openai merged commit 61098c7 into main Mar 7, 2026
31 checks passed
@rm-openai rm-openai deleted the dev/rm/websearch branch March 7, 2026 00:50
@github-actions github-actions bot locked and limited conversation to collaborators Mar 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants