Repository navigation
perf(sse): avoid rescanning and flattening fragmented SSE carry (carry #7497, #7498) - #7541
Conversation
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>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
|
✅ Deterministic PR hygiene checks passed. |
Ingwannu
left a comment
There was a problem hiding this comment.
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.
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.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).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, sonext()advances its scan cursor without passing the growing carry to the regex again. The surrogate-boundary check inappend()uses a cached last code unit instead of indexing the growing carry. Emptying or clearing the buffer resets both.append()now updates the cached last code unit and scan eligibility immediately afterbuffer += fragment, before the budget commit, so a throwing commit cannot leave them describing an older carry.structure/transports/byte-accounting.mdandstructure/providers/mimo-bare-calls.md(the owning docs). The one doc conflict with currentdevwas resolved by keeping the newercarryReplayRefusalparagraph 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_DIRandOPENCODEX_HOME(mktemp),OCX_TEST_RUNNER_BUNpointed at a Bun 1.4.3 binary, and heavy commands under/usr/bin/lockf -k /tmp/ocx-heavy-test.lock.src/reverted toorigin/devand the new tests kept, the three tests fail; with the patch they pass:TextEncoder.encodeinput characters, 256 KiB tool-call frame in 512-byte fragments: 67,764,469 before vs bound < 2,099,024.charCodeAtsubject 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 base516aaaaf84): 35,703 pass / 42 skip / 5 fail across 1,619 files (exit 1). All five failures are Antigravity User-Agent/onboarding cases intests/adapters/google/google-antigravity-oauth.test.tsandtests/adapters/google/gemini-4-argon-antigravity.test.ts(expected IDE UA string mismatch). Those files reproduce the same 17 pass / 5 fail on an untouchedorigin/devworktree at516aaaaf84, 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.shon a local merge commit (EVENT_NAME=pull_request PATH_SELECTED=false):windows=false(no Windows path or marker), so nolane=alldispatch is needed.bun run testsuite (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
Summary by CodeRabbit
Performance
Bug Fixes
Documentation