Repository navigation
fix: [responses] Avoid quadratic comparisons when accumulating long text - #2434
Conversation
Castiron custom code✅ No new custom-code files detected. 34 mixed files remain; 0 existing customizations changed. Compared 34 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 32425237895 --repo openai/openai-node \
--name castiron-custom-code-32425237895-1 --dir /tmp/castiron-custom-code-32425237895-1
git apply --stat /tmp/castiron-custom-code-32425237895-1/custom-code.patch
cat /tmp/castiron-custom-code-32425237895-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 4e588c2fed7f5987381495f9cca245e245ac9cbd e41c4c3d55d51bc1f97a020e79274a48314083cd
python3 scripts/castiron/custom_code_report.py report \
--base 4e588c2fed7f5987381495f9cca245e245ac9cbd \
--head e41c4c3d55d51bc1f97a020e79274a48314083cd --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-e41c4c3d55d5
cat /tmp/castiron-custom-code-e41c4c3d55d5/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
apcha-oai
changed the title
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Independently verified the response-stream accumulator removes repeated full-prefix comparisons while preserving canonical accumulated text, mixed content, ordering, caller-owned snapshots, and existing stream/error behavior. The deterministic 16,000-delta regression directly detects the previous quadratic path, and all CI checks are green.
Summary
#2354 made direct
accumulateResponse(event, snapshot)calls recount caller-owned output so that edits between calls cannot leave stale text behind. That correctness guarantee is necessary, but the recount also compared the rebuilt string with the previous aggregate. For a long response, those independently accumulated strings can be equal only after walking the entire prefix again on every delta.Assign the authoritative recount directly. A local replay of 16,000 64-byte deltas (1.024 MB) dropped from about 1.5 seconds to 14 ms on Node 22 and 24. This preserves the existing public API, snapshot-mutation behavior, and privately owned
ResponseStreamcontext; it does not introduce a cache or pretend the necessary content-part scan can be eliminated.Add a deterministic public-entrypoint regression for that long-response shape. It detects reads of the old aggregate before the recount is installed, so the original implementation fails without relying on a wall-clock threshold.