fix(examples): avoid extra requests for empty tool calls - #2672
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 34071995510 --repo openai/openai-node \
--name castiron-custom-code-34071995510-1 --dir /tmp/castiron-custom-code-34071995510-1
git apply --stat /tmp/castiron-custom-code-34071995510-1/custom-code.patch
cat /tmp/castiron-custom-code-34071995510-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin fdb038e21509264bcdf81741a40988dd694188b1 0946565a07f2537088727f6fc044fdcbb6f53b1f
python3 scripts/castiron/custom_code_report.py report \
--base fdb038e21509264bcdf81741a40988dd694188b1 \
--head 0946565a07f2537088727f6fc044fdcbb6f53b1f --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-0946565a07f2
cat /tmp/castiron-custom-code-0946565a07f2/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
There was a problem hiding this comment.
Independently reproduced the base example's unwanted second/third requests for completed empty tool-call arrays. This guard matches the SDK runner and stays after full stream reduction; it is the simplest correct fix without changing the reducer or introducing another abstraction. No correctness, security, or structural findings.
Verified the actual example through the built public SDK on Node 22.0.0, 24.20.0, and 26.7.0: omitted/empty calls terminate directly and after a tool round trip with exact counts 1/1/2/2, natural exits, complete output, and preserved fragmented arguments/IDs/results. Five additional error-path checks preserve failures without extra requests. The six-case regression fixture passes on all three runtimes, including both legacy controls; its HTTP 400 bounds unwanted requests without relying on timeout. All 41 runner tests, CJS/ESM build/import checks, TypeScript 6, lint, and diff checks pass.
Validation limit: used copied local dependencies after registry installation failed; stopped the broader handwritten run after failures outside this patch. No full-suite or generated/ecosystem-suite pass is claimed.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed exact head 0946565. Treating an empty tool_calls array as terminal matches the SDK runner and prevents unintended follow-up completions while preserving real tool-call rounds. The fixture bounds extra requests with an explicit 400 and covers omitted/empty calls both initially and after a tool round. CI is green and there are no unresolved threads. No in-scope findings.
Summary
Reproduction
A completed assistant response with
finish_reason: 'stop'andtool_calls: []leaves an empty array in the example's collected message. Because an empty array is truthy, the old guard falls through, executes zero tools, and starts another completion request.The actual example sends an unwanted second POST for a direct answer, or a third POST after one valid tool round trip. Omitted tool calls correctly stop after one or two requests. The reproduction server rejects the first unwanted request with a non-retryable HTTP 400, so the failure is bounded and does not rely on a process timeout. Repeated empty-array answers could otherwise keep the request loop running.
Validation
1, 1, 2, 2, natural successful exits, and final text printed once.The generated suite and remaining ecosystem/packaging checks run in CI.