Skip to content

Add support for stream usage in Azure OpenAi #2858

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

andresssantos
Copy link
Contributor

Description

Related issue: spring-ai#1724

This PR introduces support for streamUsage in the AzureOpenAiChatOptions class. Additionally:

  • Updates the unit test AzureOpenAiChatOptionsTests to reflect the changes.
  • Updates the documentation in azure-openai-chat.adoc.

}

public void setStreamUsage(Boolean enableStreamUsage) {
this.streamOptions = (enableStreamUsage) ? new ChatCompletionStreamOptions().setIncludeUsage(true) : null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need an explicit property to set the enableStreamUsage value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This explicit property needs to drive how the usage is included in AzureOpenAiChatModel. Please check OpenAIChatModel StreamOptions#includeUsage for reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your review and feedback, @ilayaperumalg!

Based on your comments, I have made the following changes:

  • Introduced an explicit property (enableStreamUsage) in AzureOpenAiChatOptions.
  • Updated the setter so it no longer modifies streamOptions directly.
  • Adjusted the logic in AzureOpenAiChatModel to use the enableStreamUsage property to determine whether to include usage information in streaming responses, following the pattern of OpenAiChatModel.

Could you please check if the current implementation meets the expectations?

Thanks again for your guidance!

Signed-off-by: Andres da Silva Santos <[email protected]>
Signed-off-by: Andres da Silva Santos <[email protected]>
Signed-off-by: Andres da Silva Santos <[email protected]>
@andresssantos andresssantos force-pushed the azure-openai-stream-usage branch from ffe2bed to 9b81f11 Compare April 30, 2025 01:37
@mkheck
Copy link
Contributor

mkheck commented Apr 30, 2025

Hi @ilayaperumalg, this looks good to me as well.

@ilayaperumalg
Copy link
Member

Thank you @andresssantos for the follow up with the PR and @mkheck for the review. We'll get this reviewed and merged soon.

@ilayaperumalg ilayaperumalg self-assigned this May 1, 2025
@ilayaperumalg ilayaperumalg added this to the 1.0.0-RC1 milestone May 1, 2025
@ilayaperumalg
Copy link
Member

Rebased, squashed and merged as 7bb553e

@andresssantos Thanks very much for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants