Cloonar/coding-lab

Fork 0

web: SSE-driven refetch/render churn makes sends and chat switching sluggish as the fleet grows #175

New issue

Closed

opened 2026-07-18 21:38:18 +02:00 by dominik.polakovics · 2 comments

dominik.polakovics commented

2026-07-18 21:38:18 +02:00

Owner

Symptom

With several live agents (7 at diagnosis time), the PWA feels sluggish everywhere: sending a normal message, switching between chats, general navigation. The server is NOT the bottleneck — measured on the live instance: GET messages p50 ~48ms, GET /runs/{id} ~7ms, GET /instances ~26ms, POST reply p50 ~400ms (250ms of which is the deliberate tmux keystroke pacing). No resource pressure on the host (load 0.25, 27GB free, zero PSI). The cost lands on the client and its network connection, which is why server htop looks clean.

Mechanism (code-confirmed)

While any agent streams, its tailer publishes run.messages.changed about once per second (1s poll). The SPA reacts to every event:

Open chat (web/src/routes/RunChat.tsx refetchMessages): fires the after-cursor tail fetch AND an unconditional latest-window fetch (2 sequential requests per event, each up to ~150KB with tool chips at the 2000-char truncate limit).
Nav rail (web/src/components/AppShell.tsx): refetches the whole /api/v1/instances list per event (250ms trailing debounce) — even though the event can only flip a state dot.
DOM churn (web/src/lib/chatStream.ts mergeMessages): the merge is "later window wins per seq" unconditionally, so every refetch replaces the latest ≤60 messages with fresh object identities even when nothing changed. Solid's <For> keys by reference → those 60 subtrees (markdown parse included) are torn down and rebuilt roughly every second while an agent streams.
Unbounded accumulation: a long-lived chat tab keeps every appended message in the DOM (no windowing after fetch), so per-tick layout cost grows with conversation length.
Sibling noise: run.changed is repo-scoped, so any sibling run's change triggers a full refetchRun + refetchMessages of the open chat.

Net effect: a phone with 2-3 busy agents runs a continuous 2-4 req/s fetch loop plus a 60-subtree DOM rebuild per second; the operator's own POST /reply and route navigations queue behind it. Degradation scales with active-run count and conversation length — which is why it reads as "the lab got slower over time".

Fix direction

refetchMessages: fetch only the after-cursor tail; skip the redundant latest-window request unless a back-patch is actually needed (tool status flip / dialog resolution can be signalled, or fetched at a much lower cadence).
Identity-stable merge: keep the previous message object when the incoming one is content-equal (cheap JSON-equality or a server-side content hash per message), so unchanged messages never rebuild DOM.
Coalesce event bursts in the chat view (e.g. 300-500ms trailing debounce like the rail already has).
Rail: carry the state dot in the SSE payload (run.messages.changed already knows run + state) instead of refetching the full instance list per event.
Scope the run.changed chat refetch to events for the run actually open.
Later/optional: window or virtualize the accumulated stream.

Verification

Reproduce: open the lab with 2-3 busy agents, devtools Network tab — observe the continuous refetch loop; interaction latency while it runs vs. with agents idle. After the fix the idle-chat request rate should drop to ~0 and a streaming chat should cost one small tail fetch per event burst with no rebuild of unchanged message DOM.

