fix(ai): stop global-assistant streams from flashing/clobbering on send, reload, or switch#2061
Conversation
… live streams Sending a message from the global assistant (dashboard or right-sidebar surface) could visibly flash and make the in-progress reply disappear; reloading or switching conversations mid-stream never reattached to the live content. Root cause: four "load-on-select" effects across GlobalAssistantView.tsx and SidebarChatTab.tsx (global-mode and agent-mode variants in each) reapplied a conversation's persisted messages into the surface's own local useChat state whenever the reference changed (mount, reload, conversation/agent switch) with no guard against the surface actively streaming — unlike their sibling refresh-effects, which already had one. The unconditional overwrite clobbered the in-progress assistant bubble with a stale DB snapshot that predated the reply. Add the same streaming guard already used by the sibling effects to all four call sites. No new rendering path was needed: the existing remoteStreams/inflightRemoteStreams pipeline already renders a bootstrapped stream's live content independent of local message state, once the clobber is prevented. Also flagged, not addressed here (separate, larger, already-documented effort per PR #2011/#2018/#2022): cross-instance live-token rejoin doesn't work when a reload lands on a different web instance than the one running the generation, since the multicast stream registry is single-process.
📝 WalkthroughWalkthroughGlobal and sidebar assistant views now defer refresh and initial-message snapshot updates while streaming, and advance tracking refs only when updates execute. New regression tests cover streaming guards, conversation switches, and retry behavior after streaming ends. ChangesAssistant streaming guards
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 397f9a5475
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (selectedAgent && agentConversationId && !effectiveIsStreaming) { | ||
| setAgentMessages(agentInitialMessages); |
There was a problem hiding this comment.
When this load-signal effect fires during a bootstrapped/local agent stream, the new guard skips setAgentMessages, but the effect has already copied the current agentConversationLoadSignal into prevAgentLoadSignalRef. When the stream later finishes, effectiveIsStreaming flips and re-runs the effect with the same signal, so the early return above prevents applying the loaded snapshot; after a reload or conversation switch mid-agent-stream the dashboard can keep an empty/stale useChat history because the completion path only synthesizes the assistant bubble. Move the ref update until a load is actually applied, or remember a pending load.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in fe87376 — good catch. The guard blocked the apply while streaming, but prevAgentLoadSignalRef.current was still advanced unconditionally, so once streaming ended the effect saw "no change" on the same signal and never retried. Moved the ref assignment inside the guarded branch (only advances when setAgentMessages actually runs), and added a stateful test (ref-advances-only-on-apply in GlobalAssistantView.test.tsx) that simulates the exact multi-render sequence: guard blocks -> guard passes with the same reference -> must still apply.
| if (!selectedAgent && globalIsInitialized && globalConversationId && !displayIsStreaming) { | ||
| loadGlobalMessages(globalConversationId); |
There was a problem hiding this comment.
If globalInitialMessages changes while displayIsStreaming is true, this guard skips loadGlobalMessages, but the ref was just advanced to that same message array. The re-run when streaming stops sees the same reference and returns before fetching, so selecting or reloading a global conversation during a live stream can leave the sidebar on the old/blank message list instead of loading the selected conversation. Only mark the reference as seen after the load runs, or queue the skipped load.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in fe87376 — same defect, same fix. prevSidebarGlobalMessagesRef was advanced before the guard check, so a load skipped mid-stream was marked seen and never retried once streaming ended. Moved the assignment inside the guarded branch, and added a stateful regression test (ref-advances-only-on-apply in SidebarChatTab.test.tsx) that simulates the multi-render sequence directly: guard blocks -> guard passes with the same reference -> must still apply. While fixing this I found the identical shape in the pre-existing refreshSignal effect a few lines above (~612) and applied the same fix there for consistency, since it's the same file and subject.
| if (selectedAgent && !displayIsStreaming) { | ||
| setMessages(agentInitialMessages); |
There was a problem hiding this comment.
When agentInitialMessages changes during an active sidebar agent stream, this branch skips setMessages, but the previous-reference marker was already updated. After the stream completes, the displayIsStreaming dependency causes one more run with the same array reference, which returns before applying it; switching/reloading an agent conversation mid-stream can therefore leave the sidebar's stable useChat instance with stale or empty history. Defer updating the ref until the messages are actually applied, or keep a pending load flag.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in fe87376 — same defect, same fix. prevSidebarAgentMessagesRef was advanced before checking !displayIsStreaming, so a load skipped mid-stream was marked seen and never retried once streaming ended. Moved the ref assignment inside the guarded branch, and added a stateful regression test (ref-advances-only-on-apply in SidebarChatTab.test.tsx) that simulates the exact multi-render sequence: guard blocks -> guard passes with the same reference -> must still apply.
Codex review on this PR (3 threads, P1) found that the streaming guards added to the load-on-select effects advanced their "seen" ref BEFORE checking the guard. When the guard blocked (streaming), the ref was still marked seen — so once streaming ended, the effect would see "no change" on the same reference and permanently skip the deferred load, leaving the surface on stale or empty history instead of reattaching. Fix: move the ref assignment inside the guarded branch in all three flagged effects (GlobalAssistantView's agent load-signal effect, SidebarChatTab's global and agent load-on-select effects). While touching this, applied the identical fix to the pre-existing refreshSignal effect in SidebarChatTab (same defect shape, not introduced by this PR but same file/subject), and discovered + fixed a related gap: GlobalAssistantView's refreshSignal effect had no streaming guard at all, meaning it could clobber an in-progress reply the same way the original bug did. Added a stateful "ref-advances-only-on-apply" simulator to both test files to pin the actual multi-render sequence the bug depends on (guard blocks, then guard passes with the SAME reference must still apply) — the previous single-call boolean tests couldn't express this.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/web/src/components/layout/middle-content/page-views/dashboard/GlobalAssistantView.tsx`:
- Around line 999-1012: Update the global-mode message synchronization effect
around setGlobalLocalMessages to use a previous-value ref guard like
prevSidebarGlobalMessagesRef. Apply globalInitialMessages only when the snapshot
changes, while preserving the existing initialization, conversation, agent, and
streaming guards.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e20d8fb8-450f-4363-ab6c-cf9687300be5
📒 Files selected for processing (4)
apps/web/src/components/layout/middle-content/page-views/dashboard/GlobalAssistantView.tsxapps/web/src/components/layout/middle-content/page-views/dashboard/__tests__/GlobalAssistantView.test.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/SidebarChatTab.tsxapps/web/src/components/layout/right-sidebar/ai-assistant/__tests__/SidebarChatTab.test.tsx
…sage reference
CodeRabbit review found a severe regression in the previous commit: this
effect had no prevRef/dedup check, only a streaming guard. Since
effectiveIsStreaming is itself a dependency, the effect re-fires on
every streaming transition — including the streaming-to-not-streaming
edge at the end of every ordinary send. GlobalChatContext's
onStreamComplete deliberately does not refresh globalInitialMessages
for an own fresh completion ("surface's useChat already has the
message"), so without a dedup check this effect would reapply the
stale PRE-SEND snapshot the instant a stream finished, wiping every
just-completed reply in GlobalAssistantView's global mode.
Added the same prevGlobalInitialMessagesRef dedup + ref-advances-only-
on-apply discipline already used by every other load-on-select effect
in this PR, and a regression test pinning the exact failure sequence:
same snapshot reference, effectiveIsStreaming flips true -> false,
must not re-apply.
… the surface Found via proactive review (code-review skill, not a reviewer comment): the streaming guards added earlier in this PR (`!displayIsStreaming`/ `!effectiveIsStreaming`) blocked a load-on-select/refresh effect whenever this surface's useChat was streaming AT ALL — but useChat's id is a stable constant per surface, not per conversation, so switching to a different conversation for the same agent (or a different global conversation) does NOT abort the old conversation's in-flight send. The old guard would therefore stay blocked by an unrelated stream still running in a conversation the user already left, stranding the newly selected conversation on stale/previous messages until that unrelated stream happened to finish. Fixed by comparing the surface's own held stream-start conversation id (streamConvIdRef/globalStreamConvIdRef in GlobalAssistantView, heldStreamConvIdRef in SidebarChatTab — both already existed for the Stop/abort path) against the conversation actually being loaded, so the guard only blocks when the live stream truly belongs to that conversation. Six effects updated: SidebarChatTab's refreshSignal, global load-on-select, and agent load-on-select effects; GlobalAssistantView's refreshSignal, agent load-signal, and global load-on-select effects. Added regression tests pinning the conversation-scoping property directly (own stream in conversation A + switch to idle conversation B must not block B's load) in both test files. Also removed a test that inlined a standalone reimplementation of a bug no longer present anywhere in production code, so it could never actually regress (documentation dressed as an assertion, flagged by the same review pass).
1 participant
Summary
After #2018 (server-owned AI streams), per-page chat (
AiChatView.tsx) reattaches cleanly to live streams on reload/reconnect. The global assistant — which exists as two independently-mounted surfaces,GlobalAssistantView.tsx(dashboard) andSidebarChatTab.tsx(right sidebar) — didn't get equivalent treatment:Root cause
Both files have "load-on-select" effects (one for global mode, one for agent mode, in each file — four total) that reapply a conversation's persisted messages into the surface's own local
useChatstate whenever the reference changes (mount, reload,loadConversation, conversation/agent switch). All four were missing the streaming guard their sibling refresh-effects already have, so a mount/reload/switch landing while the surface was actively streaming unconditionally overwrote the in-progress assistant bubble with a stale DB snapshot that predated the reply.Two theories I ruled out while investigating (kept here so a reviewer doesn't rediscover them):
useChatinstance — I read the installed@ai-sdk/reactsource directly;useChat'sidoption only controls when a given call site recreates its own localChatref, it is not a cross-component registry.GlobalAssistantViewandSidebarChatTabhave always had fully independent Chat instances.GlobalChatContext'sonUserMessage—useChannelStreamSocket.ts'shandleUserMessagealready filtersisOwnStreambefore invoking that callback.Fix
Added a streaming guard to all four load-on-select effects, matching the pattern their sibling refresh-effects already used. No new rendering path was needed — the existing
remoteStreams→inflightRemoteStreams/ChatMessagesAreapipeline already renders any not-yet-local stream's accumulated content regardless of ownership, so once the clobber is prevented, that pipeline already covers reattachment.Update 1: ref-ordering defect (Codex review)
Codex flagged (3 P1 threads) that the guards advanced their "seen" ref before checking the streaming condition, so a load skipped mid-stream was marked seen and permanently dropped once streaming ended. Fixed by moving the ref assignment inside the guarded branch in all three flagged effects, plus the identical pre-existing shape in
SidebarChatTab.tsx'srefreshSignaleffect, plus a related gap this surfaced:GlobalAssistantView.tsx'srefreshSignaleffect had no streaming guard at all.Update 2: missing dedup regression (CodeRabbit review)
CodeRabbit caught that
GlobalAssistantView.tsx's global-mode load-on-select effect had a streaming guard but no prevRef/dedup, so it re-fired on every streaming transition — including the streaming → not-streaming edge at the end of an ordinary, successful send — reapplying the stale pre-send snapshot and wiping the just-completed reply. Fixed with the sameprevGlobalInitialMessagesRefdedup used elsewhere.Update 3: conversation-scoping regression (found via proactive review, not a reviewer comment)
Ran this repo's
code-reviewskill (8 parallel finder angles + verification) against the diff after the first two rounds landed. Two independent finders converged on a real regression I'd introduced: all six streaming guards checkeddisplayIsStreaming/effectiveIsStreaming— a flag scoped to the surface, not the conversation. BothuseChatinstances in these files use a stable id per surface (not per conversation), so switching to a different conversation for the same agent (or a different global conversation) does not abort the old conversation's in-flight send — the flag stays true for a conversation the user has already left. The guard would then block loading the newly selected conversation's messages until that unrelated stream happened to finish, stranding the UI on stale/previous-conversation content. The codebase's own existing comments confirm this is a real, previously-encountered scenario ("switching conversation mid-stream does NOT abort the POST").Fixed by reusing the existing
streamConvIdRef/globalStreamConvIdRef(GlobalAssistantView) andheldStreamConvIdRef(SidebarChatTab) — refs that already exist for the Stop/abort path, latching the conversation a stream actually started in — to scope each guard to "is my own stream, for this specific conversation" rather than "is anything streaming." All six effects updated.Added regression tests pinning the conversation-scoping property directly (own stream in conversation A + switch to idle conversation B must not block B's load) in both test files. Also removed a test that inlined a standalone reimplementation of a bug no longer present anywhere in production code, so it could never actually regress (documentation dressed as an assertion, flagged by the same review pass).
Noted, not fixed (out of scope for this PR):
SidebarChatTab.tsx'sloadGlobalMessages/shouldApplyLoadedMessages, neither introduced by this PR (both predate it — I only changed which boolean gates the dispatch of this pre-existing function, not its internal request handling): (1) the streaming guard is checked at dispatch time but the fetch's.then()callback doesn't re-check it before writing, so a new send starting in the ~0.5-3s window before the pending-stream store registers it could still have its optimistic content overwritten; (2)shouldApplyLoadedMessagescompares only conversationId, not a per-request sequence number, so if two of this file's independently-guarded effects (refreshSignal and the globalInitialMessages load-on-select) both happen to fire in the same render for the same conversation, their two concurrent fetches aren't ordered — whichever HTTP response resolves last wins, not whichever was requested last. Both are narrow (require rare event coincidences) and would need new request-generation-counter infrastructure in shared code to close properly; flagging for awareness/a future follow-up rather than expanding this PR's scope further.Known, separate, out-of-scope limitation
Prod runs multiple web instances.
streamMulticastRegistryis single-process (documented in #2011/#2018, reconfirmed in #2022's "out of scope: multicast cross-instance join"). If a reload/switch reconnects to a different instance than the one running the generation, there's no live-token source available — with or without this fix. That's a known, deliberately-deferred, larger effort, not something this PR attempts.Test plan
bun run typecheckclean (apps/web)eslintclean on touched filesright-sidebar/ai-assistant,dashboard,contexts/GlobalChatContext,ai-page/AiChatViewSidebarChatTab.test.tsx,GlobalAssistantView.test.tsxcode-reviewskill (8 finder angles) against the diff and fixed the one confirmed correctness regression it surfaced (conversation-scoping)GlobalAssistantView(dashboard) as the active surface, and confirm the reply stays visible after it finishes (the CodeRabbit-flagged regression)🤖 Generated with Claude Code
https://claude.ai/code/session_01U8GPExjBCyxrXDtaJVYoWk