Skip to content

Revert back to iterating over lines#680

Merged
sjmonson merged 1 commit intomainfrom
fix/gptoss_streaming
Apr 1, 2026
Merged

Revert back to iterating over lines#680
sjmonson merged 1 commit intomainfrom
fix/gptoss_streaming

Conversation

@sjmonson
Copy link
Copy Markdown
Collaborator

@sjmonson sjmonson commented Apr 1, 2026

Summary

Partially reverts #663 to iterating over lines, but keeps the skipping of blank newlines.

Details

#663 switched the HTTP backend to iterating over byte strings. The problem is that is did not handle the case where a line was split over multiple iterations.

Test Plan

Run a benchmark with known errored request rate (preferably 0) and ensure that there are no failed requests due to orjson.JSONDecodeError: unexpected end of data.


  • "I certify that all code in this PR is my own, except as noted below."

Use of AI

  • Includes AI-assisted code completion
  • Includes code generated by an AI application
  • Includes AI-generated tests (NOTE: AI written tests should have a docstring that includes ## WRITTEN BY AI ##)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts the OpenAI HTTP streaming implementation to iterate over decoded text lines again (instead of splitting raw byte chunks), while still skipping blank newline separators, to avoid JSON parsing failures when a single SSE line is split across multiple network reads.

Changes:

  • Switch _aiter_lines() from Response.aiter_bytes() + split(b"\n\n") to Response.aiter_lines()
  • Preserve behavior of skipping blank/whitespace-only lines during streaming

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Collaborator

@jaredoconnell jaredoconnell left a comment

Choose a reason for hiding this comment

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

Looks fine. Are there any potential problems with skipping blank lines?

Copy link
Copy Markdown
Collaborator

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

Let's get this in and freeze v0.6.

@sjmonson
Copy link
Copy Markdown
Collaborator Author

sjmonson commented Apr 1, 2026

Are there any potential problems with skipping blank lines?

There shouldn't be. Its mainly to account for the fact that every data portion ends with \n\n. So we were counting the extra newline as an iteration even though its just an end indicator for the data. According to the spec here the extra newline is just to separate multi-part event into blocks of related information. E.g.

data: first event
id: 1

data:second event
id

data:  third event
data: more data

It does not seem like any of the OpenAI endpoints use this feature though since all of the data is on one line.

Signed-off-by: Samuel Monson <smonson@redhat.com>
@sjmonson sjmonson force-pushed the fix/gptoss_streaming branch from e097472 to 155efd1 Compare April 1, 2026 20:03
@sjmonson sjmonson merged commit a963ae9 into main Apr 1, 2026
19 checks passed
@sjmonson sjmonson deleted the fix/gptoss_streaming branch April 1, 2026 20:14
@dbutenhof dbutenhof added the bug Represents a user-visible defect label Apr 7, 2026
@dbutenhof dbutenhof modified the milestones: Backlog, v0.6.0 Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Represents a user-visible defect

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants