Skip to content

Revert "[Bugfix] Fix Qwen3CoderToolParser anyOf/oneOf type resolution for nullable params (#37831)"#38751

Merged
chaunceyjiang merged 1 commit intovllm-project:mainfrom
khluu:revert-582340f
Apr 1, 2026
Merged

Revert "[Bugfix] Fix Qwen3CoderToolParser anyOf/oneOf type resolution for nullable params (#37831)"#38751
chaunceyjiang merged 1 commit intovllm-project:mainfrom
khluu:revert-582340f

Conversation

@khluu
Copy link
Copy Markdown
Collaborator

@khluu khluu commented Apr 1, 2026

Summary

  • Reverts commit 582340f which breaks a test on main.
  • This commit introduced changes to vllm/tool_parsers/qwen3coder_tool_parser.py and added tests in tests/tool_parsers/test_qwen3coder_tool_parser.py for anyOf/oneOf type resolution for nullable params.

Test plan

  • CI should pass with the revert applied

🤖 Generated with Claude Code

@mergify mergify bot added qwen Related to Qwen models tool-calling bug Something isn't working labels Apr 1, 2026
@khluu khluu added ready ONLY add when PR is ready to merge/full CI is needed and removed bug Something isn't working tool-calling qwen Related to Qwen models labels Apr 1, 2026
@mergify mergify bot added qwen Related to Qwen models tool-calling bug Something isn't working labels Apr 1, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request simplifies the parameter type resolution logic in the qwen3coder_tool_parser and removes associated unit tests. The review feedback identifies several regressions introduced by this simplification, specifically regarding the handling of array-based types, $ref schemas, and oneOf definitions, which will cause tool call arguments to be incorrectly processed as raw strings instead of parsed objects or typed values.

Comment on lines +153 to +165
if (
isinstance(param_config[param_name], dict)
and "type" in param_config[param_name]
):
param_type = str(param_config[param_name]["type"]).strip().lower()
elif (
isinstance(param_config[param_name], dict)
and "anyOf" in param_config[param_name]
):
# anyOf has no top-level "type"; treat as object to trigger json.loads.
param_type = "object"
else:
param_type = "string"
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.

high

The simplified type resolution logic introduced in this revert has several regressions compared to the logic being removed:

  1. Array type support: If a parameter definition uses an array for the type field (e.g., "type": ["integer", "null"]), str(param_config[param_name]["type"]) will result in a string like "['integer', 'null']". This will fail the subsequent .startswith("int") and other type checks, defaulting to string or relying on the ast.literal_eval fallback, which may fail for booleans.
  2. Missing $ref and oneOf: The previous logic correctly handled $ref (treating it as an object) and oneOf. The new logic defaults these to "string", which will cause nested objects (common with Pydantic models) to be returned as raw strings instead of parsed dictionaries.

Since this is a revert intended to fix CI, if a full restoration of the previous logic is not desired, consider at least handling these common cases to avoid breaking tool calls with nested models or nullable fields.

        param_def = param_config[param_name]
        if not isinstance(param_def, dict):
            param_type = "string"
        elif "type" in param_def:
            t = param_def["type"]
            if isinstance(t, list):
                # Extract first non-null type
                param_type = next((str(x).strip().lower() for x in t if x and str(x).strip().lower() != "null"), "string")
            else:
                param_type = str(t).strip().lower()
        elif any(k in param_def for k in ("anyOf", "oneOf", "$ref")):
            # Treat complex schemas and references as objects to trigger json.loads
            param_type = "object"
        else:
            param_type = "string"

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

Labels

bug Something isn't working qwen Related to Qwen models ready ONLY add when PR is ready to merge/full CI is needed tool-calling

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants