feat: tool result callback

[carlos-verdes]

Fixes #289

[carlos-verdes]

@0xMochan @cvauclair with this PR we have tool chain properly implemented

[cvauclair]

Hey @carlos-verdes ! This is a feature we 100% do want to support so thanks for the issue/PR 🦾 !

The one concern I have with this implementation is that users lose the ability to perform a "single turn" request to the LLM, or in other words, that the Chat::chat trait method now performs multi-turn by default. It's not hard to imagine that users might want to control that.

That being said, it is still possible to perform a single turn (and get a ToolCall message from the model) by calling the Agent::completion method like so:

// response here can be used to get the `ToolCall`
let response = agent.completion(prompt, chat_history)
    .await?
    .send()
    .await?;

I'm definitely not against keeping the Prompt and Chat traits as "batteries-included" traits which handle tool result callback, but I'm curious to know your thoughts on this

[s6nqou]

@carlos-verdes This is really helpful! I actually wrote almost the exact same loop logic in my project, and now I can finally remove it!

I also noticed another issue: the completion impls of the providers always append the prompt to the end of full_history before sending it to the LLM. I'm not sure if this will cause any issues. For example:

Expected messages?:

first request:
[Prompt]
second request:
[Prompt, ToolCall, ToolResult]
third request:
[Prompt, ToolCall, ToolResult, AssistantContent]

Current messages:

first request:
[Prompt]
second request:
[ToolCall, ToolResult, Prompt]
third request:
[ToolCall, ToolResult, Prompt, AssistantContent]

I'm wondering if the prompt argument is really necessary, and could it be made optional or even removed? That way, we'd only need to manage chat_history. Would love to hear your thoughts.

[carlos-verdes]

Hey @carlos-verdes ! This is a feature we 100% do want to support so thanks for the issue/PR 🦾 !

The one concern I have with this implementation is that users lose the ability to perform a "single turn" request to the LLM, or in other words, that the Chat::chat trait method now performs multi-turn by default. It's not hard to imagine that users might want to control that.

That being said, it is still possible to perform a single turn (and get a ToolCall message from the model) by calling the Agent::completion method like so:

// response here can be used to get the `ToolCall`
let response = agent.completion(prompt, chat_history)
    .await?
    .send()
    .await?;

I'm definitely not against keeping the Prompt and Chat traits as "batteries-included" traits which handle tool result callback, but I'm curious to know your thoughts on this

The contract between the client and the LLM is like this:
"I give you a human readable prompt and you give me a human readable answer".

This PR implements this contract, whatever has to happen to comply with the contract is implementation and hence is hidden from the client. Actually the prompt method returns a String not a message right?

If you use function calls (tools) you need to understand there are going to be extra calls to the LLM, the same way as when you chose a reasoning model you know there will be extra steps too (and they are executed as part of the prompt call, not exposed to the caller).

If you read the documentation from OpenAI you also see this PR implements exactly the flow as they tell you:
OpenAI function calling

Without this logic "inside" the completion method you move the problem to the client (as @s6nqou mentioned this is what he has doing on his current project code and he was happy to find out this PR removes this problem from him).

Now imagine we are still not convinced and we want to handle the extra call outside prompt method.
First we need to return Message instead of String:

impl<M: CompletionModel> Prompt for &Agent<M> {
    async fn prompt(&self, prompt: impl Into<Message> + Send) -> Result<Message, PromptError> {
        self.chat(prompt, vec![]).await
    }
}

Then we are force to handle all type of messages in the client code:

// now I need to have the chat history vector in the client
let history = vec![Message::from("Can you please let me know title and url of rig platform?")];

loop {
    // assume we changed prompt implementation to return a Message
    let response: Message = search_agent
            .chat(history)
            .await?;

    match response {
        case Message::Text(text) => break Ok(text.text),
        case Message::ToolCall(tool_call) =>
            history.push(tool_call);

            let tool_result = ... // make the call to the tool here
            histroy.push(tool_result);
    }    
}

... it has not sense to me, I would not use this library if I have to do this.

I honestly don't see any justification to not make the extra calls to the LLM outside the method.

One option if you want low level control is to have 2 traits / methods, one is responsible to send a message and return the LLM response (also as a message).

pub trait LlmMessageHandler: Send + Sync {
    fn send_message(
        &self,
        message: impl Into<Message> + Send,
    ) -> impl std::future::Future<Output = Result<Message, PromptError>> + Send;
}

And keep the current traits as they are but with a default implementation with the loop from this PR calling the method from the previous trait:

pub trait Prompt: Send + Sync {
    fn prompt(
        &self,
        prompt: impl Into<Message> + Send,
    ) -> impl std::future::Future<Output = Result<String, PromptError>> + Send {
        // here we can implement the loop calling send_message method
    }
}

[carlos-verdes]

I also noticed another issue: the completion impls of the providers always append the prompt to the end of full_history before sending it to the LLM. I'm not sure if this will cause any issues. For example:

This is not correct, let me check this out.
The order should be as you mentioned, prompt first, then the tool call, then tool result.

[0xMochan]

I also noticed another issue: the completion impls of the providers always append the prompt to the end of full_history before sending it to the LLM. I'm not sure if this will cause any issues. For example:

Expected messages?:

first request:
[Prompt]
second request:
[Prompt, ToolCall, ToolResult]
third request:
[Prompt, ToolCall, ToolResult, AssistantContent]

Current messages:

first request:
[Prompt]
second request:
[ToolCall, ToolResult, Prompt]
third request:
[ToolCall, ToolResult, Prompt, AssistantContent]

I'm wondering if the prompt argument is really necessary, and could it be made optional or even removed? That way, we'd only need to manage chat_history. Would love to hear your thoughts.

Alright, this is a mistake, unfortuntely not caught in reviews 🫠. Prompt should always be the latest message w/ and w/o history! I'll handle this in a sep. issue + PR, good catch.

[carlos-verdes]

Prompt should always be the latest message w/ and w/o history!

I'm not sure about this, when you have function calls I think the proper order is:

• prompt
• call tool message
• tool result message

[carlos-verdes]

@s6nqou commit 7b84024 should fix the order issue.

[carlos-verdes]

Commit 4c96852 fixes issues found on DeepSeek model after we merged PR #283

[nothingalike]

ok i tested more after the filter update and this works for OpenAPI / Ollama when using the url approach.

closing: #303