[0disoft]

Summary

Command Code's NDJSON reader UTF-8 encodes the entire growing buffer twice per read and searches the retained incomplete line again even when the new fragment contains no newline. For a record arriving as n fixed-size fragments, this repeats quadratic measurement/allocation and newline-search work. In the real adapter, a 65,568-byte ASCII record delivered in 256-byte reads causes 16,908,320 encoded bytes; an EOF-terminated 65,567-byte record causes 17,105,020.

Measure only each newly decoded fragment, add its byte size to the retained total, and search for newlines only when the new decoded text contains one. After draining complete lines, remeasure only the remaining suffix. The same probes now encode exactly 65,568 and 65,567 bytes respectively, with intact output and zero retained bytes on completion. This makes that fragmented-record bookkeeping linear; it is not an end-to-end latency or process-RSS measurement.

• Preserve fix(command-code): avoid double charging live NDJSON buffers #8047's delta-only buffer resizing, turn ceiling, admission-before-concatenation, suspended-yield ownership, Unicode decoding, event order and cancellation.
• Extend the existing buffer-budget test with deterministic total-encoding and newline-search work bounds for newline/EOF termination, and exact/refused EOF replacement-byte admission. Update the existing byte-accounting contract. No new test files or layout entries.
• No GUI, auth, credential, dependency, workflow or release changes.

Duplicate search covered open and closed issues/PRs for Command Code NDJSON, incremental accounting, quadratic work, TextEncoder, fragmented buffers and NDJSON encoding. No equivalent fix was found. Merged #8047 fixes duplicate budget charges and explicitly leaves asymptotic scanning unchanged; this patch preserves that fix. Closed #4389 concerns other adapters/shared Chat and SSE paths and does not change command-code.ts. #2531/#2532 concern non-record JSON validation. This implementation is independent.

Verification

Exact head: c1b809270011a70fc58a4f058e59cdcd2440098f. Base: dev at 883bf0fd7d6ef7a208223ffd363705ba387d13ad. Native Windows, Bun 1.4.4. Refetched upstream before submission: 1 ahead / 0 behind. The tree has not changed since validation.

• Before the runtime fix, bun test tests/providers/command-code-buffer-budget.test.ts --timeout 30000: 10 passed / 2 failed, 38 assertions. Both new work-bound cases failed at the encoding-volume assertion (16,908,320 and 17,105,020 measured characters); intact output and cleanup assertions passed.
• After the runtime fix, the same command: 12 passed / 0 failed, 40 assertions.
• After adding EOF decoder-replacement admission coverage, bun test --isolate tests/providers/command-code-buffer-budget.test.ts tests/providers/command-code-provider.test.ts tests/providers/command-code-error-finish.test.ts tests/providers/command-code-tool-text.test.ts tests/providers/command-code-tool-text-prose-split.test.ts tests/ci-workflows/file-size-ratchet.test.ts --timeout 30000: 166 passed / 0 failed, 40,778 assertions. Covers provider/tool behavior, whole/one-byte Unicode reads, exact/over-limit budgets, shared ownership, drain-before-read, consumer return, cancellation and unchanged file-size caps.
• bun run typecheck: passed, no type errors in 2,330 files.
• bun run structure:check: passed.
• bun run privacy:scan: passed.
• git diff --check, staged whitespace check before commit, and git diff upstream/dev...HEAD --check: passed. Working tree clean.
• A separate synthetic real-parser measurement on the final source confirmed 65,568 -> 65,568 and 65,567 -> 65,567 wire/encoded bytes, two delivered events and zero retained bytes. This supplemental probe uses the production budget factory.

Full-suite scope exception: the full local suite and broad import-connected test:changed were not run. This adapter imports OAuth, dispatch and shared bridge surfaces, so that run is disproportionate to the bounded private NDJSON-loop change on this Windows laptop. Five focused Command Code files plus the file-size gate were run explicitly. Broader integration/platform coverage remains for repository CI; no hosted CI success is claimed. No workflow dispatch or rerun was initiated, and regression fixtures use no live provider credentials or inference.

Applied test-design-guard to work-volume, Unicode/EOF boundaries and cleanup checks.

Author review: regression sensitivity

Rechecked the runtime diff, retained-byte ownership, split UTF-8 decoding, EOF flush and newline/EOF record handling on the same exact head. No additional runtime defect was found.

Two independent temporary mutations were tested with bun test tests/providers/command-code-buffer-budget.test.ts --test-name-pattern 'bounds fragmented NDJSON' --timeout 30000:

• Replacing fragment-only sizing with encoder.encode(buffer + decoded).byteLength preserves admission values but restores cumulative-prefix allocation. Both cases failed the encoding-work assertion: 8,486,944 > 262,272 and 8,552,510 > 262,268 measured characters.
• Removing only the new-text newline guard preserves the optimized byte sizing. Both cases passed encoding/output/cleanup assertions and failed the newline-search assertion: 8,486,944 > 131,136 and 8,552,510 > 131,134 scanned-character upper bounds.
• Each mutation returned 0 passed / 2 failed (exit 1) at its intended work-bound assertion. Source was restored byte-for-byte in finally, with SHA-256 verification after each run. Neither mutation is committed. The committed source and tests remain unchanged, so the existing passing validation above still applies; no broad suite was repeated.

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. Scope review preserves those surfaces and existing limits.

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 large, fragmented NDJSON command output, including records that end without a newline.
  • Incomplete UTF-8 sequences at the end of a stream are now included in buffer-limit checks, helping prevent output from exceeding the configured limit.
  • Stream processing continues to emit valid records and completion events when output remains within the limit.

Maintainer integration verification

