fix: stamp _db_persisted at row load time so resumed transcripts never re-append (#92231)

[kshitijk4poor]

Summary

Resumed sessions no longer re-append their entire loaded transcript to state.db — message dicts are stamped _db_persisted at row load time, closing the row-amplification bug class behind #92231 (one incident session reached 15,962 active rows with only 472 distinct contents, doubling per compression cycle: 998 → 1,995 → 3,990 → 7,981).

Root cause: SessionDB.get_messages_as_conversation / get_resume_conversations returned plain dicts WITHOUT _DB_PERSISTED_MARKER. The flush dedup in _flush_messages_to_session_db only knew loaded rows were durable when the caller threaded the exact same list object through as conversation_history= (identity match). Every identity-losing handoff — compression's durable-snapshot adoption (which re-loads a fresh list), incremental tool-call persists that pass no history, rotation preflight on a cold resume — treated the whole loaded transcript as new and re-inserted it. Aborted rotations then compounded the copies.

Fix (architectural, one chokepoint): stamp the marker in SessionDB._rows_to_conversation, the shared row→dict conversion behind both loaders. A dict materialized FROM a durable row is durable by construction — no caller needs to know about identity boundaries anymore. This is the same pattern as the existing _row_id stamp in the same function.

Changes

• hermes_state.py: define _DB_PERSISTED_MARKER_KEY (local literal — importing run_agent would be circular) and stamp it per row in _rows_to_conversation.
• tests/agent/test_load_time_durability_stamp_92231.py (new): marker-literal sync guard across the three defining modules, load-stamp assertions for both loaders, 3-cycle identity-lost flush amplification repro, new-tail-still-writes guard, compaction-copy child-handoff guard.
• tests/agent/test_session_rotation_flush_cold_resume_68454.py: the control test asserting the old double-write is now asserting idempotence (the "wrong" call shape is now safe).
• tests/acp/test_session.py: restore-shape test pops the new marker like it already pops timestamp.
• tests/test_message_reactions.py: the underscore-key allow-list test now sanctions _db_persisted alongside _row_id (same transport-strip contract).

Why this is safe

• Wire safety: every transport strips underscore-prefixed keys before the API request (chat_completion_helpers.py _clean_message_for_api, anthropic_adapter.py key filter) — same contract _row_id relies on today.
• Rotation handoffs still write: compression's assembly copies strip the marker (_fresh_compaction_message_copy + the terminal _strip_persistence_markers sweep in compress()), so compacted transcripts still flush to the child session — the Rotation compression: _db_persisted marker copied via messages[i].copy() skips bulk flush to child session (cached gateway regression since #50372) #57491 invariant is preserved and covered by a new test.
• Branch/seed copies unaffected: /branch and the TUI _persist_branch_seed build fresh field-projected dicts and write via append_messages_batch directly — they never pass loaded dicts through the marker-gated flush.
• turn_finalizer's marker pop (fill-empty-tail rewrite) still works: it operates on the live dict and invalidates the flush scan cursor, unchanged.
• Staged CLI input: _pending_cli_user_message is always a freshly constructed dict, never a loaded one — its _db_persisted gates keep their semantics.

Validation

Check	Result
New regression suite (5 tests)	pass
3-agent Phase 2c review (reuse/quality/invariants)	1 must-fix found+fixed (reactions allow-list test); wire-strip verified on all 3 transports; repair-merge interaction verified pre-existing-safe
tests/agent/ + tests/test_hermes_state.py (5,379 tests)	failure set byte-identical to origin/main baseline (189 pre-existing env failures, 0 new)
Targeted: rotation-boundary #68196, concurrent fork, ACP, reactions, hermes_state (320 tests)	pass
E2E (real AIAgent + real SQLite, temp HERMES_HOME): 3 identity-lost reload+flush cycles	rows stayed 12 → [12, 12, 12]; genuine new tail still writes (→14); re-flush idempotent (14)
ruff on changed files	clean

Fixes #92231

Review-pass follow-up (commit 2)

Full /hermes-pr-review Phase 2 (2a/2b/2c) + /simplify-code ran on the final diff; three verified findings folded as a follow-up commit:

• Literal consolidation: hermes_state.py now imports _DB_PERSISTED_MARKER from agent.context_compressor (the "can't import agent layer" premise was wrong — hermes_state already imports agent.* at module level; only run_agent is circular). agent/turn_finalizer.py's raw "_db_persisted" string (previously outside the drift guard) now uses the shared constant. test_marker_constant_in_sync extended with identity assertions + the turn_finalizer surface.
• Marker-insensitive no-op check: with load-stamping, a cold-resumed stamped snapshot vs a marker-swept compress() no-op copy would compare unequal under raw == and misclassify "no progress" as progress. The commit-layer check now falls back to _strip_marker_for_comparison on both sides; raw == still runs first so engine-returned list subclasses keep their __eq__ semantics. New regression test, mutation-checked.

Follow-up validation: 334 targeted tests green (PR suites + hermes_state + turn_finalizer + micro-compaction); compression/compaction/finalizer sweep 269/269; both mutation checks fail-on-mutation/green-on-restore; ruff clean.

Second review round (commits 3-4)

Distinct 3-parallel-reviewer /simplify-code pass on the full final diff + reviewer-comment triage:

• docs: the mutate-then-persist contract (in-place content mutation of a stamped dict ⇒ pop marker + invalidate flush-scan prefix) is now stated on _DB_PERSISTED_MARKER itself, addressing the review comment's silent-staleness concern. Both existing mutators already comply.
• test: dropped two vacuous is identity assertions from the sync guard (CPython interns identifier-like literals, so is cannot detect a copy-pasted literal — verified empirically); the == assertions are the full honest guard.
• Efficiency reviewer verified the two-leg no-op comparison is immaterial (runs once per compression attempt, adjacent to a multi-second summarization LLM call); reuse reviewer: no material findings; function-local _DB_PERSISTED_MARKER import verified cycle-required (context_compressor transitively loads conversation_compression).

Competing PRs #92269 and #92272 closed with credit — both scoped narrower than the chokepoint stamp (live-replay-only marking; per-call-site baseline threading).

Infographic

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

Strong root-cause fix: stamping durability at materialization time (hermes_state.py @@ -11098) removes an entire class of identity-losing handoff bugs instead of patching each call site, and the test suite is exemplary — constant-sync guard, amplification regression, over-skip guard, and the rotation control test flipped from documenting the double-write to forbidding it.

Two residual risks worth addressing:

1. In-place mutation after load: the marker now asserts "this dict's content is durable," but nothing enforces it. Any path that loads rows via get_messages_as_conversation, mutates content in memory (steer injection, post-hoc redaction, summary substitution), and later flushes will be skipped as already-persisted — leaving the DB stale with no error. The steer path pops the marker manually, but a comment on _DB_PERSISTED_MARKER_KEY making the "mutate ⇒ pop or UPDATE" contract explicit (and ideally a debug-level log when a flush skips a row whose in-memory content differs from what was written) would prevent the next incident from being silent.

2. Every consumer of these loaders now receives the _db_persisted key. The tests pin transport stripping for gateway/reactions paths, but export/trajectory/ACP serializers that JSON-dump loader output directly would start leaking internal keys. A quick audit of json.dumps(get_messages_as_conversation(...))-style call sites — or stripping non-opt-in underscore keys centrally — would close that.

Also remember to add any future duplicate constant definitions to test_marker_constant_in_sync.

[kshitijk4poor]

Thanks — both points were verified and are addressed:

1. Mutate ⇒ pop contract: now documented explicitly on _DB_PERSISTED_MARKER in run_agent.py ("the marker asserts this dict's CONTENT is durable as written; mutate-in-place + persist ⇒ pop the marker and invalidate _db_flush_scan_prefix"), pointing at the two canonical pop sites (turn_finalizer fill-empty-tail, context_compressor micro-compaction defrag). Both existing in-place mutators already follow the contract.

2. Serializer key-leak audit: done as part of the review pass — all 12 consumers of get_messages_as_conversation/get_resume_conversations were enumerated and classified. Every JSON-serializing surface is safe: sessions export (JSONL) and the web /export endpoint use raw get_messages() rows (different, unstamped path); trace export/upload rebuilds explicit fields via build_trace_jsonl; ACP replay payloads are constructed from role/content. The only surface that ships the key is the desktop display projection that already opts into _row_id — same sanctioned-underscore contract, client ignores unknown keys, and live flushed dicts already carried the stamp pre-PR.

The constant-sync guard was also extended in the follow-up commit: hermes_state and turn_finalizer now import the constant from context_compressor (single source), leaving run_agent's predating copy as the only synced literal, with identity assertions in test_marker_constant_in_sync.