[2/3] feat(ai): Add extension support in ai crate#726
Conversation
ehayes2000
changed the title
synoet
left a comment
synoet
left a comment
There was a problem hiding this comment.
Mostly good, just please remove the panic.
| yield Ok(StreamPart::ToolCall(call)); | ||
| yield Ok(PartOrExt::Part(StreamPart::ToolCall(call))); | ||
| } else { | ||
| panic!("Failed to try from") |
| while let Some(item) = stream.next().await { | ||
| if let Err(e) = item { | ||
| yield Err(e); | ||
| break; |
There was a problem hiding this comment.
Is it intentional that the outer loop continues after the stream errors ?
| impl Default for AnthropicClient { | ||
| fn default() -> Self { | ||
| Self::new() | ||
| Self::new(AnthropicRequestExtensions(vec![])) |
There was a problem hiding this comment.
nit: Why does this impl have no default tools ? But the ToolLoop by default includes web search. Can we unify them.
There was a problem hiding this comment.
I don't think its correct to include web search by default any time a connection to anthropic is created. If this client is used for insights, completions, markdown edits, or explain it would be strange to see AI search the web. The tool_loop/chat interface (a rename of the preexisting ai/tools/client/chat interface) is constructed explicitly for user chats so it's appropriate to instantiate chat business requirements with this interface.
| ref part @ PartOrExt::Part(ref p) => { | ||
| yield Ok(p.to_owned()); | ||
| stream_parts.push(part.to_owned()); | ||
| } | ||
| ref part @ PartOrExt::Ext(ref e) => { | ||
| if let Some(p) = self.client.handle_extension_item(e.to_owned()) { | ||
| yield Ok(p); | ||
| } | ||
| stream_parts.push(part.to_owned()); |
There was a problem hiding this comment.
you read ref but then immediately call to_owned().
There was a problem hiding this comment.
I'm binding references to the the whole part and the destructured p / e because I need owned data for both. If I don't use ref bindings the destructure moves.
| // list of tool responses as openai items that are not returned / yielded | ||
| let mut non_yielding_responses = vec![]; |
There was a problem hiding this comment.
I'm confused why these are split up, if they are just yielded at the end anyway ?
There was a problem hiding this comment.
Yea it's a bit ugly. They're split because standard tool responses are trigger tool execution to get a response then trigger another round trip to the provider. Extension callbacks don't do either, but still need to be be transformed and appended to build the next request which will either be triggered by a user request or a standard tool call.
- Add definition for web search server tool - Add example to test anthropic web search - Add extension pattern to openai bindings
- Add trait to support streams that extend openai stream with additional items - Rename `client` -> `tool_loop` - Update tool loop to use new client trait and handle extended streams - Implement extension client for existing clients (placeholder for anthropic)
ehayes2000
force-pushed
the
ehayesdev/m-5447-ai-crate-api-extension-support
branch
from
df56c81 to
a0f178e
Compare
ehayes2000
deleted the
ehayesdev/m-5447-ai-crate-api-extension-support
branch
2 participants
items
client->tool_loopanthropic)
ExtendedClienttrait is the core change of this PR. All changes are made around this update.