Repository navigation
perf(flow-chat): cut per-flush cost on the streaming render path - #3545
Open
Raing5Days wants to merge 1 commit into
Open
Raing5Days wants to merge 1 commit into
Raing5Days wants to merge 1 commit into
Conversation
Four fixes found while profiling the Web UI streaming hot path. Each removes work that ran unconditionally on every batched flush (32ms while text streams). - TextChunkModule: newline coalescing re-scanned the whole accumulated response on every chunk, which is O(N^2) over a long answer. Both sides of an append are already clean, so only the two-character join boundary plus the incoming chunk need cleaning. Reused item content is cleaned once when recovered so the buffer invariant still holds. - EventBatcher: build the trace payload only when trace output can reach a sink. The logger drops the entry after its level check, so every flush previously allocated a full event map for a discarded log line. The slow-flush warning keeps its counts. - modernFlowChatStore: drop the immer middleware. The setters only assign whole fields, so produce() and its deep freeze walked the entire session and virtual item tree on every flush for no benefit. Also reset the needsUserAttention projection cache guard in clear(). - useConversationTimeline: remember prewarmed text items in a WeakSet so a streaming flush no longer re-walks the whole timeline. Verification: 133 vitest cases across the touched modules pass, tsc --noEmit is clean, eslint reports 0 errors. Web UI only; no remote-workspace, remote-control, peer-device, or dispatch protocol behavior changes.
Raing5Days
force-pushed
the
perf/flow-chat-streaming-hot-path
branch
from
October 24, 2026 16:34
b53bbd7 to
b4fc0d1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Profiling the Web UI streaming render path turned up four places where work ran
unconditionally on every batched flush (every 32ms while text streams). All four are
fixed here as one focused change set on the same hot path.
TextChunkModule— newline coalescing re-scanned the whole accumulated response onevery chunk:
(currentContent + text).replace(/\n{3,}/g, '\n\n'). That is O(N) perchunk and O(N^2) over a long answer, on the main thread. Both sides of an append are
already clean, so only the two-character join boundary plus the incoming chunk need
cleaning. Recovered item content is cleaned once when reused, so the buffer invariant
still holds.
EventBatcher—getBatchedEventsLogPayload()ran beforelog.trace(), and thelogger drops the entry after its level check. With the default level (WARN in
production, DEBUG in dev) every flush allocated a full per-event map for a log line
that was then thrown away. The payload is now built only when trace output can reach a
sink; the slow-flush warning keeps its counts.
modernFlowChatStore— dropped theimmermiddleware. The three setters only assignwhole fields, never a draft, so
produce()plus immer's deep freeze walked the entiresession and virtual-item tree on every flush. Plain
set()keeps the same referencesemantics.
clear()now also resets theneedsUserAttentionprojection cache guardalongside the other cached fields.
useConversationTimeline— the prewarm effect depends on[sources], which changes onevery flush, so it re-walked the whole timeline including items already queued or
parsed. A
WeakSetof seen items skips them.Fixes #
Type and Areas
Type: performance / refactor
Areas: web UI (
src/web-ui/src/flow_chat)Motivation / Impact
Streaming a long answer got progressively more expensive per flush, and part of that
cost bought nothing: a redundant full-text rescan, a log payload that was discarded, a
state-tree deep freeze nothing depended on, and a repeated timeline sweep.
User-facing effect: long streaming responses keep a steadier frame budget and the cost
per flush stops growing with the length of the conversation. No behavior, API, or
persisted-shape change.
Verification
Run on this branch, based on
origin/mainate17e1803e:pnpm --dir src/web-ui run test:run src/flow_chat/services/flow-chat-manager/TextChunkModule.test.ts src/flow_chat/services/EventBatcher.test.ts src/flow_chat/store/modernFlowChatStore.test.ts→ 109 passedpnpm --dir src/web-ui run test:run src/flow_chat/timeline/→ 24 passed across 8 filespnpm --dir src/web-ui exec tsc --noEmit -p tsconfig.json→ cleanpnpm --dir src/web-ui run lint→ 0 errors (14 pre-existing warnings, none in the changed files)TextChunkModule.test.tsdid not assert newline coalescing directly before this change;behavior equivalence there is covered by the existing streaming tests. A dedicated
"chunk boundary crosses 3+ newlines" case would be a good follow-up.
Reviewer Notes
Two points worth a careful look:
modernFlowChatStoreno longer freezes its state. Nothing reads frozen state or handsan updater to
setStateon this store, anduseFlowChatActionsselects actions throughuseShallow, so it is unaffected.canvasStorekeeps itsimmermiddleware.run". Both buffer writers go through the new helper, and recovered item content is
cleaned once on reuse.
Remote scenarios: this touches the local Web UI render path only. No changes to remote
workspace transport, remote control, Peer Device Mode command handling, or the detached
dispatch protocol, and none of those paths consume these modules.
Checklist