Skip to content

Fix stale streaming attempts emitting late deltas - #66231

Closed
davidb73-hub wants to merge 1 commit into
NousResearch:mainfrom
davidb73-hub:fix/stale-stream-attempt-deltas
Closed

Fix stale streaming attempts emitting late deltas#66231
davidb73-hub wants to merge 1 commit into
NousResearch:mainfrom
davidb73-hub:fix/stale-stream-attempt-deltas

Conversation

@davidb73-hub

Copy link
Copy Markdown
Contributor

Summary

  • mark stream attempts as cancelled when stale, interrupted, or superseded by retry cleanup
  • drop chunks from cancelled attempts before they can update callbacks or accumulated response content
  • log discarded stale chunks with chunk and byte counters

Tests

  • scripts/run_tests.sh tests/run_agent/test_stream_interrupt_retry.py -- -q
  • env UV_CACHE_DIR=.uv-cache uv run --extra dev ruff check agent/chat_completion_helpers.py tests/run_agent/test_stream_interrupt_retry.py

Split out from #66195 per maintainer request.

@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 P1 High — major feature broken, no workaround labels Jul 17, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #65991 and #66005. This PR fences stale attempts in chat-completion handling; #66005 uses a broader single-writer sink guard across streaming paths.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Comment

Fix for stale streaming attempts (+158/-2). The change addresses an edge case in streaming that can cause late deltas. Flagging for human review given the subtle nature of the fix.


Reviewed by Hermes Agent

@davidb73-hub

Copy link
Copy Markdown
Contributor Author

Thanks for the triage links — to clarify how this relates to #66005:

Both target the stale-stream dual-writer symptom from #65991, but at different layers:

  • This PR fences the chunk-consumption loop in _call_chat_completions only: each retry gets an attempt id, chunks from superseded attempts are discarded before they reach the delta handling, and a superseded attempt raises instead of assembling a final response. It does not touch the anthropic_messages or Bedrock streaming paths.
  • fix(streaming): fence superseded streams out of the delta sink (single-writer, #65991) #66005 enforces the single-writer invariant on the delta sink itself, guarded by attempt id, across all three streaming paths — and also fixes the stale-kill path not setting the cancellation flag before force-close.

So #66005 is the more complete fix for #65991, and if maintainers prefer it I'm happy to close this one in its favour. The only case for this PR is as a smaller, lower-risk diff (+158/−2) confined to the chat-completions path if a targeted fix is wanted sooner. The two overlap in chat_completion_helpers.py, so they shouldn't both land as-is.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved (LGTM)

Overview

Fixes stale streaming attempts emitting late deltas. +158/0.

Security

  • No hardcoded secrets or credentials

Code Quality

  • Clean streaming state management

Looks Good

  • Well-scoped streaming fix

Reviewed by Hermes Agent

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via #66947. Your commits cherry-picked with authorship preserved — the stale-stream-attempt late-chunk fix is now on main, plus a tiny follow-up gating the debug log on first-discard to avoid double-logging. Thanks for the clean fix!

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 P1 High — major feature broken, no workaround type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants