Skip to content

fix(agent): invalidate flush-scan cursor when finalizer pops db marker - #75170

Closed
spfcraze wants to merge 1 commit into
NousResearch:mainfrom
spfcraze:fix/flush-cursor-marker-pop
Closed

fix(agent): invalidate flush-scan cursor when finalizer pops db marker#75170
spfcraze wants to merge 1 commit into
NousResearch:mainfrom
spfcraze:fix/flush-cursor-marker-pop

Conversation

@spfcraze

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the delivered final response silently going missing from the durable transcript when a turn ends on a pure tool-call tail.

Two mechanisms collide:

  1. finalize_turn's tail fill (agent/turn_finalizer.py): when the transcript tail is a pure tool-call assistant row with empty content but a final_response was delivered, it fills that row's content in place and pops _db_persisted — explicitly so the next _persist_session re-writes the filled content (the fix for Gateway can deliver assistant response without persisting assistant row #43849/Telegram: assistant responses not persisted to session DB (model re-answers old messages) #44100).
  2. The bounded flush scan (run_agent.py:_flush_messages_to_session_db_unlocked, perf commit 30c783589): skips the identity-matched prefix of the previous flush's snapshot, on the documented assumption that "no code path pops _DB_PERSISTED_MARKER from a live dict in place."

That assumption is false — the tail fill is the one place that does. The filled row is the same dict object flushed mid-turn, so the identity match skips it and the marker pop has no effect: the user sees the answer, but state.db keeps content="" and /resume replays an unanswered backlog, so the model re-answers — the exact #43849/#44100 symptom, resurfacing via the perf cursor.

The fix invalidates the cursor at the pop site (agent._db_flush_scan_prefix = None), so the filled row is re-examined. The perf optimization is untouched everywhere else; the append-only flush design is preserved (the re-persist appends the filled row, same as pre-cursor behavior).

Related Issue

No GitHub issue — discovered via code review and reproduced live (see below). Happy to file one first if preferred.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • agent/turn_finalizer.py: after popping _db_persisted in the tail fill, invalidate agent._db_flush_scan_prefix (with a comment explaining the interaction).
  • tests/agent/test_turn_finalizer_final_response_persistence.py: new test — the fill path invalidates the cursor (and pops the marker) on a pure tool-call tail.
  • tests/run_agent/test_identity_flush.py: new end-to-end test — incremental flush stamps the tool-call row; after fill + pop + invalidation, the turn-end flush persists the filled answer to a real SessionDB.

How to Test

Reproduction (against pre-fix code, real AIAgent + real SessionDB):

agent._flush_messages_to_session_db_unlocked(messages)   # mid-loop: tool-call row written, stamped, cursor set
messages[0]["content"] = "the final answer"              # what finalize_turn's fill does
messages[0].pop("_db_persisted", None)
agent._flush_messages_to_session_db_unlocked(messages)   # turn-end persist
# pre-fix:  DB contents are ['', 'next question']  — the answer never lands
# post-fix: DB contents are ['', 'the final answer', 'next question']

Focused validation completed:

  1. bash scripts/run_tests.sh tests/agent/test_turn_finalizer_final_response_persistence.py tests/run_agent/test_identity_flush.py — 9/9 pass.
  2. Sabotage check: stashing the agent/turn_finalizer.py change makes exactly the cursor-invalidation test fail (8 pass); restoring returns to 9/9.
  3. Wider suites: all tests/agent/test_turn_finalizer_* + the full tests/run_agent/ directory — 1278/1278 pass.
  4. Full repo-wide suite, bash scripts/run_tests.sh (branch): 22,817 pass / 100 fail. Baseline on clean main (this branch's parent), same machine: 22,816 pass / 99 fail — failing sets identical except tests/honcho_plugin/test_pin_peer_name.py, which fails identically standalone on clean main (pre-existing flake, no agent-persistence imports). Nothing in the persistence/finalizer surface fails.
  5. uvx --from ruff==0.15.10 ruff check agent/turn_finalizer.py tests/agent/test_turn_finalizer_final_response_persistence.py tests/run_agent/test_identity_flush.py — clean.
  6. git diff --check — clean.
  7. Adversarial cases executed: pre-cursor behavior parity (the re-persist appends the filled row, matching the append-only design); marker-pop without fill (no tail fill) is unaffected; the existing test_final_response_fills_pure_tool_call_tail still passes unchanged.

The full repo-wide suite was run locally (item 4) with results verified against clean main; GitHub CI remains the final confirmation environment.

Checklist

Code

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A (inline comment documents the cursor/marker interaction)
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — no platform surface
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Logs

Sabotage verification output:

# fix stashed (pre-fix behavior):
=== Summary: 2 files, 8 tests passed, 1 failed ===   (the cursor-invalidation test)
# fix restored:
=== Summary: 2 files, 9 tests passed, 0 failed ===

The bounded flush-scan in _flush_messages_to_session_db_unlocked skips
the identity-matched prefix of its previous snapshot, on the documented
assumption that no code path pops _DB_PERSISTED_MARKER from a live dict
in place. finalize_turn's pure-tool-call-tail fill is exactly that path:
it pops the marker so the filled content gets re-persisted — but the
cursor then skips the row anyway, so the delivered final response never
reaches state.db and /resume replays content="" (the NousResearch#43849/NousResearch#44100
class resurfacing via the perf cursor). Invalidate the cursor at the
pop site so the filled row is re-examined.
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 31, 2026
@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 31, 2026
teknium1 added a commit that referenced this pull request Aug 1, 2026
…ling site

The micro-compaction defrag pass (_defrag_rolling_summary) rewrites the
newest MICRO marker's content and pops _DB_PERSISTED_MARKER from the
LIVE dict in place — the same in-place pop class finalize_turn's fill
site was fixed for in #75170. Without invalidation the bounded
flush-scan cursor identity-skips the rewritten marker row and the
defragged rolling summary never reaches state.db (resume rehydrates a
stale summary).

The compressor holds no agent reference, so the pop site raises
_flush_scan_cursor_invalidated and the finalize_turn micro-compaction
block consumes it, setting agent._db_flush_scan_prefix = None.

The module-scope pop sites (context_compressor.py:175/224) operate on
fresh copies — identity-breaking by construction — and need no flag.

Follow-up to #75170 (fix-the-class sweep of _DB_PERSISTED_MARKER
in-place pops).
teknium1 added a commit that referenced this pull request Aug 1, 2026
…ling site

The micro-compaction defrag pass (_defrag_rolling_summary) rewrites the
newest MICRO marker's content and pops _DB_PERSISTED_MARKER from the
LIVE dict in place — the same in-place pop class finalize_turn's fill
site was fixed for in #75170. Without invalidation the bounded
flush-scan cursor identity-skips the rewritten marker row and the
defragged rolling summary never reaches state.db (resume rehydrates a
stale summary).

The compressor holds no agent reference, so the pop site raises
_flush_scan_cursor_invalidated and the finalize_turn micro-compaction
block consumes it, setting agent._db_flush_scan_prefix = None.

The module-scope pop sites (context_compressor.py:175/224) operate on
fresh copies — identity-breaking by construction — and need no flag.

Follow-up to #75170 (fix-the-class sweep of _DB_PERSISTED_MARKER
in-place pops).
teknium1 added a commit that referenced this pull request Aug 1, 2026
…ling site

