feat: add client tool search support - #186
Conversation
Signed-off-by: haoshan98 <haoshanw@gmail.com>
Signed-off-by: haoshan98 <haoshanw@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 447f734c49
ℹ️ 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".
| let rows = item::get_items_by_conversation_in_tx(&mut tx, conversation_id).await?; | ||
| let latest_item_id = rows.last().map(|row| row.id.as_str()); | ||
| let conversation_turns = response::get_conversation_turns_in_tx(&mut tx, conversation_id).await?; | ||
| let latest_response = latest_item_id.and_then(|latest_item_id| { |
There was a problem hiding this comment.
Preserve metadata for zero-item conversation turns
When a conversation turn persists no items—most notably a generate:false prewarm with input: []—latest_item_id is None, so this expression always discards the response's metadata. A subsequent request using the same conversation_id therefore fails to inherit the prewarmed tool-search declarations and loaded-tool state; if older items exist, it can instead inherit stale metadata from the prior non-empty turn. The latest conversation response must remain identifiable even when its history_item_ids is empty.
Useful? React with 👍 / 👎.
| if self.parallel_tool_calls == Some(true) { | ||
| return Err(ToolError::Config( | ||
| "parallel_tool_calls must be false when tool search is active".to_owned(), | ||
| )); |
There was a problem hiding this comment.
Force serial calls when tool search is active
When the client omits the optional parallel_tool_calls field, this check accepts the request and the private upstream request also leaves the field absent, allowing the backend's enabled/default behavior to produce multiple or mixed tool calls. The translator permits only one tool-search call and rejects a second as an invalid upstream response, turning an otherwise valid request into a 502. Active tool search should normalize None to Some(false) rather than only rejecting explicit true.
Useful? React with 👍 / 👎.
Signed-off-by: haoshan98 <haoshanw@gmail.com>
Signed-off-by: haoshan98 <haoshanw@gmail.com>
Signed-off-by: haoshan98 <haoshanw@gmail.com>
Signed-off-by: haoshan98 <haoshanw@gmail.com>
Signed-off-by: haoshan98 <haoshanw@gmail.com>
| ) -> DbResult<Option<Response>> { | ||
| let escaped_item_id = item_id.replace('!', "!!").replace('%', "!%").replace('_', "!_"); | ||
| let history_suffix = format!("%\"{escaped_item_id}\"]"); | ||
| sqlx::query_as::<_, Response>( |
There was a problem hiding this comment.
I understand why we need metadata from the response associated with the captured conversation version: compaction can remove the tool_search_call/tool_search_output history, and selecting the latest response would be unsafe if another turn is persisted after rehydration.
However, this query reconstructs the response-to-item relationship by applying LIKE to the serialized history_item_ids JSON. This is unindexed, depends on the exact JSON encoding, and suggests that the storage schema is missing an explicit relationship.
Could we model this association directly—for example, with a response_id/turn_id on stored items or a normalized response_items relation—and resolve it using an indexed lookup? At minimum, ConversationSnapshot could retain the last item ID already loaded by rehydrate_snapshot, avoiding the additional get_id_by_conversation_sequence query. I would keep the exact-version behavior but avoid introducing CRUD based on textual matching over JSON.
This reverts commit ef79009. Signed-off-by: haoshan98 <haoshanw@gmail.com>
Signed-off-by: haoshan98 <haoshanw@gmail.com>
Summary
Add client-executed tool search support to the Responses API.
tool_search_callas a first-class item for blocking and streaming responses.tool_searchdeclarations into upstream-compatible function tools.