Skip to content

perf(google): avoid rescanning fragmented SSE records - #7766

Merged
lidge-jun merged 1 commit into
devfrom
perf/google-sse-incremental-buffer-20261026
Oct 27, 2026
Merged

perf(google): avoid rescanning fragmented SSE records#7766
lidge-jun merged 1 commit into
devfrom
perf/google-sse-incremental-buffer-20261026

Conversation

@luvs01

@luvs01 luvs01 commented Oct 26, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Collaborator

Summary

Fragmented Google SSE input repeatedly measures and splits the entire unfinished line, making small chunks of a large record increasingly expensive. Measure only newly decoded text and split the retained line only when a fragment adds a newline.

Replacement storage remains reserved before concatenation, old/new overlap stays charged, and decoder EOF output uses the same accounting path. Add operation-count, split UTF-8, EOF, overflow, and reader-cleanup regressions, and update the owning contracts and test registry.

Verification

  • The unchanged commit 44f1b1db0a7733aeca1850d695c3305d7542efee completed the four Linux CI shards, Catalog async contracts, and required gates before submission. Job logs confirm all 10 new google-stream-fragmentation.test.ts cases ran and passed.
  • The author's Bun 1.4.4 focused run passed 101 Google adapter tests and 38 layout/tooling/ratchet tests. Typecheck, structure, privacy, and staged whitespace checks passed. The new operation-count tests fail on the unchanged implementation and pass with this fix; they measure parser work, not end-to-end throughput.
  • Those unchanged checks were not repeated during transfer because of shared-worktree storage/I/O constraints. A prior broader local run hit its 180-second bound with 19 failures: 18 reproduced on the base and were resolved by local proxy exclusions; one sidecar timeout passed in isolation but its broad-run cause remains unproven. No full local success is claimed.
  • Native macOS/Windows jobs were skipped by the PR workflow. Upstream Cross-platform CI and Catalog async contracts completed successfully on the exact head; no actionable review findings remain. The change is ready for maintainer review under the documented focused-validation exception.

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.

Co-authored-by: Epinephrine [email protected]

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of Google streaming responses when UTF-8 characters or records arrive across multiple chunks, including incomplete records at the end of a stream.
    • Strengthened memory-limit enforcement while processing streamed text, including decoded replacement characters.
    • Improved cleanup of buffered data and response resources when a stream is cancelled or stopped early.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •
edited
Loading

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 816e84b and 44f1b1d.

Files selected for processing: 6
  • scripts/test-layout/layout-adapters.json
  • src/adapters/google.ts
  • structure/providers/google.md
  • structure/transports/byte-accounting.md
  • tests/adapters/google/google-stream-fragmentation.test.ts
  • tests/fixtures/test-layout-expected.json

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

Walkthrough

The Google adapter now accounts for decoded SSE fragments incrementally, checks retained text for complete lines only when new text contains a newline, and applies the same accounting to decoder EOF output. New tests cover fragmentation, UTF-8, budget limits, cancellation, and cleanup.

Changes: Google SSE fragment accounting

Layer / File(s) Summary
Fragment accounting and validation
src/adapters/google.ts, tests/adapters/google/google-stream-fragmentation.test.ts, structure/providers/google.md, structure/transports/byte-accounting.md, scripts/test-layout/layout-adapters.json, tests/fixtures/test-layout-expected.json		 parseStream reserves byte budget for each decoded fragment before appending it. It skips splitting retained text when the fragment has no newline and measures the residual suffix after complete lines are consumed. Tests cover large fragmented records, split UTF-8, EOF budget boundaries, overflow, cancellation, and early-iteration cleanup. The documentation and test layout describe and register this coverage.

Priority: Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Merge Risk: Minimal · up to 44f1b

No actionable issue is established for this change; it is mergeable 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 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (4 skipped: 4… 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 primary change: reducing repeated scanning of fragmented Google SSE records. It matches the implementation, tests, and stated objectives.
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: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (4 skipped: 4 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 github-actions Bot added the enhancement New feature or request label Oct 26, 2026
@github-actions

github-actions Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@luvs01
luvs01 marked this pull request as ready for review October 26, 2026 15:11
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
🔒 Security Review ✅ Completed 2026-10-26T15:15:51.482633Z 44f1b1d Draft marked ready
ℹ️ 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.

@lidge-jun

lidge-jun commented Oct 27, 2026

Copy link
Copy Markdown
Owner

Maintainer integration into dev (MAINTAINERS.md, dev-only maintainer integration; not a self-approval).

  • Head merged: 44f1b1db0a7733aeca1850d695c3305d7542efee. PR CI run 40341118889 (pull_request) concluded success on this exact head: gates, structure gate, test 1/4 to 4/4, api usage, docker smoke, npm-global, storage policy, Catalog async contracts.
  • Independent review (L8 lane, GPT-6.1 Sol): APPROVE, zero blocking findings. The new fragmentation test fails against dev source (6 pass / 4 fail) and passes with this change. A differential probe over 2,287 split points across 12 fixtures found no event mismatches; the only intended difference is that tiny-budget EOF replacement now enforces byte admission.
  • Current dev (2b5148b12) merged locally into this head: tests/adapters/google/ plus the test-layout, layout-tooling and file-size-ratchet guards, and typecheck, pass. The one failure in a whole-folder google run (Antigravity live model discovery > uses the CCA agent list...) reproduces identically on unchanged dev and passes when its file runs alone on both, so it is pre-existing cross-file interference, not this change.
  • No submitted reviews, inline threads or change requests are outstanding.

@lidge-jun
lidge-jun merged commit 9a757f5 into dev Oct 27, 2026
37 of 39 checks passed
@lidge-jun
lidge-jun deleted the perf/google-sse-incremental-buffer-20261026 branch October 27, 2026 14:04
This was referenced Oct 28, 2026
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lidge-jun lidge-jun Awaiting requested review from lidge-jun lidge-jun is a code owner
@Ingwannu Ingwannu Awaiting requested review from Ingwannu Ingwannu is a code owner

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@luvs01 @lidge-jun