Skip to content

fix(translation): accept SSE data without a space - #404

Open
ting-hong-shieh wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
ting-hong-shieh:fix/399-sse-data-optional-space
Open

fix(translation): accept SSE data without a space#404
ting-hong-shieh wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
ting-hong-shieh:fix/399-sse-data-optional-space

Conversation

@ting-hong-shieh

@ting-hong-shieh ting-hong-shieh commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

What

  • accept both data:{...} and data: {...} SSE fields
  • remove at most one optional ASCII space after the field-name colon
  • cover a no-space JSON payload and data:[DONE] in the shared stream decoder

Why

The SSE parser matched only the literal data: prefix, so valid upstream events without the optional space were silently ignored.

Closes #399

Validation

  • cargo test -p switchyard-translation decode_stream_accepts_sse_data_without_optional_space -- --nocapture — passed
  • cargo test -p switchyard-translation — passed
  • cargo fmt --all --check — passed
  • cargo clippy --workspace --all-targets -- -D warnings — passed
  • cargo test --workspace — passed with loopback access enabled for the repository's local mock servers
  • git diff --check — passed

No live provider calls or secrets were used.

Summary by CodeRabbit

  • Bug Fixes
    • Improved streaming response parsing to accept data fields with or without a space after the colon.
    • Preserved support for termination markers such as [DONE].
  • Tests
    • Added regression coverage for both supported data field formats.

Signed-off-by: Ting-Hong Shieh <32212900+ting-hong-shieh@users.noreply.github.com>
@ting-hong-shieh

Copy link
Copy Markdown
Contributor Author

Before / after output snapshot

I ran the same in-process SSE decode at the PR base (b256d936) and current head (a1f747bd). No HTTP or provider call was involved.

Input:

data:{"choices":[{"delta":{"content":"Hello"}}]}

data:[DONE]

Before (b256d936):

{"decoded_text":"","event_count":0}

Both no-space data: fields were ignored, so the text and terminal marker disappeared.

After (a1f747bd):

{"decoded_text":"Hello","event_count":1}

The JSON field is decoded and the no-space data:[DONE] marker terminates the stream without producing an extra event.

@ting-hong-shieh
ting-hong-shieh marked this pull request as ready for review August 13, 2026 15:39
@ting-hong-shieh
ting-hong-shieh requested a review from a team as a code owner August 13, 2026 15:39
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 437ae787-0175-4936-815c-386a3d93c3e1

📥 Commits

Reviewing files that changed from the base of the PR and between b256d93 and a1f747b.

📒 Files selected for processing (2)
  • crates/switchyard-translation/src/helpers.rs
  • crates/switchyard-translation/src/sse.rs

Walkthrough

The SSE parser now accepts data: fields with or without one space after the colon. A regression test covers space-less payload and [DONE] frames.

Changes

SSE parser fix

Layer / File(s) Summary
SSE data parsing and regression coverage
crates/switchyard-translation/src/sse.rs, crates/switchyard-translation/src/helpers.rs
The parser accepts both data: and data: prefixes and removes at most one optional space. The regression test validates payload decoding and [DONE] termination without the space.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: ⚪ Minimal · up to a1f74

The decoder now accepts valid SSE data fields with or without one optional space, with no actionable merge-blocking risk remaining after normal checks and review.

Poem

I’m a rabbit with ears held high,
Parsing data as it hops by.
With one space or none in sight,
[DONE] ends the stream just right.
Carrots cheer the passing test!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: accepting SSE data fields without a space.
Linked Issues check ✅ Passed The parser change and regression test satisfy issue #399 by accepting both SSE data formats and handling data:[DONE].
Out of Scope Changes check ✅ Passed The changes are limited to the SSE parser fix and focused regression coverage required by issue #399.

Comment @coderabbitai help to get the list of available commands.

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.

[bug] SSE decoder drops data: fields without a space after the colon

1 participant