Skip to content

fix(gateway): keep overflow stream chunks editable - #45938

Closed
2001Y wants to merge 2 commits into
NousResearch:mainfrom
2001Y:fix/slack-stream-overflow-rolling-edit
Closed

fix(gateway): keep overflow stream chunks editable#45938
2001Y wants to merge 2 commits into
NousResearch:mainfrom
2001Y:fix/slack-stream-overflow-rolling-edit

Conversation

@2001Y

@2001Y 2001Y commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Keeps the newest overflow chunk editable while gateway responses stream past a platform message limit.

Previously, when a streamed response overflowed before an editable message existed, the consumer split the entire accumulated response through truncate_message() and posted every chunk as an immutable standalone message. That meant the second Slack chunk could not keep updating while more tokens arrived. This PR seals only the overflowing head chunks, leaves the trailing chunk as the active preview, and lets later deltas edit that newest chunk in place.

It also fixes the same split helper fallback to use the computed codepoint budget when adapters count length in non-codepoint units (for example Telegram UTF-16 units), so overflow chunks do not accidentally exceed the adapter's real limit.

Related Issue

No issue filed; reported from live Slack gateway behavior.

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

  • gateway/stream_consumer.py
    • For first-send/after-segment overflow, seal only completed head chunks.
    • Keep the remaining tail in _accumulated so _send_or_edit() creates it as the active editable chunk.
    • Preserve final-delivery guards by only marking final delivery when sealed chunks and the tail both land.
    • Use _custom_unit_to_cp(...)'s returned codepoint budget for split fallbacks, instead of mixing adapter length units with Python slice indexes.
  • tests/gateway/test_stream_consumer.py
    • Added a regression test proving the second overflow chunk remains the edit target and later deltas update it.
    • Adjusted UTF-16 overflow coverage to assert the real delivered chunks stay within the adapter length limit.

How to Test

  1. RED: python -m pytest tests/gateway/test_stream_consumer.py::TestInitialOverflowRollingEdit::test_initial_overflow_keeps_last_chunk_as_edit_target -q
    • Failed before the fix with: AssertionError: the second overflow chunk should be edited...
  2. GREEN: python -m pytest tests/gateway/test_stream_consumer.py::TestInitialOverflowRollingEdit::test_initial_overflow_keeps_last_chunk_as_edit_target -q
    • 1 passed
  3. Targeted gateway suite: python -m pytest tests/gateway/test_stream_consumer.py -q
    • 94 passed
  4. CI-style targeted gateway run: scripts/run_tests.sh tests/gateway/test_stream_consumer.py tests/gateway/test_stream_consumer_fresh_final.py tests/gateway/test_stream_consumer_thread_routing.py tests/gateway/test_update_streaming.py -- -q
    • 150 tests passed, 0 failed
  5. Cross-platform scan: python scripts/check-windows-footguns.py gateway/stream_consumer.py
    • ✓ No Windows footguns found (1 file(s) scanned).
  6. Syntax check: python -m py_compile gateway/stream_consumer.py tests/gateway/test_stream_consumer.py
    • Passed with no output.
  7. Full suite attempt: scripts/run_tests.sh -j 8 -- -q
    • Attempted, but this local macOS worktree has unrelated existing failures outside the touched gateway files, including Anthropic adapter import/path issues, systemctl-only live guard tests on macOS, /tmp vs /private/tmp macOS path assertions, and a timing-sensitive delegate heartbeat assertion. The targeted gateway suites above passed.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A, no user-facing docs changed
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

For New Skills

N/A — no new skill.

Screenshots / Logs

Not applicable; behavior is covered by the gateway stream consumer regression tests above.

@liuhao1024

Copy link
Copy Markdown
Contributor

Verified clean — gateway stream consumer overflow refactor + chunk-boundary fix.

