Repository navigation
fix(streaming): preserve literal double brackets in ChatAdapter output - #10459
brennanbutler01 wants to merge 3 commits into
Conversation
|
This comment has been minimized.
This comment has been minimized.
|
@greptileai Please re-review d6e88bb. The inline-marker regression no longer includes a second completion marker; mutation verification catches broken inline termination. |
VANDRANKI
left a comment
There was a problem hiding this comment.
Community review, does not clear the merge gate. Note: I tried to leave this as an approval but GitHub rejected it with "Can not approve a pull request without explicit repository access", so this is a comment instead.
I read the diff and the full StreamListener class in dspy/streaming/streaming_listener.py on main to check flush() against how end-of-field detection works everywhere else in that file.
The bug: the old ChatAdapter branch of flush() did last_tokens.find("[["), a bare substring search over the buffered tail of the stream. Any legitimate model output containing a literal [[ (a matrix like [[1, 2], [3, 4]], nested JSON-ish text, etc.) would trigger a false match on the first [[ it contains, truncating the flushed output right there and silently dropping everything the model actually generated after that point. Meanwhile every other place in this same class (_could_form_end_identifier, receive, _default_handle_stream_chunk) already detects the real end-of-field marker using the compiled regex re.compile(r"\[\[ ## (\w+) ## \]\]") stored as self.adapter_identifiers["ChatAdapter"]["end_identifier"], which requires the full [[ ## word ## ]] shape, not just two brackets. flush() was the one place still using the naive substring check.
The fix switches flush() to use that same compiled regex via .search() and takes .start() as the boundary, falling back to len(last_tokens) when there's no match, which preserves the old fallback behavior for the no-marker case.
I traced the new parametrized test by hand against this regex: "Matrix: [[1, 2], [3, 4]]" contains [[1, 2], which does not match \[\[ ## (\w+) ## \]\] (no ## after the brackets), so the regex correctly skips past it, while "[[ ## completed ## ]]" does match, giving the correct truncation point. This holds across all three ways the test splits the content across chunks (all-at-once, split mid-bracket, split mid-marker), and both with an explicit ChatAdapter() and with adapter=None (which the same branch's isinstance(...) or settings.adapter is None handles identically).
This looks correct and fixes a real, easy-to-hit data-loss bug for any ChatAdapter-streamed field whose content happens to contain double brackets.
Changes Description
ChatAdapter streaming truncates ordinary nested-array text such as
Matrix: [[1, 2], [3, 4]]becauseStreamListener.flush()treats any[[as a field boundary. Use the existing complete field-marker pattern instead, preserving literal brackets while still removing actual markers.Six regression cases fail before the fix and pass afterward. They cover default and explicit ChatAdapter settings, brackets split across chunks, and a completion marker sharing the answer chunk.
The inline-marker cases contain no later completion marker. Temporarily disabling inline-marker recognition makes both cases fail, confirming the regression cannot be masked by a backup marker.
Local verification: 44 tests passed across the two streaming test files, three provider-dependent cases were skipped, and two proxy-server tests were excluded. Ruff lint, applicable pre-commit hooks, and
git diff --checkpassed.Contributor Checklist
{label}(dspy): {message}(current commit usesfix(streaming))Warnings
No live-provider validation was performed. The existing test file has formatting differences on the base revision; unrelated formatting was left untouched.
OpenAI Codex helped investigate the bug, prepare the patch and regression tests, and run local checks. The task was to inspect DSPy's streaming/cancellation paths for a reproducible defect, check for overlapping work, and prepare a focused fix. The follow-up prompt approving that investigation was “let's do it please”.