Repository navigation
fix(chat): don't wipe fresh streaming/tool-timeline on a completed hydrate snapshot - #4903
Conversation
…drate snapshot hydrateRuntimeFromSnapshot unconditionally cleared streamingAssistantByThread and rewrote toolTimelineByThread for any terminal snapshot (interrupted OR completed). A completed snapshot can lag behind live state this session already has for the thread — e.g. the socket-disconnect reconciliation path deliberately keeps streamingAssistantByThread set across endInferenceTurn so a partial reply stays visible while the socket reconnects, and a late fetchAndHydrateTurnState rehydration can then land and wipe it. Only let the snapshot clobber those two lanes when it's unambiguously the authority: an interrupted snapshot (genuine crash, nothing fresher can exist) or there's no live streaming state to lose (cold boot / new window). Mirrors the interrupted-only guard already applied to the workflow-proposal clear in tinyhumansai#4886.
📝 WalkthroughWalkthrough
ChangesRuntime snapshot hydration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/store/chatRuntimeSlice.ts (1)
1993-2014: 📐 Maintainability & Code Quality | 🔵 TrivialAdd a branch-level
chatRuntime.turnStatelog in this completed/interrupted guard.
fetchAndHydrateTurnStatealready logs the hydrate attempt, but this reconciliation still has no signal for whether the live stream was preserved or cleared. Log only lifecycle andhasFresherLiveStreamso reconnect races stay grep-friendly.🤖 Prompt for 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. In `@app/src/store/chatRuntimeSlice.ts` around lines 1993 - 2014, Add a branch-level chatRuntime.turnState log within the snapshot lifecycle guard around hasFresherLiveStream, recording only snapshot.lifecycle and hasFresherLiveStream. Emit it whether the live stream is preserved or cleared, without logging snapshot contents or other state.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@app/src/store/chatRuntimeSlice.ts`:
- Around line 1993-2014: Add a branch-level chatRuntime.turnState log within the
snapshot lifecycle guard around hasFresherLiveStream, recording only
snapshot.lifecycle and hasFresherLiveStream. Emit it whether the live stream is
preserved or cleared, without logging snapshot contents or other state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 932ec7bc-4ebd-4e00-a0b6-747a6eb0a0b7
📒 Files selected for processing (2)
app/src/store/chatRuntimeSlice.test.tsapp/src/store/chatRuntimeSlice.ts
Bug
hydrateRuntimeFromSnapshotinapp/src/store/chatRuntimeSlice.tswas fixed in #4886 to only clearpendingWorkflowProposalsByThreadonsnapshot.lifecycle === 'interrupted'(not'completed') — acompletedsnapshot can be this session's own just-settled turn racing against the live path.The same reducer still unconditionally cleared
streamingAssistantByThread[threadId]and rewrotetoolTimelineByThread[threadId]for any terminal snapshot, includingcompleted. Concretely,ChatRuntimeProvider's socket-disconnect reconciliation deliberately keepsstreamingAssistantByThreadset acrossendInferenceTurnso a partial reply stays visible while the socket reconnects (see the comment atChatRuntimeProvider.tsx:1403). If acompletedfetchAndHydrateTurnStaterehydration then lands for that thread, it wiped the just-preserved streaming bubble and replaced the live (richer) tool timeline with the (behind) persisted snapshot — a transient flicker / loss of just-streamed narration, even though the proposal card itself now survives.Fix
Split the terminal-snapshot block in
hydrateRuntimeFromSnapshot:inferenceStatusByThreadclear andprocessingByThreadassignment still happen unconditionally for bothinterrupted/completed(unaffected by this bug).streamingAssistantByThreaddelete +toolTimelineByThreadoverwrite now only run whensnapshot.lifecycle === 'interrupted'(genuine crash — nothing fresher can exist) or there is no live streaming state for the thread (!state.streamingAssistantByThread[threadId]), which covers cold boot / new window so history still hydrates.This mirrors the
interrupted-only invariant already used for the workflow-proposal clear from #4886, keeping the three guards (proposal / streaming lane / tool timeline) consistent.Tests
Added to
app/src/store/chatRuntimeSlice.test.ts(hydrateRuntimeFromSnapshot — streaming/timeline race guard):does not wipe a fresher streaming/tool-timeline lane on a completed snapshotclears the streaming/tool-timeline lanes on an interrupted snapshot (crash cleanup)still hydrates the timeline from a completed snapshot on cold boot (no prior live state)Test plan
corepack pnpm typecheckcleancorepack pnpm lintclean (0 errors; pre-existing unrelated warnings in other files)corepack pnpm test --config app/test/vitest.config.ts src/store/chatRuntimeSlice.test.ts— 30/30 passedcorepack pnpm format— no diff beyond the intended changeSummary by CodeRabbit
Bug Fixes
Tests