Skip to content

fix(stream): a seal must not leave an oversized leftover on the non-final lane - #114151

Closed
dasgltd wants to merge 1 commit into
NousResearch:mainfrom
dasgltd:upstream-stream-dup
Closed

dasgltd wants to merge 1 commit into
NousResearch:mainfrom
dasgltd:upstream-stream-dup

Conversation

@dasgltd

@dasgltd dasgltd commented Sep 17, 2026

Copy link
Copy Markdown

Problem

_seal_overflow_heads clears the edit target mid-iteration, so the overflow gate evaluated at the top of run() is already stale by the time the leftover is pushed:

if not self._use_native_streaming and self._first_send_overflows():
    if await self._split_first_send(tick): return
    ...
await self._seal_overflow_heads()
await self._push_update(tick)      # <- gate is stale here

The seal loop exits after one successful seal (while self._overflows() and self._message_id is not None ... sets _message_id = None), so a buffer far past the limit leaves most of itself behind. That leftover reaches _push_update -> _first_send with no message to edit, on a non-final tick, and adapter.send() chunks and caps it itself, numbering the pieces (i/n). The turn-final lane then publishes the same text again with its own denominator - the two lanes do not share a budget (MAX_MESSAGE_LENGTH vs the consumer's _safe_limit) nor the same payload shape (a notify=True send strips the rich tail before chunking).

Evidence

Measured in production on Discord, from the platform API rather than from a screenshot:

  • one inbound message, one API call, tool_turns=0, a 36642-char answer
  • two interleaved sequences on screen, (i/10) and (i/9)
  • chunk 1 of each sequence byte-identical (sha256 of the bodies with the (i/n) indicator stripped), and the concatenations share a prefix - one answer, split twice, not two answers
  • the gateway's normal final send was correctly suppressed (streamed=True previewed=False content_delivered=True), which places the duplicate inside the consumer, not in the gateway's delivery ledger
  • no content was lost: the closing embeds arrived intact. What was lost is readability - the two sequences interleave, so a reader sees a TL;DR in the middle of another chunk

Likely related: #25349.

Fix

Re-check the overflow gate after the seal and hand a still-oversized leftover to _split_first_send, the consumer's own cap-aware, ledger-aware splitter, which already owns sealing. This removes the same-iteration invalidation rather than compensating for it downstream.

Deliberately NOT done: no per-turn message cap and no new "already published" flag. The existing flag family (_final_response_sent / _final_content_delivered / _turn_split_delivery) behaved correctly in this incident - a fourth state would be more surface, not less.

Tests

New file tests/gateway/test_stream_consumer_oversized_leftover.py, two cases:

  1. on a non-final lane the adapter is never handed a payload larger than one message
  2. no line reaches the channel twice across new messages (the reported symptom)

The toy adapter uses a 2000-char limit on purpose: the consumer floors its own budget at 500, so a smaller limit makes it legitimately emit chunks above the limit - a measurement artifact, not a leak.

Verification on this branch

  • 2 passed with the change
  • negative control: with the change reverted, both tests fail, including the duplicate-line one
  • 567 passed, 4 skipped across the streaming/split/overflow/orphan tests in tests/gateway/
  • uvx ruff check . -> All checks passed!

Searched open and merged PRs for an existing fix to this lane before opening; the nearby ones (#103160, #107643, #111296) tune the duplicate-send warning, not the seal/leftover path.

🤖 Generated with Claude Code

…inal lane

_seal_overflow_heads clears the edit target mid-iteration, so the overflow gate
evaluated at the top of run() is already stale when the leftover is pushed. The
seal loop exits after ONE successful seal (its condition includes
_message_id is not None), so a buffer far past the limit leaves most of itself
behind. That leftover then reaches _push_update -> _first_send with no message to
edit, on a NON-final tick, and adapter.send() chunks and caps it itself, numbering
the pieces (i/n). The turn-final lane later publishes the same text again with its
own denominator, because the two lanes use different budgets and different payload
shapes.

Observed in production on Discord: one inbound message, one API call, no tool
turns, a 36642-char answer, and two interleaved sequences on screen - (i/10) and
(i/9) - whose first chunks were byte-identical once the indicator was stripped and
whose concatenations shared a prefix. The gateway's normal final send was
correctly suppressed (streamed=True, content_delivered=True), which places the
duplicate inside the consumer rather than in the gateway's delivery ledger.

Fix: re-check the overflow gate AFTER the seal and hand a still-oversized leftover
to _split_first_send, which is the consumer's own cap-aware, ledger-aware splitter
and already owns sealing. This removes the same-iteration invalidation rather than
compensating for it downstream, and it does not add a fourth delivery flag: the
existing flags behaved correctly here.

Negative control on this branch: with the change reverted, both new tests fail,
including the one asserting that no line is published twice.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Sep 17, 2026
@kshitijk4poor

Copy link
Copy Markdown

Thanks @dasgltd. Current main already fixes this via 4f24811 ("fix(gateway/stream): no stray "(n/n)" in the live overflow preview"), so this PR is superseded and I'm closing it. Your PR predates that commit, so credit to you for pinning it first.

If you see a remaining gap on current main, a fresh PR against it is welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants