Merge PR #2068 into stage-338
fix(ui): prevent stuck sidebar spinner on completed sessions (closes #2066) by @franksong2702
This commit is contained in:
commit
4388cb1a10
4 files changed
+106
-8
No files matched your search
+30
-4
@@ -231,16 +231,38 @@ function _isSessionActivelyViewedForList(sid) {
|
||||
function _isSessionLocallyStreaming(s) {
|
||||
if (!s || !s.session_id) return false;
|
||||
const isActive = S.session && s.session_id === S.session.session_id;
|
||||
return Boolean(
|
||||
(isActive && S.busy)
|
||||
|| (typeof INFLIGHT === 'object' && INFLIGHT && INFLIGHT[s.session_id])
|
||||
);
|
||||
// For the active session, rely on S.busy to indicate an ongoing stream.
|
||||
// INFLIGHT entries for non-active sessions are artifacts of interrupted
|
||||
// streams (page refresh, network disconnect, gateway restart) where
|
||||
// `delete INFLIGHT[sid]` was never reached — they should NOT cause the
|
||||
// sidebar spinner to appear on completed sessions. (#2066)
|
||||
return isActive && Boolean(S.busy);
|
||||
}
|
||||
|
||||
function _isSessionEffectivelyStreaming(s) {
|
||||
return Boolean(s && (s.is_streaming || _isSessionLocallyStreaming(s)));
|
||||
}
|
||||
|
||||
function _purgeStaleInflightEntries() {
|
||||
// Clean up INFLIGHT entries for sessions the server confirms are NOT
|
||||
// streaming. This prevents the in-memory cache from growing unbounded
|
||||
// when streams end abnormally. (#2066)
|
||||
if (typeof INFLIGHT !== 'object' || !INFLIGHT) return;
|
||||
const sessionsById = new Map();
|
||||
if (Array.isArray(_allSessions)) {
|
||||
for (const s of _allSessions) {
|
||||
if (s && s.session_id) sessionsById.set(s.session_id, s);
|
||||
}
|
||||
}
|
||||
for (const sid of Object.keys(INFLIGHT)) {
|
||||
const s = sessionsById.get(sid);
|
||||
if (s && !s.is_streaming) {
|
||||
delete INFLIGHT[sid];
|
||||
if (typeof clearInflightState === 'function') clearInflightState(sid);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function _rememberRenderedStreamingState(s, isStreaming) {
|
||||
if (!s || !s.session_id || !isStreaming) return;
|
||||
_sessionStreamingById.set(s.session_id, true);
|
||||
@@ -2257,6 +2279,10 @@ function renderSessionListFromCache(){
|
||||
// Don't re-render while user is actively renaming a session (would destroy the input)
|
||||
if(_renamingSid) return;
|
||||
closeSessionActionMenu();
|
||||
// Purge stale INFLIGHT entries for sessions the server confirms are NOT
|
||||
// streaming. This runs on every list refresh to prevent memory leaks from
|
||||
// interrupted streams. (#2066)
|
||||
_purgeStaleInflightEntries();
|
||||
const q=($('sessionSearch').value||'').toLowerCase();
|
||||
const activeSidForSidebar=_activeSessionIdForSidebar();
|
||||
const titleMatches=q?_allSessions.filter(s=>(s.title||'Untitled').toLowerCase().includes(q)):_allSessions;
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
"""Regression checks for #2066 stale sidebar spinner state."""
|
||||
|
||||
import json
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
SESSIONS_JS = (Path(__file__).resolve().parent.parent / "static" / "sessions.js").read_text()
|
||||
|
||||
|
||||
def _function_block(name: str, next_name: str) -> str:
|
||||
start = SESSIONS_JS.find(f"function {name}")
|
||||
assert start != -1, f"{name} not found in sessions.js"
|
||||
end = SESSIONS_JS.find(f"function {next_name}", start)
|
||||
assert end != -1, f"{next_name} not found after {name}"
|
||||
return SESSIONS_JS[start:end]
|
||||
|
||||
|
||||
def test_local_streaming_only_uses_active_session_busy_state():
|
||||
block = _function_block("_isSessionLocallyStreaming", "_isSessionEffectivelyStreaming")
|
||||
|
||||
assert "const isActive = S.session && s.session_id === S.session.session_id;" in block
|
||||
assert "return isActive && Boolean(S.busy);" in block
|
||||
assert "INFLIGHT[s.session_id]" not in block
|
||||
assert "INFLIGHT && INFLIGHT[s.session_id]" not in block
|
||||
|
||||
|
||||
def test_cache_render_purges_stale_non_streaming_inflight_entries():
|
||||
purge_block = _function_block("_purgeStaleInflightEntries", "_rememberRenderedStreamingState")
|
||||
render_block = _function_block("renderSessionListFromCache", "_showProjectPicker")
|
||||
|
||||
assert "const sessionsById = new Map();" in purge_block
|
||||
assert "if (s && s.session_id) sessionsById.set(s.session_id, s);" in purge_block
|
||||
assert "const s = sessionsById.get(sid);" in purge_block
|
||||
assert "_allSessionsById" not in purge_block
|
||||
assert "if (s && !s.is_streaming)" in purge_block
|
||||
assert "delete INFLIGHT[sid];" in purge_block
|
||||
assert "clearInflightState(sid);" in purge_block
|
||||
assert "_purgeStaleInflightEntries();" in render_block
|
||||
|
||||
|
||||
def test_stale_inflight_purge_executes_without_undeclared_session_map():
|
||||
purge_block = _function_block("_purgeStaleInflightEntries", "_rememberRenderedStreamingState")
|
||||
script = f"""
|
||||
let _allSessions = [
|
||||
{{session_id: 'done-session', is_streaming: false}},
|
||||
{{session_id: 'running-session', is_streaming: true}}
|
||||
];
|
||||
let INFLIGHT = {{
|
||||
'done-session': true,
|
||||
'running-session': true,
|
||||
'unknown-session': true
|
||||
}};
|
||||
let cleared = [];
|
||||
function clearInflightState(sid) {{
|
||||
cleared.push(sid);
|
||||
}}
|
||||
{purge_block}
|
||||
_purgeStaleInflightEntries();
|
||||
console.log(JSON.stringify({{inflight: INFLIGHT, cleared}}));
|
||||
"""
|
||||
result = subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
payload = json.loads(result.stdout)
|
||||
|
||||
assert payload == {
|
||||
"inflight": {
|
||||
"running-session": True,
|
||||
"unknown-session": True,
|
||||
},
|
||||
"cleared": ["done-session"],
|
||||
}
|
||||
@@ -186,8 +186,8 @@ def test_polling_transition_tracks_the_same_effective_streaming_state_as_sidebar
|
||||
assert render_idx != -1, "_renderOneSession not found"
|
||||
render_block = SESSIONS_JS[render_idx:SESSIONS_JS.find("const hasUnread=", render_idx)]
|
||||
|
||||
assert "(isActive && S.busy)" in local_block
|
||||
assert "INFLIGHT && INFLIGHT[s.session_id]" in local_block
|
||||
assert "isActive && Boolean(S.busy)" in local_block
|
||||
assert "INFLIGHT && INFLIGHT[s.session_id]" not in local_block
|
||||
assert "s.is_streaming || _isSessionLocallyStreaming(s)" in effective_block
|
||||
assert "const isStreaming=_isSessionEffectivelyStreaming(s);" in render_block, (
|
||||
"the row spinner and polling completion transition must use the same "
|
||||
|
||||
@@ -129,8 +129,9 @@ def test_sidebar_uses_local_inflight_state_for_immediate_spinner():
|
||||
messages_js = (Path(__file__).resolve().parent.parent / "static" / "messages.js").read_text()
|
||||
|
||||
assert "function _isSessionLocallyStreaming(s)" in SESSIONS_JS
|
||||
assert "(isActive && S.busy)" in SESSIONS_JS
|
||||
assert "INFLIGHT[s.session_id]" in SESSIONS_JS
|
||||
assert "isActive && Boolean(S.busy)" in SESSIONS_JS
|
||||
assert "function _purgeStaleInflightEntries()" in SESSIONS_JS
|
||||
assert "delete INFLIGHT[sid];" in SESSIONS_JS
|
||||
assert "function _isSessionEffectivelyStreaming(s)" in SESSIONS_JS
|
||||
assert "const isStreaming=_isSessionEffectivelyStreaming(s);" in SESSIONS_JS
|
||||
assert "if(typeof renderSessionListFromCache==='function') renderSessionListFromCache();" in messages_js
|
||||
|
||||
Reference in new issue
Block a user