fix(examples): preserve background stream resume cursors - #2671
Conversation
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. |
Castiron custom code✅ No new custom-code files detected. 32 mixed files remain; 0 existing customizations changed. Compared 32 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 34071425990 --repo openai/openai-node \
--name castiron-custom-code-34071425990-1 --dir /tmp/castiron-custom-code-34071425990-1
git apply --stat /tmp/castiron-custom-code-34071425990-1/custom-code.patch
cat /tmp/castiron-custom-code-34071425990-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin fdb038e21509264bcdf81741a40988dd694188b1 bcb1378d7f8722ecb7530797865c40fddbf730a3
python3 scripts/castiron/custom_code_report.py report \
--base fdb038e21509264bcdf81741a40988dd694188b1 \
--head bcb1378d7f8722ecb7530797865c40fddbf730a3 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-bcb1378d7f87
cat /tmp/castiron-custom-code-bcb1378d7f87/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed bcb1378 against fdb038e. No substantive correctness or maintainability findings.
The caller-owned cursor matches docs/streaming.md and records consumed events rather than SDK read-ahead. Independently reproduced the distinction: after EOF at event 3, the base example prints 0,1,2,3,11,12 while this head prints 0–12; both retain complete final text because the helper accumulates the full replay before filtering emission. The direct cursor update is the simplest fit here, without new branches or abstractions.
Validation I executed: 185 focused example/ResponseStream/accumulator tests; all 34 fixture cases again with FORCE_COLOR=1; 38 independent base/head loopback checks on each of Node 22.0.0, 24.20.0, and 26.7.0. These cover cutoff/early EOF, ordered duplicate-free events, completion reuse, final snapshots, request counts, failed/incomplete responses, empty/missing-ID streams, SSE errors, and socket failures. CJS/ESM build/imports, repository lint/format, TypeScript 6, and diff whitespace checks passed.
Validation limits: registry installation failed, so checks used an isolated dependency copy from a checkout with an identical lockfile. No live API, full generated suite, or ecosystem matrix was run.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed exact head bcb1378. Tracking the last sequence number actually delivered to the caller and resuming after it fixes the hard-coded cursor gap without changing terminal handling or final response accumulation. The regression now disconnects before the final delta and asserts the complete ordered sequence set; CI is green and there are no unresolved threads. No in-scope findings.
3 participants
Summary
starting_after: 10.Reproduction
When the first connection ends cleanly after event 3 and the replay contains events 0–12, the current example prints events
0, 1, 2, 3, 11, 12. Its hard-coded cursor suppresses unseen text deltas 4–10. An interruption immediately afterresponse.createdsimilarly skips events 1–10.The SDK still replays all events internally and produces a complete final response. This is an example-level event-continuity bug, not lost final-response data or a problem with the SDK accumulator. Tracking the last observed sequence uses the same small pattern already documented in
docs/streaming.md.Validation
FORCE_COLORin the child makes all 34 cases pass with a forced-color parent without adding an ANSI parser.The generated suite and remaining ecosystem/packaging checks run in CI. This is separate from the completion-reuse and final-status fixes in #2557 and #2600.