Skip to content

fix(gateway,agent): persist delivered responses that recovery paths drop from the transcript - #44120

Closed
AIalliAI wants to merge 1 commit into
NousResearch:mainfrom
AIalliAI:fix/44100-persist-delivered-response
Closed

fix(gateway,agent): persist delivered responses that recovery paths drop from the transcript#44120
AIalliAI wants to merge 1 commit into
NousResearch:mainfrom
AIalliAI:fix/44100-persist-delivered-response

Conversation

@AIalliAI

@AIalliAI AIalliAI commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Problem

Gateway delivers assistant responses to the platform (confirmed in gateway.log), but the session DB ends up with no assistant rows between the user messages. When the next message arrives, the model loads a transcript full of "unanswered" user messages and re-answers all of them in one turn.

Root cause

Two pieces interact:

  1. Agent — partial-stream recovery drops the assistant turn. In agent/conversation_loop.py, when the final assembled assistant message has no visible content but text was already streamed to the user, the recovery path sets final_response from the streamed text and breaks without appending an assistant message to messages. The turn-end _persist_session() then flushes a transcript whose tail is the user message — the user row survives (written by the turn-start crash-resilience flush), the assistant row never exists. This matches the issue's evidence exactly: response ready (…, 48 chars) logs a non-empty final_response while the DB has zero assistant rows.

  2. Gateway — every fallback write is a silent no-op. Since state.db became the canonical transcript store (spec 002), append_to_transcript(..., skip_db=True) does nothing at all. The gateway skips all post-turn DB writes via skip_db=agent_persisted to avoid the bug: SQLite session transcript accumulates duplicate messages (3-4x token inflation) #860/Bug: User messages stored twice in state.db when agent and gateway both write to SQLite #42039 duplicate-write bug — correct for messages the agent flushed, but it means the gateway cannot backfill anything the agent's flush missed. The delivered response is silently dropped with no error anywhere.

Fix

One invariant, enforced at both layers: a delivered final_response must end up in the session transcript.

The gateway backfill also covers the fallback_prior_turn_content recovery (response sourced from an earlier tool-call turn's content, transcript tail ends at a tool message) and any future agent path that returns a response without representing it in messages — with an INFO log so occurrences are visible instead of silent.

Tests

tests/gateway/ + the neighboring run_agent persistence/streaming suites pass; the handful of failures present are identical with and without this diff (pre-existing, environment-dependent: shutdown forensics/systemd, Telegram MarkdownV2 escaping).

Related

Canonical issue: #43849 (#44100 is the Telegram-specific duplicate). This applies the gateway backfill proposed in #43853 and the agent-side root-cause fix a gateway-only backfill can't reach — the partial-stream recovery path that returns a delivered response without ever appending it to messages. Happy to consolidate onto whichever thread maintainers prefer to track.

Fixes #44100

@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 Jun 11, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

Verification: thorough fix for transcript persistence gap with comprehensive regression tests

Reviewed both the conversation_loop.py and gateway/run.py changes — the fix correctly addresses #44100 from two complementary angles:

Agent-side fix (conversation_loop.py): When partial-stream recovery fires (empty final message but content already streamed), the recovered text is now appended as a real assistant message before break. Without this, _persist_session writes no assistant row and the transcript loses role alternation.

Gateway-side fix (gateway/run.py): Three changes:

  1. skip_db=agent_persistedskip_db=False for the assistant response write in the new_messages path — a response generated this turn cannot be in loaded history, so there's no duplicate risk
  2. A safety-net backfill when the turn's new_messages contain no assistant text — catches any recovery path the agent-side fix might miss
  3. The not new_messages fallback also uses skip_db=False

The safety-net check _turn_has_assistant_text correctly filters for role == "assistant" with non-empty content and no tool_calls, so it won't false-positive on tool-call assistant messages.

Test coverage is strong: 4 gateway tests (backfill/no-backfill/tool-turn/fallback) + 2 agent tests (recovery persistence/normal control). The _bootstrap helper correctly mocks the session store to verify skip_db values.

@AIalliAI

Copy link
Copy Markdown
Contributor Author

Requesting maintainer review — this is ready to land from my side. Standalone fork CI is pending first-run approval here; the rollup branch in #44061 carrying this session's batch is fully green on upstream CI (all test shards, typecheck, e2e).

…rop from the transcript

Gateway delivered assistant responses to the platform but never persisted
them to the session DB, so the model saw consecutive "unanswered" user
messages and re-answered all of them on the next turn (NousResearch#44100).

Two layers, one invariant — a delivered final_response must end up in the
session transcript:

1. agent: the partial-stream recovery path (final message empty/thinking-
   only but content already streamed to the user) set final_response and
   broke out of the loop WITHOUT appending an assistant message. The
   turn-end _persist_session then wrote no assistant row — only the user
   message (persisted by the turn-start crash-resilience flush) survived.
   Append the recovered text as a real assistant turn before breaking.

2. gateway: state.db is the canonical transcript store (spec 002), so
   append_to_transcript(..., skip_db=True) is a complete no-op — the
   gateway's "fallback" writes could never backfill anything. When a
   turn's new messages contain no assistant text but a response was
   delivered, write the assistant row with skip_db=False. A response
   generated this turn cannot already be in the loaded history, so the
   NousResearch#860/NousResearch#42039 duplicate-write protection (which concerns the user entry
   and agent-flushed messages) is preserved — covered by regression
   tests.

Fixes NousResearch#44100
@AIalliAI
AIalliAI force-pushed the fix/44100-persist-delivered-response branch from 3023751 to 488452b Compare June 20, 2026 04:49
@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jun 21, 2026
@teknium1

teknium1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Thanks @AIalliAI — the root-cause analysis is accurate and the evidence trail (delivered in gateway.log, zero assistant rows in state.db, next turn re-answering "unanswered" user messages) is exactly the failure. Closing because both halves of the fix land on approaches we've already resolved a different way.

Agent-side half — appending the recovered text as a real assistant turn in conversation_loop.py before break — is the _ensure_final_response_in_messages() mechanism proposed in #31291 / #31314 and deliberately not adopted. Origin issue #31269 was closed NOT_PLANNED; #31291 was closed implemented_on_main with "the proposed helper was not adopted… current main now fixes that persistence failure with stronger flush semantics." The flush-layer fix is #46071 (identity-based SessionDB flushing) — that's the real cause the cursor diagnostic pointed at, and it's on main.

Gateway-side half — backfilling the assistant row with skip_db=False when the turn has no assistant text — overrides the skip_db=agent_persisted guard that exists specifically for the #860/#42039 duplicate-write protection. Gateway-side recovery delivery is already handled by #34336 (scope stream-delivery flags at tool boundaries, deliver recovered originals) and #53987 (partial-stream recovery leaves response_was_previewed=False so the gateway sends the fragment + a "send continue" explainer instead of persisting a truncated turn as final).

So the delivered-response-in-transcript invariant you're after is enforced on current main — at the flush layer for durability, and at the gateway-delivery layer for what the user sees — without the finalize-time backfill. Closing as superseded by #46071 + #34336 + #53987. Genuinely solid PR; it just arrives after we'd settled the design.

@teknium1 teknium1 closed this Jul 1, 2026
@teknium1

teknium1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Fixed on main via PR #56279, which merged @WXBR's #46183. That approach fixes the same "delivered response dropped from transcript" bug at the single finalize_turn chokepoint every recovery path flows through — covering your two hand-patched break sites (partial_stream_recovery, fallback_prior_turn_content) plus any future recovery path, without needing the gateway-side backfill.

Your diagnosis was correct and matched ours exactly (the skip_db=agent_persisted no-op on the gateway side, and the recovery break that never appends an assistant row). We went with the chokepoint fix for smaller footprint. Credited in the merged PR. Thanks for the thorough writeup and repro.

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:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

Telegram: assistant responses not persisted to session DB (model re-answers old messages)

4 participants