One event stops costing eight round-trips: scoped, coalesced room fetches - #45
Merged
Conversation
… world One SSE event used to ring a doorbell that reloaded everything: any spec-scoped non-message event triggered specReviews + listRuns + a reviewDetail per review, and a new message re-fetched a full page. Since presence landed, agent_tool_* and agent_presence stream during live runs, so an open room fired that 2+N storm on every tool call the agent made. Route by event type instead: - Telemetry and presence events (agent_tool_*, agent_log_chunk, agent_presence, heartbeat) cost zero fetches, zero merges, zero renders in the room; the presence store keeps the chips live. - spec_message_posted fetches with the server's exclusive after cursor (now exposed through Gateway.listSpecMessages) and falls back to one full page if the announced message_id doesn't land in the merge. - review_* refreshes the one review detail when the event carries the review id and it's loaded; the list otherwise (sail's stage events carry no review_id today) and on list-shape boundaries. - Run-lifecycle events refresh listRuns only; spec_status_changed and snapshot rows render straight from the payload with no fetch. - Unrecognized spec-scoped events keep the conservative refresh so a new server event type is never silently dropped. Bursts coalesce per (kind, spec): microtask batching plus an in-flight dirty flag, so five mixed events cost one fetch per kind. The reconnect gap-fill's full reconcile is unchanged. Behavior tests count gateway calls per the acceptance list.
The live catch-up's after-cursor skips a message that synced in out of order (a cross-node id older than the newest confirmed). Its fallback then refetched the latest page — which, in a room with more than a page of history, need not reach back far enough to include that message, so it stayed lost until a reconnect. Recover by anchoring on the missing id's position instead: fetch the page ending at its loaded successor (before=<successor>), which contains it. Falls back to the latest page only when no successor exists (impossible while a newer confirmed message is loaded). Convergence now genuinely comes from the fetch, as the spec's edge promised. Proven red-first with a >PAGE_SIZE room and a late out-of-order announcement.
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.
Why
The room's SSE handling was resilient but coarse: any spec-scoped event that wasn't a message triggered
specReviews+listRuns+ onereviewDetailper review, and a new message re-fetched a full page. Since presence landed,agent_tool_*andagent_presenceevents stream during live runs — so an open room fired the 2+N fetch storm on every tool call the agent made, each fetch its own SSH round-trip.What
Route by event type instead of reloading on any match (spec
mast-refetch-diet):agent_tool_*,agent_log_chunk,agent_presence,heartbeat) → zero fetches, zero timeline merges, zero renders in the room. Presence chips keep updating from the app-wide presence store, which was already wired.spec_message_posted→ one messages fetch with the server's exclusiveaftercursor, now exposed throughGateway.listSpecMessages({before, after, limit}options; the endpoint already supported it). If the announcedmessage_iddoesn't land in the merge — a cross-node message can sync in with an id older than the cursor — one full latest-page fetch recovers it. Echo suppression for your own posts is unchanged.review_*→ that review's detail only, when the event carries the review id and the detail is loaded; the spec's review list otherwise. Verified in sail source: stage events carry noreview_idtoday (onlyreview_approved/finding_dismisseddo), so the fallback is the common live path until sail adds it.review_iteration_started/review_completedare list-shape boundaries and refresh the list.spec_dispatched,agent_session_*,agent_failed,agent_cancelled, …) →listRunsfor that spec only.spec_status_changed/ snapshot / guardrail rows → render straight from the event payload, no fetch. The board's own reload path is untouched.Bursts coalesce per (kind, spec): microtask batching plus an in-flight dirty flag (the
useBoardpattern extended to cover the in-flight window), so five mixed events cost one fetch per kind and an event landing mid-flight re-runs exactly once.The reconnect gap-fill (#44) keeps its full reconcile — routing applies only to live events.
Tests
Behavior tests count gateway calls, per the spec's acceptance list:
spec_message_posted→ exactly one messages call carryingafter, zero review/run callsreview_stage_passedwith a review id → one review-detail call, nothing else; without the id → one list callbun test(512 pass) andbun run typecheckgreen; no new dependencies; no sail changes.