fix(core): use dedicated types for responsesapi web search tool config (#14136)
This changes the web_search tool spec in codex-core to use dedicated Responses-API payload structs instead of shared config types and custom serializers. Previously, `ToolSpec::WebSearch` stored `WebSearchFilters` and `WebSearchUserLocation` directly and relied on hand-written serializers to shape the outgoing JSON. This worked, but it mixed config/schema types with the OpenAI Responses payload contract and created an easy place for drift if those shared types changed later. ### Why This keeps the boundary clearer: - app-server/config/schema types stay focused on config - Responses tool payload types stay focused on the OpenAI wire format It also makes the serialization behavior obvious from the structs themselves, instead of hiding it in custom serializer functions.
This commit is contained in:
Owen Lin
GitHub
parent
d241dc598c
commit
d309c102ef
2 changed files with 44 additions and 67 deletions
|
|
@ -155,12 +155,11 @@ fn strip_total_output_header(output: &str) -> Option<(&str, u32)> {
|
|||
pub(crate) mod tools {
|
||||
use crate::tools::spec::JsonSchema;
|
||||
use codex_protocol::config_types::WebSearchContextSize;
|
||||
use codex_protocol::config_types::WebSearchFilters;
|
||||
use codex_protocol::config_types::WebSearchUserLocation;
|
||||
use codex_protocol::config_types::WebSearchFilters as ConfigWebSearchFilters;
|
||||
use codex_protocol::config_types::WebSearchUserLocation as ConfigWebSearchUserLocation;
|
||||
use codex_protocol::config_types::WebSearchUserLocationType;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use serde::Serializer;
|
||||
|
||||
/// When serialized as JSON, this produces a valid "Tool" in the OpenAI
|
||||
/// Responses API.
|
||||
|
|
@ -181,16 +180,10 @@ pub(crate) mod tools {
|
|||
WebSearch {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
external_web_access: Option<bool>,
|
||||
#[serde(
|
||||
skip_serializing_if = "Option::is_none",
|
||||
serialize_with = "serialize_web_search_filters"
|
||||
)]
|
||||
filters: Option<WebSearchFilters>,
|
||||
#[serde(
|
||||
skip_serializing_if = "Option::is_none",
|
||||
serialize_with = "serialize_web_search_user_location"
|
||||
)]
|
||||
user_location: Option<WebSearchUserLocation>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
filters: Option<ResponsesApiWebSearchFilters>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
user_location: Option<ResponsesApiWebSearchUserLocation>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
search_context_size: Option<WebSearchContextSize>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
|
|
@ -212,63 +205,43 @@ pub(crate) mod tools {
|
|||
}
|
||||
}
|
||||
|
||||
fn serialize_web_search_filters<S>(
|
||||
filters: &Option<WebSearchFilters>,
|
||||
serializer: S,
|
||||
) -> Result<S::Ok, S::Error>
|
||||
where
|
||||
S: Serializer,
|
||||
{
|
||||
match filters {
|
||||
Some(filters) => {
|
||||
#[derive(Serialize)]
|
||||
struct SerializableWebSearchFilters<'a> {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
allowed_domains: Option<&'a Vec<String>>,
|
||||
}
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
pub(crate) struct ResponsesApiWebSearchFilters {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) allowed_domains: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
SerializableWebSearchFilters {
|
||||
allowed_domains: filters.allowed_domains.as_ref(),
|
||||
}
|
||||
.serialize(serializer)
|
||||
impl From<ConfigWebSearchFilters> for ResponsesApiWebSearchFilters {
|
||||
fn from(filters: ConfigWebSearchFilters) -> Self {
|
||||
Self {
|
||||
allowed_domains: filters.allowed_domains,
|
||||
}
|
||||
None => serializer.serialize_none(),
|
||||
}
|
||||
}
|
||||
|
||||
fn serialize_web_search_user_location<S>(
|
||||
user_location: &Option<WebSearchUserLocation>,
|
||||
serializer: S,
|
||||
) -> Result<S::Ok, S::Error>
|
||||
where
|
||||
S: Serializer,
|
||||
{
|
||||
match user_location {
|
||||
Some(user_location) => {
|
||||
#[derive(Serialize)]
|
||||
struct SerializableWebSearchUserLocation<'a> {
|
||||
#[serde(rename = "type")]
|
||||
r#type: WebSearchUserLocationType,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
country: Option<&'a String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
region: Option<&'a String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
city: Option<&'a String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
timezone: Option<&'a String>,
|
||||
}
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
pub(crate) struct ResponsesApiWebSearchUserLocation {
|
||||
#[serde(rename = "type")]
|
||||
pub(crate) r#type: WebSearchUserLocationType,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) country: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) region: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) city: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub(crate) timezone: Option<String>,
|
||||
}
|
||||
|
||||
SerializableWebSearchUserLocation {
|
||||
r#type: user_location.r#type,
|
||||
country: user_location.country.as_ref(),
|
||||
region: user_location.region.as_ref(),
|
||||
city: user_location.city.as_ref(),
|
||||
timezone: user_location.timezone.as_ref(),
|
||||
}
|
||||
.serialize(serializer)
|
||||
impl From<ConfigWebSearchUserLocation> for ResponsesApiWebSearchUserLocation {
|
||||
fn from(user_location: ConfigWebSearchUserLocation) -> Self {
|
||||
Self {
|
||||
r#type: user_location.r#type,
|
||||
country: user_location.country,
|
||||
region: user_location.region,
|
||||
city: user_location.city,
|
||||
timezone: user_location.timezone,
|
||||
}
|
||||
None => serializer.serialize_none(),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2017,11 +2017,11 @@ pub(crate) fn build_specs(
|
|||
filters: config
|
||||
.web_search_config
|
||||
.as_ref()
|
||||
.and_then(|cfg| cfg.filters.clone()),
|
||||
.and_then(|cfg| cfg.filters.clone().map(Into::into)),
|
||||
user_location: config
|
||||
.web_search_config
|
||||
.as_ref()
|
||||
.and_then(|cfg| cfg.user_location.clone()),
|
||||
.and_then(|cfg| cfg.user_location.clone().map(Into::into)),
|
||||
search_context_size: config
|
||||
.web_search_config
|
||||
.as_ref()
|
||||
|
|
@ -2766,8 +2766,12 @@ mod tests {
|
|||
tool.spec,
|
||||
ToolSpec::WebSearch {
|
||||
external_web_access: Some(true),
|
||||
filters: web_search_config.filters,
|
||||
user_location: web_search_config.user_location,
|
||||
filters: web_search_config
|
||||
.filters
|
||||
.map(crate::client_common::tools::ResponsesApiWebSearchFilters::from),
|
||||
user_location: web_search_config
|
||||
.user_location
|
||||
.map(crate::client_common::tools::ResponsesApiWebSearchUserLocation::from),
|
||||
search_context_size: web_search_config.search_context_size,
|
||||
search_content_types: None,
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue