Fix bug where playgroud conversation is was not reset when playground…#2473
Fix bug where playgroud conversation is was not reset when playground…#2473sarah-inkeep merged 2 commits intomainfrom
Conversation
… was closed or when navigating between agents
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: f51d8ec The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 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 |
|
Fixes a stale
|
There was a problem hiding this comment.
Clean, well-targeted fix. The two changes cover both scenarios correctly: unmount cleanup handles playground close, and the generateId() in resetAgentState handles agent navigation. No issues found.
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
💭 Consider (1) 💭
Inline Comments:
- 💭 Consider:
playground.tsx:54MissingresetPlaygroundConversationIddependency in useEffect
✅ APPROVE
Summary: Clean, focused bug fix that correctly addresses the conversation ID persistence issue across playground close/agent navigation. The implementation follows established zustand patterns in the codebase. The useEffect cleanup approach is appropriate for component unmount handling.
Nice work! 🎉
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-frontend |
1 | 0 | 0 | 0 | 1 | 0 | 0 |
| Total | 1 | 0 | 0 | 0 | 1 | 0 | 0 |
| useEffect(() => { | ||
| // when the playground is closed the chat widget is unmounted so we need to reset the conversation id | ||
| return () => resetPlaygroundConversationId(); | ||
| }, []); |
There was a problem hiding this comment.
💭 Consider Missing dependency in useEffect
Issue: The resetPlaygroundConversationId function is referenced in the cleanup but not listed in the dependency array.
Why: While this works correctly in practice (zustand actions are stable references, as noted in the store's TKDodo pattern comments), adding the dependency is cleaner and satisfies exhaustive-deps expectations. The file already uses biome-ignore directives for similar patterns (line 74), so either approach is acceptable.
Fix: (1-click apply)
| }, []); | |
| }, [resetPlaygroundConversationId]); |
Refs:
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
This is a delta re-review triggered by a merge from main (f51d8ec46). The actual bug fix code (c834801bd) has not changed since the prior review.
🕐 Pending Recommendations (1)
- 💭
playground.tsx:54MissingresetPlaygroundConversationIddependency in useEffect
✅ APPROVE
Summary: No new code changes in this PR's scope since the prior review. The merge from main only brought in version bumps and unrelated changes. The original fix remains clean and well-targeted. The prior Consider item about the missing dependency is noted but non-blocking (zustand actions are stable references per the TKDodo pattern).
Ship it! 🚀
Reviewers (0)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
| Total | 0 | 0 | 0 | 0 | 0 | 1 | 0 |
Note: No reviewers dispatched for this delta re-review — only a merge commit was added since the prior review.
… was closed or when navigating between agents