Updated streaming tools for OpenAI

[rubencagnie-toast]

👋 - I had a similar challenge, but then I saw your PR on the koog repo - so I figured I'd contribute to your repo

With the changes I made, I was successful in creating a strategy that does the streaming + tool calls appropriately

edge(nodeStart forwardTo mapStringToRequests)
edge(mapStringToRequests forwardTo nodeStreaming)
edge(nodeStreaming forwardTo executeMultipleTools onMultipleToolCalls { true })
edge(executeMultipleTools forwardTo mapToolCallsToRequests)
edge(mapToolCallsToRequests forwardTo nodeStreaming)
edge(nodeStreaming forwardTo nodeFinish onNoToolCalls  { true })

[dosier]

Oo I just made some breaking changes, someone on the JB team said it's fine to introduce breaking changes, so I changed the sig of all the executeStreaming funcs to return a flow of streamframe objects (also fixed refactor issues). Are you able to merge my changes in fine @rubencagnie-toast ?

[rubencagnie]

Looking into that now

[dosier]

Anyways I am off to bed now, if u haven't done it yet, I'll merge my latest changes in this pr tmmrw :)

[dosier]

Heya, thanks, I like the StreamHandler integration, just some of the other changes I think I am a bit confused on.

I think my mistake was that I didn't realise a tool call can be chunked, I assumed they were always carrying the full payload.
After reading the OpenAI docs I see they do support delta tool calls.

Though idk if we should handle the combining of delta tool calls under the hood (the contract being that a tool call will only be emitted when/if it's fully streamed by the underlying llm api).

Maybe we should make it so there are two types of StreamFrame flows, a low level and a high level one. Not sure on the best approach here. We can also keep the design very low-level (matching openAI standard) and just introduce fancy extension functions for user friendliness.
E.g. stream.autoMergeToolCalls().onEach since I think most users don't care about partial tool calls and then having to combine them themselves every-time.

[dosier]

I am not sure if this is in-scope for my original PR, might be best added as part of a separate PR?

[dosier]

Okay after reviewing the other changes I understand this method more now, as in you can access the streamed data through the newly introduced StreamHandler in EventHandler, I am not really familiar with the whole koog api yet for building strategies and what not so this was a bit confusing to me at first.

I still feel like this may be a feature that should probably be part of it's own PR or do you think this should be considered a fundamental feature of tool calling support in the api?

[rubencagnie-toast]

I'm happy to take this out. I am using this in my application, but it might be too specific to my use-case. Happy to remove it

[dosier]

Do u think it's worth it making text nullable? I feel like those frames can be omitted.
However, I do like to have support for finishReason, maybe it would be nicer to introduce a new StreamFrame type, like StreamFrame.End or something that contains more meta information (perhaps also token count etc).

    /**
     * Represents a frame of a streaming response from a LLM that signals the end of the stream.
     *
     * @property finishReason The reason for the stream to end.
     */
    @Serializable
    public data class End(
        val finishReason: String? = null
    ) : StreamFrame

I am not very familiar with the standard when it comes to this in llm apis. But keeping Append.text null-safe seams like such a massive benefit for the vast majority of use-cases.

[rubencagnie-toast]

Oh, I like that idea. Are we always guaranteed that the event with "finish-reason" won't have content left? If not, we might need to consider have the function return List<StreamFrame> when parsing it

[rubencagnie-toast]

Heya, thanks, I like the StreamHandler integration, just some of the other changes I think I am a bit confused on.

I think my mistake was that I didn't realise a tool call can be chunked, I assumed they were always carrying the full payload. After reading the OpenAI docs I see they do support delta tool calls.

Though idk if we should handle the combining of delta tool calls under the hood (the contract being that a tool call will only be emitted when/if it's fully streamed by the underlying llm api).

Maybe we should make it so there are two types of StreamFrame flows, a low level and a high level one. Not sure on the best approach here. We can also keep the design very low-level (matching openAI standard) and just introduce fancy extension functions for user friendliness. E.g. stream.autoMergeToolCalls().onEach since I think most users don't care about partial tool calls and then having to combine them themselves every-time.

I've updated the PR with your comments. I will be off for the next 24 hours, so will catch back up on Monday

[dosier]

Okay I will be off until monday too probably, I also made some changes, introducing StreamFrame.End and some flow related extensions. Nice, now I think the only thing I am not sure on how to resolve is partial tool calls.

I am not sure if every supported llm client has support for tool call streaming, if they do then we should probably place the responsibility on the user to merge and validate the tool call chunks and refactor StreamFrame.Append and StreamFrame.ToolCall to StreamFrame.Delta.Text and StreamFrame.Delta.ToolCall respectively, then offer some syntactic sugar (extension functions) to transform a flow of stream frames to a more user friendly flow.

[dosier]

I opened another PR for u hahah rubencagnie#2 where I merged my latest changes in.

[dosier]

Also opened another version with the removal of partial tool call streaming, since only openai's official api seems to support that, but they also send an event when a tool call is finished that contains all the required info.
rubencagnie#3