Release exp-v0.52.179: gateway watcher index-covered fingerprint + failure-state preservation (#6821) (#6850)

* perf: keep gateway watcher fingerprint index-covered

* [verified] fix: preserve watcher state on projection failure

* Release exp-v0.52.179: keep gateway watcher fingerprint index-covered + preserve state on projection failure (#6821)

---------

Co-authored-by: StefanPieter <stefan@noble-pro.com>
Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>

CHANGELOG.md

@@ -27,6 +27,8 @@
 
 ### Fixed
 
+- **The gateway sidebar watcher no longer wipes session state on a transient database hiccup, and its 5-second poll is cheaper on large histories.** The watcher's every-5-second fingerprint aggregate read `m.role`, which forced table-page access for historical message rows even when the `(session_id, timestamp)` message index could otherwise cover the query — expensive on a large `state.db`. The hot fingerprint now stays index-covered (`COUNT(m.id)` + `MAX(m.timestamp)` keeps same-count rewrite detection), a query-plan check on a 1.07 GB database confirms `USING COVERING INDEX` (≈20.9 ms cold, 1.1–1.4 ms warm), and a bounded full projection every 60 seconds keeps role-only sidebar visibility changes eventually consistent. Separately, a transient projection failure is now distinguished from a genuinely empty result, so a database error preserves the prior sidebar state and every watcher checkpoint instead of blanking the list, and no empty initial snapshot is published before `state.db` exists. Thanks @stefanpieter. (#6821, #3506)
+
 - **A configured default theme/skin now actually reaches a brand-new user with no settings file.** `load_settings()` read the appearance keys through an `isinstance(stored, dict)` arm, but a *missing* `settings.json` yields `{}` (still a dict), so `stored.get("theme")` was `None` and normalization fell through to a hardcoded `dark`/`default` — making the configured `_SETTINGS_DEFAULTS["theme"]`/`["skin"]` unreachable for exactly the case they exist to serve. This was invisible on the stock `dark`/`default` values (both paths agree) but meant any deployment or fork that changed the appearance defaults still handed every new user `dark`. The fallback is now gated on whether an appearance key is actually stored (the pair, not per field — so a stored legacy theme with no skin keeps its migration instead of having the default skin injected), so a truly-empty profile gets the configured defaults. Pairs with #6808 (the boot script no longer fabricates an explicit choice, letting the server default win). Thanks @tomtong2015. (#6834, #6808)
 
 - **The profile dropdown no longer serves stale rows sooner than intended.** The 4-second cache behind the profile list stamped its timestamp *before* the profile-row build (an `os.walk` across every profile's skill tree) rather than after, so the TTL clock started early — the cache effectively expired sooner than 4s by however long the build took, and a build slower than the TTL could produce an entry that was already stale on arrival. The timestamp is now taken after the rows are built (still inside the single-flight lock), so the full TTL measures from when the data actually became valid. Thanks @starship-s. (#6846)

api/gateway_watcher.py

@@ -9,7 +9,6 @@ This enables real-time session list updates in the sidebar without
 requiring any changes to hermes-agent.
 """
 import hashlib
-import json
 import logging
 import os
 import queue
@@ -44,7 +43,7 @@ _WATCHER_EXCLUDED_SOURCES = ("cron", "webui")
 
 
 def _cheap_change_fingerprint(db_path: Path) -> str | None:
-    """Compute a cheap change-detection fingerprint without the messages JOIN.
+    """Compute a cheap fingerprint with an index-covered message aggregate.
 
     The expensive projection (``read_importable_agent_session_rows``) runs a CTE
     plus a per-session ``MAX(messages.timestamp)`` aggregation over an oversampled
@@ -53,25 +52,15 @@ def _cheap_change_fingerprint(db_path: Path) -> str | None:
     scan, and the watcher runs it forever on a 5s timer even when nothing changed
     (issue #3506).
 
-    This computes a fingerprint from a ``sessions``-table-only scan (no messages
-    JOIN), scoped to the same non-cron/webui rows as the projection. To guarantee
-    it never skips a change the projection would reflect, it hashes **every
-    sessions-table column the projection reads or uses for visibility/collapse**
-    -- not just the columns surfaced to the sidebar. That matters because the
-    projection collapses compression lineage and hides/shows rows based on
-    ``parent_session_id`` / ``ended_at`` / ``end_reason`` / ``source``, so a change
-    to one of those alters *which rows* appear even when no displayed field on a
-    given row moved.
+    This hashes every sessions-table column the projection uses, plus a
+    per-session ``COUNT`` / ``MAX(messages.timestamp)`` aggregate scoped to the
+    same non-cron/webui rows. The message aggregate stays on the agent's existing
+    ``(session_id, timestamp)`` covering index, avoiding a table-page lookup for
+    every historical message.
 
-    The one projection input that does not live in the ``sessions`` table is the
-    per-session message aggregate (``COUNT`` / ``MAX(messages.timestamp)`` ->
-    ``last_activity``). That is fully proxied by ``sessions.message_count``: the
-    agent's state layer bumps ``message_count`` on every appended message and
-    rewrites it to the absolute count on truncate/rewind/compaction, so a message
-    insert or delete (the only events that can move ``MAX(timestamp)``) always
-    changes ``message_count``. The fingerprint is therefore a strict superset of
-    the projection's change surface (it also fires on out-of-order inserts that
-    would not raise ``MAX(timestamp)``).
+    ``role`` is intentionally absent because it is not in that index. A bounded
+    periodic full projection in ``GatewayWatcher._poll_once`` covers rare
+    role-only visibility mutations without restoring the five-second table scan.
 
     Returns the fingerprint string, or ``None`` on any error / a pre-source
     schema so the caller falls back to running the expensive projection rather
@@ -110,21 +99,17 @@ def _cheap_change_fingerprint(db_path: Path) -> str | None:
             # timestamps but can leave sessions.message_count unchanged — so the
             # sessions-only scan above would miss it and the watcher would skip a
             # projection whose last_activity (MAX(messages.timestamp)) actually
-            # moved. Fold in a PER-SESSION message aggregate, scoped to the same
-            # non-excluded sessions as the projection. It must be per-session
-            # (grouped), NOT a single global MAX: rewriting an OLDER, non-newest
-            # session moves that session's last_activity but not the global max,
-            # so a global aggregate would still miss it (#3536 review round 2).
-            # cron/webui churn is excluded by the JOIN filter so it still does
-            # NOT trigger a re-projection. This is one GROUP BY over the already-
-            # filtered set — far cheaper than the projection's oversampled
-            # correlated CTE — so it preserves the cheap-fingerprint property.
+            # moved. Fold in a PER-SESSION COUNT/MAX aggregate, scoped to the same
+            # non-excluded sessions as the projection. COUNT preserves drift
+            # detection; MAX catches same-count rewrites because replacement rows
+            # receive fresh timestamps. Do not read ``role`` here: the normal
+            # (session_id, timestamp) index can then cover this five-second scan
+            # instead of forcing a table-page lookup for every historical row.
             if 'messages' in {r[0] for r in conn.execute(
                     "SELECT name FROM sqlite_master WHERE type='table'").fetchall()}:
                 try:
                     msg_rows = conn.execute(
                         "SELECT s.id, COUNT(m.id), "
-                        "COUNT(CASE WHEN LOWER(m.role) = 'user' THEN 1 END), "
                         "COALESCE(MAX(m.timestamp), 0) "
                         "FROM sessions s LEFT JOIN messages m ON m.session_id = s.id "
                         f"WHERE s.source IS NOT NULL AND s.source NOT IN ({placeholders}) "
@@ -157,9 +142,11 @@ def _get_state_db_path(hermes_home: Path | None = None) -> Path:
     return hermes_home / 'state.db'
 
 
-def _get_agent_sessions_from_db(db_path: Path | None = None) -> list:
+def _get_agent_sessions_from_db(db_path: Path | None = None) -> list | None:
     """Read all non-webui sessions from state.db.
-    Returns list of session dicts, or empty list on any error.
+
+    Returns a list of session dicts (including an empty list for a successful
+    empty projection), or ``None`` when the projection fails.
     """
     db_path = Path(db_path) if db_path is not None else _get_state_db_path()
     if not db_path.exists():
@@ -182,7 +169,7 @@ def _get_agent_sessions_from_db(db_path: Path | None = None) -> list:
             })
         return sessions
     except Exception:
-        return []
+        return None
 
 
 # ── GatewayWatcher ──────────────────────────────────────────────────────────
@@ -200,6 +187,11 @@ class GatewayWatcher:
     """
 
     POLL_INTERVAL = 5  # seconds between polls
+    # ``messages.role`` is not present in the agent's covering
+    # ``(session_id, timestamp)`` index, but the full projection uses it for CLI
+    # visibility. Keep the hot poll index-only and bound detection of rare
+    # role-only mutations with a periodic parity projection.
+    PROJECTION_PARITY_INTERVAL = 60.0
     SUBSCRIBER_TIMEOUT = 30  # seconds before sending keepalive comment
 
     def __init__(
@@ -226,6 +218,7 @@ class GatewayWatcher:
         # unchanged we skip the expensive messages-JOIN projection entirely
         # (issue #3506). Empty string forces the first poll to run the full read.
         self._last_cheap_fp: str = ''
+        self._last_full_projection_at: float | None = None
 
     def start(self):
         """Start the watcher daemon thread."""
@@ -317,34 +310,55 @@ class GatewayWatcher:
                 except Exception:
                     logger.debug("Failed to send sentinel to dead subscriber")
 
+    def _poll_once(self, *, now: float | None = None) -> bool:
+        """Run one change-detection pass and report whether projection ran.
+
+        Most passes stay on the covering fingerprint. A bounded parity pass
+        protects projection fields (notably role-derived CLI visibility) that
+        the agent's existing index cannot see.
+        """
+        db_path = self._state_db_path
+        # A watcher may start before the agent has created state.db. Publishing an
+        # empty first snapshot would make an already-rendered sidebar disappear;
+        # wait for the first real database instead. If a previously observed DB
+        # disappears, the normal projection path still publishes that change.
+        if (
+            not db_path.exists()
+            and self._last_full_projection_at is None
+            and not self._last_hash
+        ):
+            return False
+
+        cheap_fp = _cheap_change_fingerprint(db_path) if db_path.exists() else ''
+        current_time = time.monotonic() if now is None else now
+        fingerprint_changed = cheap_fp is None or cheap_fp != self._last_cheap_fp
+        parity_due = (
+            self._last_full_projection_at is None
+            or current_time - self._last_full_projection_at
+            >= self.PROJECTION_PARITY_INTERVAL
+        )
+        if not fingerprint_changed and not parity_due:
+            return False
+
+        sessions = _get_agent_sessions_from_db(db_path)
+        if sessions is None:
+            return False
+        current_hash = _snapshot_hash(sessions)
+        if cheap_fp is not None:
+            self._last_cheap_fp = cheap_fp
+        self._last_full_projection_at = current_time
+
+        if current_hash != self._last_hash:
+            self._last_hash = current_hash
+            self._last_sessions = sessions
+            self._notify_subscribers(sessions)
+        return True
+
     def _poll_loop(self):
         """Main polling loop. Runs in a daemon thread."""
         while not self._stop_event.is_set():
             try:
-                # Phase 1: cheap sessions-only fingerprint. The expensive
-                # messages-JOIN projection (_get_agent_sessions_from_db) only
-                # runs when this fingerprint actually changes, so an idle server
-                # with a large state.db stops re-aggregating tens of thousands
-                # of message rows every 5 seconds (issue #3506). A None
-                # fingerprint (error / unreadable db) forces the full read so we
-                # never silently skip a real change.
-                db_path = self._state_db_path
-                cheap_fp = _cheap_change_fingerprint(db_path) if db_path.exists() else ''
-                if cheap_fp is not None and cheap_fp == self._last_cheap_fp:
-                    # Nothing changed in the sidebar-visible session set; skip
-                    # the expensive projection and the notify entirely.
-                    pass
-                else:
-                    # Phase 2: only now pay for the full projection.
-                    sessions = _get_agent_sessions_from_db(db_path)
-                    current_hash = _snapshot_hash(sessions)
-                    if cheap_fp is not None:
-                        self._last_cheap_fp = cheap_fp
-
-                    if current_hash != self._last_hash:
-                        self._last_hash = current_hash
-                        self._last_sessions = sessions
-                        self._notify_subscribers(sessions)
+                self._poll_once()
             except Exception:
                 logger.debug("Error in gateway watcher poll loop", exc_info=True)
 

tests/test_gateway_sse_reconnect_dedupe.py

@@ -22,13 +22,12 @@ def _block(src: str, start: str, end: str) -> str:
 def test_gateway_watcher_remains_hash_only():
     """The watcher should not try to infer restarts from state.db mtime."""
     src = _read(GATEWAY_WATCHER)
-    poll = _block(src, "    def _poll_loop(self):", "\n_watchers:")
+    poll_once = _block(src, "    def _poll_once(", "\n    def _poll_loop")
 
     assert "_get_db_mtime" not in src
     assert "_detect_gateway_restart" not in src
-    assert "current_hash = _snapshot_hash(sessions)" in poll
-    assert "if current_hash != self._last_hash:" in poll
-    assert "_notify_subscribers(sessions)" in poll
+    assert "current_hash = _snapshot_hash(sessions)" in poll_once
+    assert "if current_hash != self._last_hash:" in poll_once
 
 
 def test_gateway_sse_dedupes_reconnect_snapshot_before_refresh():

tests/test_issue3506_memory_and_watcher.py

@@ -282,6 +282,169 @@ def test_cheap_fingerprint_detects_same_count_message_rewrite(tmp_path):
     )
 
 
+def test_cheap_fingerprint_message_aggregate_does_not_read_role_payload(
+    tmp_path, monkeypatch
+):
+    """The five-second fingerprint must stay on the covering session/timestamp
+    index. Reading ``role`` forces a table lookup for every message row and made
+    a 10 GB state.db fingerprint take tens of seconds even while WebUI was idle.
+    Role-only changes are deliberately handled by the bounded periodic full
+    projection, so this query can stay on the existing covering index.
+    """
+    gw = importlib.import_module("api.gateway_watcher")
+    db, conn = _make_db(tmp_path)
+    conn.execute(
+        "CREATE INDEX idx_messages_session ON messages(session_id, timestamp)"
+    )
+    conn.commit()
+    _add_session(conn, "s1", "telegram", mc=3)
+    statements = []
+    real_open = gw.open_state_db_readonly
+
+    def traced_open(path):
+        traced = real_open(path)
+        traced.set_trace_callback(statements.append)
+        return traced
+
+    monkeypatch.setattr(gw, "open_state_db_readonly", traced_open)
+
+    assert gw._cheap_change_fingerprint(db) is not None
+    aggregate = next(
+        statement
+        for statement in statements
+        if "LEFT JOIN messages" in statement
+    )
+    assert "m.role" not in aggregate.lower()
+    assert "COUNT(m.id)" in aggregate
+    assert "MAX(m.timestamp)" in aggregate
+    plan = conn.execute("EXPLAIN QUERY PLAN " + aggregate).fetchall()
+    assert any(
+        "COVERING INDEX idx_messages_session" in str(row[3]) for row in plan
+    ), plan
+
+
+def test_periodic_projection_recovers_role_only_sidebar_visibility_change(tmp_path):
+    """Role-only mutations must not remain invisible forever."""
+    gw = importlib.import_module("api.gateway_watcher")
+    db, conn = _make_db(tmp_path)
+    _add_session(conn, "cli1", "cli", mc=1, title="Untitled")
+    conn.execute(
+        "CREATE INDEX idx_messages_session ON messages(session_id, timestamp)"
+    )
+    conn.commit()
+
+    watcher = gw.GatewayWatcher(state_db_path=db)
+    subscriber = watcher.subscribe()
+    assert watcher._poll_once(now=1.0) is True
+    initial = subscriber.get_nowait()
+    assert [row["session_id"] for row in initial["sessions"]] == ["cli1"]
+    initial_fingerprint = watcher._last_cheap_fp
+
+    conn.execute("UPDATE messages SET role = 'assistant' WHERE session_id = 'cli1'")
+    conn.commit()
+    assert gw._cheap_change_fingerprint(db) == initial_fingerprint
+
+    before_deadline = 1.0 + watcher.PROJECTION_PARITY_INTERVAL - 1.0
+    assert watcher._poll_once(now=before_deadline) is False
+    assert subscriber.empty()
+
+    at_deadline = 1.0 + watcher.PROJECTION_PARITY_INTERVAL
+    assert watcher._poll_once(now=at_deadline) is True
+    event = subscriber.get_nowait()
+    assert event["sessions"] == []
+    assert subscriber.empty()
+
+
+def test_initial_missing_db_does_not_publish_an_empty_snapshot(tmp_path):
+    gw = importlib.import_module("api.gateway_watcher")
+    watcher = gw.GatewayWatcher(state_db_path=tmp_path / "missing.db")
+    subscriber = watcher.subscribe()
+
+    assert watcher._poll_once(now=1.0) is False
+    assert subscriber.empty()
+    assert watcher._last_full_projection_at is None
+
+
+def test_initial_projection_failure_does_not_publish_empty_and_retries(
+    tmp_path, monkeypatch
+):
+    gw = importlib.import_module("api.gateway_watcher")
+    db, conn = _make_db(tmp_path)
+    conn.close()
+    attempts = []
+
+    def fail_once_then_project_empty(*args, **kwargs):
+        attempts.append(True)
+        if len(attempts) == 1:
+            raise sqlite3.OperationalError("projection failed")
+        return []
+
+    monkeypatch.setattr(
+        gw, "read_importable_agent_session_rows", fail_once_then_project_empty
+    )
+    watcher = gw.GatewayWatcher(state_db_path=db)
+    subscriber = watcher.subscribe()
+
+    assert watcher._poll_once(now=1.0) is False
+    assert attempts == [True]
+    assert subscriber.empty()
+    assert watcher._last_sessions == []
+    assert watcher._last_hash == ""
+    assert watcher._last_cheap_fp == ""
+    assert watcher._last_full_projection_at is None
+
+    assert watcher._poll_once(now=2.0) is True
+    assert attempts == [True, True]
+    assert subscriber.get_nowait()["sessions"] == []
+    assert watcher._last_full_projection_at == 2.0
+
+
+def test_projection_failure_preserves_populated_state_and_parity_retry(
+    tmp_path, monkeypatch
+):
+    gw = importlib.import_module("api.gateway_watcher")
+    db, conn = _make_db(tmp_path)
+    _add_session(conn, "tg1", "telegram", mc=2)
+
+    watcher = gw.GatewayWatcher(state_db_path=db)
+    subscriber = watcher.subscribe()
+    assert watcher._poll_once(now=1.0) is True
+    initial_event = subscriber.get_nowait()
+    assert [row["session_id"] for row in initial_event["sessions"]] == ["tg1"]
+    initial_sessions = watcher._last_sessions
+    initial_hash = watcher._last_hash
+    initial_fingerprint = watcher._last_cheap_fp
+    real_projection = gw.read_importable_agent_session_rows
+    attempts = []
+
+    def fail_once_then_project(*args, **kwargs):
+        attempts.append(True)
+        if len(attempts) == 1:
+            raise sqlite3.OperationalError("projection failed")
+        return real_projection(*args, **kwargs)
+
+    monkeypatch.setattr(
+        gw, "read_importable_agent_session_rows", fail_once_then_project
+    )
+    at_deadline = 1.0 + watcher.PROJECTION_PARITY_INTERVAL
+
+    assert watcher._poll_once(now=at_deadline) is False
+    assert attempts == [True]
+    assert subscriber.empty()
+    assert watcher._last_sessions is initial_sessions
+    assert watcher._last_hash == initial_hash
+    assert watcher._last_cheap_fp == initial_fingerprint
+    assert watcher._last_full_projection_at == 1.0
+
+    assert watcher._poll_once(now=at_deadline + 1.0) is True
+    assert attempts == [True, True]
+    assert subscriber.empty()
+    assert watcher._last_sessions is initial_sessions
+    assert watcher._last_hash == initial_hash
+    assert watcher._last_cheap_fp == initial_fingerprint
+    assert watcher._last_full_projection_at == at_deadline + 1.0
+
+
 def test_cheap_fingerprint_detects_lineage_only_change(tmp_path):
     """Lineage/visibility fields the projection uses for collapse (parent_session_id,
     end_reason, ended_at) must be part of the fingerprint."""
@@ -363,39 +526,27 @@ def test_poll_loop_skips_projection_when_unchanged(tmp_path, monkeypatch):
     db, conn = _make_db(tmp_path)
     _add_session(conn, "tg1", "telegram", mc=2)
 
-    monkeypatch.setattr(gw, "_get_state_db_path", lambda: db)
+    projected = []
 
-    calls = {"n": 0}
-    real = gw._get_agent_sessions_from_db
+    def fake_projection(_path):
+        projected.append(True)
+        return [{"session_id": "tg1"}]
 
-    def counting(db_path=None):
-        calls["n"] += 1
-        return real(db_path)
+    monkeypatch.setattr(gw, "_get_agent_sessions_from_db", fake_projection)
+    w = gw.GatewayWatcher(state_db_path=db)
 
-    monkeypatch.setattr(gw, "_get_agent_sessions_from_db", counting)
+    assert w._poll_once(now=1.0) is True
+    assert projected == [True]
 
-    w = gw.GatewayWatcher()
-
-    # Run the change-detection body directly (one iteration) without the thread.
-    def one_iteration():
-        db_path = w._state_db_path
-        cheap_fp = gw._cheap_change_fingerprint(db_path) if db_path.exists() else ''
-        if cheap_fp is not None and cheap_fp == w._last_cheap_fp:
-            return
-        sessions = gw._get_agent_sessions_from_db(db_path)
-        if cheap_fp is not None:
-            w._last_cheap_fp = cheap_fp
-        _ = gw._snapshot_hash(sessions)
-
-    one_iteration()  # first poll: must read
-    assert calls["n"] == 1
-    one_iteration()  # unchanged: must skip
-    one_iteration()  # still unchanged: must skip
-    assert calls["n"] == 1, "expensive projection must not run while state is unchanged"
+    # Mutate only an excluded WebUI row: irrelevant churn must not trigger another
+    # expensive projection before the parity deadline.
+    _add_session(conn, "webui2", "webui", mc=99, title="WebUI run 2")
+    assert w._poll_once(now=2.0) is False
+    assert projected == [True]
 
     _add_session(conn, "tg1", "telegram", mc=3)  # a real change
-    one_iteration()
-    assert calls["n"] == 2, "expensive projection must run again after a real change"
+    assert w._poll_once(now=3.0) is True
+    assert projected == [True, True]
 
 
 def test_lru_eviction_skips_active_runs():