Fix quadratic thread-reopen replay: batch client apply + cap server resume gap#4605
Fix quadratic thread-reopen replay: batch client apply + cap server resume gap#4605colonelpanic8 wants to merge 6 commits into
Conversation
Move the pure derive steps (display message attachment mapping, preview handoff/optimistic merge, turn diff summary index, revert turn counts) out of ChatView's inline useMemos into threadTimelineModel.ts so the thread-state -> timeline pipeline can run outside the app shell (perf harnesses, tests). Behavior-identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dev-served page (/perf.html) that mounts the real MessagesTimeline with a synthetic thread and replays N streaming thread.message-sent deltas through applyThreadDetailEvent, publishing one state per macrotask to mirror the one-event-at-a-time resume replay in threads.ts. Reports wall time, React commit count/time, long tasks, and worst stall; a batch param previews what client-side coalescing buys. Measured (Firefox headless, dev build): 2000 deltas at batch=1 take ~212s wall / 4697 commits; batch=32 takes ~8s / 179 commits. Exposes @t3tools/client-runtime/state/thread-reducer for the page. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- threadReplayPerf.test.ts times the applyThreadDetailEvent fold over N streaming deltas and counts per-event state publications through the real resume path (2000 deltas -> 2002 emissions today). - threadsSyncHarness.test-util.ts extracts the threads-sync fake-socket harness for reuse. - acp-mock-agent gains T3_ACP_PROMPT_RESPONSE_CHUNK_COUNT / _CHUNK_TEXT / T3_ACP_PROMPT_CHUNK_DELAY_MS to emit a response as many small agent_message_chunk updates, so a fake provider can generate heavy streaming histories end to end when enableAssistantStreaming is on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Real agentic threads are dominated by thread.activity-appended events (e.g. 4170 of 4830 in a sampled thread) even with assistant streaming off, and the reducer re-sorts the whole activities array per event. Add an activities param that interleaves tool activity events with message deltas to reproduce that mix: 350+4200 events at batch=1 take ~37s / 7114 commits; batch=32 takes ~5.4s / 325 commits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eplay linear Route subscription items through a queue and fold each drained batch (plus a few scheduler turns of an in-flight burst) through the reducer with a single state publication, instead of one SubscriptionRef.set per event. A resume replay of thousands of persisted events previously published one state per event, re-deriving and re-rendering the full timeline each time — reopening a large thread froze the UI for tens of seconds (pingdotgg#4596). Live one-at-a-time updates still publish immediately; no timers are involved, so a quiet stream gains no latency. The threadReplayPerf gauge now enforces the collapse: 2000 replayed deltas must produce fewer than 200 publications (previously 2002). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
subscribeThread with afterSequence replayed every persisted event since the cursor, unbounded — a thread left idle through a few long agentic turns replays thousands of events on reopen (pingdotgg#4596). Mirror the shell stream's SHELL_RESUME_MAX_GAP: when the cursor is more than THREAD_RESUME_MAX_GAP (1000) behind the head (or ahead of it), send a fresh snapshot followed by the buffered live tail instead. In-range replays are now also bounded to the captured head rather than Number.MAX_SAFE_INTEGER. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
vouch:unvouched
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| // publish immediately; a replay backlog folds into large batches with a | ||
| // single publication each, keeping thread reopen cost linear instead of | ||
| // quadratic. No timers, so batching never delays a quiet stream. | ||
| yield* Effect.forkScoped( |
There was a problem hiding this comment.
🟡 Medium state/threads.ts:334
Stale items from a previous session linger in pendingItems after resubscription and race with the new session's HTTP snapshot, so a reconnect can publish a partially replayed thread as live or overwrite a fresh snapshot with stale events and move lastSequence backwards.
The drain fiber and makeSubscribeInput both run non-atomic read-modify-write cycles over lastSequence and state through applyItems. When subscribeDynamic switches sessions, items the old session already enqueued are never cleared, so the drain fiber can apply them after the new session has set awaitingCompletion and loaded its HTTP snapshot — clearing the flag early or overwriting the snapshot with older data. Consider draining or recreating pendingItems on each resubscription so old-session items cannot leak into the new session's state.
🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/state/threads.ts around line 334:
Stale items from a previous session linger in `pendingItems` after resubscription and race with the new session's HTTP snapshot, so a reconnect can publish a partially replayed thread as `live` or overwrite a fresh snapshot with stale events and move `lastSequence` backwards.
The drain fiber and `makeSubscribeInput` both run non-atomic read-modify-write cycles over `lastSequence` and `state` through `applyItems`. When `subscribeDynamic` switches sessions, items the old session already enqueued are never cleared, so the drain fiber can apply them after the new session has set `awaitingCompletion` and loaded its HTTP snapshot — clearing the flag early or overwriting the snapshot with older data. Consider draining or recreating `pendingItems` on each resubscription so old-session items cannot leak into the new session's state.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 543cd7d. Configure here.
| } | ||
| yield* applyItems(batch); | ||
| }), | ||
| ), |
There was a problem hiding this comment.
Concurrent snapshot apply race
High Severity
Direct updates (e.g., HTTP snapshots) and queued stream items (from WS) race to modify shared thread state and lastSequence on different fibers. Without mutual exclusion, operations can interleave, causing lastSequence to go backward or events to apply out of order, leading to incorrect thread state, especially during resubscriptions or connection instability.
Reviewed by Cursor Bugbot for commit 543cd7d. Configure here.
| } | ||
| yield* applyItems(batch); | ||
| }), | ||
| ), |
There was a problem hiding this comment.
Stale queue across resubscribe
Medium Severity
pendingItems is never cleared when subscribeDynamic switch-maps on session replace or foreground wakeup. Leftover items from the prior stream—especially a synchronized marker—can still drain afterward. That can flip status to live and clear awaitingCompletion while the new subscription is still catching up, breaking the completion-marker contract during the exact reopen/resubscribe window this change targets.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 543cd7d. Configure here.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. Unresolved review comments identify high-severity race conditions in the new client-side batching logic (concurrent snapshot applies and stale queue items across resubscriptions). These potential correctness bugs in the core state synchronization changes require human review. You can customize Macroscope's approvability policy. Learn more. |
…h client apply + cap server resume gap)
…h client apply + cap server resume gap)
1 participant
Fixes #4596.
Reopening a thread with a large persisted-event backlog visibly re-replayed the whole backlog: the server streamed every event since the client's cursor individually, and the client published a new state (full timeline re-derive + re-render) per event. Real threads carry thousands of events — a sampled database had a thread with 4,830 (4,170 of them
thread.activity-appended, with assistant streaming off) — making reopen cost quadratic and freezing the UI for tens of seconds to minutes.The fix (two independent halves)
packages/client-runtime/src/state/threads.ts): subscription items now flow through a queue; the apply loop drains one item plus everything that accumulated (giving an in-flight burst a few scheduler turns to land) and folds the whole batch through the reducer with a single state publication. Live one-at-a-time updates publish immediately; no timers, so a quiet stream gains no latency.apps/server/src/ws.ts):subscribeThreadresume now mirrors the shell stream's gap cap — beyondTHREAD_RESUME_MAX_GAP(1000) events (or a cursor ahead of the head), it sends a fresh snapshot plus the live tail instead of an unbounded replay. In-range replays are bounded to the captured head instead ofNumber.MAX_SAFE_INTEGER.Evidence
threadReplayPerfgauge (added here) drives the real resume path: 2000 replayed deltas previously produced 2,002 state publications; the test now enforces < 200./perf.htmlplayground (added here) mounts the realMessagesTimelineand showed one-publication-per-event costs ~37s for a real-thread mix (350 message + 4,200 activity events) and ~212s for 2000 streaming deltas; batched application measured ~5.4s / ~8.1s respectively (7–26×).subscribeThread sends a fresh snapshot instead of replaying a large gap.Also in this PR (repro tooling, first four commits)
threadTimelineModel.ts: pure extraction of ChatView's timeline derivation (behavior-identical) so the pipeline runs outside the app shell./perf.htmldev playground replaying synthetic delta/activity backlogs through the real reducer + timeline with commit/stall metrics.threadReplayPerf.test.tsunit gauge and a chunked-streaming mode foracp-mock-agent.ts(T3_ACP_PROMPT_RESPONSE_CHUNK_COUNT) for end-to-end reproduction.Tests: client-runtime 473/473, server
server.test.ts113/113, web timeline suites 101/101; typechecks clean across touched packages.🤖 Generated with Claude Code
Note
Medium Risk
Changes core WebSocket thread resume and client state publication semantics; behavior is bounded and mirrors the existing shell gap cap, but incorrect batching or gap logic could cause stale or missed thread state on reconnect.
Overview
Fixes thread reopen freezing when thousands of persisted events sit behind the client cursor by attacking server replay volume and client apply/render churn separately.
Server (
subscribeThread) addsTHREAD_RESUME_MAX_GAP(1000), matching the shell stream: if the cursor is too far behind (or ahead of head), the server skips unbounded event replay and sends a fresh thread snapshot plus the live tail. In-range catch-up now reads only through the captured head instead of an unbounded range.Client (
threads.ts) routes subscription items through a queue and folds bursts with drain-based batching: many replayed events collapse into one reducer pass and one state publication per batch, while isolated live updates still publish immediately (no timers).Supporting work: timeline derivation moved to
threadTimelineModel.tsfor reuse;/perf.htmlplayground andthreadReplayPerf.test.tsbenchmark the regression;acp-mock-agentcan emit many streaming chunks for E2E repro; new server test asserts large-gap thread subscribe returns snapshot + synchronized withoutreadEvents.Reviewed by Cursor Bugbot for commit 543cd7d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix quadratic thread-reopen replay by batching client state emissions and capping server replay gap
threads.tsreplaces per-event state publication with a drain-based batching loop: events are queued and coalesced across severalyieldNowcycles, emitting at most one state update per batch instead of one per event.ws.tsintroducesTHREAD_RESUME_MAX_GAP(1,000 events):subscribeThreadnow falls back to sending a fresh snapshot when the client's cursor is more than 1,000 events behind (or ahead), avoiding unbounded catch-up replays.ThreadReplayPlayground.tsx) and a benchmark test suite (threadReplayPerf.test.ts) are added to measure and validate replay cost.ChatView.tsxis extracted into pure helpers inthreadTimelineModel.ts.📊 Macroscope summarized 543cd7d. 8 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.