Skip to content

fix(streaming): stop losing the tail of commentary truncated at a tool-call boundary - #88963

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-88954
Open

fix(streaming): stop losing the tail of commentary truncated at a tool-call boundary#88963
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-88954

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops permanently losing the tail of streamed commentary when the stream is truncated at the text→tool_calls boundary (#88954). With streaming enabled on Telegram, the streamed commentary is routinely cut mid-text when the tool_calls finish arrives — the partial stream is a prefix of the full commentary. The interim-message path used the prefix-based _interim_content_was_streamed verdict, so the gateway's _interim_assistant_cb received already_streamed=True and called on_segment_break() — finalizing the truncated bubble as-is. The tail ("...to pick it u" instead of "...to pick it up") was never delivered. The reporter verified the provider is not at fault: the same proxy with tools streams the complete text.

The fix keeps the two consumers' semantics distinct:

  • _interim_content_was_streamed (prefix match) stays unchanged — for the conversation-loop "previewed" marks the streamed prefix IS on the user's screen, and the contract test (test_interim_content_was_streamed_matches_prefix_not_exact, per the fix(desktop, ink): don't wipe messages before final message #65919 review) pins that behavior.
  • New _interim_content_fully_streamed (exact normalized equality) is used for the interim-message verdict that feeds the gateway callback. Only an exact match may skip the full-text resend; a partial prefix falls through to on_commentary(), which re-delivers the complete text — a benign duplicate, never lost text (the outcome the old comment claimed but did not deliver on this path).

Related Issue

Fixes #88954

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • run_agent.py
    • New _interim_content_fully_streamed(): same normalization pipeline, exact-equality verdict, with a docstring explaining why the gateway decision needs the stricter test.
    • _emit_interim_assistant_message uses the exact verdict for already_streamed.
  • tests/run_agent/test_run_agent_codex_responses.py
    • test_interim_commentary_partial_stream_receives_full_text: a streamed prefix ("...pick it u") of the full commentary must yield already_streamed=False with the FULL text, so the gateway re-delivers it.
    • test_interim_commentary_exact_stream_still_marks_streamed: an exactly-streamed commentary keeps already_streamed=True (no duplicate resend).
    • The existing prefix-contract test is untouched and still passes.

How to Test

  1. python -m pytest tests/run_agent/test_run_agent_codex_responses.py -q
    Observed result: 49 passed (includes the untouched prefix-contract test).
  2. Fail-on-main check: git stash the run_agent.py change and rerun test_interim_commentary_partial_stream_receives_full_text — Observed result: 1 failed (the truncated prefix is marked already_streamed, matching the reported loss).
  3. Adjacent consumers: pytest tests/run_agent/test_plugin_stream_hooks.py tests/run_agent/test_verification_continuation_budget.py tests/run_agent/test_tool_call_incremental_persistence.py -q — Observed result: 25 passed, 7 failed; the same 7 fail identically on a stashed baseline (pre-existing local-environment failures, unrelated).

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 (codex responses suite: 49 passed; adjacent suites match the stashed baseline)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15.4 (arm64), Python 3.14

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — the new helper's docstring documents the two-consumer split; or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — pure string-comparison change; or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

…l-call boundary

With streaming enabled on Telegram, the streamed commentary is routinely
cut mid-text when the tool_calls finish arrives; the partial stream is a
prefix of the full commentary. The interim-message path used the
prefix-based _interim_content_was_streamed verdict, so the gateway's
_interim_assistant_cb called on_segment_break() — finalizing the
truncated bubble — and the tail ("...pick it u" vs "...pick it up") was
permanently lost (NousResearch#88954).

That prefix semantics stays correct for the conversation-loop
"previewed" marks (the streamed prefix IS on the user's screen there,
and the contract test pins it per the NousResearch#65919 review). The gateway
decision needs the stricter test: add _interim_content_fully_streamed
(exact normalized equality) and use it for the interim-message verdict.
Only an exact match may skip the full-text resend; a partial prefix
falls through to on_commentary() and re-delivers the complete text —
a benign duplicate, never lost text.
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter area/streaming Streaming responses: gateway delivery, provider wire P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 18, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor Author

The slice-7 failure (tests/gateway/test_loop_command.py::test_gateway_loop_goal_note_when_goal_active) passes locally on this PR's exact head (1 passed in 3.30s) — it's another intermittent gateway/goal-family flake, same family as the goal-verdict pair that has flaked across PRs today. This PR touches only the interim-message streaming verdict in run_agent.py plus its own tests; there is no path into the /loop command's goal-note rendering.

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/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter 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.

Telegram streaming: intermediate commentary truncated at tool-call boundary (last chunk lost)

2 participants