Repository navigation
[Bugfix][Parser] Keep DeepSeek DSML tool arguments valid JSON - #59585
bojiang-li wants to merge 1 commit into
Conversation
Signed-off-by: bojiang-li <327132355+bojiang-li@users.noreply.github.com>
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment Once the PR is approved or has the If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
Chebaleomkar
left a comment
There was a problem hiding this comment.
Nice, focused fix. I read the diff and ran the new tests locally on CPU (Linux, Python 3.12, no GPU):
- With the patch,
tests/parser/engine/test_dsml_finite.pygives 38 passed. - The same test file against unpatched
maingives 38 failed, so the tests do guard the fix. (I put the PR's test file onmainonly for this check and removed it afterwards. The failures there are expected, sincemaindoesn't have the fix.)
What I like:
- Falling back to the raw string for non-finite values (
NaN,Infinity, overflowing exponents such as1e999) keepsargumentsvalid JSON._is_json_finitealso catches nested values in arrays and objects. - Guarding
_unwrap_wrapper_argsstops unwrapping from reintroducing a non-finite value. - The tests cover the V4, V3.2 and V4.1 parsers, batch and several streaming chunk sizes, both
string=attribute forms, a>inside a JSON string, and a partial overflow (1e99then1e999).
Minor points:
_is_json_finiteis a private name imported across modules, and the tests import private helpers (_dsml_arg_converter,_test_request). A public name would be a little cleaner._is_json_finiterunsjson.dumpson each decoded value, including on the partial streaming path. It is probably negligible, but worth a mention.- The Rust frontend DSML path (#55166 touches it) may have the same non-finite behavior. It could be worth checking in a follow-up.
- The PR is still a draft with no update since Oct 1.
I only ran the PR's tests and did not run a model end to end. AI assistance (Claude Code) was used for the code reading.
Overview
Keep DeepSeek DSML tool arguments valid JSON when model output contains
NaN,Infinity, or overflowing exponents such as1e999. Preserve the original parameter text instead of producing non-JSON constants or losing it as"Infinity".Claims
Validation
The 38 new regression cases fail on unchanged source and pass with this patch. They exercise the production batch and streaming parsers at several chunk boundaries, strict JSON decoding, omitted/explicit string attributes, a
>inside a JSON string before the non-finite value, wrapper handling, and an exponent that overflows as more digits arrive.Result: 5,376 passed. Existing tokenizer fixtures were used on macOS arm64 CPU; no GPU or model-inference test was run. A source-checkout version warning was present. An additional comparison against the base implementation matched all 29 existing DSML corpus samples in batch and three streaming chunk sizes (116 comparisons).
Details
Python's
json.loadsaccepts non-finite constants and overflows large exponents to infinity;json.dumpsthen emitsInfinity/NaNby default. Existing schema coercion does not necessarily inspect already-decoded nested containers. This reuses_is_json_finite, introduced for schema coercion in #43984, before retaining decoded DSML values and before unwrapping a decoded string wrapper. Literal fallback follows the converter's existing handling of values it cannot parse as JSON.This is an AI-assisted draft, with source, tests, and validation prepared by agents for maintainer review.
Pull Request Checklist
I used vLLM's
/pr-checklistskill. (Mandatory for agents, optional for humans).AI assistance was used during the creation of this PR.
Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.
Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.