fix(dashboard): make optimistic↔canonical bubble convergence testable behind a pure seam#324
Merged
Merged
Conversation
2c719b8 to
b3bb5ff
Compare
… behind a pure seam
ChatState reconciled three overlapping state layers (optimistic / streaming /
canonical) into the rendered bubble list as a render-time $derived filter plus
convergence side effects scattered across four SSE handlers and applyZeroBlocks.
That merge — the exact spot the project's state-machine testing discipline
points at (both optimistic↔canonical interleavings; reload-mid-state) — had no
isolated test surface: every case required standing up the whole store (runes,
Zero wiring, SSE plumbing, transport injection), so the bug class lived in code
nothing could unit-test.
Lift the merge into one pure, deterministic, rune-free module:
services/dashboard/src/lib/stores/bubble-convergence.ts. ChatState keeps
transport, focus, pagination, timers, and all reactivity; it now calls the
convergence core across a narrow seam.
The core owns the shared types (ChatMessage, OverlayKey/overlayKey) and the
previously in-store pure helpers (parse*, *IdForTurn, filterRowsToCurrentSession,
dropSupersededNoResponseSafetyNet, oldestBlockCursor, ...) — moved, not
imported, so the core imports neither store and the chat<->zero cycle is not
reintroduced. ChatState re-exports the public surface for existing consumers.
Pure interface:
- mergeBubbles(legacy, streaming, optimistic, focus) -> ChatMessage[]
(entries by reference; reads only identity fields so per-delta $state
mutations never re-run #derivedMessages — the hot-path reactivity contract)
- pruneConverged(legacy, streaming, focusAgent) -> {keep, drop}
- reconcileComplete(snapshot, event) -> ReconcilePlan (one dispatcher + 5
unexported per-kind helpers; the shell applies the discriminated plan)
- reconcileCanceled / userBubbleAlreadyLanded / mergeZeroSnapshot
The wall clock is threaded in, never read implicitly: mergeZeroSnapshot REQUIRES
a `now` the shell pins at its IO boundary, so the FRI-85/FRI-91 grace-window
suppressions are a deterministic function of inputs and unit-testable without
touching Date.now(). parseBlocks accepts opts.now (shell callers that set grace
fields pass it) and keeps a Date.now() fallback only for time-agnostic callers
where now cannot affect the result. The shell's transport-failure clock stays
out of the module.
Convergence behavior is preserved (the 228-test chat.test.ts regression suite
passes untouched); the fix is closing the test gap and pinning the invariants.
Ships with +29 pure convergence tests and +4 store-level integration tests
(POST-then-SSE and SSE-then-POST interleavings, reload-mid-state, agent-agnostic
multi-agent cancel, a discriminating perf-subscription contract probe, and a
deterministic grace-window test) in the same change, per the repo's state-machine
testing discipline. The pure extraction also pins several latent convergence
invariants that were previously unguarded (the reachedOldest two-writer
pagination reset; zeroSeenBlockIds write-back ordering; agent-agnostic cancel;
merged-vs-raw dedup read surface). chat.svelte.ts net -1455 lines.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjsfRm96p4MW2xz8ExHU6n
b3bb5ff to
dacbf75
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.
What & why
ChatStatereconciled three overlapping state layers (optimistic / streaming / canonical) into the rendered bubble list as a render-time$derivedfilter plus convergence side effects scattered across four SSE handlers andapplyZeroBlocks. That merge — the exact spot the project's state-machine testing discipline points at (both optimistic↔canonical interleavings; reload-mid-state) — had no isolated test surface: every case required standing up the whole store (runes, Zero wiring, SSE plumbing, transport injection), so the bug class lived in code nothing could unit-test.This lifts the merge into one pure, clock-free, rune-free module —
services/dashboard/src/lib/stores/bubble-convergence.ts— with a narrow interface.ChatStatekeeps transport, focus, pagination, timers, and all reactivity; it now calls the convergence core across a real seam.Shape
The core owns the shared types (
ChatMessage,OverlayKey/overlayKey) and the previously in-store pure helpers (parse*,*IdForTurn,filterRowsToCurrentSession,dropSupersededNoResponseSafetyNet,oldestBlockCursor, …) — moved, not imported, so the core imports neither store and thechat↔zerocircular import is not reintroduced.ChatStatere-exports the public surface for existing consumers.Pure interface:
mergeBubbles(legacy, streaming, optimistic, focus) -> ChatMessage[]— entries returned by reference; reads only identity fields (id/agent/sessionId) so a per-deltaentry.text += …$statemutation never re-runs#derivedMessages(the hot-path reactivity contract).pruneConverged(legacy, streaming, focusAgent) -> {keep, drop}reconcileComplete(snapshot, event) -> ReconcilePlan— one dispatcher + 5 unexported per-kind helpers; the shell applies the discriminated plan via oneswitch.reconcileCanceled/userBubbleAlreadyLanded/mergeZeroSnapshotBehavior
Convergence behavior is preserved — the 228-test
chat.test.tsregression suite passes untouched. The fix is closing the test gap and pinning the invariants.Tests (ship in the same change, per the state-machine testing discipline)
bubble-convergence.test.ts) — pinned shapes/counts and by-reference identity, no store/runes/Zero.chat.test.ts: POST-then-SSE and SSE-then-POST interleavings, reload-mid-state, agent-agnostic multi-agent cancel, and a discriminating perf-subscription contract probe.The pure extraction also pins several latent convergence invariants that were previously unguarded: the
reachedOldesttwo-writer pagination reset,zeroSeenBlockIdswrite-back ordering, agent-agnostic cancel, and the merged-vs-raw dedup read surface.chat.svelte.tsnet −1455 lines.Verification
chat.test.ts+bubble-convergence.test.ts: 256 passing.svelte-check: 0 errors (1 pre-existing unrelated warning).prettier --check: clean. Core imports neither store (acyclicity verified).Type is
fix→ release-please patch bump.🤖 Generated with Claude Code