I am integrating this PR into dev under the maintainer-integration policy without a second maintainer approval. Independent review of head c1b809270011a70fc58a4f058e59cdcd2440098f found no issues; no review threads or maintainer objections were open at inspection.

• Exact-head PR CI: Cross-platform CI, attempt 2 completed successfully, including ci, gates, test changed, and the applicable Ubuntu jobs. The full five-shard matrix and platform-specific jobs were filtered by PR policy. Catalog async contracts, attempt 2 passed.
• With temporary isolated HOME, OPENCODEX_HOME, CODEX_HOME, and CLAUDE_CONFIG_DIR: the five Command Code test files plus test-layout, file-size ratchet, and Lab boundary guards passed (220 tests, 0 failures); typecheck, privacy scan, and structure check passed. The two fragmented-record work-bound cases failed against the parent implementation and passed on the PR head.
• A temporary merge with dev at 1b87631d51c2547fe4e061e771cf3f46114ba269 applied cleanly and passed the same 220-test focused/guard set, typecheck, privacy scan, and structure check. The temporary merge was discarded afterward.
• Full local suite was not run: concurrent repository work and the cost of the full suite make the targeted tests and PR CI the practical local scope; broader coverage remains with the scheduled batch CI. No skipped platform jobs are claimed as passing.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The Command Code NDJSON parser now measures newly decoded text incrementally and searches for records only when a chunk contains a newline. Tests cover fragmented records and EOF replacement bytes. The accounting documentation describes the behavior and test bounds.

Changes: Command Code NDJSON accounting

Layer / File(s)	Summary
Incremental accounting and validation src/adapters/command-code.ts:440-453, tests/providers/command-code-buffer-budget.test.ts:1-45, tests/providers/command-code-buffer-budget.test.ts:62-81, structure/transports/byte-accounting.md:158-161	The parser adds each decoded chunk’s encoded byte length to the retained-buffer count and scans for records only when new text contains a newline. Tests check measurement and scan bounds for fragmented records, plus buffer-limit behavior for incomplete UTF-8 at EOF. The documentation describes the accounting rules and test coverage.

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

Change: Refactor

Possibly related PRs

• lidge-jun/opencodex#8047: Changes NDJSON retained-buffer charge resizing in the same Command Code adapter and has tests extended by this PR.
• lidge-jun/opencodex#8015: Applies incremental measurement and newline-search changes to the separate Ollama Native adapter.
• lidge-jun/opencodex#8069: Includes an Ollama Native adapter change for incremental NDJSON measurement and newline scanning.

Merge Risk: Minimal · up to c1b80

No actionable merge blocker is established for the incremental NDJSON accounting change. Merge after normal CI checks succeed.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the incremental measurement of fragmented NDJSON in src/adapters/command-code.ts.
Linked Issues check		No active directly linked issue remains. Issue #4389 is closed and provides historical context only. No linked-issue coding requirements apply.
Out of Scope Changes check		The changes match the stated NDJSON optimization. src/adapters/command-code.ts measures decoded fragments and limits newline searches. tests/providers/command-code-buffer-budget.test.ts tests work…
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…

Full details: Out of Scope Changes check

Explanation

The changes match the stated NDJSON optimization. src/adapters/command-code.ts measures decoded fragments and limits newline searches. tests/providers/command-code-buffer-budget.test.ts tests work bounds and EOF admission. structure/transports/byte-accounting.md documents the accounting contract. Issue #4389 is historical context, not a scope requirement.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. 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 0 functions across 2 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

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

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: lidge-jun/opencodex
• Branch: dev
• File: .coderabbit.yaml
• Commit: 883bf0f
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 22715b78-a1cd-460e-9601-9c7f4256f55c

Commits

Reviewing files that changed from the base of the PR and between 883bf0f and c1b8092.

Files selected for processing: 3

• src/adapters/command-code.ts
• structure/transports/byte-accounting.md
• tests/providers/command-code-buffer-budget.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.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ 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.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

Hygiene

✅ Deterministic PR hygiene checks passed.

[Ingwannu]

Approving head c1b809270011a70fc58a4f058e59cdcd2440098f (full diff, 3 files).

Checked:

• src/adapters/command-code.ts ndjson(): the invariant bufferBytes === utf8Bytes(buffer) holds on every path. TextDecoder (streaming) only emits complete code points and maps invalid/incomplete-at-EOF input to U+FFFD, so the UTF-8 size of buffer + decoded is exactly bufferBytes + utf8Bytes(decoded); when no newline arrives the buffer is unchanged after the append, so skipping the remeasure is exact, and the drained residual is remeasured only when lines were removed.
• The "retained suffix has no newline" guard is sound: after each drain loop buffer contains no \n, so only newly decoded text can complete a record.
• #8047's ordering is preserved: resizeBuffer (which can throw on overflow) still runs before buffer += decoded, the single live charge is resized by delta, and finally releases bufferBytes. Event order, the suspended yield* during draining, EOF flush and cancellation are unchanged.
• Tests: the work-bound cases exercise the real adapter for newline- and EOF-terminated 64 KiB records in 256-byte reads and assert intact output, zero retained bytes, and linear encode/scan volume; the EOF U+FFFD cases pin replacement-byte admission at 2 (refused) and 3 (admitted) bytes. The author's parent-run and mutation results show the bounds fail on the quadratic variants.
• structure/transports/byte-accounting.md matches the new behavior.
• Hosted CI on this exact head: Cross-platform CI gates, test changed, structure gate and ci green; Catalog async contracts green. dev has not touched command-code.ts, its tests or the structure doc since the base 883bf0fd7d.

Non-blocking: when a fragment does contain a newline, buffer.indexOf("\n") still rescans the retained prefix once (it could start at the pre-append length), but that is one pass per newline-bearing read and stays within the asserted bound.