🪲 BUG-#62: Persist already-shown text when an SSE chunk is malformed (closes #62, #64) - #67
Merged
Conversation
FernandoCelmer
commented
Aug 16, 2026
FernandoCelmer
left a comment
Member
Author
There was a problem hiding this comment.
🔍 Code Review
Code issues found: 2
| # | Severity | Comment |
|---|---|---|
| 1 | [Blocking] | stop_reason is overwritten when a malformed chunk follows a clean finish_reason |
| 2 | [Suggestion] | Missing test for the finish_reason → malformed sequence |
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
pycodeloop/providers/generic.py—_stream(): wrapsjson.loads(payload)intry/except json.JSONDecodeError, breaking out of the loop and returning whatever text/tool-calls were already accumulated (stop_reason="malformed_stream") instead of letting the exception discard everything the caller already streamed to the user. Also tracks whether the loop ended via an explicit terminal marker ([DONE]or afinish_reasonchunk) — if the iterator just runs out without one (connection dropped mid-response),stop_reasonis now"connection_lost"instead of silently defaulting to"stop", which used to make a truncated response indistinguishable from a clean completion.tests/providers/test_generic.py— Addstest_streaming_keeps_already_shown_text_on_malformed_chunkandtest_streaming_flags_a_connection_dropped_mid_response. Verified both fail without the fix (the first with an unhandledJSONDecodeError, the second withstop_reason == "stop"instead of"connection_lost") and pass with it.Motivation and Context
Two related integrity bugs in the same streaming path:
data:line mid-stream raised unhandled. By that pointon_delta(...)had already fired for every prior chunk — those deltas already reached the client and rendered on screen — but the exception propagated up, the turn failed, andsession.add_assistant(...)was never called, so the text the user already saw never entered the persisted session.finish_reason/[DONE]marker, theforloop just ended andstop_reasonsilently defaulted to"stop", making a truncated response look like the model finished on its own — future turns build on a "complete" message that's actually cut off mid-thought.Closes #62. Closes #64.
Types of changes
Checklist