Reviewed the full diff and test additions. Key observations:

  1. _cp_budget // 2 fix (lines 568 and 825): The old code compared split_at < limit // 2 but limit is the raw character limit while _cp_budget is the adjusted budget from _custom_unit_to_cp() (which differs for UTF-16 codepoints). Using limit was a latent bug for platforms with non-trivial length functions. The fix is correct.

  2. Rolling-edit overflow pattern: The old approach sealed ALL overflow chunks as immutable messages via truncate_message. The new approach seals only the head chunks and keeps the trailing chunk as the active edit target, so subsequent deltas update the last message in-place instead of posting new immutable messages. This is a significant UX improvement for long streaming responses on Telegram and similar platforms.

  3. _accumulated.lstrip("\n") after split: Correct — prevents empty first-line messages when splitting at a newline boundary.

  4. Test TestInitialOverflowRollingEdit: Validates the key invariant — the second overflow chunk receives edits from later deltas, confirming the rolling-edit pattern works.

  5. UTF-16 test refactor: Changed from asserting truncate_message was called with utf16_len to asserting actual split results respect the 4096 limit. Stronger test — validates outcomes, not implementation details.

No issues found. The _cp_budget fix is a real bug fix for UTF-16 platforms; the overflow refactor improves streaming UX without changing semantics.

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter P2 Medium — degraded but workaround exists labels Jun 14, 2026
@2001Y
2001Y force-pushed the fix/slack-stream-overflow-rolling-edit branch from 85739f6 to 79b8e7a Compare June 17, 2026 12:45
@2001Y

2001Y commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Rebased this onto current main and force-pushed head 79b8e7ad.

Verification:

  • git diff --check — passed
  • python -m py_compile gateway/stream_consumer.py tests/gateway/test_stream_consumer.py — passed
  • python scripts/check-windows-footguns.py gateway/stream_consumer.py tests/gateway/test_stream_consumer.py — passed
  • scripts/run_tests.sh tests/gateway/test_stream_consumer.py -- -q -o addopts= — 94 passed

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for identifying a real current-main streaming issue: gateway/stream_consumer.py:672-701 sends every initial-overflow chunk and clears _accumulated, so it leaves no editable trailing preview.

Problems

  • gateway/stream_consumer.py:538 in this PR replaces adapter.truncate_message() with newline-only direct slicing. That bypasses the documented splitter contract in gateway/platforms/base.py:5521-5648, including word boundaries, balanced/reopened code fences, and chunk indicators. It also bypasses Yuanbao's table/fence-aware override at gateway/platforms/yuanbao.py:4982-5008.

Suggested changes

  • Preserve the adapter splitter contract while implementing the rolling editable tail; do not duplicate a reduced splitter in the stream consumer.
  • Add a first-send rolling-edit regression using an overflowed fenced block. Existing coverage explicitly protects balanced fences at tests/gateway/test_platform_base.py:1505-1535 and UTF-16 fence handling at tests/gateway/test_platform_base.py:1718-1728.

Automated hermes-sweeper review.

Comment thread gateway/stream_consumer.py Outdated
reply_to = self._message_id or self._initial_reply_to_id
for chunk in chunks:
reply_to = self._initial_reply_to_id
while _len_fn(self._accumulated) > _safe_limit:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop replaces adapter.truncate_message() with newline-only slicing. That loses the shared splitter's word-boundary, balanced/reopened code-fence, and chunk-indicator behavior (gateway/platforms/base.py:5521-5648) and bypasses adapter overrides such as Yuanbao's table-aware splitter. Please retain that contract while keeping the editable tail.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/streaming Streaming responses: gateway delivery, provider wire labels Jul 14, 2026
@2001Y
2001Y force-pushed the fix/slack-stream-overflow-rolling-edit branch from 79b8e7a to e0ed971 Compare July 23, 2026 13:26
@2001Y

2001Y commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto the current upstream main and force-pushed head e0ed9710f06a1efc942f32984e55473f74e629e2.

The stream overflow path now preserves the platform adapter contract instead of directly slicing oversized text. The rolling editable tail remains intact, including adapter-specific fence/table handling.

Verification on this head:

  • stream/base targeted suite: 327 passed;
  • Yuanbao-related suite: 169 passed;
  • Python compilation: passed;
  • Windows footgun scan: passed;
  • git diff --check: passed.

The PR is ready for review of this head.

teknium1 added a commit that referenced this pull request Jul 23, 2026
Widen #48476's fence guarantees to the two splitters that still emitted
fence-broken chunks:

