From f5ef3688c6562920fa3dec2a9bab6c9d120c86a7 Mon Sep 17 00:00:00 2001 From: starship-s <45587122+starship-s@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:59:59 +0000 Subject: [PATCH] fix(cache): keep streaming TTLs above poll cadence --- api/models.py | 25 +++---- api/route_session_list_cache.py | 21 +++--- tests/test_issue4842_cron_projection_perf.py | 2 +- .../test_issue856_pinned_indicator_layout.py | 4 +- tests/test_session_sidebar_cache.py | 4 +- tests/test_streaming_cache_ttl_vs_poll.py | 65 +++++++++++++++++++ 6 files changed, 95 insertions(+), 26 deletions(-) create mode 100644 tests/test_streaming_cache_ttl_vs_poll.py diff --git a/api/models.py b/api/models.py index e30e52b48..257a41449 100644 --- a/api/models.py +++ b/api/models.py @@ -54,10 +54,12 @@ WEBHOOK_PROJECT_CHIP_LIMIT = 200 _CLI_SESSIONS_CACHE_TTL_SECONDS = 5.0 # While a turn is actively streaming, hold the CLI/cron projection longer than # one poll interval (mirrors the route-level #4808 hold-down). The frontend -# polls /api/sessions every ~5s during a stream; without a wider window the -# CLI cache key advances on every streamed message row (see below) and the -# expensive state.db CLI/cron projection is re-run on every poll. (#4842) -_CLI_SESSIONS_CACHE_STREAMING_TTL_SECONDS = 30.0 +# polls /api/sessions on the static/sessions.js `_streamingPollMs` cadence. +# Pair this wider window with the stable streaming cache key below so repeated +# polls reuse the projection instead of re-running the expensive state.db +# CLI/cron projection. (#4842) Keep this strictly greater than +# `_streamingPollMs`/1000 (see tests/test_streaming_cache_ttl_vs_poll.py). +_CLI_SESSIONS_CACHE_STREAMING_TTL_SECONDS = 45.0 _CLI_SESSIONS_CACHE_LOCK = threading.Lock() _CLI_SESSIONS_CACHE_INFLIGHT: "dict[tuple, threading.Event]" = {} _CLI_SESSIONS_CACHE_INVALIDATION_VERSION = 0 @@ -93,7 +95,7 @@ _CLAUDE_CODE_PARSE_CACHE_MAX = 1000 # ~200 sidecar file reads per /api/sessions build — and because the enclosing # _CLI_SESSIONS_CACHE is keyed on a state.db content fingerprint that advances # on every streamed message row, that whole scan was re-paid on essentially -# every 5s poll during a live turn (the "100% CPU / multi-second get_cli_sessions" +# every streaming poll during a live turn (the "100% CPU / multi-second get_cli_sessions" # in #4842/#4808/#4672). This memoizes the parse result keyed by the sidecar's # (path, mtime_ns, size, ctime_ns) stat signature: a warm projection re-stats # each file (~1 stat) instead of re-reading+parsing it, while any genuine @@ -6473,7 +6475,7 @@ def _reload_cli_sessions_after_inflight( def _cli_sessions_cache_ttl_seconds() -> float: # #4842: widen the freshness window while a turn is streaming so the fixed - # ~5s streaming poll cadence doesn't force a rebuild on every poll. Paired + # streaming poll cadence doesn't force a rebuild on every poll. Paired # with the streaming-freeze cache key (so the key is stable across polls # mid-stream), this bounds the heavy CLI/cron projection to one rebuild per # streaming-TTL window instead of one per poll. Mirrors the route-level @@ -6631,8 +6633,9 @@ def _cli_sessions_streaming_freeze_marker(): in-app structural mutation invalidates the cache directly via ``clear_cli_sessions_cache``. Externally-driven changes that don't fire that listener (a scheduled cron completing, an external CLI writing rows) surface - within one streaming-TTL window (≤30s) rather than instantly — a bounded, - self-healing lag that is the deliberate latency/CPU trade-off of the freeze. (#4842) + after the streaming TTL expires and a subsequent refresh occurs rather than + instantly — a bounded, self-healing lag that is the deliberate latency/CPU + trade-off of the freeze. (#4842) """ try: active = _active_stream_ids() @@ -6672,9 +6675,9 @@ def _resolve_cli_sessions_context(source_filter=None, include_claude_code: bool # #4842: while a turn streams, freeze the volatile state.db component of the # key so per-message writes don't bust the CLI cache and re-run the heavy # CLI/cron projection on every poll (mirrors the route-level #4808 freeze). - # The wider streaming TTL in get_cli_sessions() still forces a periodic - # rebuild so a streaming session's own count stays fresh within that window, - # and structural mutations invalidate via clear_cli_sessions_cache(). + # The wider streaming TTL in get_cli_sessions() still permits a periodic + # rebuild after that window, and structural mutations invalidate via + # clear_cli_sessions_cache(). _streaming_marker = _cli_sessions_streaming_freeze_marker() db_state_key = _streaming_marker if _streaming_marker is not None else _sqlite_file_stat_cache_key(db_path) cache_key = ( diff --git a/api/route_session_list_cache.py b/api/route_session_list_cache.py index 82b424670..ae4c03373 100644 --- a/api/route_session_list_cache.py +++ b/api/route_session_list_cache.py @@ -14,15 +14,16 @@ from api.profiles import _profiles_match _SESSIONS_CACHE_TTL_SECONDS = 2.5 # #4808: while a turn is actively streaming the frontend polls /api/sessions on a -# fixed cadence (static/sessions.js `_streamingPollMs` = 5000ms). With the idle TTL -# of 2.5s, every streaming poll lands in a fresh window and forces a full -# all_sessions() rebuild on the hot path under the global store LOCK — pinning CPU -# and starving token rendering on large stores (recurrence of #4672). Hold the -# sidebar cache steady for longer than one poll interval while streaming; live -# runtime state (active stream, sort order, pending flags) is overlaid on every -# response regardless of cache, and structural/settings changes still evict -# immediately via the source stamp. -_SESSIONS_CACHE_STREAMING_TTL_SECONDS = 10.0 +# fixed cadence (static/sessions.js `_streamingPollMs`). With the idle TTL of +# 2.5s, the entry expires between streaming polls, so each poll can find it stale +# and force a full all_sessions() rebuild on the hot path under the global store +# LOCK — pinning CPU and starving token rendering on large stores (recurrence of +# #4672). Hold the sidebar cache steady for longer than one poll interval while +# streaming; live runtime state (active stream, sort order, pending flags) is +# overlaid on every response regardless of cache, and structural/settings changes +# still invalidate immediately. Keep this strictly greater than +# `_streamingPollMs`/1000 (see tests/test_streaming_cache_ttl_vs_poll.py). +_SESSIONS_CACHE_STREAMING_TTL_SECONDS = 45.0 _SESSIONS_CACHE_MAX_ENTRIES = 64 _SESSIONS_CACHE_WAIT_SECONDS = 0.25 _SESSIONS_CACHE_STALE_WAIT_SECONDS = 0.10 @@ -176,7 +177,7 @@ def _session_list_cache_get( _SESSIONS_CACHE.pop(key, None) return None, False # #4808: widen the freshness window while a turn is streaming so the fixed - # 5s streaming poll cadence doesn't force a full rebuild on every poll. + # streaming poll cadence doesn't force a full rebuild on every poll. ttl = _SESSIONS_CACHE_TTL_SECONDS if _session_list_cache_streaming_freeze_marker() is not None: ttl = _SESSIONS_CACHE_STREAMING_TTL_SECONDS diff --git a/tests/test_issue4842_cron_projection_perf.py b/tests/test_issue4842_cron_projection_perf.py index db6e821eb..8b15d1b9a 100644 --- a/tests/test_issue4842_cron_projection_perf.py +++ b/tests/test_issue4842_cron_projection_perf.py @@ -15,7 +15,7 @@ On a cron-heavy profile (the #4842 reporter had 200+ cron sessions) that was hundreds of file reads per `/api/sessions` build, and because the enclosing `_CLI_SESSIONS_CACHE` is keyed on a state.db content fingerprint that advances on every streamed message row, the whole scan was re-paid on essentially every -5s poll during a live turn — pinning CPU to 100% and making `get_cli_sessions` +streaming poll during a live turn — pinning CPU to 100% and making `get_cli_sessions` take multiple seconds (#4842 / #4808 / #4672). These tests pin the fix: the expensive per-row work is now O(unique files), the diff --git a/tests/test_issue856_pinned_indicator_layout.py b/tests/test_issue856_pinned_indicator_layout.py index 5dfaa5006..9dad7e133 100644 --- a/tests/test_issue856_pinned_indicator_layout.py +++ b/tests/test_issue856_pinned_indicator_layout.py @@ -157,7 +157,7 @@ def test_date_group_caret_expanded_down_collapsed_right(): def test_apperror_path_calls_render_session_list(): """apperror handler must call renderSessionList() to clear the streaming indicator - immediately rather than waiting for the 5s streaming poll interval.""" + immediately rather than waiting for the streaming poll interval.""" messages_js = (Path(__file__).resolve().parent.parent / "static" / "messages.js").read_text(encoding="utf-8") apperror_idx = messages_js.find("source.addEventListener('apperror'") assert apperror_idx != -1, "apperror handler not found in messages.js" @@ -166,7 +166,7 @@ def test_apperror_path_calls_render_session_list(): apperror_block = messages_js[apperror_idx:warning_idx] assert "renderSessionList()" in apperror_block, ( "apperror handler must call renderSessionList() so the streaming indicator " - "clears immediately on server errors, not after a 5s poll delay" + "clears immediately on server errors, not after the polling fallback delay" ) diff --git a/tests/test_session_sidebar_cache.py b/tests/test_session_sidebar_cache.py index 7834f6136..2798861ce 100644 --- a/tests/test_session_sidebar_cache.py +++ b/tests/test_session_sidebar_cache.py @@ -721,7 +721,7 @@ def _age_cache_entry(key, seconds): def test_streaming_widens_cache_freshness_window(monkeypatch): """#4808: while a turn streams, an entry older than the idle TTL but younger - than the streaming TTL must still read FRESH, so the fixed 5s streaming poll + than the streaming TTL must still read FRESH, so the fixed streaming poll does not force a rebuild every poll.""" routes._session_list_cache_clear() key = _streaming_ttl_key() @@ -729,7 +729,7 @@ def test_streaming_widens_cache_freshness_window(monkeypatch): monkeypatch.setattr(routes, "_session_list_cache_source_stamp", lambda k: ("stable",)) routes._session_list_cache_set(key, _session_cache_payload("live")) - # Age it past the idle 2.5s TTL but under the 10s streaming TTL. + # Age it past the idle TTL but keep it under the streaming TTL. _age_cache_entry(key, routes._SESSIONS_CACHE_TTL_SECONDS + 1.0) # Idle (no active stream) → stale → miss. diff --git a/tests/test_streaming_cache_ttl_vs_poll.py b/tests/test_streaming_cache_ttl_vs_poll.py new file mode 100644 index 000000000..b494f5505 --- /dev/null +++ b/tests/test_streaming_cache_ttl_vs_poll.py @@ -0,0 +1,65 @@ +"""The streaming session-list caches must outlive one streaming poll interval. + +While a turn is actively streaming, the frontend re-polls ``/api/sessions`` on a +fixed cadence (``static/sessions.js`` ``_streamingPollMs``). Both streaming-cache +hold-downs — the route-level ``_SESSIONS_CACHE_STREAMING_TTL_SECONDS`` (#4808) and +the CLI/cron ``_CLI_SESSIONS_CACHE_STREAMING_TTL_SECONDS`` (#4842) — exist so that +those repeated polls read a held entry instead of forcing a full, LOCK-contending +rebuild on the streaming hot path (#4672). + +That only holds if each streaming TTL is strictly longer than the poll interval. +If a TTL is <= the poll interval, the entry expires between polls, every poll can +find it stale, and the hold-down collapses back into per-poll rebuilds — the exact +regression these constants were added to prevent. + +This test derives the poll interval from the single source of truth (the JS +constant) so the invariant can never silently drift when either side is retuned. +""" + +import re +from pathlib import Path + +import pytest + +from api import models +from api import route_session_list_cache + +_REPO_ROOT = Path(__file__).resolve().parent.parent +_SESSIONS_JS = _REPO_ROOT / "static" / "sessions.js" + + +def _streaming_poll_seconds() -> float: + """Parse the real ``const _streamingPollMs = ;`` from static/sessions.js. + + Kept as the single source of the poll magic number so the test never encodes + a second, drift-prone copy of it. + """ + source = _SESSIONS_JS.read_text(encoding="utf-8") + match = re.search(r"\bconst\s+_streamingPollMs\s*=\s*(\d+)\s*;", source) + assert match, "could not find `const _streamingPollMs = ;` in static/sessions.js" + poll_ms = int(match.group(1)) + assert poll_ms > 0, f"_streamingPollMs must be a positive integer, got {poll_ms!r}" + return poll_ms / 1000.0 + + +@pytest.mark.parametrize( + "ttl_seconds, label", + [ + ( + route_session_list_cache._SESSIONS_CACHE_STREAMING_TTL_SECONDS, + "route_session_list_cache._SESSIONS_CACHE_STREAMING_TTL_SECONDS", + ), + ( + models._CLI_SESSIONS_CACHE_STREAMING_TTL_SECONDS, + "models._CLI_SESSIONS_CACHE_STREAMING_TTL_SECONDS", + ), + ], +) +def test_streaming_ttl_strictly_exceeds_poll_interval(ttl_seconds, label): + poll_seconds = _streaming_poll_seconds() + assert ttl_seconds > poll_seconds, ( + f"{label} ({ttl_seconds}s) must be strictly greater than the streaming " + f"poll interval ({poll_seconds}s from static/sessions.js `_streamingPollMs`), " + "otherwise the streaming hold-down expires between polls and every poll " + "forces a full rebuild on the streaming hot path (#4672/#4808/#4842)." + )