Skip to content

Optimize extractValueForKey with early guard and streaming line scan - #1215

Open
nordicnode wants to merge 1 commit into
CodebuffAI:mainfrom
nordicnode:perf-extract-value-for-key
Open

Optimize extractValueForKey with early guard and streaming line scan#1215
nordicnode wants to merge 1 commit into
CodebuffAI:mainfrom
nordicnode:perf-extract-value-for-key

Conversation

@nordicnode

Copy link
Copy Markdown

Optimize extractValueForKey with early guard and streaming line scan

Summary

• In cli/src/utils/implementor-helpers.ts, optimize extractValueForKey to avoid allocating an array of all lines on every invocation.
extractValueForKey is called repeatedly for every tool and agent block during message rendering to extract file, message, unifiedDiff, patch, and errorMessage.
• Previously, extractValueForKey unconditionally called output.split('\n'). For multi-thousand line tool results or diffs, this allocated an array of thousands of strings and performed regex checks across all lines even when the searched key was completely absent, churning megabytes of garbage per render.
• Added an instant substring guard (if (!output || !output.includes(key + ':')) return null) and replaced output.split('\n') with an index-based line scanner (indexOf('\n')). Keys located early in the output (e.g. message: ... on line 1 or 2) now return immediately without parsing or allocating the remainder of the output.
• Benchmark: In a 5,000-line output, lookups for missing keys dropped from 11.02ms down to 0.20ms (55x speedup), and finding a key on line 1 dropped from 17.41ms down to 0.31ms (56x speedup across 100 runs).
• Adds unit tests in cli/src/utils/__tests__/implementor-helpers.test.ts verifying colon-less key rejection, line termination handling, and performance on large outputs.

Test plan

[✓] bun test --config=/dev/null src/utils/__tests__/implementor-helpers.test.ts — 102 pass, 0 fail
[✓] bun run --cwd cli typecheck — 0 errors
[✓] PR hygiene check passed

@codebuff-team

Copy link
Copy Markdown
Contributor

Good change. extractValueForKey is called per tool/agent block during rendering, so avoiding split('\n') on multi-KB outputs is a real win, and the benchmark numbers you cite (11ms → 0.2ms) are believable given the earlier code always materialized the whole line array.

Correctness check: the early guard output.includes(key + ':') is safe as a filter — since the per-line regex ^\s*([A-Za-z0-9_]+):\s*(.*)$ requires the exact key followed immediately by :, any real match implies the literal substring key: exists somewhere in output. So there's no false-negative risk, only occasional wasted scans when the substring appears inside unrelated content (e.g. somefile:), which is harmless.

The streaming rewrite of the pipe-continuation block (multi-line | values) preserves the original semantics: empty lines still push '' and continue, indent < baseIndent still breaks, and the last-line-without-trailing-newline case is handled via the lineEnd === -1 ? len fallback. I traced through the added tests by hand and they match the implementation's behavior.

Tests are appropriately scoped (colon-less rejection, no-trailing-newline extraction, 5000-line perf/correctness) and live in the existing __tests__ file, matching repo conventions. Diff is small, in-scope (cli/src/utils), and doesn't touch forbidden paths.

Minor nit for future PRs: the benchmark numbers in the description aren't reproducible from the test file itself (no timing assertions), so if a reviewer wants performance regression protection, consider adding a rough timing bound test — not blocking here.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree labels Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants