Skip to content

server: Fix the status of finish_reason if the stream value is False #10382

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

Closed
wants to merge 1 commit into from

Conversation

SeongBeomLEE
Copy link

@SeongBeomLEE SeongBeomLEE commented Nov 18, 2024

"finish_reason" returns "stop" in all cases because the "stop_word" value is always "True" when using "stream false mode" in /chat/completions API.

So modify the "stopped_word" value so that "length" can also be returned.

Thanks.

@SeongBeomLEE SeongBeomLEE changed the title [fix] Modify the status of finish_reason if the stream value is False server: Fix the status of finish_reason if the stream value is False Nov 18, 2024
Copy link
Collaborator

@ngxson ngxson left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Collaborator

@ngxson ngxson left a comment

Choose a reason for hiding this comment

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

I temporary revoke my approval because the CI failed.

It's likely due to another bug in the code, unrelated to this PR. In anyway, the 3 bool stopped_eos, stopped_word, stopped_limit are too hacky and they should be refactored into one single enum.

I would appreciate if you (or someone) can take time to refactor this whole stopped_* mechanism.

@SeongBeomLEE
Copy link
Author

@ngxson

can you merge the current PR to resolve the issue?

I will leave another PR later on what you talked about.

thanks.

@ngxson
Copy link
Collaborator

ngxson commented Dec 6, 2024

I'm closing this because I moved stopped_* to an enum in #10643

finish_reason should now be correct.

@ngxson ngxson closed this Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants