Skip to content

Fix stale session render cache - #2682

Closed
heelee912 wants to merge 2 commits into
nesquena:masterfrom
heelee912:codex/fix-stale-session-render-cache
Closed

heelee912 wants to merge 2 commits into
nesquena:masterfrom
heelee912:codex/fix-stale-session-render-cache

Conversation

@heelee912

@heelee912 heelee912 commented May 21, 2026 •

Copy link
Copy Markdown

Summary

Fixes a WebUI transcript rendering cache issue where session HTML could be reused after switching sessions when only
the message count and render window size matched. This could make the visible transcript stale, including recently
sent messages disappearing or old messages appearing in the wrong position after navigation or context compaction.

This is a follow-up to #963 and fixes the known limitation where the session render cache was keyed only by session
id, message count, and render window size.

Root Cause

renderMessages() in static/ui.js used a count-based cache guard. When two render states had the same message
count and render window size, cached innerHTML could be reused even if the message content, live/pending state,
tool metadata, or compression anchor had changed.

Changes

  • Added a deterministic render-cache signature for session messages.
  • Included message identity/content, live and pending flags, status cards, tool calls, attachments, reasoning,
    truncation/window state, and compression anchor metadata in the cache key.
  • Required the cached signature to match before reusing session HTML.
  • Added regression coverage for same-count transcript changes and compression-anchor changes.
  • Updated the changelog.

Validation

pytest tests/test_session_render_cache_signature.py \
  tests/test_auto_compression_card.py \
  tests/test_regressions.py \
  -q

pytest tests/test_session_runtime_ownership_invariants.py \
  tests/test_streaming_markdown.py \
  tests/test_stale_stream_cleanup.py \
  tests/test_session_metadata_fast_path.py \
  -q

Combined result: 170 passed, 1 skipped locally. The skipped test was environment-gated, not a failure.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Summary

Reading static/ui.js:5537-5611 on the PR branch against origin/master, this is a clean tightening of the session-render cache guard. The previous key was (sid, msgCount, renderWindowSize), so any edit/retry/tool-result patch that kept the message count constant could win the cache lookup on cross-session navigation and replay a stale innerHTML. The new _messageRenderCacheSignature(messages, renderWindowSize) hashes per-message identity, content, live/pending flags, status cards, tool calls, attachments, reasoning, plus the session compression-anchor fields — exactly the inputs renderMessages later branches on. The cache key check at static/ui.js:5770 now requires cached.signature===cacheSignature in addition to count + window size:

const cached=_sessionHtmlCache.get(sid);
if(cached&&cached.msgCount===msgCount&&cached.renderWindowSize===renderWindowSize&&cached.signature===cacheSignature){
  inner.innerHTML=cached.html;
  _sessionHtmlCacheSid=sid;
  ...
}

The store at static/ui.js:6381 writes the same signature into the cache entry, so cold renders self-tag for the next lookup. The _messageRenderCachePart helper uses an FNV-1a-like hash to fold the per-message payload to a short string, which keeps the signature compact even for large transcripts.

One observation worth thinking about

cacheSignature is now computed unconditionally on every renderMessages call (line 5758), even when the fast-path will be skipped (e.g. same-session in-flight stream, transient compression UI present, or INFLIGHT[sid] set). For a 50-message visible window with substantial assistant content, that means hashing each msgContent(m) plus a JSON.stringify(m.content||'') per message on every settled render. In the warm path that's fine; during active streaming renderMessages can be called per chunk for some flows, and the hash work is O(N·content). If you see perf regressions on long, actively-streaming sessions, the cheap mitigation is to skip the signature compute when the cache-eligibility precondition (sid && sid!==_sessionHtmlCacheSid && !INFLIGHT[sid] && !hasTransientTranscriptUi) is false:

const cacheEligible=!!(sid&&sid!==_sessionHtmlCacheSid&&!INFLIGHT[sid]&&!hasTransientTranscriptUi);
const cacheSignature=cacheEligible?_messageRenderCacheSignature(S.messages,renderWindowSize):null;

Then guard the write-back at line 6381 with if(cacheSignature){...} so we only store when we'd have read. Non-blocking — the correctness fix is what matters and is right; this is just a follow-up if anyone reports a slowdown on long streams.

Tests

tests/test_session_render_cache_signature.py pins the source structure rather than running the function — it greps for the new helper, the signature comparison in renderMessages, the cache-write shape, and the removal of the stale "cache key is session_id + message count" limitation comment. That's the right shape for a vanilla-JS static-only repo with no JS test harness, and matches the conventions of tests/test_regressions.py already in the tree. The assertions are tight enough that a regression that reintroduces the count-only key would fail.

Verdict

LGTM on the cache-correctness change. The signature inputs cover the cases that #963 left behind (edits, tool-result patches in place, compression-anchor reassignment, reasoning that appeared after an out-of-order patch). The 300KB + 8-session eviction bound at line 6382-6384 is unchanged so memory footprint stays the same. Worth a release-note line because the symptom — "old message reappears in wrong slot after switching sessions" — is exactly the kind of thing a user reports as data loss when it's actually just a stale render.

@heelee912

Copy link
Copy Markdown
Author

Addressed the non-blocking perf observation by only computing the cache signature when the cache fast path is eligible. Validation: node --check static/ui.js, python3 -m pytest tests/test_session_render_cache_signature.py -q, and git diff --cached --check.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Thanks for the careful work on this!

Superseded by #2692 (shipped in v0.51.104, Release CB / stage-397). Both PRs target the same root cause — the session render cache key was (session_id, message_count, render_window_size), so same-count content changes (edits, retries, tool-output swaps) silently reused stale HTML on back-navigation.

The shipped fix (#2692 by @ai-ag2026) folds a content signature into the cache key — including m._ts, m._error, m._statusCard, m._partial_tool_calls, reasoning payload presence, and JSON.stringify(tc.args) per settled tool call. Functionally equivalent to your _messageRenderCachePart approach.

I reviewed both diffs side-by-side and either fix would have closed the underlying #2613 class. We ran #2692 first because it landed in the lowest-risk batch and its CHANGELOG accuracy passed Opus pre-merge review without code edits. Closing this as superseded — no work lost, the same fix is live in master.

Happy to take follow-up PRs that further tighten the render cache contract if you've spotted additional cases (compression-anchor changes, sidecar reconciliation edges) that the shipped signature doesn't cover. Thanks again for the catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants