Repository navigation
[Rust Frontend] Share SchemaRoot between argument coercion and grammars - #59408
Merged
Merged
Conversation
BugenZhao
force-pushed
the
bz/tool-schema-view
branch
2 times, most recently
from
October 1, 2026 08:33
0fa9e16 to
6dc430f
Compare
BugenZhao
added this pull request to stack #59574
October 1, 2026 08:48
BugenZhao
changed the title
SchemaRoot between argument coercion and grammars
BugenZhao
marked this pull request as ready for review
October 1, 2026 11:53
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
njhill
approved these changes
Oct 6, 2026
BugenZhao
force-pushed
the
bz/tool-schema-view
branch
from
October 7, 2026 07:07
74fc44b to
18790cb
Compare
Member
Author
|
/ci run |
|
✅ Triggered Buildkite CI #93249 for commit |
Contributor
CI selector (shadow): no narrower answer, every step runs: 193 test steps (269 jobs) instead of 7 (7 jobs)Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works. Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.
Why: rust: rust/src/parser/src/lib.rs: root crate bucket; 16 gate-env steps + 12 hardware-image steps; + the PyO3 bridge claim (vllm/tool_parsers/rust_tool_parser.py) Selector would run (193)
Would skip (today's rules run them) (0)none Would add (today's rules do not run them) (186)
AMD mirrors: would skip (0)none AMD mirrors: would add (156)
6 changed files · base |
… grammars Tool parsers coerce raw parameter text by the tool's JSON schema, and argument grammars constrain the same parameters, but each interpreted the schema separately. Coercion ignored `$ref`, so Pydantic-generated nested models, `Optional[Model]`, and referenced scalars came back as JSON text strings. The two also disagreed on `nullable`, `const`, single-schema `allOf`, and type-name aliases. Add `schema::SchemaView`, which resolves local references on demand and splits a value schema into typed options, expanding each reference target at most once per value as the argument grammar walker did. Argument grammars and coercion both read schemas through it. Coercion now normalizes one level at a time as a structured value reaches it, so a recursive schema costs only as deep as the value; expanding it ahead of time is exponential in its branching. Kimi K3 labels values through its own `xtml_type` mapping, which follows the checkpoint's `encoding_k3.py`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Bugen Zhao <i@bugenzhao.com>
`SchemaView` is the tool's parameters schema that references resolve against, so call it `SchemaRoot`. Its callers only resolved that root itself, which they had to pass again; `resolved()` returns it, and resolving another schema stays private. `typed` takes the parsed type, so inferred object and array options no longer round-trip through a type name, and `ToolSchema::from_schema` takes the schema by value, leaving the copy to the caller that owns the decision. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Coercion tried a mixed enum's types in schema order, so under
`{"enum": ["fast", 1]}` the string type took `1` before the integer did,
although the enum has no string "1" and the argument grammar already
constrains that text to the number. Non-string `const` and `enum` values
are now tried first, and text that spells one as JSON decodes to it. The
types still follow, so text matching no literal converts as before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
The test builds a `Tool` literal, which no longer compiles after `defer_loading` was added to `Tool` on main. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Bugen Zhao <i@bugenzhao.com>
BugenZhao
force-pushed
the
bz/tool-schema-view
branch
from
October 7, 2026 07:48
18790cb to
793e407
Compare
Member
Author
|
/ci run |
|
✅ Triggered Buildkite CI #93256 for commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Rust tool parsers coerce raw parameter text by the tool's JSON schema (
tool/parameters.rs), and argument grammars (#59395) constrain the same parameters by it, but each interpreted the schema on its own. Coercion ignored$ref: a parameter whose schema is{"$ref": "#/$defs/Place"}came back as the JSON text string"{\"city\": \"Paris\"}"instead of an object. Pydantic v2model_json_schema(), which most SDKs use to build tool schemas, emits exactly this for nested models, enums, andOptional[Model]:{"$defs": {"Place": {...}, "Unit": {"enum": ["celsius", "fahrenheit"], "type": "string"}}, "properties": {"place": {"$ref": "#/$defs/Place"}, "unit": {"$ref": "#/$defs/Unit", "default": "celsius"}, "backup": {"anyOf": [{"$ref": "#/$defs/Place"}, {"type": "null"}], "default": null}}}This affects every schema-coercing parser: Qwen Coder and MiMo, GLM, MiniMax M2/M3, Hy. DeepSeek DSML and Kimi K3 decode by their
string=/type=labels instead. Coercion and grammars also disagreed onnullable,const, single-schemaallOf, and type-name aliases such as"int", so a grammar could admit a value that the parser then decodes as another type.The open-PR search found #50933, which resolves
$ref/$defsfor the Python Qwen3-Coder path; there is no Rust counterpart. Python parity is not a goal here. #59190 (the Kimi K3 full-output grammar) is stacked on this PR.schema::SchemaRoot, the tool's parameters schema that references resolve against, owns how a parameter's schema is read:resolvedreturns the root after following local$refchains and single-schemaallOf; unresolvable, cyclic, and remote references accept any value and are never fetched;optionsexpands each reference target at most once per value, as the argument grammar walker from [Rust Frontend] Split argument grammars into schema resolution and per-model rendering #59395 already did, so shared definitions cannot blow up exponentially;optionssplits a value schema into typed options:const/enumliterals, one option peranyOf/oneOf/type-array alternative,object/arrayinferred fromproperties/items,nullable: trueaddingnull, and aliases such asintordictparsed to the canonical type;JsonTypegainsInteger, since coercion distinguishes integers.SchemaRootinstead of their own walker.ToolSchemakeeps the parameters schema, and object and array types resolve their fields and items only when a structured value reaches them. A recursive schema therefore costs only as deep as the value; expanding it ahead of time is exponential in its branching, and a tree with two recursive children took over 60 s to build that way.xtml_typemapping, which follows_xtml_typein the checkpoint'sencoding_k3.py; integers and floats are bothnumber. Its grammar is unchanged.Behavior changes in coercion:
$refresolves at the parameter level, in nested fields and items, and insideanyOf.constand single-schemaallOfare understood, andenumvalues coerce by their own type, so"2"under{"enum": [1, 2, 3]}becomes2.constorenumvalue as JSON decodes to that value before any type is tried, so1under{"enum": ["fast", 1]}becomes the number1, as the argument grammar already constrains it, instead of the string"1". Text matching no literal converts as before.anyOf/oneOfwhose alternatives all have unknown types now falls back to the raw string, like an unknowntype, instead of being treated as an object.Behavior changes in argument grammars (Kimi K3):
nullable: trueadmitstype="null", an aliased type such as"int"constrains its value instead of dropping the parameter, and a schema withpropertiesoritemsbut notypeadmits only objects or arrays.Test Plan
cd rust cargo nextest run -p vllm-parser -p vllm-chat cargo clippy --workspace --all-targets --all-features -- -D warningsschemaunit tests: the Pydantic schema above, unresolvable, cyclic, and remote references, aliases in type arrays,nullable, mixed enums, singleallOf, anditemswithouttype.Optionaltakingnull), recursive and branching recursive schemas converted from structured values, and integer enums.parse_completewith referenced parameters returns an object and an integer.Test Result
vllm-parser+vllm-chat: 940 tests passed, including all 36 XGrammar replay cases; the roundtrip grammar files are unchanged.AI assistance: implementation and validation assisted by Claude Code.
🤖 Generated with Claude Code