Skip to content

Batch reconnect replay into a single observable mutation#65

Merged
mcintyre94 merged 4 commits intomainfrom
chat-reconnect-delay-troubleshoot-cd59686f
Mar 12, 2026
Merged

Batch reconnect replay into a single observable mutation#65
mcintyre94 merged 4 commits intomainfrom
chat-reconnect-delay-troubleshoot-cd59686f

Conversation

@mcintyre94
Copy link
Copy Markdown
Owner

Summary

  • During reconnect, handleEvent was appending to @Observable ChatMessage.content for every replayed event, causing a full SwiftUI re-render (and a haptic) per event. For a chat catching up on many messages this meant N expensive render passes blocked on the main actor before the user saw anything.
  • Introduced isReplaying, replayContentBuffer, and replayToolUseBuffer in ChatViewModel. While runReconnectLoop is active, .assistant and .user events accumulate into these local arrays instead of mutating the observable model directly.
  • After each processServiceStream iteration, applyReplayBuffer() flushes the buffer with a single append(contentsOf:) — one @Observable mutation → one render pass instead of N.
  • Tool-use/result linking is preserved inside the buffer before commit, and replayToolUseBuffer is merged into the main toolUseIndex on flush.
  • Haptics (.medium for first text, .light per tool result) and the auto-checkpoint are suppressed during replay — both are only appropriate for live streaming.

Test plan

  • New tests: runReconnectLoop_buffersContentAndAppliesInOneBatch and runReconnectLoop_buffersToolUseAndResultWithLinking verify content lands correctly after buffered replay
  • Existing runReconnectLoop tests still pass
  • Manually open a chat with many completed turns and verify reconnect is noticeably faster with no jank

🤖 Generated with Claude Code

mcintyre94 and others added 2 commits March 12, 2026 07:32
During reconnect, handleEvent was modifying @observable ChatMessage.content
for every replayed event, causing a SwiftUI re-render (and a haptic) per
event. For a long chat catching up on many messages this meant hundreds of
expensive render passes on the main actor before the user saw anything.

Fix: buffer all content changes during runReconnectLoop into local value-type
arrays (replayContentBuffer, replayToolUseBuffer), then flush in one shot via
applyReplayBuffer() after each stream iteration. Tool-use/result linking is
preserved inside the buffer before it is committed. Haptics and the
auto-checkpoint are also suppressed during replay.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…g after reconnect

- Add `lastSessionComplete` to SpriteChat: when true, skip the network
  call entirely on reconnect (content is already in persistence)
- Pass `serviceIsRunning` through the reconnect path so the replay
  buffer is flushed and `isReplaying` is cleared on the first new event
  when the service is still running — fixes live events buffering until
  the connection drops instead of streaming incrementally
- Make `ChatStatus` Equatable to support direct == comparisons in tests
- Add 4 tests covering lastSessionComplete and live-service reconnect

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Mar 12, 2026

Code review

Text fragmentation across retry iterationsWisp/ViewModels/ChatViewModel.swift

applyReplayBuffer() uses append(contentsOf:) to flush the buffer into message.content, but this does not apply the same consecutive-text-block merging that the live-streaming path performs.

The reconnect loop is designed to run multiple iterations — explicitly, when the service is still running (isRunning == true) or when the "one extra retry after service stopped" path fires. After iteration 1 flushes its buffer and clears replayContentBuffer = [], iteration 2 accumulates a fresh buffer that starts with a new .text(...) entry. When iteration 2 calls applyReplayBuffer(), that text is appended directly to message.content with no check of message.content.last, leaving two separate .text entries where the live path would have produced one.

This matters because AssistantMessageView renders each .text element in message.content as a separate AssistantTextBubble — a distinct rounded-rectangle chat bubble. Text that belongs to a single assistant response will render as two visually separated bubbles instead of one, specifically in the "one extra retry after service stop" race-condition path that this PR explicitly handles.

Suggested fix — replace the single append(contentsOf:) call with element-by-element append that mirrors the live-streaming merge logic:

// In applyReplayBuffer(), replace:
message.content.append(contentsOf: replayContentBuffer)

// With:
for item in replayContentBuffer {
    if case .text(let newText) = item,
       case .text(let existing) = message.content.last {
        message.content[message.content.count - 1] = .text(existing + newText)
    } else {
        message.content.append(item)
    }
}

applyReplayBuffer was using append(contentsOf:) which skips the
consecutive-text-block merging logic. On the retry-after-service-stop
path the reconnect loop runs two iterations, each flushing its own
buffer. The leading .text from the second buffer would land as a
separate entry instead of being merged with the trailing .text from the
first, producing two chat bubbles for a single assistant response.

Replace append(contentsOf:) with element-by-element appending that
mirrors the live-streaming merge: if the incoming item and the last
item in message.content are both .text, concatenate them in place.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Owner Author

@mcintyre94 mcintyre94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — replaced append(contentsOf:) with element-by-element appending that mirrors the live-streaming merge logic. Also added a test (runReconnectLoop_mergesTextAcrossRetryIterations) that exercises exactly the two-iteration scenario described in the review.

…m1 events

The mergesTextAcrossRetryIterations test was missing uuid fields on its
events. Without UUIDs, textLine1 in stream2 isn't recognised as already-
processed and gets replayed again, producing "Hello Hello world". Real
log events always carry UUIDs — add them to the test fixtures so the
deduplication path that the fix in applyReplayBuffer depends on actually
fires.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mcintyre94 mcintyre94 merged commit 5639de6 into main Mar 12, 2026
2 checks passed
@mcintyre94 mcintyre94 deleted the chat-reconnect-delay-troubleshoot-cd59686f branch March 12, 2026 22:35
@mcintyre94 mcintyre94 restored the chat-reconnect-delay-troubleshoot-cd59686f branch March 15, 2026 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant