Skip to content

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

Open
spfcraze wants to merge 1 commit into
mainfrom
fix/flush-cursor-marker-pop
Open

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

Conversation

@spfcraze

@spfcraze spfcraze commented Jul 31, 2026

Copy link
Copy Markdown
Owner

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 NousResearch/hermes-agent#43849/Telegram: assistant responses not persisted to session DB (model re-answers old messages) NousResearch/hermes-agent#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 NousResearch#43849/NousResearch#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.
spfcraze pushed a commit that referenced this pull request Aug 10, 2026
…lves

A session title had no notion of who set it, so two bugs followed. An
auto-generated title could clobber a name the user typed, and every
compression rotation renumbered the conversation it forked - one piece of
work reaching 'Smallville Map Architecture Plan #10' in the sidebar.

Titles now carry a source (derived < llm < user) enforced by one
compare-and-swap, so an automatic write can only ever replace a title of
strictly lower authority. Compression carries the name across unchanged.
Legacy NULL rows rank as user, so auto-titling only fills genuinely
empty titles on existing data.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant