-
Notifications
You must be signed in to change notification settings - Fork 305
AI sdk v5 migration #391
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
AI sdk v5 migration #391
Conversation
🦋 Changeset detectedLatest commit: 8527613 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
7bd4b8b to
a451b1b
Compare
threepointone
left a comment
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 did a first pass. the code seems ok, but the typecasting seems too much. I'll test this out tomorrow and see if I can fix the types.
|
When this lands, how do we migrate existing persisted messages to use the new message types? |
|
good point. we should add an automatic migration. |
2dee41d to
8d0ebdc
Compare
|
@whoiskatrin Looking at the code to migrate v4 messages. The latest version of v4 messages actually had content and parts. But v5 modified the part types as well so a full migration would likely required mapping the part types as well |
|
@ryleyrandall22 thanks for checking, this is work in progress pretty much and not ready for review |
|
Do you think it makes sense to add only send latest message functionality to this PR? I wrote a PR for this functionality for the v4 sdk and I'm happy to make another one for v5 if this PR lands before that. Or it might make sense to just include it in this PR since we're refactoring things anyway. See: |
b637e7a to
dd9b71c
Compare
Just tested it across multiple apps, and it works perfectly. Thanks a lot for the fix and your efforts! |
packages/agents/src/index.ts
Outdated
| export { | ||
| useAgentChat, | ||
| detectToolsRequiringConfirmation, | ||
| type AITool | ||
| } from "./ai-react"; |
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.
@whoiskatrin These exports from ./ai-react are causing errors when building my RSC project which imports from agents (in a server context) and from agents/react or agents/ai-react (in a use client context).
Would it be ok to remove this re-export and always import these from agents/react or agents/ai-react.
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.
@threepointone should we? You asked me to do the opposite, so referring to you here
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.
yeah let's not export these from the root
|
@jldec i think most of the things should be solved now, persistance is fixed, just need to do the export for you |
threepointone
left a comment
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.
tentative approval, looks good, pending nits and notes
|
I had a quick question; Is there any way to send warnings to ui without writing |
https://ai-sdk.dev/docs/ai-sdk-ui/streaming-data#transient-data-parts-ephemeral should work |
|
@deathbyknowledge i noticed, when i used https://ai-sdk.dev/docs/ai-sdk-ui/streaming-data#transient-data-parts-ephemeral, it only sent the data to the user that triggered it, and it didnt broadcast to all connections. I'm assuming thats intended? |
I believe that transient data parts are designed to be delivered only to the user connection that triggered the event, yes. |
|
@whoiskatrin I think i figured out the cause of the issue I noticed earlier When invoking the time tool in guides/human-in-the-loop, the tool is running twice, once on the server and once on the client. The UI shows the server tool output, but it sends a message back with the client tool output, which is what is persisted. If you make the tool outputs different in guides/human-in-the-loop/src/tools.ts you'll see that it's the client result which is saved and appears on reload. |
this has been addressed btw, you shouldn't be experiencing this. there was a root cause and we've eliminated that |
|
Thanks @whoiskatrin - I confirmed that the issues I saw previously are now fixed. I did do a little bit of digging to figure out how to show errors in the client UI, and when I expose the error value returned by the useAgentChat hook, I noticed this error appearing after processing any input message (confirmed by adding error output to /guides/human-in-the-loop/src/app.tsx as in the screenshot below)
|
Oh, good catch, I've just pushed the fix. |
|
Thank you! |
|
Haven't been as excited for a PR in a while 🙏🏼 This is a-mazing 😄 ❤️ |
|
@whoiskatrin sorry this is after merged but can we add a way to override the type of |
|
Can you open a new issue for this? We probably need a new signature because we already have a third aegument incoming there |
|
@threepointone created! #433 |
|
@whoiskatrin This is really amazing, thank you for this update. Are there plans to update the starter template to use the latest version? |
|
@sscowden yes! it's almost done, we will try to publish it on monday |




Updated from AI SDK v4 → v5 (createUIMessageStream, UIMessageStreamWriter, etc.)