server: infer JSON types for GLM tool-call arguments - #729
Open
Flor1an-B wants to merge 1 commit into
Open
Conversation
GLM's <arg_key>/<arg_value> wire format carries no type attribute, unlike DSML's string="true|false" that the DeepSeek path already honors. Every argument was therefore quoted as a JSON string regardless of its actual type: numbers became "10", arrays became "[]", booleans became "true". Clients that validate tool-call arguments against the tool's JSON schema (zod/ajv-style validators) reject the call (issue antirez#569). Fix: a value that fully parses as one JSON value (number, bool, null, array, or object) with nothing left over is passed through raw instead of being quoted -- the same heuristic the report proposed and the same whole-string validation semantics json_minify_raw_value's callers already rely on for DSML's explicit string="false" path. A plain string still gets quoted exactly as before. Root cause and fix approach identified by @solodddd in antirez#569, including the exact repro used for the new test. Tested: full `make ds4_test` suite passes, including the exact WebSearch repro from the issue (string/number/array argument mix) asserting the client-visible JSON now has unquoted numbers/arrays and still-quoted strings.
|
Verified on M3 Ultra 512GB (Metal) — this fixes the #569 repro for the numeric case. Environment:
Results:
Before this PR: "max_results":"10" Server log shows a single WebSearch tool call with arguments One honest caveat: in this run the model did not emit exclude_domains, Thanks for the fix — and for crediting the original report in #569. |
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 #569.
GLM's
<arg_key>/<arg_value>wire format carries no type attribute, unlike DSML'sstring="true|false"that the DeepSeek path already honors. Every argument was therefore quoted as a JSON string regardless of its actual type — numbers became"10", arrays became"[]", booleans became"true". Any client that validates tool-call arguments against the tool's JSON schema (zod/ajv-style validators) rejects the call.Fix (the heuristic option from the issue): a value that fully parses as one JSON value (number, bool, null, array, or object) with nothing left over after trimming whitespace is passed through raw instead of being quoted — reusing the same whole-string validation semantics
json_minify_raw_value's existing callers already rely on for DSML's explicitstring="false"path. A plain string is unaffected and still gets quoted exactly as before.Root cause and the fix approach were identified by @solodddd in #569, including the exact repro (
WebSearchwith a string/number/array argument mix) that the new test is built from.Testing: full
make ds4_testsuite passes. Addedtest_json_text_is_whole_value(the new helper in isolation) andtest_parse_glm_tool_call_infers_json_value_types(the exact issue repro end-to-end throughparse_generated_message_ex_for_syntax, assertingmax_resultsandexclude_domainsare now unquoted whilequerystays a quoted string). Updated the one pre-existing assertion that encoded the old behavior (test_parse_glm_tool_call_messageexpected"timeout": "10", now"timeout": 10).