Skip to content

fix(gpt-oss): Reject failed GPT-OSS grammars for mandatory tool choices - #5056

Merged
silencelamb merged 1 commit into
InternLM:mainfrom
emme1t:emme1t/fix-api-edge-cases-20261022
Oct 24, 2026
Merged

silencelamb merged 1 commit into
InternLM:mainfrom
emme1t:emme1t/fix-api-edge-cases-20261022

Conversation

@emme1t

@emme1t emme1t commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Motivation

The GPT-OSS parser rejects tool_choice="required" when XGrammar cannot build the tool grammar. The equivalent mandatory forms, a named function and allowed_tools.mode="required", currently fall back to unconstrained generation. A request that requires a tool call can therefore proceed with a plain-text response.

This completes the grammar-failure behavior introduced in #4997.

Modification

  • Recognize ToolChoice and required AllowedToolChoice alongside the string required when grammar construction fails.
  • Raise the existing request-validation error for all three mandatory forms before generation starts.
  • Preserve prompt-only fallback for auto and allowed_tools.mode="auto", including response-format prompt injection.
  • Extend the existing regression to five tool-choice modes with and without a structured response format; update the parser docstring.

Validation

  • The expanded regression on upstream aba987ad produced 4 failed / 6 passed. Both named-function and allowed-required modes failed with and without a response format.
  • python -m pytest tests/test_lmdeploy/serve/parsers/test_gpt_oss_parser.py tests/test_lmdeploy/serve/parsers/test_required_tool_choice.py -q: 110 passed.
  • All applicable pre-commit hooks and git diff --check passed.

These are parser-level CPU tests on Python 3.13.13 with real LMDeploy imports and XGrammar 0.2.8. The existing Harmony test fixture supplies a scripted token parser, and the regression simulates grammar construction failure. GPU model inference was not exercised.

Checklist

  • Pre-commit checks passed.
  • Added regression coverage for mandatory and automatic tool choices.
  • Updated the relevant docstring.
  • No dependency requirement changes.

@windreamer
windreamer requested review from windreamer and a balanced review from Copilot October 23, 2026 11:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused validation change covers all mandatory forms, preserves automatic fallback, and includes targeted regression coverage.

No open issues 🎉

What changed in this PR

Completes the GPT-OSS grammar-failure handling introduced in #4997 so mandatory tool choices cannot fall back to unconstrained generation.

Changes:

  • Reject failed grammar construction for named functions and required allowed-tool choices.
  • Preserve prompt-only fallback for automatic modes and clarify the parser docstring.
  • Expand regression coverage to five tool-choice modes, with and without a structured response format.
File Description
tests/​test_lmdeploy/​serve/​parsers/​test_gpt_oss_parser.py Covers mandatory rejection, automatic fallback, and response-format prompt injection.
lmdeploy/​serve/​parsers/​_openai_harmony.py Extends mandatory-choice validation and documents grammar-failure behavior.

Review effort: Balanced

@windreamer

Copy link
Copy Markdown
Collaborator

LGTM

@silencelamb silencelamb changed the title [Fix] Reject failed GPT-OSS grammars for mandatory tool choices fix(gpt-oss): Reject failed GPT-OSS grammars for mandatory tool choices Oct 24, 2026
@silencelamb
silencelamb merged commit 7998cdb into InternLM:main Oct 24, 2026
4 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.

4 participants