Repository navigation
Retain GLM tool arguments that do not match their declared type - #179
Merged
Merged
Conversation
When GLM generated a tool argument whose text did not parse as the schema-declared JSON type, for example "true" for a boolean parameter, the argument-tags parser failed the whole call and the server aborted the stream with "tool parameter value does not match its declared JSON type" (NVIDIA#39). Clients retried the same request and hit the same abort. Strict tool schemas are already rejected at request decoding, so every parsed call is non-strict and its arguments are the client's to validate. GLM was the only dialect that validated generated values against the schema: the function-tag and MiniMax parsers retain invalid values, and Gemma, DeepSeek and XTML do not check the client schema. Declared types still guide conversion, and every anyOf/oneOf branch is tried in order. When no declared type matches, the parser now retains the generated JSON value, or the raw text as a string when it is not JSON. It does not coerce values: "true" stays the string "true". Invalid type declarations still fail the call, including a union branch with an invalid type when no other branch matches the value. Calls with missing or unterminated tags, empty names, or empty or duplicate argument keys still fail. No package or metadata change. Validation on base cc696a6, run as the CI workflow does (release builds, FLARE_TEST_TEMPLATE_DIR prepared by prepare_templates.py for gemma4, glm53, mimo_v2, nemotron_h, qwen38 and step5): - The GLM parser tests (7) pass, including a streaming test split at every character boundary, a test for invalid top-level and nested union type declarations, and a protocol test that streams a mismatched GLM call through the Chat Completions, Messages and Responses encoders. Both mismatch tests fail on the base with the reported error. - Helios lib: 1229 passed, 24 ignored, 5 failed. All 12 integration targets pass. package::cache failed_default_clear_keeps_the_selected_entry fails only as root and passes as an unprivileged user. The four engram::reader staging tests fail with ENOMEM because the build container's hard RLIMIT_MEMLOCK is 8 MiB; CI raises it to 64 MiB. - Flare CLI: 295 passed, 7 ignored, 1 failed. The failure, distribution cache_tests unreadable_marker_directory_still_serves_the_verified_rank, fails only as root and passes as an unprivileged user. - The ignored glm_frontend checkpoint test passes against nvidia/GLM-5.3-Flash-FP4-Local at 76348ddb, with its reference generated by generate_glm_fixtures.py. It parses the checkpoint template's parallel tool call through the GLM parser. - rustfmt reports no changes in the added code. Not run: the four engram::reader tests with a 64 MiB memlock limit, a live GLM model reproduction, Heliopause, Python and CUDA tests. Clients now receive off-schema argument values from GLM instead of an aborted stream, matching the other tool dialects. Design-Signed-Off-By: Mark Yong via Cursor Agent (Auto); design-review=1c01
Collaborator
|
Related review: #59 also changes GLM tool-argument handling, but adds array coercion. This PR instead retains off-schema values without coercion. Please resolve that semantic difference before combining either implementation. #179 remains the focused review candidate for the stream-abort defect in #39; #59 is marked for design review. |
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
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.
fixes #39
When GLM generated a tool argument whose text did not parse as the schema-declared JSON type, for example "true" for a boolean parameter, the argument-tags parser failed the whole call and the server aborted the stream with "tool parameter value does not match its declared JSON type" (#39). Clients retried the same request and hit the same abort.
Strict tool schemas are already rejected at request decoding, so every parsed call is non-strict and its arguments are the client's to validate. GLM was the only dialect that validated generated values against the schema: the function-tag and MiniMax parsers retain invalid values, and Gemma, DeepSeek and XTML do not check the client schema.
Declared types still guide conversion, and every anyOf/oneOf branch is tried in order. When no declared type matches, the parser now retains the generated JSON value, or the raw text as a string when it is not JSON. It does not coerce values: "true" stays the string "true". Invalid type declarations still fail the call, including a union branch with an invalid type when no other branch matches the value. Calls with missing or unterminated tags, empty names, or empty or duplicate argument keys still fail. No package or metadata change.
Validation on base cc696a6, run as the CI workflow does (release builds, FLARE_TEST_TEMPLATE_DIR prepared by prepare_templates.py for gemma4, glm53, mimo_v2, nemotron_h, qwen38 and step5):
Not run: the four engram::reader tests with a 64 MiB memlock limit, a live GLM model reproduction, Heliopause, Python and CUDA tests. Clients now receive off-schema argument values from GLM instead of an aborted stream, matching the other tool dialects.
Design-Signed-Off-By: Mark Yong via Cursor Agent (Auto); design-review=1c01