* GatewayStreamConsumer._split_text_chunks (fallback final send): close
  the orphaned ``` at each chunk boundary and reopen it — with the
  original language tag — on the next chunk, mirroring
  BasePlatformAdapter.truncate_message's contract.  Headroom is reserved
  so balanced chunks stay within the platform limit.
* Slack block_kit._split_text (3000-char section chunking): same
  close/reopen balancing for mrkdwn section text carrying fences.

With these, every chunk boundary — non-streaming send
(truncate_message), streaming overflow (_truncate_for_stream via
adapter.truncate_message per #45938), fallback final
(_split_text_chunks), final-send balance (ensure_closed_code_fences),
and Block Kit section splits — delivers fence-balanced chunks.

Regression tests probe each path with fenced fixtures, assert per-chunk
balance, limit compliance, language-tag reopening, and prose passthrough.
teknium1 added a commit that referenced this pull request Jul 23, 2026
Widen #48476's fence guarantees to the two splitters that still emitted
fence-broken chunks:

* GatewayStreamConsumer._split_text_chunks (fallback final send): close
  the orphaned ``` at each chunk boundary and reopen it — with the
  original language tag — on the next chunk, mirroring
  BasePlatformAdapter.truncate_message's contract.  Headroom is reserved
  so balanced chunks stay within the platform limit.
* Slack block_kit._split_text (3000-char section chunking): same
  close/reopen balancing for mrkdwn section text carrying fences.

With these, every chunk boundary — non-streaming send
(truncate_message), streaming overflow (_truncate_for_stream via
adapter.truncate_message per #45938), fallback final
(_split_text_chunks), final-send balance (ensure_closed_code_fences),
and Block Kit section splits — delivers fence-balanced chunks.

Regression tests probe each path with fenced fixtures, assert per-chunk
balance, limit compliance, language-tag reopening, and prose passthrough.
teknium1 added a commit that referenced this pull request Jul 23, 2026
Widen #48476's fence guarantees to the two splitters that still emitted
fence-broken chunks:

* GatewayStreamConsumer._split_text_chunks (fallback final send): close
  the orphaned ``` at each chunk boundary and reopen it — with the
  original language tag — on the next chunk, mirroring
  BasePlatformAdapter.truncate_message's contract.  Headroom is reserved
  so balanced chunks stay within the platform limit.
* Slack block_kit._split_text (3000-char section chunking): same
  close/reopen balancing for mrkdwn section text carrying fences.

With these, every chunk boundary — non-streaming send
(truncate_message), streaming overflow (_truncate_for_stream via
adapter.truncate_message per #45938), fallback final
(_split_text_chunks), final-send balance (ensure_closed_code_fences),
and Block Kit section splits — delivers fence-balanced chunks.

Regression tests probe each path with fenced fixtures, assert per-chunk
balance, limit compliance, language-tag reopening, and prose passthrough.
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #70191 — your commit was cherry-picked/reapplied onto current main with your authorship preserved in git history: your editable overflow stream chunks fix was cherry-picked.

Thanks for the contribution!

@teknium1 teknium1 closed this Jul 23, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
Widen NousResearch#48476's fence guarantees to the two splitters that still emitted
fence-broken chunks:

* GatewayStreamConsumer._split_text_chunks (fallback final send): close
  the orphaned ``` at each chunk boundary and reopen it — with the
  original language tag — on the next chunk, mirroring
  BasePlatformAdapter.truncate_message's contract.  Headroom is reserved
  so balanced chunks stay within the platform limit.
* Slack block_kit._split_text (3000-char section chunking): same
  close/reopen balancing for mrkdwn section text carrying fences.

With these, every chunk boundary — non-streaming send
(truncate_message), streaming overflow (_truncate_for_stream via
adapter.truncate_message per NousResearch#45938), fallback final
(_split_text_chunks), final-send balance (ensure_closed_code_fences),
and Block Kit section splits — delivers fence-balanced chunks.

Regression tests probe each path with fenced fixtures, assert per-chunk
balance, limit compliance, language-tag reopening, and prose passthrough.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/streaming Streaming responses: gateway delivery, provider wire comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

4 participants