The micro-compaction defrag pass (_defrag_rolling_summary) rewrites the
newest MICRO marker's content and pops _DB_PERSISTED_MARKER from the
LIVE dict in place — the same in-place pop class finalize_turn's fill
site was fixed for in #75170. Without invalidation the bounded
flush-scan cursor identity-skips the rewritten marker row and the
defragged rolling summary never reaches state.db (resume rehydrates a
stale summary).

The compressor holds no agent reference, so the pop site raises
_flush_scan_cursor_invalidated and the finalize_turn micro-compaction
block consumes it, setting agent._db_flush_scan_prefix = None.

The module-scope pop sites (context_compressor.py:175/224) operate on
fresh copies — identity-breaking by construction — and need no flag.

Follow-up to #75170 (fix-the-class sweep of _DB_PERSISTED_MARKER
in-place pops).
@teknium1

teknium1 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Merged via salvage PR #75883 (#75883) — your commit cherry-picked with authorship preserved, plus a follow-up applying the same cursor invalidation at the sibling in-place marker-pop site in context_compressor's defrag path (fix-the-class). The sabotage-verified test methodology in your PR made verification easy. Thanks!

randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…ling site

The micro-compaction defrag pass (_defrag_rolling_summary) rewrites the
newest MICRO marker's content and pops _DB_PERSISTED_MARKER from the
LIVE dict in place — the same in-place pop class finalize_turn's fill
site was fixed for in NousResearch#75170. Without invalidation the bounded
flush-scan cursor identity-skips the rewritten marker row and the
defragged rolling summary never reaches state.db (resume rehydrates a
stale summary).

The compressor holds no agent reference, so the pop site raises
_flush_scan_cursor_invalidated and the finalize_turn micro-compaction
block consumes it, setting agent._db_flush_scan_prefix = None.

The module-scope pop sites (context_compressor.py:175/224) operate on
fresh copies — identity-breaking by construction — and need no flag.

Follow-up to NousResearch#75170 (fix-the-class sweep of _DB_PERSISTED_MARKER
in-place pops).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants