General fixes for tool calling

[Trofleb]

General fixes for tool calling

Tool calling was incompatible with the openai api on both the stream and non stream implementation.

Fixes:

• tool calling should return a string in arguments
• tool calling streaming should return an array as tool_calls
• tool calling streaming was not streaming the arguments in a readable way (more on that further down)

Small changes

• Better differentiation in tool types
• arguments/properies differentiation

Fixes the following issues I think:
#2461
#2480
#2864

Before submitting

• This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
• Did you read the contributor guideline,
  Pull Request section?
• Was this discussed/approved via a Github issue or the forum? Please add a link
  to it if that's the case.
• Did you make sure to update the documentation with your changes? Here are the
  documentation guidelines, and
  here are tips on formatting docstrings.
• [~] Did you write any new necessary tests?

Details

Arguments are now strings, as per openai's API

Open AI - non stream

"tool_calls": [
  {
    "id": "call_7c9bnn08VbcmznYinyg5H9Do",
    "type": "function",
    "function": {
      "name": "get_weather",
      "arguments": "{\"location\":\"Zurich, France\"}"
    }
  }
],

Before - non stream

"tool_calls": [
  {
    "id": "0",
    "type": "function",
    "function": {
      "description": null,
      "name": "get_weather",
      "arguments": {
        "location": "Zurich, France"
      }
    }
  }
]

Now - non stream

"tool_calls": [
          {
            "id": "0",
            "type": "function",
            "function": {
              "name": "get_weather",
              "arguments": "{\"location\":\"Zurich, France\"}"
            }
          }
        ]

Streaming has a particular behaviour for tool calling, I did my best to conform to what the openai api was giving out.

Open AI - stream

{"role":"assistant","content":null,"tool_calls":[{"index":0,"id":"call_TCt8rccpnTreOYMxv16lefdg","type":"function","function":{"name":"get_weather","arguments":""}}],"refusal":null}
{"tool_calls":[{"index":0,"function":{"arguments":"{\""}}]}
{"tool_calls":[{"index":0,"function":{"arguments":"location"}}]}
{"tool_calls":[{"index":0,"function":{"arguments":"\":\""}}]}
{"tool_calls":[{"index":0,"function":{"arguments":"Zuri"}}]}
{"tool_calls":[{"index":0,"function":{"arguments":"ch"}}]}
{"tool_calls":[{"index":0,"function":{"arguments":","}}]}
{"tool_calls":[{"index":0,"function":{"arguments":" France"}}]}
{"tool_calls":[{"index":0,"function":{"arguments":"\"}"}}]}
{}

Before - stream

{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"{\""}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"function"}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"\":"}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":" {\""}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"_name"}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"\":"}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":" \""}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"get"}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"_weather"}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"\","}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":" \""}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"location"}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"\":"}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":" \""}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"Zur"}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"ich"}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":","}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":" France"}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"\"}}"}}}
{"role":"assistant","tool_calls":{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"<|eot_id|>"}}}

Now - stream

{"role":"assistant","tool_calls":[{"index":0,"id":"","type":"function","function":{"name":"get_weather","arguments":"{"}}]}
{"role":"assistant","tool_calls":[{"index":0,"id":"","type":"function","function":{"name":null,"arguments":" \""}}]}
{"role":"assistant","tool_calls":[{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"location"}}]}
{"role":"assistant","tool_calls":[{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"\":"}}]}
{"role":"assistant","tool_calls":[{"index":0,"id":"","type":"function","function":{"name":null,"arguments":" \""}}]}
{"role":"assistant","tool_calls":[{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"Zuri"}}]}
{"role":"assistant","tool_calls":[{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"ch"}}]}
{"role":"assistant","tool_calls":[{"index":0,"id":"","type":"function","function":{"name":null,"arguments":","}}]}
{"role":"assistant","tool_calls":[{"index":0,"id":"","type":"function","function":{"name":null,"arguments":" France"}}]}
{"role":"assistant","tool_calls":[{"index":0,"id":"","type":"function","function":{"name":null,"arguments":"\"}"}}]}

Testing

The test I could run seem to still pass ( I don't have a premium hugging face account - yet ;). I adapted the tests that needed to be adapted but I'm not sure how to add test for streaming. I welcome any help I can get on that front.

I did some tests by doing the following:

1. Ran a simple tool calling model on local

text-generation-launcher --model-id MadeAgents/Hammer2.1-0.5b --port 8080

2. Tested tool calling without streaming using langchain_openai
3. Tested tool calling with streaming using langchain_openai

Other PR

A PR was created before I could make mine. As I went bit further I still wanted to finish mine. Of course I'm open to closing mine, or group our changes.

Review

Not sure who to tag here.
FYI: I'm by no means a rust expert and welcomes criticism/improvements.

[Datta0]

@Trofleb
Thanks for this.

Do you happen to have the response handy for the non streaming aka stream=False case? We noticed that TGI returns a JSON while OpenAI spec expects string...
It'd be great if you can add a sample response here for reference

[Trofleb]

@Datta0 We had the same issue, I added the samples for the non stream version to my initial message.

[Trofleb]

Looking at other PR, maybe @OlivierDehaene or @Narsil can you review this ?

[Datta0]

@Datta0 We had the same issue, I added the samples for the non stream version to my initial message.

Cool, I think the response looks fine....
Just one small question. Why do we need to rename arguments to parameters here ?

[Trofleb]

Just one small question. Why do we need to rename arguments to parameters here ?

It's one of the slight api differences. The code was allowing both arguments and parameters in function definition which is not the case with openai. It was a side effect of using the same struct for function definition and function call. By removing it I changed a bit the accepted values, making it closer to openai.

Thinking about it now, I should have made it a non breaking change. I'll fix it if anyone thinks it's a problem.