Fix DB race condition. Refresh at then end of a completed stream.
- FIxes: https://github.com/vercel/ai-chatbot/issues/302
- Fixes: https://github.com/vercel/ai-chatbot/issues/364
- Resolve race condition when aiState.done is called before onSetAIState
- This PR prevents the UI from refreshing before the db is updated.
- A similar approach proposed by @Saran33 in https://github.com/vercel/ai-chatbot/pull/399
- Implements a token based approach to refreshing instead of the hacky message length approach.
- Only refreshes when a new message finishes. No extra refreshes on a chat with 1 message.
- Only animate the side menu when a new chat is created and name the localStorage variable more appropriately.
https://github.com/user-attachments/assets/2bf3d8f4-bd5e-400e-a5b0-012279bd5f83
@shenst1 is attempting to deploy a commit to the Uncurated Tests Team on Vercel.
A member of the Team first needs to authorize it.
Nice idea, I hadn't noticed the issue with navigating between chats. The only caveat I would have about refreshing the router when aiState is done would be that if there's a delay in the generate function, for instance if calling a third party API, then refreshing the router could cause an error because the UI stream won't have completed. I'm currently working on decoupling it so that aiState.done() or aiState.update is only called after the message streams are closed. It seems like when AI state is updated mid-stream it's causing re-renders and some jumping. Also, as for refreshing the router on every message, that too can sometimes add to the jarring UX.
Looking at it again, I'm wondering if could be worth updating the Link in SidebarItem to refresh the router that way to resolve https://github.com/vercel/ai-chatbot/issues/364
Discussion on it here https://github.com/vercel/next.js/discussions/65487#discussioncomment-9350577
Following on from the last comment, a quick workaround for https://github.com/vercel/ai-chatbot/issues/364 could be to do something like:
// actions.ts
import { revalidatePath } from 'next/cache';
export async function revalidate(path: string) {
revalidatePath(path);
}
// sidebar-item.tsx
// ...
const handleLinkClick = useCallback(async () => {
await revalidate(`/chat/${chat.id}`);
}, [chat.id]);
// ...
<Link
href={`/chat/${chat.id}`}
onClick={handleLinkClick}
prefetch={false}
)}
>
This way, at least we would only be refreshing the router cache when a user clicks to change chat. That may be less frequent than doing so after each message within the same chat. However, revalidatePath currently invalidates all the routes in the client router cache, a well as purging the Data Cache and Full Route Cache, and requires a server action. In light of that, an [even more hackish] approach could be something like:
const handleLinkClick = useCallback(
async (e: React.MouseEvent) => {
e.preventDefault();
router.push(`/chat/${chat.id}`);
router.refresh();
},
[chat.id]
);
Regarding the fix using a handleLinkClick function, I believe this approach could lead to potential issues. The problem would surface with any <Link> click, not just isolated instances like the one in the sidebar-item. For example, my application includes a main menu with several navigational links to different pages. This would require adding a click handler to every single link on the page (including those in an informational footer) to ensure proper functionality.
In my experience, modifying Link click handlers can introduce unintended consequences and is generally not advisable. Instead, using refresh aligns better with the application state and follows the patterns recommended in the documentation. While this might occasionally cause some animation jankiness, it's better to address the animation itself rather than altering the fundamental state management of the router.
@shenst1 I totally agree, it's not ideal. I have the same issue if navigating to other landing pages and then clicking Back in the browser. But if navigating via mouse clicks in the UI, the chat is always refreshed. For my use case, I think the edge case is worth the tradeoff for a smoother chat experience, but currently refactoring it significantly so gonna take another look at it at some stage for sure
It looks like this repo has been refactored to use the UI library instead of ai/rsc. However, the refresh issue is still present. I was able to get the behavior i visually want by adding a client fetch in chat.tsx like this
const { data: loadedConvClientSide } = useQuery({
queryKey: ['conversation', id],
queryFn: async () => {
return await getConversation(id);
}
})
then in useChat:
initialMessages: loadedConvClientSide?.messages
I've spent a ton of time trying to find a solution that works with revalidatePath, but I couldn't come up with anything that worked consistently.
Closing this PR. The recent refactor makes this completely out of date. Still a decent solution for those stuck on the older version and useChat isn't possible. The new onFinish param in useChat is where any problems related to this issue should be handled.