You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Devin Connect-RPC buffers a fragmented frame as a queue of chunks. When the frame becomes complete, drop() currently calls Array.shift() for each fully consumed chunk. A 64 KiB text frame delivered in four-byte reads caused 16,388 queue-prefix removal calls in the real streamChatEvents parser.
Compact surviving chunks in place with copyWithin and length truncation, or clear the queue when no bytes remain. This avoids allocating an unused removed-chunk result array. Keep a partial head as its existing slice and preserve every later chunk in order. The operation-count regression bounds the same input to at most eight prefix-removal calls, observing shift, splice and copyWithin rather than depending on CPU speed.
Runtime change is limited to the private drop() helper and its nearby queue comment in src/adapters/devin/cloud-direct/chat.ts.
Preserve protobuf decoding, header/payload ordering, gzip, the frame/inflation ceilings, EOS/trailer semantics, deadlines, abort and reader cleanup. No auth, credential, endpoint, dependency, workflow, GUI or release changes.
Add a deterministic real-parser work-count regression and six framing cases covering one-byte, split and coalesced reads, Unicode, empty frames and gzip. Register the test in both layout maps and update the byte-accounting contract.
In a separate native Windows / Bun 1.4.4 probe of the original and patched drop() bodies, draining 65,536 four-byte chunks (256 KiB) while retaining a 16-byte suffix took a five-sample median 11.8604 ms -> 1.0870 ms (about 10.9x shorter). Both versions retained exactly 16 bytes in one chunk. This measures queue drainage only, not provider inference, total response latency, or RSS. The durable regression asserts operation counts; it has no timing threshold and makes no runtime-specific Big-O claim about Array.shift.
Duplicate searches covered open and closed issues/PRs for Devin queue, stream, fragmentation, Connect chunks, cloud-direct performance and chunkQueue.shift. No equivalent change or existing PR for this branch was found. Related results #4450 concern response-header deadlines and #6344 concern late reasoning signatures; they address different behavior. This change is independently implemented.
Verification
Initial verification head: 24119a360e7e288ea364a5b9d2bd19e1db145193 (current-head follow-up below). Base: dev at 63de499e1. Native Windows, Bun 1.4.4. The fetched base is current and the committed worktree is clean.
Before the runtime change, bun test tests/providers/devin-stream-queue.test.ts --timeout 30000: 6 passed / 1 failed, 8 assertions. Output/framing cases passed; the work-count assertion failed with 16,388 prefix removals against an upper bound of eight.
After the change, bun test --isolate tests/providers/devin-stream-queue.test.ts tests/providers/devin-hardening.test.ts tests/providers/devin-stream-deadline.test.ts tests/providers/devin-chat-wire-fixes.test.ts --timeout 30000: 125 passed / 0 failed, 431 assertions.
bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts --timeout 30000: 38 passed / 0 failed, 664 assertions. Both explicit layout mappings and the existing file-size ceilings pass.
bun run typecheck: passed; no type errors in 2,253 files.
bun run structure:check: passed with the new test staged for index-based path validation.
bun run privacy:scan: passed.
git diff --cached --check before commit and git diff upstream/dev...HEAD --check after commit: passed.
Applied test-design-guard to deterministic work bounds and framing preservation. The tests use a synthetic inference executor, explicit unavailable-catalog state and restored JWT opt-in environment state; no provider credential or live provider request is used.
Full-suite scope exception: local full-suite and broad import-connected test:changed runs were not performed. The cloud-direct adapter's shared imports reach wider provider, OAuth and bridge integrations, making that run disproportionate for this private queue-helper change on this Windows host. Four focused runtime/provider files and three source-as-data layout/ratchet files were exercised explicitly above. Broader integration and platform coverage remain for repository CI; no hosted CI result is claimed as passing. No workflow dispatch or rerun was initiated.
Author review follow-up
Current exact head: 36f795edc054c4f34df77ee50b0de4c84da21474.
Review found no additional framing defect, but the first implementation's splice creates an unused array containing the removed chunk references. Use copyWithin followed by length truncation to compact survivors in place. Keep the full-drain clear, partial-head slices and subsequent frame order unchanged; update the owning contract.
Extend the existing deterministic real-parser test to observe copyWithin as well as shift/splice and count removed-result array slots. Against 24119a360, bun test tests/providers/devin-stream-queue.test.ts --test-name-pattern 'bounded queue-prefix' --timeout 30000 failed at the intended assertion: 16,387 discarded chunk-reference slots instead of zero, while the removal-call bound passed. This is allocation regression evidence, not a process-RSS measurement.
bun test --isolate tests/providers/devin-stream-queue.test.ts tests/providers/devin-hardening.test.ts tests/providers/devin-stream-deadline.test.ts tests/providers/devin-chat-wire-fixes.test.ts --timeout 30000: 125 passed / 0 failed, 432 assertions. The test asserts zero discarded-result slots and at most eight prefix-removal calls, plus intact output.
bun run typecheck: passed, no type errors in 2,253 files. bun run structure:check and bun run privacy:scan: passed. Diff whitespace checks passed.
Repeated the isolated drop comparison against the original dev helper with the current implementation: 65,536 four-byte chunks plus a retained 16-byte suffix, five samples per version, medians 11.8604 ms -> 1.0870 ms. Both versions retain exactly the same suffix. These current measurements replace the first implementation's timing in Summary; no whole-response speedup is claimed.
Applied test-design-guard to allocation evidence and framing preservation. Layout mappings are unchanged by this follow-up, so their preceding 38 passing checks were not repeated. The documented full-suite exception remains; hosted results must cover this new exact head independently. No live provider request, workflow dispatch or rerun was initiated.
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; this patch does not change those surfaces.
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
Bug Fixes
Improved handling of streamed responses so fragmented content is delivered intact and in the correct order, including when data arrives in small or uneven chunks.
Reduced unnecessary processing while consuming stream data, helping large responses be handled more efficiently. Empty and compressed frames continue to be supported.
Streamed content remains correctly ordered when a read ends partway through a chunk, with no change to existing end-of-stream handling.
No actionable comments were generated in the recent review.
Walkthrough
The Devin stream queue now clears the queue when all bytes are consumed. Otherwise, it removes the consumed prefix by compacting the array in place and preserves any partially consumed chunk. New tests cover fragmented reads and frame order. The test-layout mapping and fixture include the new test.
The drain clears the queue when no bytes remain. Otherwise, it counts fully consumed chunks, slices a partially consumed chunk when needed, and compacts the queue with copyWithin. The documentation describes this behavior and notes that existing caps, EOS handling, and teardown remain unchanged.
Queue drain tests and registration tests/providers/devin-stream-queue.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
Tests cover a 64 KiB frame sent in 4-byte reads, queue-removal operations, and text event order across six read sizes. They also cover Unicode, empty frames, and compressed text. The test-layout mapping and fixture include the new test.
lidge-jun/opencodex#7607: Moves a test mapping out of scripts/test-layout/layout.json, the same layout file updated here.
Merge Risk: Minimal · previous review 36f79
The queue-drain optimization is mergeable with no established current-head risk.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: batching removal of consumed Devin stream chunks for performance.
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
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 with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
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.
Reviewing files that changed from the base of the PR and between 24119a3 and 36f795e.
Files selected for processing: 3
src/adapters/devin/cloud-direct/chat.ts
structure/transports/byte-accounting.md
tests/providers/devin-stream-queue.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
Maintainer integration into dev (account lidge-jun) at head 90bc906689a9dd4803abc8a60875fd7fb864e03a. The PR is moved out of draft by a maintainer for this integration; the contributor checklist gate re-drafted it after a maintainer dev-merge push.
perf(devin): batch consumed stream chunk removal. A maintainer merge of current dev resolved one docs-only conflict in structure/transports/byte-accounting.md by keeping both paragraphs verbatim.
Exact-head required CI passed (ci, enforce-target, hygiene, test 1/4–4/4).
Independent review (gpt-6.1-terra, high): fresh final review PASS on the reviewed code (Devin/provider tests 250/0); the merge commit adds only the dev merge.
Local on the merged head: typecheck clean, layout/ratchet 38/0, structure:check pass, Devin stream-queue tests 7/0.
No unresolved threads or changes-requested reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ci:fullRun the four full Ubuntu test shards on this pull requestenhancementNew feature or request
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
lidge-jun
added
the
ci:full
Summary
Devin Connect-RPC buffers a fragmented frame as a queue of chunks. When the frame becomes complete,
drop()currently callsArray.shift()for each fully consumed chunk. A 64 KiB text frame delivered in four-byte reads caused 16,388 queue-prefix removal calls in the realstreamChatEventsparser.Compact surviving chunks in place with
copyWithinand length truncation, or clear the queue when no bytes remain. This avoids allocating an unused removed-chunk result array. Keep a partial head as its existing slice and preserve every later chunk in order. The operation-count regression bounds the same input to at most eight prefix-removal calls, observingshift,spliceandcopyWithinrather than depending on CPU speed.drop()helper and its nearby queue comment insrc/adapters/devin/cloud-direct/chat.ts.drop()bodies, draining 65,536 four-byte chunks (256 KiB) while retaining a 16-byte suffix took a five-sample median 11.8604 ms -> 1.0870 ms (about 10.9x shorter). Both versions retained exactly 16 bytes in one chunk. This measures queue drainage only, not provider inference, total response latency, or RSS. The durable regression asserts operation counts; it has no timing threshold and makes no runtime-specific Big-O claim aboutArray.shift.Duplicate searches covered open and closed issues/PRs for Devin queue, stream, fragmentation, Connect chunks, cloud-direct performance and
chunkQueue.shift. No equivalent change or existing PR for this branch was found. Related results #4450 concern response-header deadlines and #6344 concern late reasoning signatures; they address different behavior. This change is independently implemented.Verification
Initial verification head:
24119a360e7e288ea364a5b9d2bd19e1db145193(current-head follow-up below). Base:devat63de499e1. Native Windows, Bun 1.4.4. The fetched base is current and the committed worktree is clean.bun test tests/providers/devin-stream-queue.test.ts --timeout 30000: 6 passed / 1 failed, 8 assertions. Output/framing cases passed; the work-count assertion failed with 16,388 prefix removals against an upper bound of eight.bun test --isolate tests/providers/devin-stream-queue.test.ts tests/providers/devin-hardening.test.ts tests/providers/devin-stream-deadline.test.ts tests/providers/devin-chat-wire-fixes.test.ts --timeout 30000: 125 passed / 0 failed, 431 assertions.bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts --timeout 30000: 38 passed / 0 failed, 664 assertions. Both explicit layout mappings and the existing file-size ceilings pass.bun run typecheck: passed; no type errors in 2,253 files.bun run structure:check: passed with the new test staged for index-based path validation.bun run privacy:scan: passed.git diff --cached --checkbefore commit andgit diff upstream/dev...HEAD --checkafter commit: passed.Applied test-design-guard to deterministic work bounds and framing preservation. The tests use a synthetic inference executor, explicit unavailable-catalog state and restored JWT opt-in environment state; no provider credential or live provider request is used.
Full-suite scope exception: local full-suite and broad import-connected
test:changedruns were not performed. The cloud-direct adapter's shared imports reach wider provider, OAuth and bridge integrations, making that run disproportionate for this private queue-helper change on this Windows host. Four focused runtime/provider files and three source-as-data layout/ratchet files were exercised explicitly above. Broader integration and platform coverage remain for repository CI; no hosted CI result is claimed as passing. No workflow dispatch or rerun was initiated.Author review follow-up
Current exact head:
36f795edc054c4f34df77ee50b0de4c84da21474.splicecreates an unused array containing the removed chunk references. UsecopyWithinfollowed by length truncation to compact survivors in place. Keep the full-drain clear, partial-head slices and subsequent frame order unchanged; update the owning contract.copyWithinas well asshift/spliceand count removed-result array slots. Against24119a360,bun test tests/providers/devin-stream-queue.test.ts --test-name-pattern 'bounded queue-prefix' --timeout 30000failed at the intended assertion: 16,387 discarded chunk-reference slots instead of zero, while the removal-call bound passed. This is allocation regression evidence, not a process-RSS measurement.bun test --isolate tests/providers/devin-stream-queue.test.ts tests/providers/devin-hardening.test.ts tests/providers/devin-stream-deadline.test.ts tests/providers/devin-chat-wire-fixes.test.ts --timeout 30000: 125 passed / 0 failed, 432 assertions. The test asserts zero discarded-result slots and at most eight prefix-removal calls, plus intact output.bun run typecheck: passed, no type errors in 2,253 files.bun run structure:checkandbun run privacy:scan: passed. Diff whitespace checks passed.dropcomparison against the original dev helper with the current implementation: 65,536 four-byte chunks plus a retained 16-byte suffix, five samples per version, medians 11.8604 ms -> 1.0870 ms. Both versions retain exactly the same suffix. These current measurements replace the first implementation's timing in Summary; no whole-response speedup is claimed.Applied test-design-guard to allocation evidence and framing preservation. Layout mappings are unchanged by this follow-up, so their preceding 38 passing checks were not repeated. The documented full-suite exception remains; hosted results must cover this new exact head independently. No live provider request, workflow dispatch or rerun was initiated.
Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit