mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
* 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>
68 lines
3.6 KiB
Python
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)))
|