## Symptom With several live agents (7 at diagnosis time), the PWA feels sluggish everywhere: sending a normal message, switching between chats, general navigation. The server is NOT the bottleneck — measured on the live instance: GET messages p50 ~48ms, GET /runs/{id} ~7ms, GET /instances ~26ms, POST reply p50 ~400ms (250ms of which is the deliberate tmux keystroke pacing). No resource pressure on the host (load 0.25, 27GB free, zero PSI). The cost lands on the client and its network connection, which is why server htop looks clean. ## Mechanism (code-confirmed) While any agent streams, its tailer publishes `run.messages.changed` about once per second (1s poll). The SPA reacts to every event: 1. **Open chat** (`web/src/routes/RunChat.tsx` `refetchMessages`): fires the after-cursor tail fetch AND an unconditional latest-window fetch (2 sequential requests per event, each up to ~150KB with tool chips at the 2000-char truncate limit). 2. **Nav rail** (`web/src/components/AppShell.tsx`): refetches the whole `/api/v1/instances` list per event (250ms trailing debounce) — even though the event can only flip a state dot. 3. **DOM churn** (`web/src/lib/chatStream.ts` `mergeMessages`): the merge is "later window wins per seq" *unconditionally*, so every refetch replaces the latest ≤60 messages with fresh object identities even when nothing changed. Solid's `<For>` keys by reference → those 60 subtrees (markdown parse included) are torn down and rebuilt roughly every second while an agent streams. 4. **Unbounded accumulation**: a long-lived chat tab keeps every appended message in the DOM (no windowing after fetch), so per-tick layout cost grows with conversation length. 5. **Sibling noise**: `run.changed` is repo-scoped, so any sibling run's change triggers a full `refetchRun` + `refetchMessages` of the open chat. Net effect: a phone with 2-3 busy agents runs a continuous 2-4 req/s fetch loop plus a 60-subtree DOM rebuild per second; the operator's own POST /reply and route navigations queue behind it. Degradation scales with active-run count and conversation length — which is why it reads as "the lab got slower over time". ## Fix direction - `refetchMessages`: fetch only the after-cursor tail; skip the redundant latest-window request unless a back-patch is actually needed (tool status flip / dialog resolution can be signalled, or fetched at a much lower cadence). - Identity-stable merge: keep the previous message object when the incoming one is content-equal (cheap JSON-equality or a server-side content hash per message), so unchanged messages never rebuild DOM. - Coalesce event bursts in the chat view (e.g. 300-500ms trailing debounce like the rail already has). - Rail: carry the state dot in the SSE payload (`run.messages.changed` already knows run + state) instead of refetching the full instance list per event. - Scope the `run.changed` chat refetch to events for the run actually open. - Later/optional: window or virtualize the accumulated stream. ## Verification Reproduce: open the lab with 2-3 busy agents, devtools Network tab — observe the continuous refetch loop; interaction latency while it runs vs. with agents idle. After the fix the idle-chat request rate should drop to ~0 and a streaming chat should cost one small tail fetch per event burst with no rebuild of unchanged message DOM.

dominik.polakovics added the

needs-triage

bug

labels

2026-07-18 21:38:18 +02:00

dominik.polakovics commented

2026-07-18 21:46:57 +02:00

Author

Owner

This was generated by AI during triage.

Agent Brief

Category: bug
Summary: Stop the SSE-driven refetch/render churn in the SPA: tail-only message fetches, identity-stable merges backed by a server content hash, state-carrying events for the nav rail, and run-scoped chat refetches.

Current behavior:
See the issue body — the mechanism is code-confirmed and was re-verified at triage time. In short, while any agent streams, run.messages.changed fires ~1/s and the SPA reacts by (1) fetching both the after-cursor tail AND an unconditional latest window in the open chat, (2) refetching the entire instances list for the nav rail, and (3) merging with "later window wins per seq" semantics that replace the latest ~60 message object identities even when content is unchanged — Solid's reference-keyed <For> then tears down and rebuilds those subtrees every tick. Additionally, the repo-scoped run.changed event triggers a full refetch of the open chat even when it concerns a sibling run. Server timings are healthy; the cost is client-side and scales with active-run count and conversation length.

Desired behavior:

