softwaremill / sttp-openai

Apache License 2.0
41 stars 9 forks source link

Direct-style streaming using ox #193

Closed adamw closed 1 month ago

adamw commented 1 month ago

Closes #192

There's a couple of TODOs (waiting for missing Ox features), but it works.

One issue I have is that the signature of the stream-chat-completion request is complex:

Request[Either[OpenAIException, Ox ?=> IO ?=> Source[Either[DeserializationOpenAIException, ChatChunkResponse]]]]

Although it says exactly what might happen ;). That is:

I don't think I'd call such a signature "simple" ;). But once you understand the basic building blocks it makes sense ...

Alternatives:

  1. require IO & Ox as part of the createStreamedChatCompletion method. Simpler return type, more familiar signature, but not as precise: creating the request description doesn't in itself require a concurrency scope
  2. independently, any parsing errors might be propagated to the return Source as errors.

Combining these two we would end up with:

def createStreamedChatCompletion(
   chatBody: ChatBody
)(using Ox, IO): Request[Either[OpenAIException, Source[ChatChunkResponse]]] =

cc @lbialy

adamw commented 1 month ago

just one nitpick in docs, beside that looks fine, didn't know SSEs are already supported by ox-based sttp client

So you'd keep the signature of createStreamedChatCompletion as-is?

lbialy commented 1 month ago

I mean, it does model what can happen very precisely and the inclusion of capabilities bound to context functions in that type is something to behold (maybe @odersky should see this, this is a good example of what people will build with caps). Your proposals for simplification throw out some useful properties through the window. The only thing that could possibly happen here is just to compact the complexity by hiding some details with aliases, maybe opaque types that are subtypes, eg.: Source[Either[DeserializationOpenAIException, ChatChunkResponse]] could be ChatChunkResponseStream or ChatChunkResponseSource.

lbialy commented 1 month ago

independently, any parsing errors might be propagated to the return Source as errors.

rrrright, but that would close the Source on first serde error (because you have to return ChannelClosedException.Error) so the possibility that just part of the stream is borked or just unsupported on serde layer would disappear