Skip to content

Fix Responses tool_choice compatibility for LobeHub - #18

Merged
reonokiy merged 2 commits into
mainfrom
fix/tool-choice
Oct 11, 2026
Merged

reonokiy merged 2 commits into
mainfrom
fix/tool-choice

Conversation

@reonokiy

@reonokiy reonokiy commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

LobeHub tool-backed generation sends tool_choice=required, which the gateway currently rejects before contacting upstream. Accept and preserve auto, none and required through the existing Codex serializers, retaining validation for unknown choices.

Validation:

  • cargo test --workspace --locked passed, including standard/Lite conversion and public HTTP forwarding regressions.
  • Official Python SDK compatibility suite passed against local fixtures, with positive cases for all three choices and rejection of an unknown choice.
  • Minimal requests through the deployed native endpoint confirmed upstream accepts required (one function call) and none (zero calls), both completing successfully.

No credential or authentication changes.

Why
LobeHub tool-backed generation sends required, which the gateway rejected before contacting a capable upstream.

How
Accept and preserve auto, none and required through the existing Codex serializers. Cover standard/lite conversion and public HTTP forwarding; retain rejection of unknown choices.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-11T01:20:50.042028Z 16533e6 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Why: The SDK compatibility suite still expected required to be rejected. How: Cover auto, none and required with function tools and reserve the rejection case for an unknown choice.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16533e6a74

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/request.rs
if self
.tool_choice
.as_deref()
.is_some_and(|v| !["auto", "none", "required"].contains(&v))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Update the SDK suite for the newly accepted tool choice

When the sdk job in .github/workflows/image.yml runs cargo test --locked --test gateway actual_ -- --ignored, actual_openai_sdk_compatibility_suite executes test_compatibility.py::test_unsupported_values[tool-choice], which still expects tool_choice='required' to raise a 400/422 error. This allowlist now forwards that request to the successful fake upstream, so the expected exception is not raised and the required CI job fails deterministically; move this case to an accepted-value test and update the compatibility matrix accordingly.

Useful? React with 👍 / 👎.

@reonokiy
reonokiy merged commit 991fb8c into main Oct 11, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant