Decode SSE chunks incrementally - #1068
PeterDaveHello merged 1 commit into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe eventsource parser now uses one streaming ChangesEventsource decoding
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Refactor Merge Risk: ⚪ Minimal · up to Incremental SSE decoding preserves chunk-boundary behavior and reset isolation with direct regression coverage. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoDecode SSE chunks incrementally with a streaming TextDecoder
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can switch off images and animations for a plain-text comment Previous reviewsReview updated until commit 61f630c Results up to commit e73d3a1
|
There was a problem hiding this comment.
🟢 Approval recommended
The changes are fully reviewed with no unresolved blocking issues.
Pull request overview
Updates SSE parsing to decode UTF-8 data incrementally while preserving decoder state across chunks.
Changes:
- Reuses a streaming
TextDecoder. - Resets decoder state with parser state.
- Adds split-sequence and reset tests.
File summaries
| File | Description |
|---|---|
tests/unit/utils/eventsource-parser.test.mjs |
Tests split UTF-8 sequences and decoder reset behavior. |
src/utils/eventsource-parser.mjs |
Implements incremental decoding and decoder reset handling. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Streaming SSE decode:
src/utils/eventsource-parser.mjsnow keeps oneTextDecoderand decodes only newly received bytes (decoder.decode(chunk, { stream: true })) instead of accumulating and re-decoding the whole byte buffer on every feed. The parallelbytestracking (and itsTextEncoderre-encode bookkeeping on partial consumption) is removed entirely. - Decoder reset:
reset()reassignsdecoder = new TextDecoder(), discarding any buffered incomplete UTF-8 sequence together with the clearedbuffer. - Tests: a byte-boundary test asserting
data: 台灣🙂 café\n\nparses identically when split at every byte position, and a test thatreset()drops pending decoder bytes so a subsequent clean event parses without residue.
I verified the refactor is behavior-preserving: all 21 unit tests pass (including the existing every-chunk-boundary and CRLF-split tests), and the byte-boundary test also holds at every real split of multi-byte content against the old implementation — the change is a genuine CPU/allocation win (drops the O(chunks × full-buffer) re-decodes) with the streaming decoder carrying the incomplete UTF-8 state that the old bytes array had to re-track manually. The only caller, src/utils/fetch-sse.mjs, feeds Uint8Array chunks from reader.read(), which is exactly what TextDecoder.decode expects. Lint and Prettier are clean on both changed files.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
223cea5 to
61f630c
Compare
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This run covers the delta since the prior Pullfrog review (e73d3a1 → 223cea5, "Strengthen SSE parser regression coverage"), which touches only tests:
- Replaced the self-referential
const expected = parseChunks(stream)baselines in the every-chunk-boundary and UTF-8 byte-boundary tests with hard-coded event arrays, so both tests can now actually fail on a parser regression instead of always matching the code under test. - Added a test that pending SSE data survives a chunk split when the stream begins with a UTF-8 BOM (exercises the streaming decoder's BOM handling at stream start).
- Added a test that an invalid UTF-8 byte (
0xff) is replaced with U+FFFD mid-stream without losing the partially-buffered event across the split.
I verified all 1049 unit tests pass via npm test, traced the hand-written expected arrays against the parser's parse/emit semantics (they match), and confirmed both new tests pin genuine behavior of the streaming decoder (BOM-stripping and U+FFFD replacement interaction with the pending-data path). The hard-coded expectations close the tautology noted on the earlier review, and the new tests are meaningful coverage for the incremental-decode change.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Code review by qodo was updated up to the latest commit 61f630c |
Summary
TextDecoderper SSE parser instance.Why
The parser currently copies pending bytes and decodes the full accumulated buffer again whenever a new chunk arrives. For fragmented or long SSE lines, this repeats work and allocations.
Streaming decode preserves incomplete UTF-8 sequences across chunk boundaries while allowing the parser to keep only the unconsumed decoded text.