streaming: render chat from MessageStore#32454
Conversation
![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: de45b04186
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for snapshot in messageStore.orderedMessages { | ||
| guard !consumedDaemonIds.contains(snapshot.id) else { continue } | ||
| result.append(MessageStore.chatMessage(from: snapshot)) |
There was a problem hiding this comment.
Because each ChatViewModel starts a MessageStreamReducer that calls eventStreamClient.subscribe() without the existing conversation filter, every cached chat view model receives streaming events for all conversations. Now that rendering appends every unconsumed snapshot here, opening or returning to another conversation can show assistant messages streamed in a different conversation. This needs to scope the reducer/store by conversationId (or subscribe with broadcastFilter) before using these snapshots for rendering.
Useful? React with 👍 / 👎.
| } else { | ||
| // Streaming-bubble lazy-created by the legacy code path. | ||
| // Dropped: the corresponding MessageStore snapshot (if | ||
| // any) is the authoritative source and is appended | ||
| // below. This is what closes the duplication failure | ||
| // mode — the legacy "anonymous delta -> new bubble" | ||
| // branch no longer reaches the renderer. | ||
| continue |
There was a problem hiding this comment.
When any MessageStore snapshot exists, this branch drops every legacy assistant row that lacks daemonMessageId. That is broader than just the old lazy streaming bubble: inline surface rows, attachment warning rows, and inline conversation-error rows are also assistant messages created without a daemon ID, and the reducer ignores those event types, so they disappear from the rendered transcript as soon as a streamed snapshot is present.
Useful? React with 👍 / 👎.
1 participant
Summary
ChatViewModel.renderedMessagesbridge that materializesMessageStoresnapshots intoChatMessagevalues and drops the legacy lazy-bubble-creation rows that the streaming helpers would otherwise duplicate.lastAppliedSeqper conversation in a newLastAppliedSeqStore(UserDefaults), updated by every event the reducer applies, and threads it throughEventStreamClient→GatewayHTTPClient.stream(lastEventId:)as theLast-Event-Idheader on SSE (re)connect (PR 2 wired the server side; the daemon honors it on per-conversation subscriptions).messageId), and the reducer's idempotent apply is the sole writer for what the renderer sees.Scope note
This PR removes the legacy
currentAssistantMessageIdfield andclearCurrentTurnTracking()helper from the renderer source-of-truth — they no longer influence which rows the view shows. Physical deletion of those symbols is deferred to PR 6 (the "retire legacy vocabulary" cleanup), which is where the legacymessagesarray goes away entirely. Removing them in PR 4 would touch ~136 call sites acrossMessageSendCoordinator,ChatActionHandler, andChatViewModel+SurfaceHandling— work that belongs in the same PR that retires the array those helpers mutate.Part of plan: streaming-message-architecture.md (PR 4 of 6)