mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-27 12:10:40 +00:00
Fix background completion unread markers
This commit is contained in:
@@ -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
|
||||
|
||||
+15
-2
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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"
|
||||
)
|
||||
Reference in New Issue
Block a user