[PR #911] [MERGED] perf(providers): assemble fragmented responses once per line #2341

New issue

Closed

opened 2026-10-16 21:17:33 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:17:33 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/911
Author: @morluto
Created: 10/7/2026
Status: ✅ Merged
Merged: 10/7/2026
Merged by: @morluto

Base: main ← Head: codex/provider-response-framing

📝 Commits (1)

78dc917 perf(providers): assemble fragmented responses once per line

📊 Changes

4 files changed (+135 additions, -2 deletions)

View changed files

📝 src/ghidra/GhidraResponseBuffer.test.ts (+49 -0)
📝 src/ghidra/GhidraResponseBuffer.ts (+6 -1)
📝 src/hopper/HopperResponseStream.test.ts (+74 -0)
📝 src/hopper/HopperResponseStream.ts (+6 -1)

📄 Description

Summary

Hopper and Ghidra response adapters now collect fragments until a newline arrives, then assemble the line for their existing parsers. Large fragmented responses no longer repeatedly concatenate and scan the accumulated payload.

Discovered and reproduced during the requested whole-codebase performance audit; no separate tracking issue was opened.

Problem and expected behavior

For a 16 MiB response arriving in 4 KiB fragments, the adapters spend several seconds rebuilding/scanning increasingly large strings. Doubling the response size roughly quadruples the cost. Framing should process incoming bytes proportionally while preserving complete responses, validation, correlation, and cleanup.

Change and scope

Retain fragments until the incoming chunk contains a line terminator, join once, and use the existing line-delivery loop. Reset clears pending fragments as well as assembled text. Provider-specific parsing remains in each adapter; no new shared protocol abstraction or frame cap is introduced.

Contract and boundary impact

Semantic owner/earliest stage: decoded UTF-8 provider socket response framing.
CLI/MCP/tool catalog: no contract or inventory change.
Provider/bridge/platform compatibility: unchanged Hopper/Ghidra NDJSON contracts and producer bridges.
Evidence/provenance: complete payloads retained; whitespace, Unicode, correlation, and errors keep existing meanings.
Process/authorization/cleanup: no new authority; reset releases both pending and assembled data.
Generated metadata/package/installation: none.

Evidence and regression coverage

Regressions cover fragmented Unicode/CRLF frames, blank lines, multiple responses plus a trailing partial response, reset during delivery, malformed Hopper abort, and fragmented 8 MiB responses. Existing tests accepting responses above former caps still pass.
Synthetic production-adapter benchmark, Node 24.18.0/macOS arm64, three samples per workload, fixed input hashes and 4 KiB fragments: 16 MiB Ghidra median 4,476 → 3.53 ms; Hopper median 3,628 → 18.80 ms. Includes output equality checks and Hopper protocol parsing, excludes payload construction. These measure framing rather than complete engine operations.
Real installed Hopper verification passed with source-owned host-native fixtures, provider semantics, target switching, unrelated-process preservation, and clean shutdown.
Remaining proof gaps: real Ghidra was not run because GHIDRA_INSTALL_DIR is not configured. Fake transport checks establish narrower framing/lifecycle behavior. Windows provider hosts and arbitrary workload distributions are unverified.
Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm run test:focused -- src/ghidra/GhidraResponseBuffer.test.ts src/hopper/HopperResponseStream.test.ts tests/boundary/providers/ghidra/ghidraClient.test.ts tests/boundary/providers/hopper/hopperClient.cleanup.test.ts tests/boundary/providers/hopper/hopperClient.restart.test.ts tests/boundary/providers/hopper/hopperClient.protocol.test.ts tests/boundary/providers/hopper/hopperClient.progress.test.ts — 59 passed.
npm run check:fast and npm run build:cached — passed.
npm run verify:hopper — source-owned fixture validation and real Hopper lane passed on macOS arm64; clean shutdown and unrelated Hopper survival confirmed.
git diff --check — passed.
Independent Standards and Spec reviews — no actionable findings; reviewer reran all eight framing tests.

Compatibility, safety, and release

Breaking changes/migration: none.
Real provider/OS coverage: Hopper on macOS arm64; real Ghidra and other hosts unverified.
Package/release metadata: none; no binaries or profiling artifacts included.
Safety review: complete payloads, validation/abort semantics, reset during callbacks, and lifecycle cleanup preserved.

Review checklist

One focused outcome and Conventional Commit title.
Origin of finding and absence of separate issue explained.
Observable behavior and failure regressions covered.
No owning docs, contract, or generated metadata changes needed.
Existing provider-result contracts preserved.
Final diff checked for secrets, unrelated edits, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/911 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/7/2026 **Status:** ✅ Merged **Merged:** 10/7/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/provider-response-framing` --- ### 📝 Commits (1) - [`78dc917`](https://github.com/morluto/rea/commit/78dc917bf2119eedb3a7a23c2d9d5e54e6891021) perf(providers): assemble fragmented responses once per line ### 📊 Changes **4 files changed** (+135 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/ghidra/GhidraResponseBuffer.test.ts` (+49 -0) 📝 `src/ghidra/GhidraResponseBuffer.ts` (+6 -1) 📝 `src/hopper/HopperResponseStream.test.ts` (+74 -0) 📝 `src/hopper/HopperResponseStream.ts` (+6 -1) </details> ### 📄 Description ## Summary Hopper and Ghidra response adapters now collect fragments until a newline arrives, then assemble the line for their existing parsers. Large fragmented responses no longer repeatedly concatenate and scan the accumulated payload. Discovered and reproduced during the requested whole-codebase performance audit; no separate tracking issue was opened. ## Problem and expected behavior For a 16 MiB response arriving in 4 KiB fragments, the adapters spend several seconds rebuilding/scanning increasingly large strings. Doubling the response size roughly quadruples the cost. Framing should process incoming bytes proportionally while preserving complete responses, validation, correlation, and cleanup. ## Change and scope Retain fragments until the incoming chunk contains a line terminator, join once, and use the existing line-delivery loop. Reset clears pending fragments as well as assembled text. Provider-specific parsing remains in each adapter; no new shared protocol abstraction or frame cap is introduced. ## Contract and boundary impact - Semantic owner/earliest stage: decoded UTF-8 provider socket response framing. - CLI/MCP/tool catalog: no contract or inventory change. - Provider/bridge/platform compatibility: unchanged Hopper/Ghidra NDJSON contracts and producer bridges. - Evidence/provenance: complete payloads retained; whitespace, Unicode, correlation, and errors keep existing meanings. - Process/authorization/cleanup: no new authority; reset releases both pending and assembled data. - Generated metadata/package/installation: none. ## Evidence and regression coverage - Regressions cover fragmented Unicode/CRLF frames, blank lines, multiple responses plus a trailing partial response, reset during delivery, malformed Hopper abort, and fragmented 8 MiB responses. Existing tests accepting responses above former caps still pass. - Synthetic production-adapter benchmark, Node 24.18.0/macOS arm64, three samples per workload, fixed input hashes and 4 KiB fragments: 16 MiB Ghidra median 4,476 → 3.53 ms; Hopper median 3,628 → 18.80 ms. Includes output equality checks and Hopper protocol parsing, excludes payload construction. These measure framing rather than complete engine operations. - Real installed Hopper verification passed with source-owned host-native fixtures, provider semantics, target switching, unrelated-process preservation, and clean shutdown. - Remaining proof gaps: real Ghidra was not run because `GHIDRA_INSTALL_DIR` is not configured. Fake transport checks establish narrower framing/lifecycle behavior. Windows provider hosts and arbitrary workload distributions are unverified. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:focused -- src/ghidra/GhidraResponseBuffer.test.ts src/hopper/HopperResponseStream.test.ts tests/boundary/providers/ghidra/ghidraClient.test.ts tests/boundary/providers/hopper/hopperClient.cleanup.test.ts tests/boundary/providers/hopper/hopperClient.restart.test.ts tests/boundary/providers/hopper/hopperClient.protocol.test.ts tests/boundary/providers/hopper/hopperClient.progress.test.ts` — 59 passed. - `npm run check:fast` and `npm run build:cached` — passed. - `npm run verify:hopper` — source-owned fixture validation and real Hopper lane passed on macOS arm64; clean shutdown and unrelated Hopper survival confirmed. - `git diff --check` — passed. - Independent Standards and Spec reviews — no actionable findings; reviewer reran all eight framing tests. ## Compatibility, safety, and release - Breaking changes/migration: none. - Real provider/OS coverage: Hopper on macOS arm64; real Ghidra and other hosts unverified. - Package/release metadata: none; no binaries or profiling artifacts included. - Safety review: complete payloads, validation/abort semantics, reset during callbacks, and lifecycle cleanup preserved. ## Review checklist - [x] One focused outcome and Conventional Commit title. - [x] Origin of finding and absence of separate issue explained. - [x] Observable behavior and failure regressions covered. - [x] No owning docs, contract, or generated metadata changes needed. - [x] Existing provider-result contracts preserved. - [x] Final diff checked for secrets, unrelated edits, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:17:33 +00:00