Tail-only steady state. On run.messages.changed, an open chat performs the after-cursor tail fetch only. The unconditional latest-window request per event is removed. Back-patched mutations near the tail (tool status flips, answered/resolved dialogs) must still reach the client — the event payload should carry enough signal for the client to know when a latest-window fetch is actually needed, or an equivalent mechanism with the same effect: steady streaming costs one small tail fetch per event burst.
Event bursts coalesced. The chat view debounces run.messages.changed reactions with a short trailing debounce (~300–500ms), like the rail already does.
Identity-stable merge (with server hash). The messages API includes a per-message content hash (stable for identical content, changes when the rendered content changes — including tool status flips). The client merge keeps the previous message object when the incoming hash is unchanged, so unchanged messages never produce new identities and never rebuild DOM. Decision: the server-side hash is in scope now, not deferred — don't rely on client-side deep equality as the long-term mechanism.
Rail without refetch. Decision: the run.messages.changed SSE payload carries the run's conversational state (the event source already knows run + state). The shell patches the existing instances list in place to flip the state dot — no GET /instances round trip on this event. run.changed (spawn/stop/outcome transitions) may keep refetching the list.
Run-scoped chat refetch. A run.changed event only triggers the open chat's refetch when it concerns the run being viewed. If the wire event doesn't currently identify the run, extend it so the client can filter.

Key interfaces:

The SSE event contract (run.messages.changed, run.changed) — payload gains the run's conversational state and whatever back-patch/run-identity signals the client needs. Keep the events small; they are per-second per streaming agent.
The run-messages API response — each message gains a content hash field.
The pure merge helpers in the chat-stream module (mergeMessages / mergeRefetch) — become identity-preserving under equal hashes. These are pure and unit-testable; keep them that way.
The shell's shared instances resource — gains an in-place patch path driven by event payloads alongside its existing refetch path.
Pending-dialog identity handling in the chat route already dedupes by tool_id — don't regress it.

Acceptance criteria:

With a streaming agent, an open chat issues at most one tail fetch per event burst; no per-event unconditional latest-window request (assert request counts in tests with a mocked API).
Tool status flips and pending-dialog resolutions still reach an open chat and back-patch in place (existing behavior preserved, with test coverage).
Merge helpers keep object identity for messages whose content hash is unchanged (unit test: merging an identical window returns the same object references).
run.messages.changed carries the run's state; the nav rail state dot updates with zero GET /instances requests for that event type (test asserting no refetch).
A run.changed event for a sibling run does not trigger a message refetch of the open chat.
An open chat with no streaming activity issues no periodic requests.
Existing chat-stream unit tests and web test suite pass.

Out of scope:

Windowing/virtualizing the accumulated message DOM (explicitly deferred in the issue body).
Server-side query/DB performance work — the server is measured healthy; #176 (PR walks) and #177 (/clear latency) are separate issues.
Push-notification/presence behavior (#160) beyond not breaking it.

Verification note for the agent: this sandbox has no browser. Verify via unit/integration tests with request-count assertions (jsdom + mocked fetch/SSE). Live on-device verification (devtools Network tab with 2–3 busy agents, per the issue's Verification section) stays with the maintainer.

> *This was generated by AI during triage.* ## Agent Brief **Category:** bug **Summary:** Stop the SSE-driven refetch/render churn in the SPA: tail-only message fetches, identity-stable merges backed by a server content hash, state-carrying events for the nav rail, and run-scoped chat refetches. **Current behavior:** See the issue body — the mechanism is code-confirmed and was re-verified at triage time. In short, while any agent streams, `run.messages.changed` fires ~1/s and the SPA reacts by (1) fetching both the after-cursor tail AND an unconditional latest window in the open chat, (2) refetching the entire instances list for the nav rail, and (3) merging with "later window wins per seq" semantics that replace the latest ~60 message object identities even when content is unchanged — Solid's reference-keyed `<For>` then tears down and rebuilds those subtrees every tick. Additionally, the repo-scoped `run.changed` event triggers a full refetch of the open chat even when it concerns a sibling run. Server timings are healthy; the cost is client-side and scales with active-run count and conversation length. **Desired behavior:** 1. *Tail-only steady state.* On `run.messages.changed`, an open chat performs the after-cursor tail fetch only. The unconditional latest-window request per event is removed. Back-patched mutations near the tail (tool status flips, answered/resolved dialogs) must still reach the client — the event payload should carry enough signal for the client to know when a latest-window fetch is actually needed, or an equivalent mechanism with the same effect: steady streaming costs one small tail fetch per event burst. 2. *Event bursts coalesced.* The chat view debounces `run.messages.changed` reactions with a short trailing debounce (~300–500ms), like the rail already does. 3. *Identity-stable merge (with server hash).* The messages API includes a per-message content hash (stable for identical content, changes when the rendered content changes — including tool status flips). The client merge keeps the previous message object when the incoming hash is unchanged, so unchanged messages never produce new identities and never rebuild DOM. Decision: the server-side hash is in scope now, not deferred — don't rely on client-side deep equality as the long-term mechanism. 4. *Rail without refetch.* Decision: the `run.messages.changed` SSE payload carries the run's conversational state (the event source already knows run + state). The shell patches the existing instances list in place to flip the state dot — no `GET /instances` round trip on this event. `run.changed` (spawn/stop/outcome transitions) may keep refetching the list. 5. *Run-scoped chat refetch.* A `run.changed` event only triggers the open chat's refetch when it concerns the run being viewed. If the wire event doesn't currently identify the run, extend it so the client can filter. **Key interfaces:** - The SSE event contract (`run.messages.changed`, `run.changed`) — payload gains the run's conversational state and whatever back-patch/run-identity signals the client needs. Keep the events small; they are per-second per streaming agent. - The run-messages API response — each message gains a content hash field. - The pure merge helpers in the chat-stream module (`mergeMessages` / `mergeRefetch`) — become identity-preserving under equal hashes. These are pure and unit-testable; keep them that way. - The shell's shared instances resource — gains an in-place patch path driven by event payloads alongside its existing refetch path. - Pending-dialog identity handling in the chat route already dedupes by `tool_id` — don't regress it. **Acceptance criteria:** - [ ] With a streaming agent, an open chat issues at most one tail fetch per event burst; no per-event unconditional latest-window request (assert request counts in tests with a mocked API). - [ ] Tool status flips and pending-dialog resolutions still reach an open chat and back-patch in place (existing behavior preserved, with test coverage). - [ ] Merge helpers keep object identity for messages whose content hash is unchanged (unit test: merging an identical window returns the same object references). - [ ] `run.messages.changed` carries the run's state; the nav rail state dot updates with zero `GET /instances` requests for that event type (test asserting no refetch). - [ ] A `run.changed` event for a sibling run does not trigger a message refetch of the open chat. - [ ] An open chat with no streaming activity issues no periodic requests. - [ ] Existing chat-stream unit tests and web test suite pass. **Out of scope:** - Windowing/virtualizing the accumulated message DOM (explicitly deferred in the issue body). - Server-side query/DB performance work — the server is measured healthy; #176 (PR walks) and #177 (/clear latency) are separate issues. - Push-notification/presence behavior (#160) beyond not breaking it. **Verification note for the agent:** this sandbox has no browser. Verify via unit/integration tests with request-count assertions (jsdom + mocked fetch/SSE). Live on-device verification (devtools Network tab with 2–3 busy agents, per the issue's Verification section) stays with the maintainer.

dominik.polakovics added

ready-for-agent

and removed

needs-triage

labels

2026-07-18 21:47:02 +02:00

dominik.polakovics referenced this issue from a commit

2026-07-18 22:22:51 +02:00

perf(chat): tail-only SSE refetch with content-hash identity to stop render churn

dominik.polakovics referenced this issue from a pull request that will close it,

2026-07-18 22:23:16 +02:00

perf(chat): tail-only SSE refetch with content-hash identity to stop render churn #178

dominik.polakovics referenced this issue from a commit

2026-07-18 22:56:33 +02:00

style(web): prettier formatting on two issue #175 test files

dominik.polakovics referenced this issue from a commit

2026-07-18 23:41:31 +02:00

fix(chat): never lose a backpatchSeq announcement — publish, absorb, settle-heal

dominik.polakovics commented

2026-07-18 23:53:59 +02:00

Author

Owner

This was generated by AI while landing a PR.

Landing audit for PR #178 (head afk/175, verdict PASS → merging on the maintainer's free-text go-ahead).

Verification signal relied on: Forgejo Actions ci / native — green on the amended head d82213c (run 198, 6m14s), which exercised the full gate including golangci-lint. Not re-run locally beyond that; independently, the full local gate was also green before push: gofmt/go vet/go build -tags ui/go test -tags ui ./... (incl. tmuxx), prettier, eslint, vitest 865/865, tsc + vite build, labctl secret scan. The ci-nix gate did not run — correct, since the PR touches no nix/dependency files.

Conventions: Conventional Commits title ✓ · Closes #175 ✓ · ADR-0047 included ✓ · merges cleanly into main @ 28de985 ✓.

Review outcome: the original PR (86bfed0) was correct on the perf mechanics but lost the chat's self-heal — backpatchSeq is a delta on a drop-by-design bus, with three loss channels (publish gate swallowing a sig-only-tick backpatch; bus drops; a FULL refetch discarding the queued seq). All three were fixed in the amendment d82213c (publish-gate leg + FULL absorbs the queued seq + turn-settle escalation), each pinned by a regression test proven red pre-fix, and ADR-0047 now states the level/delta loss split instead of contradicting it. b1f214e was a formatting-only fix for the initially-red format:check.

Known non-blockers left open: providertest.Fake.ReadChat shares its Messages backing array (a -race flake risk if CI ever adds -race — latent, values converge); rail-dot stale-resolve clobber (cosmetic, self-heals on next event); back-patch deletion undetectable (pre-existing under ADR-0016 semantics).

> *This was generated by AI while landing a PR.* **Landing audit for PR #178** (head `afk/175`, verdict **PASS** → merging on the maintainer's free-text go-ahead). **Verification signal relied on:** Forgejo Actions `ci / native` — green on the amended head `d82213c` (run 198, 6m14s), which exercised the full gate including golangci-lint. Not re-run locally beyond that; independently, the full local gate was also green before push: `gofmt`/`go vet`/`go build -tags ui`/`go test -tags ui ./...` (incl. `tmuxx`), prettier, eslint, vitest 865/865, `tsc` + `vite build`, `labctl secret scan`. The `ci-nix` gate did not run — correct, since the PR touches no nix/dependency files. **Conventions:** Conventional Commits title ✓ · `Closes #175` ✓ · ADR-0047 included ✓ · merges cleanly into `main` @ `28de985` ✓. **Review outcome:** the original PR (`86bfed0`) was correct on the perf mechanics but lost the chat's self-heal — `backpatchSeq` is a delta on a drop-by-design bus, with three loss channels (publish gate swallowing a sig-only-tick backpatch; bus drops; a FULL refetch discarding the queued seq). All three were fixed in the amendment `d82213c` (publish-gate leg + FULL absorbs the queued seq + turn-settle escalation), each pinned by a regression test proven red pre-fix, and ADR-0047 now states the level/delta loss split instead of contradicting it. `b1f214e` was a formatting-only fix for the initially-red `format:check`. Known non-blockers left open: `providertest.Fake.ReadChat` shares its `Messages` backing array (a `-race` flake risk if CI ever adds `-race` — latent, values converge); rail-dot stale-resolve clobber (cosmetic, self-heals on next event); back-patch deletion undetectable (pre-existing under ADR-0016 semantics).

dominik.polakovics closed this issue

2026-07-18 23:54:04 +02:00