fix(session): retire stale truncation watermark on new committed turn (#3831) (#3890)

* fix(session): retire stale truncation watermark on new committed turn (#3831)

retry_last / undo_last / the Edit-truncate handler set truncation_watermark
to suppress the *replaced* tail from the append-only state.db merge.
Session.save() deliberately never auto-clears it (#2914), but nothing retired
it when the user then sent a genuinely NEW turn either — so it froze at the old
edit boundary. A frozen watermark then dropped post-watermark state.db rows
whenever the sidecar was later reconstructed empty (recovery/reconcile),
permanently losing the turns sent after the edit (state.db still had them).

Retire a POSITIVE watermark to None once the new user turn is COMMITTED to
session.messages — at the success-merge (3 sites), eager-checkpoint, error/
recovery materialization, and cold-load repair commit points. Not at chat-start:
in deferred mode the new row isn't in messages yet, so a merge in that window
would resurrect the replaced tail (the max-sidecar guard hasn't risen past the
old boundary). Once committed, max_sidecar_timestamp rises past the replaced
tail and the merge suppresses it without the watermark, so retiring is safe.

Cleared to None, never 0.0 — 0.0 is the truncate-to-empty sentinel (#2914) that
must keep blocking all state replay, so the clear is falsy-gated.

Closes #3831

* chore(changelog): clarify watermark-retirement timing to commit-time

Greptile review noted the original phrase "retires the watermark at the
start of a new user turn" was timing-imprecise. The retirement actually
fires when the new turn is durably committed to session.messages —
at the agent-result merge, the eager user-message checkpoint, or the
cold-load recovery commit. Reword for accuracy; semantics unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* test(#3831): add regression tests for the two inline watermark-clear paths (greptile P2)

Cover the error/cancel materialization path (_materialize_pending_user_turn_before_error)
and the eager first-turn checkpoint path (_checkpoint_user_message_for_eager_session_save),
which inline the falsy-gated watermark clear instead of calling the tested helper.
The error path is precisely the #3831 failure mode (recovery/reconcile after a
crash), so a dedicated regression test closes that gap. Both assert a positive
watermark clears to None while the 0.0 truncate-to-empty sentinel (#2914) is
preserved.

---------

Co-authored-by: nesquena-hermes <[email protected]>
Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

CHANGELOG.md

@@ -6,6 +6,7 @@
 ### Fixed
 
 - **Sidebar no longer stalls for seconds on a state.db that is missing the agent's message index.** The CLI-session scan orders candidate sessions by a correlated `MAX(timestamp)` subquery over `messages`, which collapses to a full per-session table scan when `idx_messages_session ON messages(session_id, timestamp)` is absent — stalling `/api/sessions` for several seconds on every refresh (the 5s cache TTL never settles, so repeated "Slow WebUI request still running" warnings appear). A normally-migrated hermes-agent state.db has this index, but a db that lost its migrations (older hermes-agent, or a hand-rebuilt/reimported db) does not. WebUI now primes the index with `CREATE INDEX IF NOT EXISTS` before the scan — a no-op when it already exists, and a ~20ms self-heal otherwise (measured 13.3s → 0.009s on a no-index 8k-session db). Best-effort: degrades silently on a read-only db, a locked db, or a minimal schema without a `timestamp` column. (#3887)
+- **A stale truncation watermark no longer hides messages sent after a retry/undo/edit.** `retry`, `undo`, and the Edit-truncate handler set a `truncation_watermark` to suppress the *replaced* tail from the append-only state.db merge, but nothing cleared it when the user then sent a genuinely new turn — so it froze at the old edit boundary. A frozen watermark then dropped the post-edit turns whenever the sidecar was later reconstructed empty (recovery/reconcile), permanently losing them from the WebUI transcript even though state.db still had the full data. WebUI now retires the watermark once the new turn is durably committed to `session.messages` — at the agent-result merge, the eager user-message checkpoint, or the cold-load recovery commit (the edit boundary is historical once a real turn lands). Cleared to `None`, distinct from the `0.0` truncate-to-empty sentinel, so the #2914 "don't replay a deleted transcript" behavior is unchanged. (#3831)
 
 ## [v0.51.341] — 2026-06-09 — Release LE (stale thinking-dot placeholder fix)
 

api/models.py

@@ -389,6 +389,12 @@ def _append_recovered_pending_turn(session, *, timestamp: int | None = None) ->
         recovered['attachments'] = list(session.pending_attachments)
     session.messages.append(recovered)
     _append_recovered_turn_to_context(session, recovered)
+    # The new user turn is now committed to messages (#3831): retire a positive
+    # truncation watermark from a prior retry/undo/edit so it can't freeze at the
+    # old edit boundary and drop these post-edit turns on a later empty-sidecar
+    # reconcile. None, never 0.0 (the truncate-to-empty sentinel, #2914).
+    if getattr(session, 'truncation_watermark', None):
+        session.truncation_watermark = None
     return recovered
 
 

api/routes.py

@@ -11630,6 +11630,15 @@ def _checkpoint_user_message_for_eager_session_save(s, msg: str, attachments, st
     if attachments:
         user_msg["attachments"] = list(attachments)
     s.messages.append(user_msg)
+    # The new user turn is now committed to messages (#3831): a positive
+    # truncation watermark from a prior retry/undo/edit has been superseded and
+    # must retire, else it freezes at the old edit boundary and later drops these
+    # post-edit turns on an empty-sidecar reconcile. Safe here (not at chat-start)
+    # because the row is durably in messages, so the merge's max-sidecar guard now
+    # suppresses the replaced tail without the watermark. Cleared to None — never
+    # 0.0, which is the truncate-to-empty sentinel (#2914).
+    if getattr(s, "truncation_watermark", None):
+        s.truncation_watermark = None
 
 
 def _is_default_or_empty_session_title(title) -> bool:

api/streaming.py

@@ -3817,6 +3817,23 @@ def _stream_writeback_can_supersede_recovery_marker(session, msg_text):
     return False
 
 
+def _retire_truncation_watermark_after_commit(session) -> None:
+    """Clear a positive truncation watermark once a new user turn is committed
+    to ``session.messages`` (#3831).
+
+    retry/undo/Edit set a positive watermark to suppress the *replaced* tail from
+    the append-only state.db merge; Session.save() deliberately never auto-clears
+    it (#2914). But nothing retired it when the user then sent a NEW turn, so it
+    froze at the old edit boundary and later dropped those post-edit turns on an
+    empty-sidecar reconcile. Once the new turn is durably in messages the merge's
+    max-sidecar guard suppresses the replaced tail without the watermark, so it is
+    safe — and necessary — to retire it here. Cleared to None, never 0.0 (the
+    truncate-to-empty sentinel that must keep blocking replay, #2914).
+    """
+    if getattr(session, 'truncation_watermark', None):
+        session.truncation_watermark = None
+
+
 def _merge_display_messages_after_agent_result(previous_display, previous_context, result_messages, msg_text):
     """Keep UI transcript durable while allowing model context to compact.
 
@@ -4412,6 +4429,14 @@ def _materialize_pending_user_turn_before_error(session) -> bool:
     if pending_attachments:
         recovered['attachments'] = list(pending_attachments)
     session.messages.append(recovered)
+    # The new user turn is now committed to messages (#3831): retire a positive
+    # truncation watermark left over from a prior retry/undo/edit so it cannot
+    # freeze at the old edit boundary and later drop these post-edit turns on an
+    # empty-sidecar reconcile. Cleared to None — never 0.0 (the truncate-to-empty
+    # sentinel, #2914). Safe here because the row is durably in messages, so the
+    # merge's max-sidecar guard suppresses the replaced tail without the watermark.
+    if getattr(session, 'truncation_watermark', None):
+        session.truncation_watermark = None
     return True
 
 
@@ -6444,6 +6469,7 @@ def _run_agent_streaming(
                     _restore_display_reasoning_metadata(_previous_messages, _result_messages),
                     msg_text,
                 )
+                _retire_truncation_watermark_after_commit(s)  # #3831
                 # Strip XML tool-call blocks from assistant message content.
                 # DeepSeek and some other providers emit <function_calls>...</function_calls>
                 # in the raw response text; this must be removed before the content is
@@ -6723,6 +6749,7 @@ def _run_agent_streaming(
                                     _restore_reasoning_metadata(_previous_messages, _result_messages),
                                     msg_text,
                                 )
+                                _retire_truncation_watermark_after_commit(s)  # #3831
                                 # Skip the error block — jump directly to the
                                 # normal post-result persistence path by
                                 # leaving _assistant_added truthy (set below).
@@ -7671,6 +7698,7 @@ def _run_agent_streaming(
                                     _restore_reasoning_metadata(_previous_messages, _result_messages),
                                     msg_text,
                                 )
+                                _retire_truncation_watermark_after_commit(s)  # #3831
                                 s.save()
                         logger.info('[webui] self-heal (except path): retry succeeded')
                         return  # skip error emission

tests/test_issue3831_watermark_clear.py

@@ -0,0 +1,172 @@
+"""Regression tests for #3831 — stale truncation_watermark data loss.
+
+Root cause: retry_last / undo_last / the Edit-truncate handler set
+``truncation_watermark`` to suppress the *replaced* tail from the append-only
+state.db merge. ``Session.save()`` deliberately does not auto-clear it (#2914),
+but nothing cleared it when the user then sent a genuinely new turn either — so
+it froze at the old edit boundary. A frozen watermark then dropped post-watermark
+state.db rows whenever the sidecar was later reconstructed empty
+(recovery/reconcile), permanently losing the turns sent after the edit.
+
+Fix: retire a *positive* watermark to ``None`` once the new user turn is
+COMMITTED to ``session.messages`` (the success-merge, eager-checkpoint, and
+recovery/cold-load commit points) — NOT at chat-start. Clearing at chat-start
+was unsafe: in deferred mode (the default) the new user row isn't in messages
+yet, so a merge in that window would resurrect the replaced tail (the
+max-sidecar guard hasn't risen past the old boundary). Once the row is committed,
+``max_sidecar_timestamp`` rises past the replaced tail and the merge suppresses
+it without the watermark, so retiring it is both safe and necessary.
+
+``None`` is distinct from ``0.0``: ``0.0`` is the truncate-to-empty sentinel
+(#2914) that must keep blocking all state replay, so the clear is falsy-gated and
+never touches ``0.0``.
+"""
+import api.models as models
+import api.streaming as streaming
+
+
+def _rows(*specs):
+    return [
+        {"role": role, "content": content, "timestamp": ts}
+        for (role, content, ts) in specs
+    ]
+
+
+class _FakeSession:
+    def __init__(self, watermark):
+        self.truncation_watermark = watermark
+        self.messages = []
+        self.context_messages = []
+        self.pending_user_message = None
+        self.pending_attachments = None
+        self.pending_started_at = None
+
+
+# --- The fix: a committed new turn retires a stale positive watermark ---------
+
+def test_retire_helper_clears_positive_watermark():
+    s = _FakeSession(100.0)  # stale, from a prior retry/undo/edit
+    s.messages = _rows(("user", "new turn", 200))
+    streaming._retire_truncation_watermark_after_commit(s)
+    assert s.truncation_watermark is None
+
+
+def test_retire_helper_leaves_zero_watermark_untouched():
+    """0.0 is the truncate-to-empty sentinel (#2914), not a stale boundary — the
+    falsy guard must leave it alone so #2914 replay-blocking is preserved."""
+    s = _FakeSession(0.0)
+    s.messages = _rows(("user", "new turn", 200))
+    streaming._retire_truncation_watermark_after_commit(s)
+    assert s.truncation_watermark == 0.0
+
+
+def test_retire_helper_noop_when_unset():
+    s = _FakeSession(None)
+    streaming._retire_truncation_watermark_after_commit(s)
+    assert s.truncation_watermark is None
+
+
+def test_recovery_commit_clears_watermark():
+    """The cold-load / recovery commit path (_append_recovered_pending_turn)
+    retires a stale positive watermark when it materializes the pending turn."""
+    s = _FakeSession(100.0)
+    s.pending_user_message = "recovered new turn"
+    s.pending_attachments = None
+    models._append_recovered_pending_turn(s, timestamp=200)
+    assert s.truncation_watermark is None
+    # The pending turn was committed to messages.
+    assert any(m.get("content") == "recovered new turn" for m in s.messages)
+
+
+# --- The effect: cleared (None) watermark stops dropping post-edit turns -------
+
+def test_cleared_watermark_keeps_post_edit_state_rows_with_empty_sidecar():
+    """Once the watermark is cleared (None), an empty-sidecar reconcile keeps the
+    genuinely-new post-edit turns instead of dropping them (the #3831 outcome)."""
+    state = _rows(
+        ("user", "q1", 50),
+        ("assistant", "a1", 100),     # old edit boundary
+        ("user", "q2-new", 200),      # sent AFTER the retry/edit
+        ("assistant", "a2-new", 300),
+    )
+    merged = models.merge_session_messages_append_only(
+        [], state, truncation_watermark=None
+    )
+    assert [m["content"] for m in merged] == ["q1", "a1", "q2-new", "a2-new"]
+
+
+# --- The guardrails: the fix must NOT regress #2914/#3102 ---------------------
+
+def test_active_watermark_still_filters_replaced_tail_empty_sidecar():
+    """A still-active (positive) watermark with an empty sidecar still suppresses
+    rows above the boundary — the replaced/edited tail (#2914). The fix only
+    changes WHEN the watermark is retired, never the merge semantics."""
+    state = _rows(
+        ("user", "q1", 50),
+        ("assistant", "a1", 100),
+        ("user", "replaced-q2", 200),   # above watermark -> filtered
+        ("assistant", "replaced-a2", 300),
+    )
+    merged = models.merge_session_messages_append_only(
+        [], state, truncation_watermark=100.0
+    )
+    assert [m["content"] for m in merged] == ["q1", "a1"]
+
+
+def test_zero_watermark_still_blocks_all_replay_empty_sidecar():
+    """A 0.0 watermark on a truncate-to-empty session must STILL block all state
+    replay (#2914) — unchanged by this fix."""
+    state = _rows(
+        ("user", "only prompt", 1.0),
+        ("assistant", "only reply", 2.0),
+    )
+    merged = models.merge_session_messages_append_only(
+        [], state, truncation_watermark=0.0
+    )
+    assert merged == []
+
+
+# --- Inline commit-path coverage (greptile): the two sites that inline the -----
+# --- falsy-gated clear instead of calling the helper must behave identically. --
+
+def test_error_path_materialize_clears_positive_watermark():
+    """The error/cancel materialization path (_materialize_pending_user_turn_before_error)
+    inlines the watermark clear. #3831 was triggered on recovery/reconcile after a
+    crash, so the error path is precisely the failure mode — lock it down: a pending
+    user turn committed on the error path retires a stale positive watermark."""
+    s = _FakeSession(100.0)  # stale, from a prior retry/undo/edit
+    s.pending_user_message = "new turn after edit"
+    appended = streaming._materialize_pending_user_turn_before_error(s)
+    assert appended is True
+    assert s.truncation_watermark is None
+    assert any(m.get("content") == "new turn after edit" for m in s.messages)
+
+
+def test_error_path_materialize_preserves_zero_sentinel():
+    """The error-path inline must use the same falsy guard as the helper: the 0.0
+    truncate-to-empty sentinel (#2914) is preserved, not cleared."""
+    s = _FakeSession(0.0)
+    s.pending_user_message = "new turn"
+    streaming._materialize_pending_user_turn_before_error(s)
+    assert s.truncation_watermark == 0.0
+
+
+def test_eager_checkpoint_clears_positive_watermark():
+    """The eager first-turn checkpoint path (_checkpoint_user_message_for_eager_session_save
+    in routes.py) inlines the same clear. A committed user turn retires a stale
+    positive watermark; the 0.0 sentinel is preserved."""
+    import api.routes as routes
+
+    s = _FakeSession(100.0)
+    routes._checkpoint_user_message_for_eager_session_save(
+        s, "eager new turn", None, started_at=200.0
+    )
+    assert s.truncation_watermark is None
+    assert any(m.get("content") == "eager new turn" for m in s.messages)
+
+    s0 = _FakeSession(0.0)
+    routes._checkpoint_user_message_for_eager_session_save(
+        s0, "eager new turn", None, started_at=200.0
+    )
+    assert s0.truncation_watermark == 0.0
+