- Extend ChatResponseMetadata for Anthropic (blocking, streaming)
- Add ChatResponseMetadata for Mistral AI (blocking)
- Extend ChatResponseMetadata for OpenAI (blocking)
- Deprecate gpt-4-vision-preview and replace its usage in tests because OpenAI rejects the calls, making the tests fail (see: https://platform.openai.com/docs/deprecations)
Fixes gh-936
Comment From: ThomasVitale
The OpenAiChatModel
doesn't currently support getting the Usage
metadata when using the streaming mode. After https://github.com/spring-projects/spring-ai/pull/920 is fixed, the OpenAiChatResponseMetadata
can be included in the streaming response as well.
Similarly, MistralAiChatModel
doesn't currently support getting the Usage
metadata when using the streaming mode.
Comment From: tzolov
Hey @ThomasVitale , Glad you've taken the responsibility of fixing this gap in our response format. The IDs are important. Here: https://github.com/spring-projects/spring-ai/pull/550 is an early attempt to address the missing IDs and other response handling issue (like correct handling for multiple choices response), but didn't like the result and haven't had time to give it another try. So thank your for looking at it. Btw, in my experiments I've noticed that not all Ai Models provide ID in their response, so In those cases i've opted for generating synthetic Ids. But maybe there are better solutions.
Comment From: ThomasVitale
@tzolov thanks for the link, I'll have a look at that PR. Some providers don't return a response ID, but in that case I would keep it empty in ChatResponseMetadata
since it's a provider-specific piece of information to trace a specific request on the provider side. If it's not provided, then it should be fine to ignore it.
For other client-driven scenarios where an ID is required on all requests (supplemented by synthetic IDs), we might need a separate concept to model that, distinct from the server-driven response ID. What do you think?
Comment From: tzolov
@ThomasVitale you have a point. When did my experiments I had the wrong assumption that need the server side ID in order to aggregate streaming messages. But this assumptions was incorrect we can do this.
Comment From: tzolov
Rebased, squashed and merged at 3227311314ca49f0ed9c36f4a67df86822c6055c
Comment From: ThomasVitale
Thank you!