-
Notifications
You must be signed in to change notification settings - Fork 3.2k
feat(ui): add finishReason in useChat onFinish callback #9857
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the pull request.
For manual verification, I would use one examples/next-openai and update one of the examples in there to some how handle the finish reason, to make sure it works as expected for the most common cases
| @@ -1,3 +1,4 @@ | |||
| import { FinishReason } from '../types/language-model'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gr2m
My miss.. removed it.
| expect(letOnFinishArgs).toMatchInlineSnapshot(` | ||
| [ | ||
| { | ||
| "finishReason": undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't looked in detail yet, but is there a case where finishReason would be undefined? We might need to update our fixtures in the tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so, finishReason can be undefined when there’s no finish event (abort, disconnect, server error). For normal completions it should be set (usually 'stop').
I’ve updated fixtures: normal paths include finishReason: 'stop'; abnormal paths keep it undefined.
This PR adds the finishReason parameter to the onFinish callback in useChat, allowing developers to know why a stream finished (stop, length, content-filter, tool-calls, etc.). Changes: - Added finishReason to ChatOnFinishCallback type - Updated UI message stream handling to propagate finishReason - Updated tests to verify finishReason is passed correctly - Added changeset for patch release
3b8a2ad to
fe0f666
Compare
I updated one of a example file in 'examples/next-openai'(use-chat-data-ui-parts). Sorry for late update after requesting review. |
Background
#9780
In order to use
finishReasonwithuseChaton the frontend, users currently have to manually send it from the server—either by adding it to metadata or usingcreateUIMessageStream.I raised a question in the discussion tab, and it seems worthwhile to include
finishReasonby default so developers can access it without additional setup.Summary
This PR adds the finishReason parameter to the onFinish callback in useChat, allowing developers to know why a stream finished (stop, length, content-filter, tool-calls, etc.).
Changes:
Manual Verification
Checklist
pnpm changesetin the project root)Future Work
Related Issues
closes #9780