Improve tool call message processing #3036

drbh · 2025-02-19T00:52:54Z

this PR allows incoming chat requests to specify tool_calls and no content, which allows tools to be passed from a tool response call into the messages of a subsequent request

for example if the following set of messages is sent without these changes an error is thrown attempting to deserialize the message missing field 'content'

[
    {"role": "user", "content": "What's the weather like in Paris today?"},
    {
        "content": "",
        "role": "assistant",
        "tool_calls": [
            {
                "id": "0",
                "function": {
                    "arguments": '{"longitude": 2.2945, "latitude": 48.8567}',
                    "name": "get_weather",
                    "description": None,
                },
                "type": "function",
            }
        ],
    },
    {"role": "tool", "tool_call_id": "0", "content": "6.7"},
]

response

I'm an AI and do not have access to real-time data. However, based on location information (Paris) I can provide general information. \n\nThe temperature in Paris varies widely throughout the year. In the summer (June to August), the average high temperature is around 23°C (73°F), while in the winter (December to February), the average low temperature is around -1°C (30°F). \n\nTo get the current weather in Paris, I recommend checking a weather website or

this PR allows messages without tools to be sent without error.

notes:

tools are included in the template by serializing the calls as a JSON string and replacing the content
thank you @saileshd1402 for a opening this draft PR Optional content field in Chat Completions Request #3021 all of your changes are included in this PR!

saileshd1402 · 2025-02-19T07:26:17Z

Thank you for this change! I'll test it out and update here

saileshd1402 · 2025-02-19T10:52:37Z

I've tested this change, it is working well!

alvarobartt · 2025-02-19T11:06:53Z

router/src/lib.rs

@@ -1221,9 +1223,15 @@ pub struct TextMessage {

 impl From<Message> for TextMessage {
    fn from(value: Message) -> Self {
+        let content = value


Maybe we could add an additional check here to prevent this type of messages from anything other than the assistant, as well as if both content and tool_calls are provided, WDYT? Maybe even a custom new error as you suggested recently.

good point! I've updated the PR to prefer a none optional enum that ensure either a content or tools are provided - looking into how errors may be improved now

alvarobartt · 2025-02-19T11:12:45Z

router/src/server.rs

-("application/json" = Vec<GenerateResponse>),
-("text/event-stream" = StreamResponse),
+(Vec<GenerateResponse> = "application/json"),
+(Vec<GenerateResponse> = "application/json"),


Duplicated here!

Suggested change

(Vec<GenerateResponse> = "application/json"),

thanks, avoided these issues/changes, by avoiding bumping the utopia version

Narsil · 2025-02-19T16:07:49Z

router/src/lib.rs

+    pub content: Option<MessageContent>,
    #[serde(default, skip_serializing_if = "Option::is_none")]
    #[schema(example = "\"David\"")]
    name: Option<String>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    tool_calls: Option<Vec<ToolCall>>,


Do we agree there can never be both a content and a tool_calls ?
Also never neither.

If true, then this begs to become an Enum of Either content or tool calling.
And that should simplify all the underlying code.

Untagged enum are not great for the error message so we should probably implement our own deserialize method instead. (Since there doesn't seem to be a type field equivalent serde could use to know which enum to deserialize)

Narsil · 2025-02-19T16:11:28Z

...ration-tests/models/__snapshots__/test_tools_llama/test_flash_llama_tool_reply_response.json

+        "content": "I can't access real-time data, but I can provide you with current conditions and forecast for Paris, France:\n\nThe current conditions in Paris are mostly cloudy with a temperature of 6.7°C (44.1°F). \n\nPlease note that the actual weather may differ from this information, and I recommend checking the forecast on a reliable weather website for the most up-to-date information.",
+        "name": null,
+        "role": "assistant",
+        "tool_calls": null


Shouldn't we skip that?

name and tool_calls are actually skipped in the response, but the Python client library adds None when deserializing the response.

the actually response payload is

{ "object": "chat.completion", "id": "", "created": 1740011163, "model": "meta-llama/Llama-3.1-8B-Instruct", "system_fingerprint": "3.1.1-dev0-native", "choices": [ { "index": 0, "message": { "role": "assistant", "content": "I can't access real-time data, but I can provide you with current conditions and forecast for Paris, France:\n\nThe current conditions in Paris are mostly cloudy with a temperature of 6.7\u00b0C (44.1\u00b0F). \n\nPlease note that the actual weather may differ from this information, and I recommend checking the forecast on a reliable weather website for the most up-to-date information." }, "logprobs": null, "finish_reason": "stop" } ], "usage": { "prompt_tokens": 103, "completion_tokens": 79, "total_tokens": 182 } }

Narsil · 2025-02-19T16:11:40Z

backends/v3/Cargo.toml

@@ -16,7 +16,7 @@ path = "src/main.rs"
 [dependencies]
 async-trait = "0.1.74"
 async-stream = "0.3.5"
-axum = { version = "0.7", features = ["json"] }
+axum = { version = "0.8", features = ["json"] }


Why do we need upgrades ?

only needed to generate valid openapi docs for the previous approach of Option<MessageContent> this is no longer needed with improved typing (using an enum instead of two optionals) in the latest commits

Narsil · 2025-02-19T16:13:53Z

integration-tests/models/test_tools_llama.py

+            {
+                "content": "",
+                "role": "assistant",
+                "tool_calls": [
+                    {
+                        "id": "0",
+                        "function": {
+                            "arguments": '{"longitude": 2.2945, "latitude": 48.8567}',
+                            "name": "get_weather",
+                            "description": None,
+                        },
+                        "type": "function",
+                    }
+                ],
+            },
+            {"role": "tool", "tool_call_id": "0", "content": "6.7"},


I really don't get that structure.

Assistant is saying it's calling a tool, and the tool is another participant in the conversation saying it has the answer for the call, is that correct ?
How does this play with chat templates ?? Are they ready for that ?

Relevant internal discussion : https://huggingface.slack.com/archives/C06JKEMK6BZ/p1739982025532149

updated to include tool_call_id in the chat template as some models will expect this template input in the latest commits

alvarobartt reviewed Feb 19, 2025

View reviewed changes

Narsil reviewed Feb 19, 2025

View reviewed changes

Narsil mentioned this pull request Feb 19, 2025

make content field optional in Message for role=assistant #3035

Closed

5 tasks

saileshd1402 and others added 6 commits February 19, 2025 19:39

make content field optional in chat request

e14617d

add tool_calls field to Message struct

f5e1a16

feat: add test and serialize tool messages

ac50b14

fix: bump utopia, openapi doc version and improve test

bddcf9b

fix: rerun update docs

56f2d66

fix: suppoer tool call id in template and remove unnecessary changes

bcc4489

drbh force-pushed the improve-tool-call-message-processing branch from db23337 to bcc4489 Compare February 20, 2025 00:40

drbh added 2 commits February 19, 2025 19:41

fix: ruff lint remove unused import

4fa8512

fix: adjust message types in tests

3770344

Narsil approved these changes Feb 21, 2025

View reviewed changes

Narsil merged commit 1cae319 into main Feb 21, 2025
20 checks passed

Narsil deleted the improve-tool-call-message-processing branch February 21, 2025 09:30

Narsil mentioned this pull request Feb 21, 2025

Optional content field in Chat Completions Request #3021

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve tool call message processing #3036

Improve tool call message processing #3036

drbh commented Feb 19, 2025

saileshd1402 commented Feb 19, 2025

saileshd1402 commented Feb 19, 2025

alvarobartt Feb 19, 2025

drbh Feb 20, 2025

alvarobartt Feb 19, 2025

drbh Feb 20, 2025

Narsil Feb 19, 2025

Narsil Feb 19, 2025

Narsil Feb 19, 2025

drbh Feb 20, 2025

Narsil Feb 19, 2025

drbh Feb 20, 2025

Narsil Feb 19, 2025

Narsil Feb 19, 2025

drbh Feb 20, 2025

Improve tool call message processing #3036

Improve tool call message processing #3036

Conversation

drbh commented Feb 19, 2025

saileshd1402 commented Feb 19, 2025

saileshd1402 commented Feb 19, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment