Fix multiple renders from cached up-to-date messages on page refresh #3358
Conversation
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
Addresses issue where multiple up-to-date control messages from HTTP cache trigger excessive renders on page refresh. When a shape receives multiple updates within the 60-second cache window (s-maxage), each update cycle ends with an up-to-date control message. These responses are cached by HTTP proxies/browser. On page refresh, the client retrieves multiple cached responses in quick succession, with each up-to-date message triggering a render, resulting in sloppy UI updates. Solution: Hybrid approach with localStorage persistence and time-window deduplication: 1. **UpToDateTracker class**: Tracks up-to-date messages per shape using: - localStorage persistence (60s TTL matching HTTP cache) - In-memory suppression window (1s for rapid messages) - LRU eviction for cache size management 2. **ShapeStream integration**: Before notifying subscribers of an up-to-date message, checks the tracker to determine if notification should be suppressed. Always allows live (SSE) messages through. 3. **State preservation**: Internal state (#isUpToDate, #lastSyncedAt) is updated even when notifications are suppressed, ensuring correct stream behavior. Benefits: - Solves page refresh problem with localStorage persistence - Adds defense-in-depth with rapid-message deduplication - Gracefully degrades when localStorage unavailable - Follows existing codebase patterns (ExpiredShapesCache) - Maintains backward compatibility Testing: Added comprehensive unit tests covering TTL expiration, localStorage persistence, LRU eviction, and edge cases. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Implements a simpler "replay mode" approach to prevent multiple renders from cached up-to-date messages on page refresh. ## Problem When a shape receives multiple updates within the 60-second HTTP cache window, each ends with an up-to-date control message. On page refresh, these cached responses replay rapidly, causing multiple renders: Response 1 (cached): [data] + up-to-date → RENDER Response 2 (cached): [data] + up-to-date → RENDER Response 3 (cached): [data] + up-to-date → RENDER Result: 3 renders in ~50ms = sloppy UI ## Solution: Binary Replay Mode Flag Instead of counting up-to-dates (error-prone), we use a simple flag: ### On receiving up-to-date (normal operation): - Record timestamp in localStorage (60s TTL) ### On page load (offset=-1): - Check if recent up-to-date exists in localStorage - If yes → enter "replay mode" ### During replay mode: - Suppress ALL cached up-to-dates - Exit mode when receiving live/SSE message - Notify subscribers only on live up-to-date ### Result: Response 1 (cached): [data] + up-to-date → suppress Response 2 (cached): [data] + up-to-date → suppress Response 3 (live): [data] + up-to-date → RENDER ONCE ## Changes 1. **UpToDateTracker** (simplified): - `recordUpToDate(shapeKey)`: Record timestamp - `shouldEnterReplayMode(shapeKey)`: Check for recent timestamp - No counting, no complex logic 2. **ShapeStream**: - Add `#replayMode` flag - Check for replay mode on startup (in #fetchShape) - Suppress cached up-to-dates in #onMessages - Exit replay mode on first live/SSE message - Record up-to-date when allowing notification 3. **Tests**: Updated to reflect replay mode behavior ## Benefits - No off-by-one errors (no counting) - Natural termination (exits on live message) - Guarantees fresh data (always does live round-trip) - Simple binary flag logic - Follows existing patterns (ExpiredShapesCache) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Fixes critical bug where replay mode never exits for long-polling users.
## Bug
Previous logic only exited replay mode for SSE messages:
```typescript
if (this.#replayMode && !isSseMessage) {
return // Suppress
}
this.#replayMode = false // Only reached if SSE
```
For long-polling users:
- isSseMessage is always false
- All up-to-dates suppressed, even fresh ones
- Replay mode never exits
- No notifications ever sent! 😱
## Fix
Check if request is in live mode (has `live=true` param):
```typescript
const isLiveRequest =
this.#currentFetchUrl?.searchParams.has(LIVE_QUERY_PARAM) ?? false
if (this.#replayMode && !isSseMessage && !isLiveRequest) {
return // Suppress cached responses during initial sync
}
this.#replayMode = false // Exit on live request OR SSE
```
Now works for both:
- SSE users: exits on first SSE message
- Long-poll users: exits when switching to live mode
## Also Fixed
- Clarified s-maxage comment (CDN cache, not browser cache)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Implements cursor-based detection to properly exit replay mode and prevent
multiple renders from cached up-to-date messages.
## Problem with Previous Approach
Previous fix checked for `live=true` param, but live requests are also cached!
Scenario:
```
Session 1:
- Request live=true&cursor=100 → up-to-date (cached)
- Request live=true&cursor=101 → up-to-date (cached)
- Request live=true&cursor=102 → up-to-date (cached)
Session 2 (page refresh):
- Request live=true&cursor=100 → CACHED, but has live=true!
- ❌ Exit replay mode, notify subscribers (WRONG!)
- Request live=true&cursor=101 → CACHED
- ❌ Not in replay mode, notify again (WRONG!)
- Result: Still getting multiple renders!
```
## Solution: Cursor-Based Detection
Cursors are time-based intervals that always increment (server code analysis):
```elixir
next_interval = ceil(time_since_epoch / timeout) * timeout
if next_interval == prev_interval:
next_interval + random(1..3600) # Always moves forward
```
**Key insight:** Fresh responses have a DIFFERENT cursor than cached ones.
### Implementation
1. **Store cursor in localStorage** (not just timestamp):
```typescript
recordUpToDate(shapeKey, cursor: string)
```
2. **Enter replay mode with last cursor**:
```typescript
const lastSeenCursor = upToDateTracker.shouldEnterReplayMode(shapeKey)
if (lastSeenCursor) {
this.#replayMode = lastSeenCursor // Store cursor, not just boolean
}
```
3. **Exit on cursor change**:
```typescript
if (currentCursor === lastSeenCursor) {
return // Still replaying cached, same cursor
}
// Different cursor = fresh from server, exit replay mode!
```
### Why This Works
- Multiple cached responses: **same cursor** → suppress all
- First fresh response: **different cursor** → exit replay mode, notify once!
- Works for both long-polling and SSE
- No complex timing logic needed
## Changes
- **UpToDateTracker**: Store cursor with timestamp
- **ShapeStream**: Store cursor in `#replayMode`, compare on up-to-date
- **Type change**: `#replayMode: false | string` (cursor when active)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
Adds throttling to avoid excessive localStorage writes as every incremental message includes an up-to-date control message. ## Why Throttle? Up-to-date messages arrive frequently: - Every incremental sync cycle ends with up-to-date - High-activity shapes can trigger 10-60+ writes/minute - localStorage writes are synchronous and block main thread (1-10ms typical) ## Implementation - **In-memory always current**: Data updates immediately - **localStorage throttled**: Writes max once per 60 seconds - **Smart scheduling**: If within throttle window, schedules write for later - **No data loss risk**: Only affects persistence, not current session ## Trade-off Worst case: User refreshes page within 60s of last write before a crash. They might see a brief flash of slightly older cached data. This is acceptable as it's rare and non-critical. ## Performance Benefit Before: Up to 60 localStorage writes/minute = potential jank After: Max 1 localStorage write/minute = smooth 60s throttle matches the cache TTL, making it a natural boundary. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3358 +/- ##
==========================================
- Coverage 70.14% 70.08% -0.06%
==========================================
Files 178 182 +4
Lines 9606 9893 +287
Branches 290 363 +73
==========================================
+ Hits 6738 6934 +196
- Misses 2866 2957 +91
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Updated all test cases to use the new recordUpToDate signature that requires a cursor argument. Also updated assertions to expect cursor strings instead of booleans, and fixed localStorage mock data structures to match the new UpToDateEntry interface with timestamp and cursor fields. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…-011CUdHvX6vr3kNkZ6HUEjv1
kevin-dp
approved these changes
Nov 13, 2025
| #activeSnapshotRequests = 0 // counter for concurrent snapshot requests | ||
| #midStreamPromise?: Promise<void> | ||
| #midStreamPromiseResolver?: () => void | ||
| #replayMode: false | string = false // False or last seen cursor when replaying cached responses |
Contributor
There was a problem hiding this comment.
nit: this variable serves 2 purposes: storing the last seen cursor and indicating whether we're in replay mode or not. I'd go for #lastSeenCursor: string | undefined and then we can derive replay mode, e.g. get #replayMode() = #lastSeenCursor !== undefined.
| // Check if the cursor has changed - cursors are time-based and always | ||
| // increment, so a new cursor means fresh data from the server. | ||
| const currentCursor = this.#liveCacheBuster | ||
| const lastSeenCursor = |
Contributor
There was a problem hiding this comment.
Why do we need this logic? The if test on L843 if (this.#replayMode && ...) checks the replay mode, so if we're here, the #replayMode must be a string. So the ternary seems unnecessary and the lastSeenCursor check in in the if test on L851 seems unnecessary too.
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.
When a shape receives multiple updates within the HTTP cache window (60s), each update ends with an up-to-date control message that gets cached. On page refresh, these cached responses replay rapidly, causing multiple renders. This change implements cursor-based detection to suppress cached up-to-date notifications until a fresh response (with a new cursor) arrives from the server, ensuring only one render occurs.