Refactor chat timeline store - #68
Open
dix105 wants to merge 82 commits into
Open
Conversation
added 30 commits
May 22, 2026 15:10
renderedMessages in the closure was stale after loadOlderMessages added new messages. Now uses document.getElementById after each load attempt with 500ms delay for React to render.
- Yellow highlight flash (2s fade) on matched message with ring border - Highlight clears on search close or after 2s timeout - Ctrl+F still opens chat search (not conflicting with Ctrl+K global) - Highlight passes through onHighlightMessage callback
- onNavigateSearchMessage in AppPage: navigates to chat, then dispatches openclaw:scroll-to-message event after 1s (for chat to load) - ChatView listens for scroll-to-message event: scrolls + highlights - Same yellow highlight flash as Ctrl+F search
- Uses CSS Custom Highlight API (::highlight) to highlight matched WORDS within message text, not just the whole message - All occurrences of the search term in the message are highlighted - Yellow background (rgba(250, 204, 21, 0.4)) on matched text - Message-level background still shows as a 'which message' indicator - Highlights cleared on search close - Fallback: if CSS Highlights API not supported, message-level highlight still works
- Matched message: subtle ring border, full opacity - All other messages: dimmed to 40% opacity (focus effect) - Matched WORDS within message: yellow highlight via CSS Highlight API - Creates Telegram Desktop-like 'focus on the match' UX - Dims clear when search closes or highlight timeout
PostCSS/Next.js CSS parser doesn't support ::highlight() pseudo-element.
Moved the style to runtime injection via document.createElement('style')
when the highlight is first created.
When patch stream receives a cursor that jumps >5 from the last received cursor, dispatches openclaw:chat-bootstrap-recovery event for the affected session. The active chat hook listens for this and refetches bootstrap data to fill the gap. - Gap threshold: 5 cursors (small gaps from batching are normal) - Only fires for sessions that already exist in state - Logs gap detection as warning for debugging - lastReceivedCursor reset in test cleanup
- dataSource state in useChatMessages: 'loading' | 'warm-cache' | 'syncing' | 'fresh' - Set to 'syncing' when warm cache is applied (bootstrap still loading) - Set to 'fresh' when bootstrap completes - Subtle pill badge with pulsing yellow dot + 'Syncing...' text - Shown in chat header next to pin button - Disappears when bootstrap data arrives
… timeline 3-phase plan to replace the current 3-system message mutation (bootstrap, patch stream, warm cache) with a single ChatTimelineStore that mediates all sources, resolves conflicts by cursor, and emits one React update. Includes migration plan, edge case matrix, rollback strategy, and success criteria.
New ChatTimelineStore mediates all three data sources: - applyWarmCache(): accepted only before bootstrap settles - applyBootstrap(): replaces warm cache, becomes authoritative - applyPatchMessage(): highest priority, always applies on top - applyOptimistic() + confirmOptimistic(): user send flow Features: - Conflict resolution: higher cursor wins, messageId dedup - Batched notifications via rAF (sync fallback in tests) - Store registry: one store per session key - Sorted output by gatewayIndex Tests cover: - Warm cache → bootstrap replacement - Patch updates + removes - Optimistic → confirmed flow - Cursor conflict resolution - Count jump prevention (60→85 in one render) - Message ordering - Subscribe/unsubscribe - Store registry lifecycle
Integration points: - Store initialized with warm messages on mount - applyWarmCache() called when async warm cache loads from IndexedDB - applyBootstrap() called when middleware bootstrap completes - applyPatchMessage() called for each message from patch stream subscription The store now receives the same data as the existing setMessages() path. Both run in parallel — store builds the timeline while existing state continues to work. This is the incremental migration step. Next: swap the read path from React state to store subscription, eliminating the duplicate state and the race conditions. All 341 existing tests pass (3 pre-existing failures in chatMessageDedupe).
Write path: - setMessages() now writes through to ChatTimelineStore via applyPatchMessage() for each changed message - All 21 existing setMessages() call sites automatically feed the store - No individual migration needed — works via the write-through wrapper Read path: - Store subscription drives React state via setLocalMessages() - Store batches all writes (warm cache, bootstrap, patches, optimistic) into one notification per rAF frame - React gets one update per frame instead of 3+ competing updates Key behavior change: - Previously: warm cache sets 60 msgs, bootstrap sets 85 msgs = two React renders, visible count jump 60→85 - Now: both write to store, store batches into one notification = single React render with 85 msgs, no jump All 341 existing tests pass (3 pre-existing chatMessageDedupe failures).
Integration tests (8 new): - Full warm→bootstrap→patch flow (no count jump) - Optimistic send → confirmed → assistant response - Rapid chat switch (separate stores don't interfere) - Stale warm cache rejected after bootstrap - Streaming text updates (no duplicates) - Pagination (older messages merge correctly) - Message removal maintains order Cleanup: - Removed redundant applyPatchMessage loop in patch subscription (setMessages write-through handles it) - Store ref updates on session key change - Total: 29 store tests (21 unit + 8 integration), all pass - 349 existing tests pass, 138 middleware tests pass - Typecheck clean Edge cases validated: - Count jumps 0→60→85 → eliminated (store batches) - Text flicker → eliminated (bootstrap replaces warm cache) - Ghost data → eliminated (bootstrap clears store) - Rapid switch → separate stores per session - Pagination → merge without duplicates
Audited actual code: client.ts, useAppFocus.ts, cacheRealtime.ts, useChatMessages.ts handleSend, persistentCache.ts, middleware-client.ts, gateway/client.ts, store.ts, timelineStore.ts Found 8 categories of gaps with specific line references: - WebSocket lifecycle (no ping, no reconnect on focus) - App focus/background (revalidates bootstrap but not WS) - Send flow (no WS check, no fallback polling) - IndexedDB (silent quota failure) - rAF batching (paused in background) - Middleware HTTP (no retry, no offline detection) - Gateway WS (no ping, no circuit breaker) - State transition matrix (verified 12 transitions)
Three fixes for WebSocket disconnect during app background: 1. WS health check every 15s — if WS is CLOSED/CLOSING, force reconnect immediately instead of waiting for onclose (which may never fire if OS killed the socket silently) 2. WS reconnect on app focus — when user returns to the app (visibilitychange or window focus), check WS state and reconnect if dead. This is the main fix for the 'send shows only thinking' bug. 3. Fallback poll after send — 5s after send ACK, if status is still thinking/streaming/tool_running, trigger a bootstrap refresh to pick up the response via HTTP instead of relying on dead WS. Edge cases: - Multiple focus events in rapid succession → reconnectAttempt=0 resets, only one connect() runs (previous WS closed first) - Health check fires while reconnect is in progress → connect() checks closedByCaller, reconnect timer cleared - Fallback poll fires but WS already delivered → bootstrap finds no new data, no-op - App stays backgrounded → health check interval suspended by OS, but fires immediately on foreground resume
When bootstrap replaces the store (applyBootstrap), optimistic messages that haven't been confirmed yet are preserved. This prevents the user's sent message from briefly disappearing when the fallback poll triggers a re-bootstrap while WS is dead. Flow: user sends → optimistic shows → WS dead → 5s fallback poll → bootstrap replaces → optimistic message preserved → server confirms → optimistic replaced with canonical.
Lesson learned: unit tests for individual methods aren't enough. Need interaction tests for every method pair that modifies the same state. The applyOptimistic→applyBootstrap data loss bug was caused by not testing this combination.
1. Optimistic message sorting: messages without gatewayIndex (optimistic) now sort to END of list, not beginning. Prevents user message appearing at top of chat. 2. Double reconnect race: health check + focus handler could both call connect() simultaneously. Added safeReconnect() guard with reconnecting flag + closes old WS before creating new one. 3. Health check doesn't close old WS: safeReconnect now always closes existing WS (even CONNECTING state) before creating new connection. 4. Fallback poll too heavy: changed from setStreamGeneration (re-inits entire effect chain) to reconcileActiveRun (lightweight status check). Increased timeout from 5s to 8s to give WS reconnect time to work.
Fixes: - applyOptimistic now always sets isOptimistic:true on the message - getSortedMessages: optimistic messages sort by isOptimistic flag, not gatewayIndex (which could be 0, not undefined) - maxSeq excludes optimistic messages from calculation New tests (8): - applyOptimistic → applyBootstrap preserves optimistic - applyOptimistic → applyBootstrap with confirmed version replaces - applyWarmCache → applyOptimistic → applyBootstrap full flow - optimistic sorts to end, not beginning - multiple optimistic messages maintain order - removeMessage on optimistic works - applyPatchMessage after bootstrap adds to existing - applyBootstrap after patches clears patch data Total: 37 tests (21 unit + 8 integration + 8 interaction), all pass
Ensures the timeline store has the optimistic message before flushSync renders it. Prevents the store subscription from overwriting the optimistic message in the next frame. Flow now: 1. applyOptimistic → store has user message 2. flushSync → React renders with optimistic message instantly 3. Store notification (rAF) → no-op (already has same data) 4. HTTP send → background 5. Patches arrive → update store + React
When warm cache shows tool calls expanded, then bootstrap arrives with different lastTwoAssistantIds, the defaultOpen prop flips to false and tool calls collapse unexpectedly. Fix: track prevDefaultOpen via ref. If defaultOpen changes from false→true, open the section. But NEVER auto-close a section that was already open — only user click can close it.
1. Periodic ping every 30s — detects silent Gateway WS disconnects. If WS is dead (not OPEN), triggers handleDisconnect. 2. Auto-reconnect on disconnect — 2s after Gateway WS drops, automatically attempts to reconnect. No more waiting for next request() call to trigger connect(). 3. Ping stopped on close — cleanup to prevent orphaned intervals. Edge cases: - Ping on dead WS → handleDisconnect → auto-reconnect 2s later - Multiple disconnects → autoReconnectTimer guard prevents multiple timers - connect() already in progress → this.connecting promise prevents duplicate - Reconnect fails → logged, next ping/request triggers another attempt
1. idbSet now detects QuotaExceededError specifically 2. Emits console.warn with actionable message (clear browser data) 3. Dispatches openclaw:storage-quota-exceeded event for UI notification 4. Warning emitted only once per session (quotaWarningEmitted flag) 5. getStorageUsage() exported — returns used/quota/percent via StorageManager Edge cases: - Quota error on first write → warning + event emitted - Repeated quota errors → single warning (no spam) - No StorageManager (old browsers) → getStorageUsage returns null - localStorage fallback still works when IndexedDB quota hit
1. visibilitychange listener flushes pending notifications when app returns to foreground — no burst of accumulated updates 2. setTimeout(100ms) fallback alongside rAF — if rAF is throttled (backgrounded tab), setTimeout ensures notifications fire within 100ms 3. Guards for test/SSR environments (document/window checks) 4. Cleanup: both rAF and setTimeout cleared in test cleanup + flush Edge cases: - App backgrounded 30min → rAF paused, setTimeout paused by OS → visibilitychange fires on foreground → flushes all pending at once - Multiple patches during background → accumulate in Map, single flush - rAF fires before setTimeout → setTimeout cleared in flushNotifications - setTimeout fires before rAF → rAF callback is no-op (Map already empty)
Gateway sends user message echo twice: once as empty confirmation (matched by optimistic), then again as full decorated message with higher seq. For new sessions, the echo gets seq+1 which breaks the seq-based dedup (openclawSeq <= entry.openclawSeq). Fix: also match by idempotencyKey from the message's __openclaw metadata. This is the most reliable match — same idempotency key means same send, regardless of sequence number. Preserves existing seq-based matching as fallback for messages without idempotency keys.
Missed the duplicate Gateway echo because the checklist didn't cover wire-level protocol behavior. Added Section 10: audit the EXACT message sequence from external systems, don't assume one-request-one-response.
* chore: add desktop state diagnostics * chore: reduce diagnostics noise * docs: add edge case fix workflow constraints --------- Co-authored-by: Developer <dev@example.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
No description provided.