fix(dspy): Reset stream listener state per streamify call for inter-call reuse - #81
Conversation
Greptile SummaryThis PR resets listener parsing state at the beginning of each
Confidence Score: 4/5This PR is not yet safe to merge because overlapping uses of a shared streamer can reset and corrupt one another’s active listener state. Sequential reuse is repaired and the reset itself is complete, but listener instances remain shared by all invocations; the new unconditional reset can erase an in-progress invocation’s parser state when another request starts. Files Needing Attention: dspy/streaming/streamify.py, tests/streaming/test_streaming.py Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
S[streamify construction] --> L[Captured shared StreamListener]
S --> A[Invocation A]
S --> B[Invocation B]
A --> RA[Reset listener]
RA --> PA[Parse A chunks into listener state]
B --> RB[Reset same listener]
RB --> C[Clear A flags, queues, and accumulated JSON]
C --> X[Missing, duplicated, or mixed streamed output]
Reviews (1): Last reviewed commit: "fix(dspy): Reset stream listener state p..." | Re-trigger Greptile |
| for listener in stream_listeners: | ||
| listener.reset() |
There was a problem hiding this comment.
Shared Reset Corrupts Active Streams
When the same reusable streamer handles overlapping calls, both calls use the captured StreamListener objects. Starting the second call resets the first call's active parsing flags, queues, and accumulated JSON. A module-level streamer serving concurrent requests can therefore drop, duplicate, or mix chunks between streams. Listener state needs to be isolated per invocation rather than reset in place.
| # Regression test for the worst-case tier: when include_final_prediction_in_output_stream=False | ||
| # is set at streamify() construction, reusing the streamer used to yield an EMPTY stream on call | ||
| # 2+ (no chunks, no Prediction) because the final-prediction guard depended on stale listener | ||
| # state. After the fix, every call should stream the field chunks and the final Prediction. |
There was a problem hiding this comment.
Comment Contradicts Expected Output
This comment says the suppressed-output configuration streams the final Prediction, but the test correctly expects only seven incremental values. This documents the opposite of the tested contract and could mislead future maintainers.
| # state. After the fix, every call should stream the field chunks and the final Prediction. | |
| # state. After the fix, every call should stream the field chunks without the final Prediction. |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Warning
GitHub issue creation failed
Detail attempted to publish this bug to GitHub, but the issue could not be created. This fix PR was created without that issue, and missing tracker references are shown as
Unknown issue.You can review and merge this PR normally. Please review your tracker integration settings before the next publish run.
Detail bug report: View on Detail
Closes Unknown issue
📝 Changes Description
This MR/PR contains the following changes:
Bug:
dspy.streamifycaptures itsStreamListenerobjects once, outsideasync_streamer. At the end of the first call,listener.finalize()setslistener.stream_end = Trueand never resets it. On any subsequent call,StreamListener.receive()early-returns on every chunk (stream_endisTrue,allow_reusedefaults toFalse), silently dropping all incrementalStreamResponsechunks (the finalPredictionis still delivered, so the loss is invisible). Withinclude_final_prediction_in_output_stream=Falseat construction, the second call yields an empty stream (no chunks, noPrediction), because the "yield final Prediction?" guard depends on stale listener state.Fix: Add
StreamListener.reset()to clear per-stream run-time state (stream_start/stream_end/cache_hitflags,field_start_queue,field_end_queue,json_adapter_state["field_accumulated_messages"]) while preserving configuration (signature_field_name,predict,predict_name,allow_reuse,adapter_identifiers).async_streamernow callslistener.reset()for every listener at the start of each invocation, so astreamify-returned streamer is reusable by default — the natural usage for long-lived callers (web handlers, REPLs, notebook loops). Theallow_reuse=Truebranch ofreceive()is refactored to callself.reset()(behavior-preserving for the documented intra-run reuse, e.g.dspy.ReActloops); the sync path (async_streaming=False) inherits the fix viaasync_streamer. This was chosen over a "fail loudly / document as single-use" alternative because the streamer is a plain callable that is natural to reuse, and the docs never state it is single-use.✅ Contributor Checklist
ruff check(the hook's action) passes;ruff format --checkclean on all three files.Automatic Fixes PRs can be configured here.