Files
hermes-agent/hermes_state_api_content.py
20acba0376 fix(sessions): proactive prune rewrites only the changed rows, in place (#124273)
* fix(sessions): proactive prune rewrites only the changed rows, in place

A prune edits the content of a few existing messages and never adds, drops or
reorders one, yet every commit went through archive_and_compact, which
archives all active rows and re-inserts the full list: state.db grew by the
whole live transcript per commit (#124102).

SessionDB.rewrite_pruned_rows resolves each changed message to its live row
(_row_id, else its tool call id or tool-call ids, else the single active row
with the same role and stored content), checks the row still holds the
original, inserts one archived twin of the pre-prune row (active=0,
compacted=1, still searchable) and updates the changed columns under the same
id. The prune tries it after the stale-history watermark check and falls back
to archive_and_compact when a row cannot be named exactly or an unwritten
message sits before durable ones.

Fixes #124102

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

* fix(sessions): in-place prune twin keeps the live row's display slot

The content UPDATE fires messages_display_identity_update, which nulls
display_identity/display_order on the live row and on its freshly inserted
twin. The lazy backfill then gives the twin its own id as display_order, so
every demoted full body renders at the END of the transcript page, after the
newest user turn (3 x 24 KB bodies below "from desktop" in the probe).

Re-stamp both rows with the pre-update identity and order inside the same
transaction: the page projection shows one row per display_order (the live
stub, in place) and the twin stays a searchable archive. This is the same
stored-identity shape #117824 proposes for the archive writer (#117750), so
the resume projection collapses the pair once that lands.

Review-fix for #124273; one assertion added to the growth invariant
(red on the PR head: 20 page rows for 17 messages).

* fix(sessions): in-place prune keeps each display slot through later compaction and backfill

Review follow-up (ehz0ah): the rewritten live row kept the archived body's
identity, so a later archive_and_compact gave its copy a fresh slot and the
full body resurfaced in the old one. The live row now takes the identity of
what it stores and keeps the slot; the twin and any earlier generation's
copies (all nulled by the identity trigger) are restamped with their own
identity and slot, and the display backfill keeps a row's stored slot when
its stored identity is still correct. The prune pre-fills slots for legacy
rows so it never carries a missing one.

* fix(agent): fail closed when an in-place prune finds the row rewritten

rewrite_pruned_rows raised one PruneRowUnresolvedError for two different
conditions: the row cannot be named (ambiguity — the full writer handles it)
and the row IS named but its stored body changed. The second means another
surface already rewrote this generation, which is what StaleHeldHistory
exists to catch; the in-place prune preserves row ids, so
_archive_watermark_for cannot see it. Returning None there handed the stale
pruned_msgs to archive_and_compact, which archives rows it never compared
and republished the held text over a newer durable edit.

Split the stale case into a PruneRowStaleError subclass and raise
StaleHeldHistory from the in-place helper so the existing handler skips the
pass instead of committing. Ambiguity keeps the fallback.

Replaces the prune-fallback regression with the two cases review asked for:
a changed stored body skips the prune and leaves the newer body active; a
genuine ambiguity over current content still commits through the full writer.

* fix(agent): make an in-place prune visible to the other lease-less writer

rewrite_pruned_rows keeps every row id and activity flag, so the id/activity
staleness gate in held_archive_watermark cannot see that a prune already
committed. A second lease-less surface (micro-compaction) holding the pre-prune
transcript passes the gate and archive_and_compact republishes the pruned-away
bodies over the winner, undoing the prune.

stale_raises now also compares the held bodies against the stored ones via the
new SessionDB.rewritten_held_row_ids, so every lease-less writer sees the
in-place generation change, not just the prune caller that owns the exception.

* fix(state): compare durable row versions, not raw content, in the in-place stale guard

Reviewers on 631c897b converged on one root defect: the stale-write guard compared
raw SQLite bytes against a loaded/normalized view, which both missed real rewrites
and invented phantom ones.

- rewrite_pruned_rows._resolve: the staleness check is now the repo's stored-row CAS
  digest (transcript_row_snapshot over a SELECT * row, covering every owned column
  including assistant tool_calls arguments) when the held dict carries DB_ROW_SNAPSHOT,
  and the loader-lens identity compare (_row_identity, the same representation
  _stamp_kept_live_prefix uses) otherwise. Raw storage bytes are never compared
  against a loaded view.
- The rewritten row's replacement dict is stamped with the row's new digest, and the
  commit runs under _execute_transcript_write so a rollback restores the stamps.
- rewritten_held_row_ids takes full held dicts and applies the same two-leg compare;
  only tool/assistant rows (the roles the in-place writer touches) are compared.
- held_archive_watermark(stale_raises=True) checks the durable prefix BEFORE the
  ids[-1] is None early return, so an unstamped trailing message caps the watermark
  but can no longer suppress version checks for the rows the caller holds exactly.

Closes the P0 stale tool_calls path, the P2 false-stale on freshly resumed
whitespace-padded rows, and the unstamped-tail blind spot (#124102).

* fix(state): gate the lease-less stale check on every role the fallback republishes

The staleness gate behind held_archive_watermark (rewritten_held_row_ids)
compared only tool/assistant held rows, but the fallback it guards —
archive_and_compact, reached whenever the in-place rewrite refuses (an
unwritten message before later durable rows, an unnamed row) — re-inserts
held rows of every model-transcript role verbatim. A user row another view
rewrote in place (set_user_message_content, the repair batch's in-place
rewrite) slipped the gate: the stale pre-edit body was republished active
and the newer edit left with zero active copies.

Compare every model-transcript role (user/assistant/tool); the per-row
comparison (DB_ROW_SNAPSHOT CAS, else the loader-lens identity) is already
role-agnostic. Bookkeeping roles stay excluded — they never reach the
model-facing list. Covers the prune fallback and micro-compaction, the two
lease-less callers of _archive_watermark_for (#124102).

Sabotage-run: reverting the role filter makes
test_a_concurrent_user_edit_stays_active_when_the_prune_falls_back fail
(stale body published); restored, the file is 22/22 green.

* style: use builtin generics in the in-place prune commit annotations

* refactor(compression): split the proactive prune and its commit into topical siblings

The code-health ratchet (CI) flags this PR's own growth: hermes_state_messages.py
(2190 > 2000), agent/context_compressor.py (5911 > its main value 5852) and
agent/conversation_compression.py (4585 > 4564). The remedy the repo prescribes:
move the PR's own code into topical siblings, keeping the facade as the seam.

- agent/context_compressor_prune.py: ProactivePruneMixin (prune_tool_results_only
  + _rewrite_pruned_rows_in_place), bound into ContextCompressor; origin-module
  symbols read through _cc() at call time (the micro_compaction.py pattern) so
  tests patching agent.context_compressor.X stay effective; logger keeps the
  agent.context_compressor name for caplog parity.
- agent/conversation_compression_watermark.py: held_archive_watermark; the origin
  module imports it and keeps _held_watermark (its only caller).
- hermes_state_prune_rewrite.py: SessionPruneRewriteMixin (rewrite_pruned_rows +
  rewritten_held_row_ids), bound into SessionDB; every SessionDB call site and
  test patch seam (db.rewrite_pruned_rows / db.rewritten_held_row_ids) unchanged.

Verified: scripts/check --only health --base origin/main -> 0 blocking; ruff clean;
prune/compaction batches green (test_proactive_prune_restart_safety 22/22,
test_micro_compaction 48/48, five proactive-prune files 25/25). The 4 failures in
test_compaction_prompt_rebuild / test_compression_review reproduce identically on
origin/main (pre-existing).

* fix(state): re-verify held rows inside the full writer's commit transaction

The lease-less stale gate (rewritten_held_row_ids) runs outside the write
transaction, so a second SessionDB connection that rewrites a covered row
AFTER the precheck but BEFORE archive_and_compact's BEGIN IMMEDIATE was
invisible to it: the commit archived the newer row and republished the stale
held text, leaving the newer edit with zero active copies (#124102, review
finding on 8c6da464).

- archive_and_compact takes held_for_cas: the caller's held dicts,
  re-verified against the live rows INSIDE the commit's transaction before
  any row moves; a mismatch raises HeldRowRewrittenError, which the two
  lease-less callers (proactive prune fallback, micro-compaction sync)
  treat exactly like StaleHeldHistory - nothing written, the pass is a
  no-op and the newer generation keeps the session. The lease path leaves
  it None: its lease rules out an overlapping rewrite.
- held_row_mismatches(conn, ...) is the compare lifted out of
  rewritten_held_row_ids so both the read-path precheck and the
  in-transaction fence share one implementation. It now chunks the id IN
  query via _id_chunks (a history past SQLite's host-parameter limit
  failed the pass instead of reclaiming) and resolves rows that have no
  _row_id but do carry the persist marker + CAS snapshot by their logical
  message_uid (idx_messages_session_uid) - the unaddressed live-replay
  loader shape, whose rows were previously exempt from the fence entirely.
- regression: test_a_user_edit_landing_after_the_precheck_stays_active_in_the_commit
  drives a second SessionDB connection's set_user_message_content at the
  archive_and_compact call boundary; sabotage (gate disabled) makes it
  fail exactly as the finding predicts.

* refactor(compression): drop the duplicate _held_watermark from the module split

The 8c6da464 sibling extraction left two identical _held_watermark
definitions back to back in agent/conversation_compression.py; Python
silently kept only the second, so editing the first had no effect (dead
code, root AGENTS.md). Remove the added copy.

* refactor(state): split the api_content sidecar writers into a topical sibling

hermes_state_messages.py crossed the FILE_LINES ratchet (2010 > 2000) after
the commit-window fence landed in it. Move set_latest_user_api_content /
set_message_api_content into hermes_state_api_content.py
(SessionApiContentMixin, bound into SessionDB in the same MRO slot): the
two row-addressed api_content sidecar UPDATEs are one topic, no behaviour
change, every call site and test patch seam unchanged.

* fix(state): flatten _archive_and_compact_on_conn onto the caller's transaction after the main merge

Main flattened this helper to run directly on the caller's write connection
(no nested _execute_transcript_write); the merge resolution had kept the old
nested shape, which opened a second BEGIN IMMEDIATE inside the live one and
deadlocked every archive_and_compact. Re-flattened while keeping the #124102
held_for_cas in-transaction CAS fence.

* fix(state): carry the held-row CAS fence through the worker RPC path

RuntimeSessionCompressionMixin.archive_and_compact rejected held_for_cas,
so a worker-side micro-compaction commit raised TypeError inside the mixin's
generic except and silently skipped the whole persist (unstamped marker,
durable transcript left un-compacted). Accept held_for_cas, project it onto
the closed handoff fields plus the persistence marker the CAS compare reads,
validate it in worker_archive, and thread it into
archive_on_connection -> _archive_and_compact_on_conn (#124102).

* test(micro): a stale held generation without row ids aborts the micro-compact

Review follow-up (ehz0ah/greptile-apps, PR #124273): ACP restore and OpenAI
session continuation load through the unaddressed live-replay projection
(get_messages_as_conversation without include_row_ids=True) — the persist
marker, durable message_uid and the stored-row CAS digest, but no _row_id.
held_row_mismatches already resolves that shape by logical message_uid, so
the lease-less fence still sees another surface's in-place prune. Pin it with
the regression both reviewers asked for: after another surface prunes in
place, micro-compaction from that stale no-row-id history is a true no-op —
the pruned stubs stay the only live generation and the pre-prune bodies are
not republished (#124102).

Sabotage: disabling the by-uid branch in held_row_mismatches fails this
test (the sync republishes the stale bodies); restored, 49/49 pass.

Co-authored-by: ehz0ah <ehz0ah@users.noreply.github.com>

* fix(sessions): the uid-resolved stale fence CASes the newest active row

held_row_mismatches resolved an id-less held dict by its logical
message_uid with a bare fetchone(): when a carried or concurrent
compaction has cloned the row back beside its archived twin, one uid
names several active physical rows, and the uid index returns them
oldest-first. The fence then CASed the oldest copy — which still held
the payload the held digest was stamped from — and cleared a held
generation as current although another writer had rewritten the newest
row; the lease-less fallback republished the stale held payload over
that newer generation (#124102).

Resolve with the same newest-active rule as the loader's
_active_logical_message_row (ORDER BY id DESC LIMIT 1). Two
regressions pin it: two active rows sharing a uid where only the
newest was rewritten in place must abort the micro-compact naming the
newest row id, and where only an older duplicate moved the held
digest must stay current.

Review follow-up to greptile-apps' 2026-10-17 P1 on PR #124273.

---------

Co-authored-by: defurniture2025 <301948806+defurniture2025@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: teknium1 <127238744+teknium1@users.noreply.github.com>
Co-authored-by: Austin Pickett <pickett.austin@gmail.com>
Co-authored-by: Austin Pickett <austinpickett@users.noreply.github.com>
Co-authored-by: ehz0ah <ehz0ah@users.noreply.github.com>
2026-10-18 19:44:32 -04:00

68 lines
3.6 KiB
Python

"""The ``api_content`` sidecar backfill writers for SessionDB (#124102 FILE_LINES split).
Split out of :mod:`hermes_state_messages`: the two row-addressed sidecar UPDATEs the turn prologue
uses when the current turn's user row was materialized before the sidecar could be composed. Bound
into ``SessionDB`` via this mixin, so every existing ``session_db.set_message_api_content(...)``
call site and test patch seam keeps working unchanged.
"""
from __future__ import annotations
from typing import Any
class SessionApiContentMixin:
"""``SessionDB`` mixin: the ``api_content`` sidecar backfills."""
def set_latest_user_api_content(self, session_id: str, content: Any, api_content: str) -> int:
"""Backfill the ``api_content`` sidecar onto the newest ACTIVE user row (0/1 rows). Preflight compaction
inserts that row BEFORE the sidecar exists and the later persist identity-skips compacted dicts;
without this a reload reopens the prompt-cache divergence. ``content`` match guards a racing rewrite.
POSITIONAL, and only safe when the caller already knows the newest
active user row IS the message it stamped. The content match is NOT
sufficient on its own: repeated identical user turns ("ok", "y",
"continue") make an OLDER row compare equal, so calling this before
the current turn's row exists overwrites the previous turn's sidecar
with this turn's bytes — durable wrong-bytes replay, a worse cache
break than the missing sidecar. When the caller holds the durable row
id (``_row_id``, synced onto the live dict by
``sync_flushed_message_markers`` and stamped by
:meth:`_insert_message_rows`), use :meth:`set_message_api_content`
instead — it addresses the exact row and cannot land on a neighbour.
"""
from hermes_state_messages import _scrub_surrogates
return self._write_rowcount(
"UPDATE messages SET api_content = ? WHERE id = (SELECT id FROM messages "
"WHERE session_id = ? AND role = 'user' AND active = 1 ORDER BY id DESC LIMIT 1"
") AND content IS ?",
(_scrub_surrogates(api_content), session_id, self._encode_content(content)))
def set_message_api_content(
self, session_id: str, row_id: int, content: Any, api_content: str
) -> int:
"""Backfill the ``api_content`` sidecar onto ONE known durable row.
Row-addressed counterpart to :meth:`set_latest_user_api_content`: the
caller passes the ``_row_id`` the write path stamped on the live
message dict, so the update cannot drift onto a neighbouring row that
merely carries the same text.
Used by the turn prologue whenever the current turn's user row was
already materialized before the sidecar could be composed (in-place
preflight compaction, a close/early flush that raced the prologue).
The crash persist then marker-skips that message, so this is the only
way the stamped bytes reach the store.
``active = 1`` and the ``content`` match stay as defensive guards: a
row the compaction archived, or one a racing rewrite changed, is left
untouched.
"""
from hermes_state_messages import _scrub_surrogates
if not session_id or isinstance(row_id, bool) or not isinstance(row_id, int) or row_id <= 0:
return 0
return self._write_rowcount(
"UPDATE messages SET api_content = ? WHERE id = ? AND session_id = ? "
"AND role = 'user' AND active = 1 AND content IS ?",
(_scrub_surrogates(api_content), row_id, session_id, self._encode_content(content)))