fix(llma): streaming providers with tool calls

[Radu-Raicea]

There are two things happening in this PR:

1. Multiple fixes related to streaming providers and tool calls.
2. A significant refactoring that applies DRY principles to the providers by introducing a converter pattern for providers. The refactoring is not perfect, but this is a big step in a good direction, and we should continue improving on it.

The fixes are the following:

• Anthropic's streaming implementation was missing $ai_tools
• Anthropic's streaming implementation was missing tool calls in $ai_output_choices
• Gemini's streaming and non-streaming implementations were not sending text messages in the correct parsable format
• Gemini's streaming implementation was missing $ai_tools
• Gemini's streaming implementation was missing tool calls in $ai_output_choices
• OpenAI Chat Completions' streaming implementation was missing tool calls in $ai_output_choices

[greptile-apps]

_{17 files reviewed, 3 comments}

_{Edit Code Review Bot Settings | Greptile}

[carlos-marchal-ph]

I really like the direction of this PR. One of my first thoughts when I started working on this repo was that the code was more complex and repetitive than probably needed. This starts addressing this in a reasonable way.

The PR itself looks good to me, but when testing it locally I ran into some issues with OpenAI. I tried reproducing them in master, and the first one is also there:

1. OpenAI Responses streaming has 0 token count and is missing the assistant response after a tool call

3. OpenAI Chat completions is missing the assistant response after a tool call

Other than that I have some more comments on the direction we want to head towards, I'll add them as a separate comment so we can discuss. They are non blocking as they probably belong on separate PRs.

[carlos-marchal-ph]

In terms of strategy moving forward, I still think there's quite a bit of room for improvement. I don't think it belongs in this PR, but I'm just writing it out to sync on it.

There is still quite a bit of unnecessary code repetition in the repo. I think some of these utilities could probably be shared with the non-streaming implementation. The data types are the first that come to mind, but I'm sure there are many transformations that could be reused.

There's also still some code that's repeated in the sync and async implementations, such as the core event listening loop. Maybe we would benefit from having a class handling the entire event loop, which we could reuse across implementations, and which could hold some data that we are currently passing around every time.

More generally, I think some of these utilities should probably be decomposed into smaller classes in smaller files, with a clearer separation of concerns. The current approach of exporting a bunch of functions from a single file is not helping with readability or testability.

Again, these are all unordered thoughts, which I'm sure you've also had at some point. We can tackle this in other PRs down the road as we work on other stuff. Just wanted to write this down to see if you agree or if you think otherwise on some of these points.

[Radu-Raicea]

In terms of strategy moving forward, I still think there's quite a bit of room for improvement. I don't think it belongs in this PR, but I'm just writing it out to sync on it.

There is still quite a bit of unnecessary code repetition in the repo. I think some of these utilities could probably be shared with the non-streaming implementation. The data types are the first that come to mind, but I'm sure there are many transformations that could be reused.

There's also still some code that's repeated in the sync and async implementations, such as the core event listening loop. Maybe we would benefit from having a class handling the entire event loop, which we could reuse across implementations, and which could hold some data that we are currently passing around every time.

More generally, I think some of these utilities should probably be decomposed into smaller classes in smaller files, with a clearer separation of concerns. The current approach of exporting a bunch of functions from a single file is not helping with readability or testability.

Again, these are all unordered thoughts, which I'm sure you've also had at some point. We can tackle this in other PRs down the road as we work on other stuff. Just wanted to write this down to see if you agree or if you think otherwise on some of these points.

Completely agreed! If you have an opportunity to take steps in those directions, like I did in this PR, I will gladly review those PRs :D

[Radu-Raicea]

@carlos-marchal-ph

The LLM returns the tool call, but the handling of the tool call (and so the response with the weather) is not sent to LLMA unless you create a span event, or add it to the input of the next LLM call.

[carlos-marchal-ph]

Ah gotcha, missing context on my end then. Any idea about the 0 input 0 output tokens thing? In any case since it's happening on master too, it's probably unrelated to this PR, so approving 👏

[Radu-Raicea]

Fixed the Responses API streaming tokens, nice catch!