Fix: Handle $ref in json-schema in qwen3_coder tool parser#37652
Fix: Handle $ref in json-schema in qwen3_coder tool parser#37652schoennenbeck wants to merge 2 commits intovllm-project:mainfrom
Conversation
Signed-off-by: Sebastian Schönnenbeck <sebastian.schoennenbeck@comma-soft.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to add support for $ref in JSON schemas for the qwen3_coder tool parser. The implementation treats any parameter with a $ref as an object type. While this works for the common case where $ref points to a complex object, it introduces a potential correctness issue. My review points out that $ref can point to any type, and the current assumption can lead to incorrect type conversions for non-object references, particularly with the ast.literal_eval fallback. I've classified this as a high-severity issue and recommended a safer handling strategy for $ref types.
| elif isinstance(param_config[param_name], dict) and ( | ||
| "anyOf" in param_config[param_name] or "$ref" in param_config[param_name] | ||
| ): | ||
| # anyOf has no top-level "type"; treat as object to trigger json.loads. | ||
| # anyOf and $ref have no top-level "type"; treat as object to trigger | ||
| # json.loads. | ||
| param_type = "object" |
There was a problem hiding this comment.
This change correctly identifies parameters defined with $ref, but making the broad assumption that they are always of object type could lead to incorrect behavior.
A $ref can point to a definition of any type, including primitives like string or integer. If a $ref points to a string type, but the model returns a value that looks like a dictionary (e.g., "{'key': 'value'}"), the current logic would incorrectly parse it as a dictionary via ast.literal_eval in the fallback path, instead of treating it as a string. This would result in a type mismatch and potential runtime errors in the tool-using code.
A safer approach would be to handle $ref parameters separately, perhaps by attempting to parse them with json.loads and falling back to treating them as a raw string if parsing fails, without attempting ast.literal_eval. This would correctly handle object-like values while not misinterpreting string literals that happen to look like Python literals.
There was a problem hiding this comment.
Switched to more restrictive parsing, avoiding the ast.literal_eval for refs.
Signed-off-by: Sebastian Schönnenbeck <sebastian.schoennenbeck@comma-soft.com>
chaunceyjiang
left a comment
chaunceyjiang
left a comment
There was a problem hiding this comment.
test_qwen3coder_tool_parser.py
Can you add a test case for this?
Parameters using $ref (e.g. Pydantic model inputs) have no "type" field. Treat them as "object" to route through json.loads, matching the behavior proposed in vllm-project#37652 but integrated into the _resolve_param_type helper. Added ref_param test case to verify $ref → object → json.loads path.
Parameters using $ref (e.g. Pydantic model inputs) have no "type" field. Treat them as "object" to route through json.loads, matching the behavior proposed in vllm-project#37652 but integrated into the _resolve_param_type helper. Added ref_param test case to verify $ref → object → json.loads path. Signed-off-by: sejung-son <sejung.son@nhn.com>
Parameters using $ref (e.g. Pydantic model inputs) have no "type" field. Treat them as "object" to route through json.loads, matching the behavior proposed in vllm-project#37652 but integrated into the _resolve_param_type helper. Added ref_param test case to verify $ref → object → json.loads path. Signed-off-by: AAISSJ <maze0717@g.skku.edu>
Will do. |
|
Closing in favor of #37831 |
2 participants
Purpose
This PR improves the qwen3_coder tool parser by allowing it to handle modern json-schema annotations including references.
Json schemas for tool definitions might have complex types using the references annotation (e.g. when the tools are python functions using pydantic models as input). This type of tool definitions is for example now sent by pydantic-ai after a recent update (see for example pydantic/pydantic-ai#3617 for a related bug).
However, in that case the parameter definition has no "type"-field; it might for example look like this:
The qwen3_coder tool call parser so far did not handle this case and consequently treated these parameters as strings leading to downstream errors.
This PR handles this case by treating parameters with a "$ref" as objects in order to trigger a
json.loads. In principle it would be possible to fully resolve the reference. However, a reference will almost always be of object type anyhow and in case of any parsing error the tool parser falls back to treating it as a string which is the current behaviour.