From 5d16ff752251adffaf267ce276d8b053d27e9a67 Mon Sep 17 00:00:00 2001 From: Frank Song Date: Tue, 28 Apr 2026 11:20:05 +0800 Subject: [PATCH] Fix background completion unread markers --- CHANGELOG.md | 6 + static/messages.js | 17 ++- static/sessions.js | 44 ++++++++ ...t_issue856_background_completion_unread.py | 104 ++++++++++++++++++ 4 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 tests/test_issue856_background_completion_unread.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f5b1c682..b033e29d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ ## [Unreleased] ### Fixed +- **Background session completion unread dots** — sidebar unread dots no longer + depend solely on `message_count` increasing after a stream finishes. Background + `done` events now set an explicit unread-completion marker, including + session-switch races where the old session is no longer the user's intended + view, and the marker clears only when that session is opened. (`static/sessions.js`, + `static/messages.js`, `tests/test_issue856_background_completion_unread.py`) - **Auto-title generic fallback** — when the auxiliary title-generation call fails and the local fallback can only produce the generic label `Conversation topic`, the WebUI now keeps the existing provisional title diff --git a/static/messages.js b/static/messages.js index 2c15270b..5dcbc2d2 100644 --- a/static/messages.js +++ b/static/messages.js @@ -4,6 +4,15 @@ function _markSessionViewed(sid, messageCount) { _setSessionViewedCount(sid, next); } +function _isSessionActivelyViewed(sid) { + if(!sid || !S.session || S.session.session_id!==sid) return false; + // During session switching, S.session still points at the previous row until + // the next metadata request resolves. Treat that in-flight switch as + // background so a just-finished old stream still gets an unread marker. + if(typeof _loadingSessionId!=='undefined' && _loadingSessionId && _loadingSessionId!==sid) return false; + return true; +} + async function send(){ const text=$('msg').value.trim(); if(!text&&!S.pendingFiles.length)return; @@ -742,17 +751,21 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ _smdEndParser(); } const d=JSON.parse(e.data); + const isSessionViewed=_isSessionActivelyViewed(activeSid); + if(!isSessionViewed && typeof _markSessionCompletionUnread==='function'){ + _markSessionCompletionUnread(activeSid, d.session&&d.session.message_count); + } delete INFLIGHT[activeSid]; clearInflight();clearInflightState(activeSid); stopApprovalPolling(); stopClarifyPolling(); if(!_approvalSessionId || _approvalSessionId===activeSid) hideApprovalCard(true); if(!_clarifySessionId || _clarifySessionId===activeSid) hideClarifyCard(true); - if(S.session&&S.session.session_id===activeSid){ + if(isSessionViewed){ S.activeStreamId=null; const _cb=$('btnCancel');if(_cb)_cb.style.display='none'; } - if(S.session&&S.session.session_id===activeSid){ + if(isSessionViewed){ // Capture previous session totals BEFORE overwriting S.session with the new // cumulative values from the done event. prevIn/prevOut are the totals as of // the start of this turn; curIn/curOut are the full post-turn totals — the diff --git a/static/sessions.js b/static/sessions.js index 2a7cda61..75a95b32 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -16,7 +16,9 @@ const ICONS={ let _loadingSessionId = null; const SESSION_VIEWED_COUNTS_KEY = 'hermes-session-viewed-counts'; +const SESSION_COMPLETION_UNREAD_KEY = 'hermes-session-completion-unread'; let _sessionViewedCounts = null; +let _sessionCompletionUnread = null; function _getSessionViewedCounts() { if (_sessionViewedCounts !== null) return _sessionViewedCounts; @@ -45,8 +47,49 @@ function _setSessionViewedCount(sid, messageCount = 0) { _saveSessionViewedCounts(); } +function _getSessionCompletionUnread() { + if (_sessionCompletionUnread !== null) return _sessionCompletionUnread; + try { + const parsed = JSON.parse(localStorage.getItem(SESSION_COMPLETION_UNREAD_KEY) || '{}'); + _sessionCompletionUnread = parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : {}; + } catch (_){ + _sessionCompletionUnread = {}; + } + return _sessionCompletionUnread; +} + +function _saveSessionCompletionUnread() { + try { + localStorage.setItem(SESSION_COMPLETION_UNREAD_KEY, JSON.stringify(_getSessionCompletionUnread())); + } catch (_){ + // Ignore localStorage write failures. + } +} + +function _markSessionCompletionUnread(sid, messageCount = 0) { + if (!sid) return; + const unread = _getSessionCompletionUnread(); + const count = Number.isFinite(messageCount) ? Number(messageCount) : 0; + unread[sid] = {message_count: count, completed_at: Date.now()}; + _saveSessionCompletionUnread(); +} + +function _clearSessionCompletionUnread(sid) { + if (!sid) return; + const unread = _getSessionCompletionUnread(); + if (!Object.prototype.hasOwnProperty.call(unread, sid)) return; + delete unread[sid]; + _saveSessionCompletionUnread(); +} + +function _hasSessionCompletionUnread(sid) { + if (!sid) return false; + return Object.prototype.hasOwnProperty.call(_getSessionCompletionUnread(), sid); +} + function _hasUnreadForSession(s) { if (!s || !s.session_id) return false; + if (_hasSessionCompletionUnread(s.session_id)) return true; const counts = _getSessionViewedCounts(); if (!Object.prototype.hasOwnProperty.call(counts, s.session_id)) { _setSessionViewedCount(s.session_id, Number(s.message_count || 0)); @@ -148,6 +191,7 @@ async function loadSession(sid){ S.session._modelResolutionDeferred=true; S.lastUsage={...(data.session.last_usage||{})}; _setSessionViewedCount(S.session.session_id, Number(data.session.message_count || 0)); + _clearSessionCompletionUnread(S.session.session_id); localStorage.setItem('hermes-webui-session',S.session.session_id); const activeStreamId=S.session.active_stream_id||null; diff --git a/tests/test_issue856_background_completion_unread.py b/tests/test_issue856_background_completion_unread.py new file mode 100644 index 00000000..bd654176 --- /dev/null +++ b/tests/test_issue856_background_completion_unread.py @@ -0,0 +1,104 @@ +"""Regression checks for #856 background completion unread markers.""" + +from pathlib import Path + + +REPO = Path(__file__).resolve().parent.parent +SESSIONS_JS = (REPO / "static" / "sessions.js").read_text(encoding="utf-8") +MESSAGES_JS = (REPO / "static" / "messages.js").read_text(encoding="utf-8") + + +def _done_block() -> str: + start = MESSAGES_JS.find("source.addEventListener('done'") + assert start != -1, "done handler not found in messages.js" + end = MESSAGES_JS.find("source.addEventListener('stream_end'", start) + assert end != -1, "stream_end handler not found after done handler" + return MESSAGES_JS[start:end] + + +def test_background_completion_unread_uses_explicit_marker_not_message_delta(): + """A background completion must stay unread even when message_count has no delta.""" + assert "SESSION_COMPLETION_UNREAD_KEY = 'hermes-session-completion-unread'" in SESSIONS_JS + assert "function _markSessionCompletionUnread(" in SESSIONS_JS + assert "function _clearSessionCompletionUnread(" in SESSIONS_JS + assert "function _hasSessionCompletionUnread(" in SESSIONS_JS + + has_unread_idx = SESSIONS_JS.find("function _hasUnreadForSession(s)") + assert has_unread_idx != -1, "_hasUnreadForSession not found" + has_unread_block = SESSIONS_JS[has_unread_idx:SESSIONS_JS.find("async function newSession", has_unread_idx)] + + marker_idx = has_unread_block.find("_hasSessionCompletionUnread(s.session_id)") + count_idx = has_unread_block.find("s.message_count > Number") + assert marker_idx != -1, "_hasUnreadForSession must check explicit completion unread marker" + assert count_idx != -1, "_hasUnreadForSession must keep the existing message_count fallback" + assert marker_idx < count_idx, ( + "explicit completion unread marker must be checked before message_count delta, " + "because completed streams can have viewed_count == message_count" + ) + + +def test_background_done_sets_marker_when_session_not_actively_viewed(): + done_block = _done_block() + assert "const isSessionViewed=_isSessionActivelyViewed(activeSid);" in done_block + assert "if(!isSessionViewed && typeof _markSessionCompletionUnread==='function')" in done_block + assert "_markSessionCompletionUnread(activeSid, d.session&&d.session.message_count);" in done_block + + +def test_active_done_marks_viewed_without_setting_unread_marker(): + done_block = _done_block() + marker_idx = done_block.find("_markSessionCompletionUnread(activeSid") + viewed_guard_idx = done_block.find("if(isSessionViewed){", marker_idx) + viewed_mark_idx = done_block.find("_markSessionViewed(activeSid", viewed_guard_idx) + + assert marker_idx != -1, "background completion marker call missing" + assert viewed_guard_idx != -1, "done handler must guard active-session UI updates" + assert viewed_mark_idx != -1, "active/current completion must still mark session viewed" + assert viewed_guard_idx < viewed_mark_idx, ( + "active-session viewed write must remain inside isSessionViewed guard so " + "switch-away races cannot mark a background completion read" + ) + + +def test_switching_away_counts_as_background_completion(): + helper_idx = MESSAGES_JS.find("function _isSessionActivelyViewed(sid)") + assert helper_idx != -1, "_isSessionActivelyViewed helper missing" + helper_block = MESSAGES_JS[helper_idx:MESSAGES_JS.find("async function send()", helper_idx)] + + assert "S.session.session_id!==sid" in helper_block + assert "_loadingSessionId" in helper_block + assert "_loadingSessionId!==sid" in helper_block, ( + "if loadSession(B) is in flight while done(A) arrives, A must be treated " + "as background even though S.session can still temporarily point at A" + ) + + +def test_completion_unread_clears_only_when_session_is_opened(): + load_idx = SESSIONS_JS.find("async function loadSession(sid)") + assert load_idx != -1, "loadSession not found" + load_block = SESSIONS_JS[load_idx:SESSIONS_JS.find("function _resolveSessionModelForDisplaySoon", load_idx)] + + stale_guard_idx = load_block.find("if (_loadingSessionId !== sid) return;") + clear_idx = load_block.find("_clearSessionCompletionUnread(S.session.session_id);") + set_viewed_idx = load_block.find("_setSessionViewedCount(S.session.session_id") + + assert clear_idx != -1, "loadSession must clear explicit completion unread when the user opens the session" + assert stale_guard_idx != -1 and stale_guard_idx < clear_idx, ( + "stale loadSession responses must not clear unread markers for sessions the user did not actually open" + ) + assert set_viewed_idx != -1 and set_viewed_idx < clear_idx, ( + "completion unread should clear at the same point the session is marked viewed" + ) + + +def test_historical_sessions_are_not_marked_unread_on_list_render(): + """The explicit unread marker must be event-driven, not initialized by _hasUnreadForSession.""" + has_unread_idx = SESSIONS_JS.find("function _hasUnreadForSession(s)") + assert has_unread_idx != -1 + has_unread_block = SESSIONS_JS[has_unread_idx:SESSIONS_JS.find("async function newSession", has_unread_idx)] + + assert "_markSessionCompletionUnread" not in has_unread_block, ( + "rendering old historical sessions must not create completion-unread markers" + ) + assert "_setSessionViewedCount(s.session_id, Number(s.message_count || 0));" in has_unread_block, ( + "missing viewed-count baseline should still initialize as read for historical sessions" + )