Skip to content

Drop warning for empty choice content #411

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

declark1
Copy link
Collaborator

@declark1 declark1 commented Jun 4, 2025

There are scenarios where this is valid, so this PR drops the warning log message when there is no choice content as this is fine.

e.g.

  • The first message's choice only includes role: assistant with no content
  • The final message with the stop reason may not include content
  • Likely other scenarios

@declark1 declark1 marked this pull request as ready for review June 4, 2025 22:30
Copy link
Collaborator

@mdevino mdevino left a comment

Choose a reason for hiding this comment

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

LGTM.

@declark1
Copy link
Collaborator Author

declark1 commented Jun 5, 2025

Discussed further offline, so going ahead and merging this.

In the future, if we want to send warnings to the user for streaming similar to unary in #402, it will need to be done at the end and sent with the last message, similar to whole doc output detections, after all chat completion chunks have been received and chat completion state is final. We won’t know if the entire content for each choice is empty or not until this point.

@declark1 declark1 merged commit e13682f into foundation-model-stack:main Jun 5, 2025
3 checks passed
@declark1 declark1 deleted the chat-completions-streaming-warning branch June 5, 2025 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants