Skip to content

fix(openai-chat): tolerate null tool-call padding - #1468

Merged
Wibias merged 1 commit into
lidge-jun:devfrom
dbc-hbin:agent/tolerate-null-tool-calls
Aug 11, 2026
Merged

fix(openai-chat): tolerate null tool-call padding#1468
Wibias merged 1 commit into
lidge-jun:devfrom
dbc-hbin:agent/tolerate-null-tool-calls

Conversation

@dbc-hbin

@dbc-hbin dbc-hbin commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

This restores compatibility with providers that use null as padding when no tool call is present, without accepting malformed claimed tool-call payloads.

Verification

  • bun test tests/openai-chat-hardening.test.ts
  • bun run typecheck
  • bun run test
  • bun run privacy:scan

No documentation change is needed because this is an internal adapter compatibility correction with no configuration or public API change.

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.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of OpenAI chat responses where tool-call data is null.
    • Text output and completion usage are now preserved without unnecessary errors.
    • Malformed, non-null tool-call data continues to be reported appropriately.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The OpenAI Chat adapter now treats null tool-call payloads as absent in streaming and non-streaming responses. Hardening tests verify preserved text output, completion usage, and error-free processing.

Changes

OpenAI null tool-call handling

Layer / File(s) Summary
Handle null tool-call payloads
src/adapters/openai-chat.ts, tests/openai-chat-hardening.test.ts
Streaming and non-streaming responses ignore null tool-call payloads. Non-null payloads remain validated, and tests cover text output, completion usage, and error-free completion.

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

Possibly related PRs

Suggested reviewers: lidge-jun, wibias, ingwannu

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: accepting null tool-call padding in the OpenAI chat adapter.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Aug 11, 2026
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

✅ READY

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

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ 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. If no CodeRabbit review appears, comment @coderabbitai review to request one.
Maintainers: @lidge-jun @Ingwannu @Wibias

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a valid, focused compatibility correction. tool_calls: null is an absent optional value in both Chat Completions response modes, while the patch still rejects non-array containers, null array entries, and incomplete claimed calls through the existing fail-closed adapter error path.

I rechecked exact head 53c1f425 on current dev@20c513198:

  • nine adjacent OpenAI Chat / Chat Completions suites: 161 passed, 0 failed;
  • typecheck: passed;
  • privacy scan: passed;
  • git diff --check: passed.

The added streaming and buffered regressions cover the intended boundary, and no documentation change is needed for this internal wire-compatibility fix. I did not find a blocker in the diff.

I am not approving or merging while the PR is draft with the readiness checklist at 0/4 and no exact-head full CI. Once the author completes the checklist against this head, current-head automated review has run, and required CI is green, this should be a strong merge candidate.

@dbc-hbin
dbc-hbin marked this pull request as ready for review August 11, 2026 09:10
@github-actions
github-actions Bot marked this pull request as draft August 11, 2026 09:10
@github-actions
github-actions Bot marked this pull request as ready for review August 11, 2026 09:14
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

@github-actions[bot] I will review pull request #1468. The review will check buffered and streamed tool_calls: null handling, malformed non-null validation, and regression-test coverage.

⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@github-actions[bot] PR #1468 is marked ready for review. The prior CodeRabbit review request reported that the current commits were already reviewed. No additional review action is required from this status update.

You are interacting with an AI system.

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readiness follow-up: the focused null-padding fix remains a valid, minimal merge candidate, and I still find no code blocker in exact head 53c1f425. However, dev has moved 52 commits since this head and the only Actions runs attached to the current commit were cancelled before a required full cross-platform result was produced.

Please rebase onto current dev@849ab5e35 and let required exact-head CI complete. I am not requesting code expansion; this is only the integration evidence needed before maintainer approval or merge.

@dbc-hbin
dbc-hbin force-pushed the agent/tolerate-null-tool-calls branch from 53c1f42 to 98bf145 Compare August 11, 2026 15:59
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@github-actions[bot] Acknowledged. Pull request #1468 is ready for review, and the reported quality and hygiene checks passed. The current commits were already reviewed, so no additional CodeRabbit review action is required.

You are interacting with an AI system.

@github-actions
github-actions Bot marked this pull request as draft August 11, 2026 16:00
@dbc-hbin

Copy link
Copy Markdown
Contributor Author

Rebased onto current dev@849ab5e35 (new head 98bf1454d). The patch applied cleanly with no conflicts — dev did not touch src/adapters/openai-chat.ts or tests/openai-chat-hardening.test.ts in the intervening 52 commits, so the diff is byte-identical (+33/-4, 2 files).

Local verification at the rebased head:

  • bun test tests/openai-chat-hardening.test.ts — 36 pass, 0 fail (including both null-padding regressions)
  • bun run typecheck — passed
  • bun run privacy:scan — passed

CI at the new head: housekeeping (enforce-target, label, hygiene, resolve-pr) is green. The two substantive workflows — Cross-platform CI and React Doctor — are action_required because this is a fork PR and GitHub is waiting on first-time maintainer approval to run them. That is the only remaining gate; no further action is needed from the contributor side.

@dbc-hbin
dbc-hbin marked this pull request as ready for review August 11, 2026 16:17
@github-actions
github-actions Bot marked this pull request as draft August 11, 2026 16:18
@Ingwannu

Copy link
Copy Markdown
Owner

I rechecked the rebased exact head 98bf1454d216bca5bfd10c9e33ac520c38fa1be3: current dev@849ab5e35 is an ancestor, the diff remains the same focused 2-file tool_calls: null correction, and git diff --check passes.

I approved the fork-gated Cross-platform CI and React Doctor workflow runs so exact-head validation can proceed. This is approval to execute CI, not PR approval or merge approval.

The PR should remain draft until the author updates the four readiness boxes against this exact head. If the required checks finish green and no new review finding appears, the patch remains a strong merge candidate.

@github-actions
github-actions Bot marked this pull request as ready for review August 11, 2026 17:11
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@github-actions[bot] Acknowledged. Pull request #1468 is ready for review at 98bf1454d216bca5bfd10c9e33ac520c38fa1be3. The readiness checklist is complete.

You are interacting with an AI system.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@github-actions The readiness checklist for PR #1468 is complete. The PR is already marked ready for review. The current changes were previously reviewed, so no additional CodeRabbit review action is required.

You are interacting with an AI system.

@flyingsquirrel0419 flyingsquirrel0419 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at exact head 98bf1454d216bca5bfd10c9e33ac520c38fa1be3.

The rebased diff remains the focused two-file correction: tool_calls: null is treated as absence in buffered and streamed Chat Completions frames, while non-array containers, null array entries, and incomplete claimed calls continue through the existing fail-closed validation path. The focused regressions cover both response modes.

Exact-head Cross-platform CI is now fully green, including all Linux shards, gates, macOS, storage/API jobs, keyring jobs, and npm-global jobs. I found no remaining blocker in this patch.

@Ingwannu
Ingwannu dismissed flyingsquirrel0419’s stale review August 11, 2026 17:19

Wrong connector identity; resubmitting through the designated Ingwannu maintainer account.

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at exact head 98bf1454d216bca5bfd10c9e33ac520c38fa1be3.

The rebased diff remains the focused two-file correction: tool_calls: null is treated as absence in buffered and streamed Chat Completions frames, while non-array containers, null array entries, and incomplete claimed calls continue through the existing fail-closed validation path. The focused regressions cover both response modes.

Exact-head Cross-platform CI is now fully green, including all Linux shards, gates, macOS, storage/API jobs, keyring jobs, and npm-global jobs. I found no remaining blocker in this patch.

@Wibias
Wibias merged commit bc1b041 into lidge-jun:dev Aug 11, 2026
34 of 40 checks passed

@Wibias Wibias left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dbc-hbin for the focused compatibility fix. This is useful because a number of OpenAI-compatible providers legitimately send tool_calls: null as padding when there is no tool call. Treating that as absence prevents valid text responses from failing unnecessarily, while the existing fail-closed checks still reject malformed non-null tool-call payloads. Small change, but it improves provider interoperability without weakening validation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants