perf(ios/chat): coalesce assistant stream tokens#3455
Conversation
Streaming applied every assistant chunk immediately: mutate the message then onChange() on EVERY token. Because messages is an observed array on an @observable ChatThread, each mutate reassigns messages[idx] and invalidates the whole message list — so a fast stream (tokens arriving faster than a frame) drove a per-token render and scroll storm. Introduce a small @mainactor StreamCoalescer that buffers chunk text and flushes on a ~50ms cadence (~20 UI updates/sec) instead of per token. All terminal paths (.done/.error, end of the send loop, and the resume loop) flush unconditionally, so the final text is always complete; a single coalescer instance is shared across the send + resume streams so a resumed turn keeps the same buffer. Verified: xcodebuild build (scheme CovenCave, iOS Simulator) succeeds.
There was a problem hiding this comment.
Pull request overview
This PR reduces iOS chat UI churn during assistant streaming by coalescing streamed .assistantChunk tokens into periodic (~50ms) flushes, avoiding per-token messages[idx] reassignment and onChange() calls that can trigger a render/scroll storm on fast streams.
Changes:
- Introduces a
@MainActor StreamCoalescerto buffer assistant chunks and flush at most ~20x/sec. - Routes both initial send-stream and resume-stream through the same coalescer instance per turn.
- Adds explicit flushes on terminal stream events (
.done,.error) and on normal stream completion.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let resumed = await resumeInterruptedStream(runId: runId, cursor: cursor, | ||
| into: messageId, familiarId: familiarId, | ||
| sawDone: &sawDone, | ||
| sawDone: &sawDone, coalescer: coalescer, | ||
| client: client, onChange: onChange) |
| apply(frame.event, into: messageId, familiarId: familiarId, | ||
| sawDone: &sawDone, onChange: onChange) | ||
| sawDone: &sawDone, coalescer: coalescer, onChange: onChange) | ||
| if let id = frame.id { nextCursor = id } | ||
| } | ||
| flush(coalescer, into: messageId, onChange: onChange) |
There was a problem hiding this comment.
Addressed in replacement PR #3460 because this branch has maintainer edits disabled. Commit 367e13ff flushes both NoResumableRun and transport-error resume paths; final head 469081d8 passed all required CI checks.
|
I found and fixed both buffered-text loss paths from the review: the send-stream recovery now drains before resume/resync, and failed/no-resumable resume attempts drain before retry/fallback. The original head is an OpenCoven-owned branch with maintainer edits disabled; my token cannot push to it (HTTP 403). I opened #3459 targeting |
|
Because CI only runs for PRs targeting |
|
Final update: replacement #3460 is now mergeable with every required CI check green and no unresolved review threads. It carries the original performance commit plus the recovery fix; #3455 itself remains blocked only because its OpenCoven-owned head cannot be updated by maintainers (HTTP 403, maintainer edits disabled). |
Summary
Second PR in the iOS performance pass. Removes a per-token render/scroll storm during assistant streaming.
Root cause
ChatThread.apply()handled each.assistantChunkby mutating the message and firingonChange()on every token.messagesis an observed array on an@ObservableChatThread, so eachmutatereassignsmessages[idx]and invalidates the entire message list. A fast stream emits tokens faster than one per frame, so this drove a render + auto-scroll storm on every token — CPU churn and jank while a reply streams.Fix
Added a small
@MainActor StreamCoalescerthat buffers chunk text and flushes on a ~50ms cadence (~20 UI updates/sec) instead of per token. Correctness:.done,.error), the end of the send loop, and the resume loop all flush unconditionally (idempotent — no-op when empty), so the final text is always complete.Verification
xcodebuild build(schemeCovenCave, iOS Simulator) → BUILD SUCCEEDED.apply()call sites — every one passes the coalescer; every stream-exit path flushes.Blast radius
Small/medium — 1 file, streaming display path only. No protocol/network change; the stream consumption, resume, and offline-queue logic are untouched.
Notes
Stacks on top of the chat-render equatable fix (#3454). CI does not build the Swift app; local
xcodebuildis the iOS gate.