Skip to content

Fix: Bridge-worker silently drops assistant replies from state.db (31269) - #31291

Closed
dskwe wants to merge 2 commits into
NousResearch:mainfrom
dskwe:fix/31269-bridge-assistant-persist
Closed

Fix: Bridge-worker silently drops assistant replies from state.db (31269)#31291
dskwe wants to merge 2 commits into
NousResearch:mainfrom
dskwe:fix/31269-bridge-assistant-persist

Conversation

@dskwe

@dskwe dskwe commented May 24, 2026

Copy link
Copy Markdown
Contributor

Problem

When chatting through the TypeScript WebUI (Agent Bridge path), assistant replies are silently missing from state.db. The response is correctly streamed to the frontend and saved in the WebUI's own session-store, but never written to Hermes Agent's SQLite session store. This causes data inconsistency: the Python WebUI (8787) shows incomplete history for sessions created via the TypeScript WebUI.

Root Cause

In run_conversation(), several exit paths set final_response as a plain string but break out of the loop without appending a structured {"role": "assistant", "content": ...} message dict to messages:

  • partial_stream_recovery — recovers text from partial SSE chunks
  • fallback_prior_turn_content — falls back to previous turn content
  • max_iterations — iteration limit reached

The normal text_response path already appends at ~L3833, so it works fine. But the edge-case exit paths skip that append, and _persist_session(messages, ...) writes whatever is in messages — which lacks the assistant reply.

Fix

Before _persist_session(), check if final_response is a non-empty string and the messages tail doesn't already contain it. If not, inject {"role": "assistant", "content": final_response}. This is a safety net that covers all exit paths without modifying each one individually.

The check:

  • Skips when final_response is empty or "(empty)" (preserves existing empty-response handling by _drop_trailing_empty_response_scaffolding)
  • Skips when the messages tail already matches (avoids duplicates from the normal text_response path)

Changes

  • agent/conversation_loop.py: Add assistant response injection before _persist_session() call (~L3962)

How to test

  1. Run existing persistence tests:
    pytest tests/run_agent/test_session_persistence_assistant_dupes.py tests/run_agent/test_compression_persistence.py -v
    
  2. Manual: chat through bridge worker, compare state.db messages before/after — assistant replies should now be persisted

dskwe added 2 commits May 24, 2026 11:46
Several exit paths (partial_stream_recovery, fallback_prior_turn_content,
max_iterations) set final_response but break without appending a structured
assistant message dict to the messages list. _persist_session then writes
messages as-is, silently dropping the assistant reply from state.db.

Add a safety net before _persist_session that injects final_response as an
assistant message when the messages tail doesn't already contain it.

Fixes NousResearch#31269
…ore _persist_session

6 tests covering _ensure_final_response_in_messages:
- injects when messages has no assistant tail
- skips when tail already matches
- skips empty/whitespace/placeholder/(empty)
- injects even when tail has different assistant content
@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 comp/gateway Gateway runner, session dispatch, delivery labels May 24, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Automated hermes-sweeper review: current main now covers the state.db assistant-drop failure this PR was trying to guard.

Evidence:

  • run_agent.py:1610 now uses identity-based SessionDB flushing instead of the old positional slice based on max(len(conversation_history), _last_flushed_db_idx). The inline comment calls out the exact old failure mode where delivered assistant responses never reached state.db.
  • run_agent.py:1651 iterates the current messages list and writes new message dict identities, so a stale or over-advanced _last_flushed_db_idx no longer makes the assistant tail disappear.
  • tests/run_agent/test_identity_flush.py:72 has regression coverage for a stale cursor set past the message list; it verifies both current question and current answer are persisted.
  • git blame points that implementation and test at d682f320b35a13084371a541a835e1d988c982b8.

The PR's proposed _ensure_final_response_in_messages() helper was not adopted, and the linked issue discussion correctly identified the bridge/cursor path as the real failure. Since current main now fixes that observable persistence failure with stronger flush semantics, this PR is redundant.

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 comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants