Skip to content

perf(sse): avoid rescanning and flattening fragmented SSE carry (carry #7497, #7498) - #7541

Merged
lidge-jun merged 4 commits into
devfrom
sweep/w1-sse-carry
Oct 23, 2026
Merged

lidge-jun merged 4 commits into
devfrom
sweep/w1-sse-carry

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

Summary

Carries #7497 and #7498 by @Platinum-Scripts into one PR on the current dev. Both are behavior-preserving performance fixes for large SSE frames that arrive in many small network fragments.

  • Chat adapter (src/adapters/openai-chat.ts, from perf(chat): avoid rescanning fragmented SSE carry #7497). After each drain the decoded carry holds only an unfinished line with no newline. A fragment without a newline therefore cannot complete a line, so the adapter keeps the already-counted UTF-8 size of the carry instead of splitting and re-encoding the whole growing line on every fragment. A fragment that contains a newline still drains through the existing parser. Append and residual budget reservations stay in their original order, so cap failures and high-water accounting are unchanged. Net line count is zero (the file sits at its file-size cap).
  • SSE block buffer (src/server/sse-payload-rewrite.ts, from perf(sse): avoid flattening fragmented block carry #7498). After a failed delimiter search, a fragment with no CR or LF cannot create a delimiter, so next() advances its scan cursor without passing the growing carry to the regex again. The surrogate-boundary check in append() uses a cached last code unit instead of indexing the growing carry. Emptying or clearing the buffer resets both.
  • Follow-up in this carry. append() now updates the cached last code unit and scan eligibility immediately after buffer += fragment, before the budget commit, so a throwing commit cannot leave them describing an older carry.
  • Updates structure/transports/byte-accounting.md and structure/providers/mimo-bare-calls.md (the owning docs). The one doc conflict with current dev was resolved by keeping the newer carryReplayRefusal paragraph and inserting the Chat carry paragraph before it.

Original commits keep their authorship, and every commit carries the trailer below. The original PRs stay open for the maintainer to close.

Co-authored-by: Platinum-Scripts 133908028+Platinum-Scripts@users.noreply.github.com

Verification

All test commands ran with isolated HOME, CLAUDE_CONFIG_DIR and OPENCODEX_HOME (mktemp), OCX_TEST_RUNNER_BUN pointed at a Bun 1.4.3 binary, and heavy commands under /usr/bin/lockf -k /tmp/ocx-heavy-test.lock.

  • Deterministic work-bound regressions (operation counts, no wall-clock timing). With src/ reverted to origin/dev and the new tests kept, the three tests fail; with the patch they pass:
    • Chat TextEncoder.encode input characters, 256 KiB tool-call frame in 512-byte fragments: 67,764,469 before vs bound < 2,099,024.
    • Block-buffer delimiter-regex subject units, 256 KiB frame in 512-byte fragments: 67,502,090 before vs bound < 1,048,616.
    • Block-buffer charCodeAt subject units on append: 67,502,088 before vs bound < 1,048,608.
  • bun test tests/adapters/openai/openai-chat-parallel-stream.test.ts tests/responses/sse-payload-rewrite.test.ts: 67 pass / 0 fail, including the Unicode/CRLF/EOF, invalid UTF-8, failed-scan surrogate boundary, and queued-append/compaction/clear equivalence cases from both PRs.
  • bun run test:changed (merge base 516aaaaf84): 35,703 pass / 42 skip / 5 fail across 1,619 files (exit 1). All five failures are Antigravity User-Agent/onboarding cases in tests/adapters/google/google-antigravity-oauth.test.ts and tests/adapters/google/gemini-4-argon-antigravity.test.ts (expected IDE UA string mismatch). Those files reproduce the same 17 pass / 5 fail on an untouched origin/dev worktree at 516aaaaf84, so they are a base failure unrelated to this diff.
  • bun run typecheck: exit 0.
  • bun run privacy:scan: passed. bun run structure:check: passed.
  • scripts/ci/windows-sensitive-diff.sh on a local merge commit (EVENT_NAME=pull_request PATH_SELECTED=false): windows=false (no Windows path or marker), so no lane=all dispatch is needed.
  • No new test files, so no test-layout registration was needed. No file in the file-size baseline grew.
  • Not run locally: the full bun run test suite (this Mac is under elevated load; the import-connected set covers both changed modules), plus macOS/Windows CI, which per policy runs only on dispatch or release lines.

Review threads: neither source PR had CodeRabbit, Codex or human review comments (both were drafts, so CodeRabbit skipped review). Nothing was left to resolve or reject.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. No auth, credential, dependency, workflow or release changes.

Summary by CodeRabbit

  • Performance

    • Improved handling of large, fragmented event streams by avoiding repeated scans and unnecessary text processing.
  • Bug Fixes

    • Preserved correct stream framing and UTF-8 accounting across chunk boundaries, including Unicode characters, line endings, invalid text, and incomplete final frames.
  • Documentation

    • Clarified how stream buffering and byte accounting work across fragments.

StreamClash Dev and others added 4 commits October 23, 2026 04:00
Co-authored-by: Platinum-Scripts <133908028+Platinum-Scripts@users.noreply.github.com>
Co-authored-by: Platinum-Scripts <133908028+Platinum-Scripts@users.noreply.github.com>
Co-authored-by: Platinum-Scripts <133908028+Platinum-Scripts@users.noreply.github.com>
Record the cached final code unit and scan eligibility as soon as the
fragment joins the buffer, before the budget commit, so a throwing commit
cannot leave them describing an older carry.

Co-authored-by: Platinum-Scripts <133908028+Platinum-Scripts@users.noreply.github.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-22T19:09:47.389409Z daf0c1d PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 516aaaa and daf0c1d.

📒 Files selected for processing: 6
  • src/adapters/openai-chat.ts
  • src/server/sse-payload-rewrite.ts
  • structure/providers/mimo-bare-calls.md
  • structure/transports/byte-accounting.md
  • tests/adapters/openai/openai-chat-parallel-stream.test.ts
  • tests/responses/sse-payload-rewrite.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

📝 Walkthrough

OpenAI Chat and the SSE block buffer now avoid repeated work on fragmented input. The changes preserve existing byte accounting and framing behavior, with tests for chunk boundaries, Unicode, invalid UTF-8, and buffer cleanup.

Changes: Fragmented SSE accounting

Layer / File(s) Summary
OpenAI Chat carry accounting
src/adapters/openai-chat.ts, tests/adapters/openai/openai-chat-parallel-stream.test.ts, structure/transports/byte-accounting.md, structure/providers/mimo-bare-calls.md
Newline-free fragments retain the existing carry byte count instead of splitting and recounting the accumulated text. Tests cover large lines, CRLF, EOF, Unicode, invalid UTF-8, and budget release.
SSE block scanning and byte accounting
src/server/sse-payload-rewrite.ts, tests/responses/sse-payload-rewrite.test.ts, structure/transports/byte-accounting.md
Delimiter scans are skipped when appended fragments contain no CR or LF. The buffer tracks the final code unit for surrogate-boundary accounting and resets that state when emptied or cleared. Tests cover large fragmented frames, byte accounting, and queued appends.

Priority: ⬇️ Low

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

Change: Refactor

Possibly related PRs

  • lidge-jun/opencodex#4389: Introduces the SSE block buffer and related OpenAI streaming byte accounting that this change optimizes.
Merge Risk: ⚪ Minimal · up to daf0c

No merge-blocking regression is identified in the fragmented SSE changes. Merge after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed 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: performance improvements that avoid rescanning and flattening fragmented SSE carry. It matches the changes in src/adapters/openai-chat.ts a…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title clearly and concisely describes the main change: performance improvements that avoid rescanning and flattening fragmented SSE carry. It matches the changes in src/adapters/openai-chat.ts and src/server/sse-payload-rewrite.ts.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

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

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the enhancement New feature or request label Oct 22, 2026

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Reviewed the complete diff at daf0c1d897cd66bf3dd4224225c813da5c5fad3f. The stream changes preserve the existing reservation/release ordering while avoiding whole-carry splitting, UTF-8 encoding, regex subjects and boundary indexing on every fragment. The deterministic work-bound tests retain exact output, Unicode/invalid UTF-8, CRLF, EOF, queued append/compaction and clear semantics; the relevant accounting docs are synchronized.

Independent capped Linux/Bun 1.4.3 with fresh private homes: bun test tests/adapters/openai/openai-chat-parallel-stream.test.ts tests/responses/sse-payload-rewrite.test.ts — 67 pass / 0 fail / 556 assertions. Current-head hosted CI 39837224148, including the gates/Typecheck step, succeeded. The current review-thread snapshot has no unresolved findings or standing maintainer objection.

The full local suite/typecheck were not repeated on the shared capped host; the completed hosted Ubuntu run supplies broader validation. Approval is bound to this source head and diff, and does not certify a later dev union, native platforms or live provider/client behavior. All external sends in the focused behavior fixtures were stubbed; no real account, runtime configuration, service or OS trust state was changed.

@lidge-jun
lidge-jun merged commit c0d80ef into dev Oct 23, 2026
35 of 39 checks passed
@lidge-jun
lidge-jun deleted the sweep/w1-sse-carry branch October 23, 2026 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants