From fc6efc008afe4b484265721cca16e61e18699fec Mon Sep 17 00:00:00 2001 From: Michael Snoyman Date: Wed, 16 Sep 2026 05:52:02 +0300 Subject: [PATCH] fix: preserve unread state across transcript resets --- api/models.py | 10 ++ api/session_ops.py | 2 + api/webui_session_db.py | 2 + .../webui-run-state-consistency-contract.md | 28 +++- static/sessions.js | 137 ++++++++++++++---- tests/_unread_store_helpers.py | 7 +- tests/test_465_session_branching.py | 2 +- .../test_issue5273_active_done_viewed_sync.py | 4 +- .../test_issue6611_regeneration_settlement.py | 10 ++ ...est_issue6911_intentional_shrink_backup.py | 30 ++++ ...t_issue856_background_completion_unread.py | 4 +- tests/test_session_unread_dot_on_visit.py | 7 +- tests/test_unread_cross_client_sync.py | 125 ++++++++++++++-- ...ebui_run_state_consistency_contract_rfc.py | 9 ++ 14 files changed, 319 insertions(+), 58 deletions(-) diff --git a/api/models.py b/api/models.py index 775124dc..3de149ba 100644 --- a/api/models.py +++ b/api/models.py @@ -1247,6 +1247,8 @@ class Session: truncation_boundary=None, clear_generation=None, intentional_shrink_generation=None, + transcript_generation: int=0, + transcript_generation_baseline: int=0, gateway_routing=None, gateway_routing_history=None, llm_title_generated: bool=False, manual_title: bool=False, @@ -1344,6 +1346,10 @@ class Session: self.truncation_boundary = truncation_boundary self.clear_generation = clear_generation self.intentional_shrink_generation = intentional_shrink_generation + self.transcript_generation = max(0, _parse_nonnegative_int(transcript_generation) or 0) + self.transcript_generation_baseline = max( + 0, _parse_nonnegative_int(transcript_generation_baseline) or 0 + ) self.gateway_routing = gateway_routing if isinstance(gateway_routing, dict) else None self.gateway_routing_history = gateway_routing_history if isinstance(gateway_routing_history, list) else [] self.llm_title_generated = bool(llm_title_generated) @@ -1430,6 +1436,8 @@ class Session: 'truncation_boundary', 'clear_generation', 'intentional_shrink_generation', + 'transcript_generation', + 'transcript_generation_baseline', 'gateway_routing', 'gateway_routing_history', 'llm_title_generated', 'manual_title', 'parent_session_id', 'worktree_path', 'worktree_branch', 'worktree_repo_root', 'worktree_created_at', @@ -1768,6 +1776,8 @@ class Session: 'created_at': self.created_at, 'updated_at': self.updated_at, 'last_message_at': last_message_at, + 'transcript_generation': self.transcript_generation, + 'transcript_generation_baseline': self.transcript_generation_baseline, 'pinned': self.pinned, 'archived': self.archived, 'project_id': self.project_id, diff --git a/api/session_ops.py b/api/session_ops.py index c03fc09f..846075c6 100644 --- a/api/session_ops.py +++ b/api/session_ops.py @@ -608,6 +608,8 @@ def _stamp_intentional_shrink_generation(session, old_message_count: int, new_me if new_message_count >= old_message_count: return False session.intentional_shrink_generation = uuid.uuid4().hex + session.transcript_generation = max(0, int(getattr(session, 'transcript_generation', 0) or 0)) + 1 + session.transcript_generation_baseline = max(0, int(new_message_count)) return True diff --git a/api/webui_session_db.py b/api/webui_session_db.py index fa699a06..68ce2c1f 100644 --- a/api/webui_session_db.py +++ b/api/webui_session_db.py @@ -57,6 +57,8 @@ _METADATA_FIELDS = frozenset( "compression_recovery_action", "truncation_watermark", "truncation_boundary", + "transcript_generation", + "transcript_generation_baseline", "gateway_routing", "gateway_routing_history", "llm_title_generated", diff --git a/docs/rfcs/webui-run-state-consistency-contract.md b/docs/rfcs/webui-run-state-consistency-contract.md index d7da1ec3..aae3d695 100644 --- a/docs/rfcs/webui-run-state-consistency-contract.md +++ b/docs/rfcs/webui-run-state-consistency-contract.md @@ -165,13 +165,21 @@ writes. | Store | Key | Semantics | |---|---|---| -| Viewed counts | `hermes-session-viewed-counts` | `sid -> N`, meaning "seen at least up to N messages" | +| Viewed counts | `hermes-session-viewed-counts` | `sid -> {message_count, transcript_generation}`, meaning "seen up to N messages in this transcript generation" | | Completion markers | `hermes-session-completion-unread` | `sid -> {message_count, completed_at, ...}` behind the visible dot | -- **Viewed counts merge by maximum.** The fact is monotonic per session and - additive across clients, so a save reads the store, merges its cache in with a - max, and writes only when the store does not already hold the merged result. - A deletion records its own key under +- **Viewed counts are generation-scoped.** Session mutation routes increment the + persisted `transcript_generation` whenever edit, regenerate, retry, undo, clear, + or truncate reduces the visible transcript, and record the retained count as + `transcript_generation_baseline`. A newer generation replaces an older one even + when its count is lower; counts are monotonic only within one generation and + merge by maximum there. A client first observing a newer generation acknowledges + only that retained baseline, so messages added after the shrink remain unread + even when the shrink and later growth arrive in one coalesced sidebar refresh. + Legacy numeric records are generation zero and migrate to the structured + representation on their next save. This prevents an old pre-truncate high-water + mark from masking messages added after a transcript reset. A deletion records + its own key under `hermes-session-viewed-counts:deleted:v1:`; merges drop any count whose session has a live deletion record, so a client that still caches the acknowledgement prunes it instead of writing it back. List membership cannot @@ -205,7 +213,15 @@ writes. pruned by age (7 days) and supersession only — never by session existence, because the sidebar list is filtered by profile, project, and source, so an absent row may simply be hidden. They are never part of the marker map - consumers read. + consumers read. A clear order is retained in module memory even when storage + quota prevents allocating its versioned key; the client still attempts the + smaller write that removes the marker from the existing marker map, so a user + can dismiss unread state under storage pressure. Logical clear order and + retention time are separate: records compare markers using their order stamp, + but the 7-day cap uses the wall-clock time at which the record was written, so + a future logical stamp cannot extend retention indefinitely. The in-memory + fallback lasts until reload, while successfully persisted records retain the + same 7-day policy. - **Cross-client repair, not cache invalidation.** The `storage` listener routes a changed unread key (including any per-session clear key) back through the same merge instead of only dropping the local cache. A client that still holds an diff --git a/static/sessions.js b/static/sessions.js index 30005822..8efd006d 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -372,6 +372,7 @@ const SESSION_PROFILE_COUNTS_KEY = 'hermes-session-profile-counts'; let _sessionProfileCounts = null; let _sessionViewedCounts = null; let _sessionCompletionUnread = null; +let _sessionCompletionUnreadClearedMemory = {}; let _sessionObservedStreaming = null; const _sessionStreamingById = new Map(); const _sessionListSnapshotById = new Map(); @@ -528,14 +529,52 @@ function _sessionListLoaded() { && _sessionListSnapshotById.size > 0; } -// A viewed count is "seen at least up to N messages": monotonic per session and -// additive across clients, so merging is a max that never rolls a count back and -// never drops another client's entry. A value that exists only in our cache is -// written back only for a session the list still shows (see -// _sessionExistsForUnreadState) so a session deleted in another client, and -// already pruned from disk by _clearSessionViewedCount, is not resurrected. It -// is always kept in the live view regardless: dropping it here would lose the -// acknowledgement for a session that still exists. +// Viewed acknowledgements are generation-scoped. Transcript mutation routes +// increment `transcript_generation` whenever the visible message list shrinks; +// counts are monotonic only within one generation. Legacy numeric entries belong +// to generation zero and are migrated on their next save. +function _sessionViewedCountRecord(value, transcriptGeneration = 0) { + if (value && typeof value === 'object' && !Array.isArray(value)) { + return { + message_count: Number(value.message_count) || 0, + transcript_generation: Math.max(0, Number(value.transcript_generation) || 0), + }; + } + return { + message_count: Number(value) || 0, + transcript_generation: Math.max(0, Number(transcriptGeneration) || 0), + }; +} + +function _sessionViewedCountValue(value) { + return _sessionViewedCountRecord(value).message_count; +} + +function _sessionViewedRecordWins(candidate, current) { + const ours = _sessionViewedCountRecord(candidate); + const theirs = _sessionViewedCountRecord(current); + if (ours.transcript_generation !== theirs.transcript_generation) { + return ours.transcript_generation > theirs.transcript_generation; + } + return ours.message_count > theirs.message_count; +} + +function _sessionTranscriptGenerationForUnread(sid, explicit = null) { + if (explicit !== null && explicit !== undefined && Number.isFinite(Number(explicit))) { + return Math.max(0, Number(explicit) || 0); + } + let active = null; + try { + active = (S && S.session && S.session.session_id === sid) ? S.session : null; + } catch (_) {} + const snapshot = (typeof _sessionListSnapshotById !== 'undefined' && _sessionListSnapshotById) + ? _sessionListSnapshotById.get(sid) : null; + const listed = (typeof _allSessions !== 'undefined' && Array.isArray(_allSessions)) + ? _allSessions.find((s) => s && s.session_id === sid) : null; + const session = active || snapshot || listed; + return Math.max(0, Number(session && session.transcript_generation) || 0); +} + function _mergeSessionViewedCounts(disk, cache) { const deleted = _readSessionViewedCountDeletions(); const gone = (sid) => Object.prototype.hasOwnProperty.call(deleted, sid); @@ -546,19 +585,20 @@ function _mergeSessionViewedCounts(disk, cache) { changed = true; continue; } - counts[sid] = Number(disk[sid]) || 0; + const record = _sessionViewedCountRecord(disk[sid]); + counts[sid] = record; + if (!disk[sid] || typeof disk[sid] !== 'object' || Array.isArray(disk[sid])) changed = true; } for (const sid of Object.keys(cache || {})) { if (gone(sid)) continue; - const ours = Number(cache[sid]) || 0; + const ours = _sessionViewedCountRecord(cache[sid]); if (Object.prototype.hasOwnProperty.call(counts, sid)) { - if (ours > counts[sid]) { + if (_sessionViewedRecordWins(ours, counts[sid])) { counts[sid] = ours; changed = true; } continue; } - if (!ours) continue; if (_sessionListLoaded() && !_sessionExistsForUnreadState(sid)) continue; counts[sid] = ours; changed = true; @@ -567,8 +607,8 @@ function _mergeSessionViewedCounts(disk, cache) { for (const sid of Object.keys(counts)) live[sid] = counts[sid]; for (const sid of Object.keys(cache || {})) { if (gone(sid)) continue; - const ours = Number(cache[sid]) || 0; - if (ours > (Object.prototype.hasOwnProperty.call(live, sid) ? live[sid] : 0)) { + const ours = _sessionViewedCountRecord(cache[sid]); + if (!Object.prototype.hasOwnProperty.call(live, sid) || _sessionViewedRecordWins(ours, live[sid])) { live[sid] = ours; } } @@ -640,21 +680,26 @@ function _saveSessionViewedCounts() { } } -function _setSessionViewedCount(sid, messageCount = 0) { +function _setSessionViewedCount(sid, messageCount = 0, transcriptGeneration = null) { if (!sid) return; const counts = _getSessionViewedCounts(); const next = Number.isFinite(messageCount) ? Number(messageCount) : 0; + const generation = _sessionTranscriptGenerationForUnread(sid, transcriptGeneration); + const incoming = _sessionViewedCountRecord(next, generation); const hadCount = Object.prototype.hasOwnProperty.call(counts, sid); - const previous = Number(counts[sid]) || 0; - // A viewed count records "seen at least up to N messages" — keep it monotonic. - // A transient lower count (e.g. a list snapshot mid-compaction) must not - // resurrect a cleared unread dot before the save below. - counts[sid] = Math.max(previous, next); + const previous = hadCount ? _sessionViewedCountRecord(counts[sid]) : null; + if (!previous || _sessionViewedRecordWins(incoming, previous)) { + counts[sid] = incoming; + } else { + counts[sid] = previous; + } _saveSessionViewedCounts(); - // Advancing the acknowledgement records a clear even if another client has - // only prepared (not persisted) its older completion marker. Repeated polling - // at the same count does not refresh the tombstone or emit storage events. - _clearSessionCompletionUnread(sid, !hadCount || next > previous); + // A generation change or count advance acknowledges completion state even if + // another client prepared an older marker before this save. + const advanced = !previous + || generation > previous.transcript_generation + || (generation === previous.transcript_generation && next > previous.message_count); + _clearSessionCompletionUnread(sid, advanced); } function _getSessionCompletionUnread() { @@ -712,11 +757,22 @@ function _readSessionCompletionUnreadCleared() { keys = Array.from({ length: localStorage.length }, (_, i) => localStorage.key(i)); } catch (_) {} const cleared = _readStoredJsonMap(SESSION_COMPLETION_UNREAD_CLEARED_KEY); + const now = Date.now(); + for (const [sid, memoryRecord] of Object.entries(_sessionCompletionUnreadClearedMemory || {})) { + const value = Number(memoryRecord && memoryRecord.order) || 0; + const recordedAt = Number(memoryRecord && memoryRecord.recorded_at) || 0; + if (recordedAt && now - recordedAt > SESSION_COMPLETION_UNREAD_CLEARED_TTL_MS) { + delete _sessionCompletionUnreadClearedMemory[sid]; + continue; + } + if (value > (Number(cleared[sid]) || 0)) cleared[sid] = value; + } try { for (const key of keys) { const parsed = _parseSessionCompletionUnreadClearedKey(key); if (!parsed) continue; - const value = Math.max(parsed.version, Number(localStorage.getItem(key)) || 0); + const storedValue = Number(localStorage.getItem(key)) || 0; + const value = parsed.version || storedValue; if (value > (Number(cleared[parsed.sid]) || 0)) cleared[parsed.sid] = value; } } catch (_) { @@ -841,6 +897,12 @@ function _writeSessionCompletionUnreadCleared(sidOrSids, clearedAt) { (Array.isArray(sidOrSids) ? sidOrSids : [sidOrSids]).map((sid) => String(sid)) ); const at = Number(clearedAt) || 0; + for (const sid of justCleared) { + const previous = _sessionCompletionUnreadClearedMemory[sid]; + if (at > (Number(previous && previous.order) || 0)) { + _sessionCompletionUnreadClearedMemory[sid] = {order: at, recorded_at: Date.now()}; + } + } // One-way migration. The previous representation kept every clear fact in one // shared blob, so concurrent clears could replace one another and the blob @@ -855,14 +917,14 @@ function _writeSessionCompletionUnreadCleared(sidOrSids, clearedAt) { const stamp = Number(value) || 0; if (!stamp) continue; try { - localStorage.setItem(_sessionCompletionUnreadClearedKey(sid, stamp), String(stamp)); + localStorage.setItem(_sessionCompletionUnreadClearedKey(sid, stamp), String(Date.now())); } catch (_) { migrated = false; } } for (const sid of justCleared) { try { - localStorage.setItem(_sessionCompletionUnreadClearedKey(sid, at), String(at)); + localStorage.setItem(_sessionCompletionUnreadClearedKey(sid, at), String(Date.now())); } catch (_) {} } if (Object.keys(legacy).length && migrated) { @@ -878,8 +940,9 @@ function _writeSessionCompletionUnreadCleared(sidOrSids, clearedAt) { const parsed = _parseSessionCompletionUnreadClearedKey(key); if (!parsed) continue; try { - const version = Math.max(parsed.version, Number(localStorage.getItem(key)) || 0); - const expired = at - version > SESSION_COMPLETION_UNREAD_CLEARED_TTL_MS; + const recordedAt = Number(localStorage.getItem(key)) || 0; + const version = parsed.version || recordedAt; + const expired = recordedAt > 0 && Date.now() - recordedAt > SESSION_COMPLETION_UNREAD_CLEARED_TTL_MS; const superseded = version < (Number(cleared[parsed.sid]) || 0); // Records are pruned by age and supersession only. Session existence is // not a pruning signal: the sidebar list is filtered by profile, project, @@ -1123,12 +1186,22 @@ function _hasUnreadForSession(s) { if (!s || !s.session_id) return false; if (_hasSessionCompletionUnread(s.session_id)) return true; const counts = _getSessionViewedCounts(); + const generation = _sessionTranscriptGenerationForUnread(s.session_id, s.transcript_generation); if (!Object.prototype.hasOwnProperty.call(counts, s.session_id)) { - _setSessionViewedCount(s.session_id, Number(s.message_count || 0)); + _setSessionViewedCount(s.session_id, Number(s.message_count || 0), generation); return false; } - if (!Number.isFinite(s.message_count)) return false; - return s.message_count > Number(counts[s.session_id] || 0); + const viewed = _sessionViewedCountRecord(counts[s.session_id]); + if (generation > viewed.transcript_generation) { + const baseline = Math.max(0, Math.min( + Number(s.message_count || 0), + Number(s.transcript_generation_baseline) || 0 + )); + _setSessionViewedCount(s.session_id, baseline, generation); + return Number.isFinite(s.message_count) && s.message_count > baseline; + } + if (generation < viewed.transcript_generation || !Number.isFinite(s.message_count)) return false; + return s.message_count > viewed.message_count; } // Keep the sidebar polling snapshot current for a just-visited session so a diff --git a/tests/_unread_store_helpers.py b/tests/_unread_store_helpers.py index 93ba4ed6..0d187e64 100644 --- a/tests/_unread_store_helpers.py +++ b/tests/_unread_store_helpers.py @@ -19,6 +19,10 @@ _HELPER_NAMES = ( "_readStoredJsonMap", "_sessionExistsForUnreadState", "_sessionListLoaded", + "_sessionViewedCountRecord", + "_sessionViewedCountValue", + "_sessionViewedRecordWins", + "_sessionTranscriptGenerationForUnread", "_mergeSessionViewedCounts", "_sessionViewedCountDeletedKey", "_readSessionViewedCountDeletions", @@ -83,6 +87,7 @@ def _const_source(name: str) -> str: # Ready to interpolate into a node harness. BLOCK = "\n".join( - [_const_source(name) for name in _CONST_NAMES] + ["var _sessionCompletionUnreadClearedMemory = {};\n"] + + [_const_source(name) for name in _CONST_NAMES] + [_function_source(name) for name in _HELPER_NAMES if f"function {name}(" in SESSIONS_JS] ) diff --git a/tests/test_465_session_branching.py b/tests/test_465_session_branching.py index e792c0c4..2d8f1e8a 100644 --- a/tests/test_465_session_branching.py +++ b/tests/test_465_session_branching.py @@ -610,7 +610,7 @@ def test_session_compact_includes_parent(): # method name and first parameter without pinning formatting. compact_def_match = re.search(r"def compact\(\s*self", src) assert compact_def_match, "Could not find compact() method" - snippet = src[compact_def_match.start():compact_def_match.start() + 3000] + snippet = src[compact_def_match.start():compact_def_match.start() + 3500] assert "'parent_session_id'" in snippet, \ "compact() should include parent_session_id" diff --git a/tests/test_issue5273_active_done_viewed_sync.py b/tests/test_issue5273_active_done_viewed_sync.py index da5d8594..26f52552 100644 --- a/tests/test_issue5273_active_done_viewed_sync.py +++ b/tests/test_issue5273_active_done_viewed_sync.py @@ -161,7 +161,7 @@ def _run_done_compaction_harness( _markSessionCompletedInList(completedSession, activeSid); const cacheRow=_allSessions.find(s=>s&&s.session_id===completedSid); const unreadAfterCacheUpdate=_hasUnreadForSession(cacheRow); - const viewedCountAfterCacheUpdate=_getSessionViewedCounts()[completedSid] ?? null; + const viewedCountAfterCacheUpdate=_sessionViewedCountValue(_getSessionViewedCounts()[completedSid]); if ({str(is_session_viewed).lower()}) {{ _markSessionViewed(completedSid, completedMessageCount); }} @@ -172,7 +172,7 @@ def _run_done_compaction_harness( unreadAfterActiveBranchSync:_hasUnreadForSession(cacheRow), hasCompletionUnread:_hasSessionCompletionUnread(completedSid), completionUnreadMessageCount:_getSessionCompletionUnread()[completedSid]?.message_count ?? null, - viewedCount:_getSessionViewedCounts()[completedSid] ?? null, + viewedCount:_sessionViewedCountValue(_getSessionViewedCounts()[completedSid]), cacheRow, }})); """ diff --git a/tests/test_issue6611_regeneration_settlement.py b/tests/test_issue6611_regeneration_settlement.py index 77bb76fd..7ba52ec0 100644 --- a/tests/test_issue6611_regeneration_settlement.py +++ b/tests/test_issue6611_regeneration_settlement.py @@ -30,6 +30,16 @@ def test_recovered_prefix_survives_regeneration_truncation(): assert [row["content"] for row in session.messages] == ["old", "old answer", "latest"] +def test_regeneration_advances_transcript_generation_when_it_shrinks(): + session = _session() + session.transcript_generation = 7 + plan = plan_regeneration(session) + + assert apply_regeneration_plan(session, plan) + + assert session.transcript_generation == 8 + + def test_stale_revision_rejects_without_mutation(): session = _session() before = copy.deepcopy(session.__dict__) diff --git a/tests/test_issue6911_intentional_shrink_backup.py b/tests/test_issue6911_intentional_shrink_backup.py index 1cd8d1db..fc437a99 100644 --- a/tests/test_issue6911_intentional_shrink_backup.py +++ b/tests/test_issue6911_intentional_shrink_backup.py @@ -162,6 +162,36 @@ def test_generation_save_load_round_trip_is_in_metadata_prefix(monkeypatch, tmp_ assert metadata.intentional_shrink_generation == generation +def test_transcript_generation_round_trips_and_advances_only_on_shrink(monkeypatch, tmp_path): + models, _session_dir = _seed_session_dir(monkeypatch, tmp_path) + from api.session_ops import truncate_session_at_keep + from api.webui_session_db import WebUIJsonSessionDB + + session = models.Session( + session_id="issue6911_transcript_generation", + messages=_history(), + transcript_generation=4, + ) + session.save() + + truncate_session_at_keep(session, len(session.messages)) + assert session.transcript_generation == 4 + truncate_session_at_keep(session, 2) + assert session.transcript_generation == 5 + assert session.transcript_generation_baseline == 2 + session.save() + + loaded = models.Session.load(session.session_id) + metadata = models.Session.load_metadata_only(session.session_id) + listed = WebUIJsonSessionDB().list_sessions() + assert loaded.transcript_generation == 5 + assert loaded.transcript_generation_baseline == 2 + assert metadata.transcript_generation == 5 + assert metadata.transcript_generation_baseline == 2 + assert listed[0]["transcript_generation"] == 5 + assert listed[0]["transcript_generation_baseline"] == 2 + + def test_no_actual_message_shrink_keeps_generation(monkeypatch, tmp_path): models, _session_dir = _seed_session_dir(monkeypatch, tmp_path) from api.session_ops import truncate_session_at_keep diff --git a/tests/test_issue856_background_completion_unread.py b/tests/test_issue856_background_completion_unread.py index 0368a108..f11349ba 100644 --- a/tests/test_issue856_background_completion_unread.py +++ b/tests/test_issue856_background_completion_unread.py @@ -52,7 +52,7 @@ def test_background_completion_unread_uses_explicit_marker_not_message_delta(): 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") + count_idx = has_unread_block.find("s.message_count > viewed.message_count") 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, ( @@ -561,6 +561,6 @@ def test_historical_sessions_are_not_marked_unread_on_list_render(): 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, ( + assert "_setSessionViewedCount(s.session_id, Number(s.message_count || 0), generation);" in has_unread_block, ( "missing viewed-count baseline should still initialize as read for historical sessions" ) diff --git a/tests/test_session_unread_dot_on_visit.py b/tests/test_session_unread_dot_on_visit.py index 789062f9..b897f106 100644 --- a/tests/test_session_unread_dot_on_visit.py +++ b/tests/test_session_unread_dot_on_visit.py @@ -212,7 +212,7 @@ const before = Object.prototype.hasOwnProperty.call(_getSessionCompletionUnread( _acknowledgeSessionVisit('open', 5, 10); const after = Object.prototype.hasOwnProperty.call(_getSessionCompletionUnread(), 'open'); const snap = _sessionListSnapshotById.get('open'); -console.log(JSON.stringify({{before, after, repaints, viewed: _getSessionViewedCounts()['open'], snap}})); +console.log(JSON.stringify({{before, after, repaints, viewed: _sessionViewedCountValue(_getSessionViewedCounts()['open']), snap}})); """ out = _run_node(script) assert out["before"] is True, "precondition: marker seeded" @@ -419,12 +419,13 @@ function _hasMarker() {{ _releaseApi({{ session: {{ session_id: 'open', message_count: 6, messages: [{{ role: 'assistant', content: 'x' }}] }} }}); await p; const markerAfter = _hasMarker(); - const viewed = _getSessionViewedCounts()['open']; + const viewedRecord = _getSessionViewedCounts()['open']; + const viewed = viewedRecord === undefined ? null : _sessionViewedCountValue(viewedRecord); console.log(JSON.stringify({{ apiIssued, markerBefore, markerAfter, - viewed: viewed === undefined ? null : viewed, + viewed, }})); }})(); """ diff --git a/tests/test_unread_cross_client_sync.py b/tests/test_unread_cross_client_sync.py index 1e40706d..ab7b38c4 100644 --- a/tests/test_unread_cross_client_sync.py +++ b/tests/test_unread_cross_client_sync.py @@ -104,6 +104,7 @@ _CLIENT_FNS = [ "_markSessionCompletionUnread", "_clearSessionCompletionUnread", "_hasSessionCompletionUnread", + "_hasUnreadForSession", ] # Helpers that carry the merge/tombstone rules; injected when present. @@ -111,6 +112,10 @@ _HELPER_FNS = [ "_readStoredJsonMap", "_sessionExistsForUnreadState", "_sessionListLoaded", + "_sessionViewedCountRecord", + "_sessionViewedCountValue", + "_sessionViewedRecordWins", + "_sessionTranscriptGenerationForUnread", "_mergeSessionViewedCounts", "_sessionViewedCountDeletedKey", "_readSessionViewedCountDeletions", @@ -161,6 +166,12 @@ let _allSessions = []; const _sessionListSnapshotById = new Map(); const listed = (...ids) => {{ _allSessions = ids.map((sid) => ({{session_id: sid}})); }}; const readDisk = (key) => JSON.parse(_store[key] || '{{}}'); +const readViewedDisk = () => Object.fromEntries( + Object.entries(readDisk(SESSION_VIEWED_COUNTS_KEY)).map(([sid, value]) => [ + sid, + value && typeof value === 'object' ? Number(value.message_count) || 0 : Number(value) || 0, + ]) +); const setDisk = (key, value) => {{ _store[key] = JSON.stringify(value); }}; // Aggregate either the legacy whole map or independent per-session records. const readAllClears = (client) => client && typeof client.clears === 'function' @@ -189,6 +200,7 @@ def _client_factory(with_listener: bool = False) -> str: function makeClient() {{ let _sessionViewedCounts = null; let _sessionCompletionUnread = null; + let _sessionCompletionUnreadClearedMemory = {{}}; {fns} {helpers} {handler} @@ -196,7 +208,7 @@ function makeClient() {{ return {{ viewed: (sid) => {{ const counts = _getSessionViewedCounts(); - return Object.prototype.hasOwnProperty.call(counts, sid) ? counts[sid] : null; + return Object.prototype.hasOwnProperty.call(counts, sid) ? _sessionViewedCountValue(counts[sid]) : null; }}, setViewed: _setSessionViewedCount, clearViewed: _clearSessionViewedCount, @@ -204,6 +216,12 @@ function makeClient() {{ clearUnread: _clearSessionCompletionUnread, writeClear: _writeSessionCompletionUnreadCleared, hasUnread: _hasSessionCompletionUnread, + hasUnreadFor: (sid, messageCount, transcriptGeneration = 0, transcriptGenerationBaseline = 0) => _hasUnreadForSession({{ + session_id: sid, + message_count: messageCount, + transcript_generation: transcriptGeneration, + transcript_generation_baseline: transcriptGenerationBaseline, + }}), clears: (typeof _readSessionCompletionUnreadCleared === 'function') ? _readSessionCompletionUnreadCleared : () => readDisk(SESSION_COMPLETION_UNREAD_CLEARED_KEY), @@ -222,7 +240,92 @@ def _script(body: str, with_listener: bool = False) -> str: return _HEADER + _client_factory(with_listener) + body -# ── Viewed counts: monotonic, repaired, never resurrected ──────────────────── +# ── Viewed counts: generation-scoped, repaired, never resurrected ───────────── + +def test_shrink_then_growth_is_unread_in_the_new_transcript_generation(): + """A pre-shrink high-water mark must not mask new messages after truncation.""" + out = _run_node(_script(""" +listed('X'); +const A = makeClient(); +A.setViewed('X', 10, 0); +A.setViewed('X', 2, 1); +console.log(JSON.stringify({ + afterShrink: A.viewed('X'), + unreadAtThree: A.hasUnreadFor('X', 3, 1), +})); +""")) + assert out["afterShrink"] == 2 + assert out["unreadAtThree"] is True + + +def test_first_observation_of_new_generation_keeps_post_shrink_growth_unread(): + """A coalesced shrink-plus-growth refresh must acknowledge only the shrink baseline.""" + out = _run_node(_script(""" +listed('X'); +const A = makeClient(); +A.setViewed('X', 10, 0); +console.log(JSON.stringify({ + unread: A.hasUnreadFor('X', 3, 1, 2), + viewed: A.viewed('X'), +})); +""")) + assert out == {"unread": True, "viewed": 2} + + +def test_zero_message_baseline_survives_persistence_and_marks_first_message_unread(): + """Presence of a zero baseline is meaningful; it is not a missing record.""" + out = _run_node(_script(""" +listed('X'); +const A = makeClient(); +A.setViewed('X', 0, 0); +console.log(JSON.stringify({ + diskHasX: Object.prototype.hasOwnProperty.call(readViewedDisk(), 'X'), + unreadAtOne: A.hasUnreadFor('X', 1, 0), +})); +""")) + assert out["diskHasX"] is True + assert out["unreadAtOne"] is True + + +def test_in_memory_clear_expiry_uses_recorded_time_not_future_logical_order(): + """A future logical stamp must not extend the seven-day memory lifetime.""" + out = _run_node(_script(""" +const A = makeClient(); +_now = 1000; +A.writeClear('X', 999999999999); +const initially = A.clears(); +_now += 8 * 24 * 60 * 60 * 1000; +A.writeClear('Y', 5); +const expired = A.clears(); +console.log(JSON.stringify({initially, expired})); +""")) + assert out["initially"]["X"] == 999999999999 + assert "X" not in out["expired"] + + +def test_unread_clear_survives_quota_rejecting_a_new_tombstone_key(): + """Clearing must still shrink the existing marker map when allocation fails.""" + out = _run_node(_script(""" +listed('X'); +setDisk(SESSION_COMPLETION_UNREAD_KEY, { + X: {message_count: 3, completed_at: 1000, unread_order: 1}, +}); +const A = makeClient(); +const before = A.hasUnread('X'); +const ordinarySetItem = localStorage.setItem; +localStorage.setItem = (key, value) => { + if (!Object.prototype.hasOwnProperty.call(_store, key)) throw new Error('QuotaExceededError'); + ordinarySetItem(key, value); +}; +A.clearUnread('X'); +console.log(JSON.stringify({ + before, + after: A.hasUnread('X'), + diskHasX: Object.prototype.hasOwnProperty.call(readDisk(SESSION_COMPLETION_UNREAD_KEY), 'X'), +})); +""")) + assert out == {"before": True, "after": False, "diskHasX": False} + def test_stale_client_cannot_roll_a_viewed_count_back(): """Sequential rollback: a client that saw an older list snapshot must not @@ -234,7 +337,7 @@ const B = makeClient(); A.setViewed('X', 417); B.setViewed('X', 131); console.log(JSON.stringify({ - disk: readDisk(SESSION_VIEWED_COUNTS_KEY), + disk: readViewedDisk(), bView: B.viewed('X'), })); """)) @@ -252,7 +355,7 @@ const A = makeClient(); A.setViewed('D', 5); A.clearViewed('D'); console.log(JSON.stringify({ - diskHasD: Object.prototype.hasOwnProperty.call(readDisk(SESSION_VIEWED_COUNTS_KEY), 'D'), + diskHasD: Object.prototype.hasOwnProperty.call(readViewedDisk(), 'D'), })); """)) assert out["diskHasD"] is False, "clearing a viewed count must remove it from disk" @@ -275,7 +378,7 @@ const bCached = B.viewed('E'); B.clearViewed('E'); console.log(JSON.stringify({ bCached: bCached, - disk: readDisk(SESSION_VIEWED_COUNTS_KEY), + disk: readViewedDisk(), })); """)) assert out["bCached"] is None, "precondition: B's cache must not hold E" @@ -302,7 +405,7 @@ const held = A.viewed('X'); A.handleStorage({ key: SESSION_VIEWED_COUNTS_KEY }); console.log(JSON.stringify({ held: held, - disk: readDisk(SESSION_VIEWED_COUNTS_KEY), + disk: readViewedDisk(), })); """)) assert out["held"] == 417, ( @@ -329,10 +432,10 @@ B.viewed('Y'); // so A's write (built from the store it read) drops B's entry. _onRead = () => { B.setViewed('Y', 999); }; A.setViewed('X', 417); -const clobbered = readDisk(SESSION_VIEWED_COUNTS_KEY); +const clobbered = readViewedDisk(); // B still holds its own acknowledgement, and now processes A's storage event. B.handleStorage({ key: SESSION_VIEWED_COUNTS_KEY }); -const disk = readDisk(SESSION_VIEWED_COUNTS_KEY); +const disk = readViewedDisk(); const writesAfterRepair = storageWrites; B.handleStorage({ key: SESSION_VIEWED_COUNTS_KEY }); console.log(JSON.stringify({ @@ -372,7 +475,7 @@ listed('T'); B.setViewed('T', 1); B.handleStorage({ key: SESSION_VIEWED_COUNTS_KEY }); console.log(JSON.stringify({ - disk: readDisk(SESSION_VIEWED_COUNTS_KEY), + disk: readViewedDisk(), tView: B.viewed('T'), })); """)) @@ -399,7 +502,7 @@ A.clearViewed('S'); B.handleStorage({ key: SESSION_VIEWED_COUNTS_KEY }); B.setViewed('T', 1); console.log(JSON.stringify({ - disk: readDisk(SESSION_VIEWED_COUNTS_KEY), + disk: readViewedDisk(), deletions: readViewedDeletions().length, bView: B.viewed('S'), })); @@ -777,7 +880,7 @@ const disk = readDisk(SESSION_COMPLETION_UNREAD_KEY); console.log(JSON.stringify({ handlers: _storageHandlers.length, xView: A.viewed('X'), - diskX: readDisk(SESSION_VIEWED_COUNTS_KEY).X, + diskX: readViewedDisk().X, stable: repairStable, qStale: qStale, qView: A.hasUnread('Q'), diff --git a/tests/test_webui_run_state_consistency_contract_rfc.py b/tests/test_webui_run_state_consistency_contract_rfc.py index b8c38a10..dedee864 100644 --- a/tests/test_webui_run_state_consistency_contract_rfc.py +++ b/tests/test_webui_run_state_consistency_contract_rfc.py @@ -46,6 +46,15 @@ def test_contract_documents_the_client_side_unread_store_keys(): ) +def test_contract_documents_generation_scoped_viewed_counts(): + text = RFC.read_text(encoding="utf-8") + source = _sessions_js() + + assert "transcript_generation" in source + assert "transcript_generation" in text + assert "monotonic only within" in text + + def test_documented_retention_window_matches_the_implemented_cap(): source = _sessions_js() match = re.search(