Repository navigation
fix(server): keep non-finite tool argument text as strings - #864
Draft
Avicennasis wants to merge 1 commit into
Draft
Avicennasis wants to merge 1 commit into
Avicennasis wants to merge 1 commit into
Conversation
This branch has not been deployed
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.
XML/text arguments such as NaN, Infinity, -Infinity and 1e9999 could become non-finite Python floats and then invalid JSON tokens in
function.arguments. Keep the original text as a string when JSON or schema coercion would produce a non-finite value, including nested JSON and the legacy Python-literal fallback. Declared number parameters follow the existing invalid-value string fallback instead of raising OverflowError.This follows the idea in Edge0-AI/Edge0 #156, commit ae8440870a6f63e6dba2a7b6f5fa93e6e4d2585c, and the serialization guard in JustVugg/colibri #2161, commit 2ba4e69da2066fe57acd11e0919a48e7cd1d89ea. No donor code was copied.
Argument serializers use
allow_nan=Falseand handle refusals per call. The base JSON streaming parser consumes completed rejected calls so later valid calls can still be parsed. The existing whole-response fallback and unfinished-stream markup suppression policies remain in place. Tests cover every registered parser name, whole and streamed input, declared and undeclared parameters, nested overflow, and refusal followed by a valid call. JSON-only formats receive quoted numeric-looking strings in the all-parser round-trip matrix; malformed JSON calls are tested separately.Verified on x86_64 Linux (AMD Ryzen 7 5800X), Python 3.12.3 and torch 2.14.1+cpu. No checkpoint was used. Some declared runtime dependencies were missing from the test environment and were installed locally; no dependency declarations were changed. From the repository root:
export PYTHONPATH=python python -m pytest tests/server/ -k function_call -q python -m pytest tests/server/ -q python -m pytest tests/ --continue-on-collection-errors -qThe focused gate passed 651 tests with 611 deselected (exit 0); the server suite passed 1,262 (exit 0). The full suite had 2,271 passed, 20 failed, 919 skipped and one platform warning (exit 1). The same full command on the base had 1,665 passed, exactly the same 20 failing test IDs (the known CPU-only environment failures), 919 skipped and the same warning. The final non-finite regression set against the base source had 310 failed, 296 passed and 42 deselected. Reverting only the streaming refusal fix made its 24 successor-call regressions fail; restoring it made all 24 pass. The focused gate was also rerun with the optional orjson backend installed (orjson 3.13.0): 651 passed.
NOT VERIFIED: CUDA, live model serving, or real-checkpoint generation. This is a parser-only CPU verification. Arbitrary malformed raw Harmony/GPT-OSS streaming arguments are still forwarded by unchanged code; validating those requires a separate streaming policy. The independently reproduced pre-existing Mistral split-comma-space separator bug is also outside this change.
This branch starts at 628b88f and depends on none of the open PRs #334, #459, #565, #592, #593 or draft #656; file lists show no overlap with either changed file. #825 is already in the base; its Qwen reasoning recovery remains covered by the server suite.
Maintainer question: is keeping the original string for a non-finite value under a declared number schema the preferred fallback? This matches existing failed-coercion behavior. Unifying malformed-call content suppression across whole and streamed responses would be a separate policy change.