feat(tool_search): add deferred tool loading support for GPT-5.4+#2853
feat(tool_search): add deferred tool loading support for GPT-5.4+#2853amitksingh1490 wants to merge 37 commits into
Conversation
…onversation history
| let consumed_tso_indices: std::collections::HashSet<usize> = Default::default(); | ||
|
|
There was a problem hiding this comment.
The consumed_tso_indices HashSet is created but never populated. This means the check at line 435 if consumed_tso_indices.contains(&idx) will always return false, and the logic to skip already-consumed ToolSearchOutput messages will never work.
This appears to be incomplete implementation - the code creates the tracking variable but never inserts any indices into it. If ToolSearchOutput entries should be consumed inline with assistant messages and skipped later, the code needs to actually track which indices were consumed.
// Either remove the unused tracking logic:
for (idx, entry) in messages.iter().enumerate() {
match &entry.message {
// ... handle messages without the consumed_tso_indices check
}
}
// OR implement the actual tracking when consuming inline| let consumed_tso_indices: std::collections::HashSet<usize> = Default::default(); | |
| // TODO: Implement tracking of consumed ToolSearchOutput indices if needed for deduplication |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
…andling for OpenAI responses
| } | ||
| ContextMessage::ToolSearchOutput(_) => { | ||
| // Tool search output is OpenAI Responses API specific - skip for Anthropic | ||
| Message { role: Role::User, content: vec![] } |
There was a problem hiding this comment.
[CORRECTNESS — HIGH] ToolSearchOutput is emitted as an empty Anthropic message
The comment says this Responses-only item is skipped, but the branch still serializes it as a user message with an empty content array. If a persisted conversation containing ToolSearchOutput is later sent through Anthropic, this can produce an invalid or semantically empty user message instead of omitting the item.
Recommendation: Filter ContextMessage::ToolSearchOutput out before provider-specific message conversion, or change the conversion to return an optional message so this variant is truly omitted rather than serialized as an empty message.
amitksingh1490
left a comment
There was a problem hiding this comment.
Additional review after re-checking the replay path.
|
Action required: PR inactive for 5 days. |
No description provided.