Skip to content

fix(streaming): preserve literal double brackets in ChatAdapter output - #10459

Open
brennanbutler01 wants to merge 3 commits into
stanfordnlp:mainfrom
brennanbutler01:fix-streaming-literal-brackets
Open

brennanbutler01 wants to merge 3 commits into
stanfordnlp:mainfrom
brennanbutler01:fix-streaming-literal-brackets

Conversation

@brennanbutler01

@brennanbutler01 brennanbutler01 commented Sep 20, 2026 •

Copy link
Copy Markdown

Changes Description

ChatAdapter streaming truncates ordinary nested-array text such as Matrix: [[1, 2], [3, 4]] because StreamListener.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 --check passed.

Contributor Checklist

  • Pre-commit checks pass locally and remotely (local checks passed; CI pending)
  • PR title describes the fix
  • Commit message follows {label}(dspy): {message} (current commit uses fix(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”.

Copilot AI lite review requested due to automatic review settings September 20, 2026 03:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] Safe to merge.

Summary

This update preserves literal double brackets in ChatAdapter streaming output while continuing to recognize complete field and completion markers. The streaming regression coverage includes default and explicit ChatAdapter configuration, markers split across chunks, and inline completion markers without fallback markers that could mask a regression.

Reviews (3) · Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." · Reviewed by Greptile

Comment thread tests/streaming/test_streaming.py Outdated
@greptile-apps

This comment has been minimized.

@brennanbutler01

Copy link
Copy Markdown
Author

@greptileai Please re-review d6e88bb. The inline-marker regression no longer includes a second completion marker; mutation verification catches broken inline termination.

@VANDRANKI VANDRANKI 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.

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants