Skip to content

fix(agent): clamp _last_flushed_db_idx after scaffolding drop (#31507) - #40714

Closed
andrebrassfield wants to merge 1 commit into
NousResearch:mainfrom
andrebrassfield:fix/flush-index-clamp-31507
Closed

fix(agent): clamp _last_flushed_db_idx after scaffolding drop (#31507)#40714
andrebrassfield wants to merge 1 commit into
NousResearch:mainfrom
andrebrassfield:fix/flush-index-clamp-31507

Conversation

@andrebrassfield

Copy link
Copy Markdown

Summary

Intermediate _persist_session calls during tool processing set _last_flushed_db_idx to the old (higher) message count. When _drop_trailing_empty_response_scaffolding pops trailing messages, the index overshoots len(messages), causing _flush_messages_to_session_db to compute an empty slice — final assistant responses are never written to state.db.

Root Cause

In _persist_session (run_agent.py, line ~1463):

  1. Intermediate _persist_session sets _last_flushed_db_idx = len(messages) (e.g., 98)
  2. Agent generates final assistant response → messages grow to 99
  3. Final _persist_session_drop_trailing_empty_response_scaffolding pops 1-2 messages (len drops to 97)
  4. _flush_messages_to_session_db computes flush_from = max(start_idx, 98) >= 97 → empty slice → no writes

Fix

Clamp _last_flushed_db_idx = min(_last_flushed_db_idx, len(messages)) after the scaffolding drop. One line, no persistence semantics changed.

Option B (clamp) chosen over Option A (flush-before-drop) because the bug is index overshoot, not wrong order. The scaffolding drop is doing its job — what's wrong is the index getting ahead of the array.

Closes

Testing

7 regression tests covering:

  • Index clamped after scaffolding drop (core regression)
  • Index unchanged when no scaffolding dropped (no-op case)
  • Index clamped to zero on full wipe
  • Index clamped after multi-message scaffolding drop (sentinel + tool pair)
  • Post-scaffold invariant: _last_flushed_db_idx <= len(messages) (50 randomized iterations)
  • Final assistant response written after intermediate persist (integration with real SessionDB)
  • Scaffolding drop then new messages written (integration)

All 25 existing tests pass (test_860_dedup.py, test_message_sequence_repair.py, test_compression_persistence.py).

Per issue #31507: intermediate _persist_session calls during tool
processing set _last_flushed_db_idx = len(messages) at the old (higher)
count. When _drop_trailing_empty_response_scaffolding pops trailing
messages, len(messages) drops below _last_flushed_db_idx. The next
_flush_messages_to_session_db computes flush_from = max(start_idx,
old_high_idx) >= len(messages) → empty slice → final assistant
responses never written to state.db.

Fix: clamp _last_flushed_db_idx = min(_last_flushed_db_idx,
len(messages)) after the scaffolding drop in _persist_session.

7 regression tests covering:
- Index clamped after scaffolding drop (core regression)
- Index unchanged when no scaffolding dropped
- Index clamped to zero on full wipe
- Index clamped after multi-message scaffolding drop
- Post-scaffold invariant (randomized)
- Final assistant response written after intermediate persist
- Scaffolding drop then new messages written

Closes #31507
@daimon-nous daimon-nous Bot added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround labels Jun 6, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Verified: _last_flushed_db_idx clamping after scaffolding drop is correct.

Reviewed the full diff — this is a precise one-line fix for a real data-loss bug in the agent session persistence layer.

  1. Root cause: _drop_trailing_empty_response_scaffolding() pops trailing messages (sentinels, recovery messages, tool pairs), reducing len(messages). But _last_flushed_db_idx was set by a previous _persist_session() call to the old (higher) message count. The subsequent _flush_messages_to_session_db() computes flush_from = max(start_idx, old_idx) which can exceed the new len(messages), producing an empty slice — final assistant responses are never written to state.db.

  2. Fix: self._last_flushed_db_idx = min(self._last_flushed_db_idx, len(messages)) after the scaffolding drop. This ensures the invariant _last_flushed_db_idx <= len(messages) always holds before the flush.

  3. Test coverage is excellent:

    • Core regression test (index=5, scaffolding drops to len=4)
    • No-drop case (index unchanged when no scaffolding)
    • Full-wipe case (all messages dropped, index clamps to 0)
    • Multi-message drop (sentinel + tool pair + assistant)
    • Randomized invariant test (50 iterations with random indices and scaffolding)
    • Integration tests with real SessionDB verifying messages are actually persisted after scaffolding drops
  4. No side effects: The clamp only affects the flush index — it doesn't change what messages are kept or dropped by the scaffolding logic itself.

LGTM.

@teknium1

Copy link
Copy Markdown
Contributor

Resolved by #46071. This PR addresses stale _last_flushed_db_idx causing an empty slice after scaffolding/message-list shrink. #46071 no longer uses that cursor to decide what writes, and live/testing covered the stale-cursor case.

@teknium1 teknium1 closed this Jun 14, 2026
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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants