Skip to content

[Bugfix][Parser] kimi_k2: route streaming through arg_converter so schema type coercion applies - #49318

Open
muhammadfawaz1 wants to merge 1 commit into
vllm-project:mainfrom
muhammadfawaz1:fix-kimi-k2-streaming-type-coercion
Open

muhammadfawaz1 wants to merge 1 commit into
vllm-project:mainfrom
muhammadfawaz1:fix-kimi-k2-streaming-type-coercion

Conversation

@muhammadfawaz1

@muhammadfawaz1 muhammadfawaz1 commented Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

kimi_k2 was the only Streaming Parser Engine backend with tool_args_json=True and no arg_converter. Because _compute_arg_delta and _flush_arg_converter both return early when arg_converter is None (before ever reaching _fix_arg_types), streaming emitted the model's raw JSON value types unchanged, while non-streaming always coerces to the tool schema via _build_extracted_result. Same model output, different typed values depending on whether the client streams or not — silent, since the output is still valid JSON.

Full root cause and reproduction: #49316

Fix

Entirely in vllm/parser/kimi_k2.py. Zero changes to parser_engine.py.

  • Added _kimi_k2_arg_converter(raw_args, partial): normalizes kimi's raw JSON to canonical json.dumps output, using partial_json_parser (already a project dependency — see requirements/common.txt, used by 7 other tool parsers) to complete incomplete objects when json.loads fails on partial input. Unparsable text is returned unchanged so no argument bytes are lost.
  • Wired it in via arg_converter=_kimi_k2_arg_converter in kimi_k2_config.
  • Removed the now-incompatible _handle_arg_chunk override, which emitted the first arg chunk as raw event.value, bypassing any converter. Once a converter exists this duplicated output.

Why this approach

Once kimi has a converter, it flows through the same converter-present machinery PR #48706 already hardened: _fix_arg_types coercion, _safe_arg_prefix's withholding of unstable trailing values, and the startswith prefix invariant. Because the converter re-emits canonical json.dumps output on every tick (both partial and flush), and non-streaming routes through the same converter, both paths produce byte-identical concatenated output by construction, rather than needing to be kept in sync by hand.

Alternative considered and rejected: raw-text withholding

Adapting PR #48706's non_string_clip idea directly to the no-converter path (withhold non-string field values from the raw stream, emit them corrected at flush) was the first approach tried. It doesn't work: once a field coerces, _fix_arg_types re-serializes the entire object with json.dumps spacing ({"a":"x",...} → {"a": "x", ...}), but partial raw JSON can't be run through _fix_arg_types mid-stream (it isn't valid JSON yet), so the raw no-space prefix already streamed is never a valid prefix of the eventual coerced-and-spaced output. The startswith check then fails and silently truncates — reproducing the exact #48702 failure mode in a new place. The only prefix that's safe under both outcomes turns out to be the opening {"firstkey":, which defeats progressive streaming entirely. This is unavoidable specifically because of middle (not just trailing) non-string fields — verified by direct construction, not just argued.

Alternative considered and rejected: minimal in-place coercion

Coercing values in place while preserving the model's original raw formatting (no re-spacing) would require either changing _fix_arg_types itself (shared with the converter-present path — out of scope and risks regressing #48706's behavior) or accepting that kimi's non-streaming output diverges from the canonical format everywhere else in the engine.

Known behavior change (intentional, flagged for review)

Non-streaming output for untyped/already-correct fields is now whitespace-normalized ({"city":"Tokyo"} → {"city": "Tokyo"}) — a direct consequence of routing both paths through the same canonical json.dumps converter. Already-coerced field output is unchanged ({"count": 3} stays {"count": 3}). No existing test asserts on the old compact non-streaming format for kimi_k2 (checked: tests/parser/, tests/reasoning/test_kimi_k2_reasoning_parser.py, tests/entrypoints/unit_tests/test_chat_utils.py all pass unchanged). Flagging this explicitly in case a maintainer considers exact non-streaming byte format a compatibility concern.

Testing

  • New file: tests/parser/engine/test_kimi_k2_streaming.py, 27 deterministic, byte-level tests at chunk sizes 1/3/1000, covering: single non-string field (int/bool/number), middle (non-trailing) non-string field, nested non-string field, parallel tool calls, string field with a numeric-looking literal, already-correct-types control, and truncated/never-closed JSON.
  • Confirmed red→green directly: git stash + run → 21/27 fail on unpatched main (every coercion scenario); git stash pop + run → 27/27 pass.
  • Independently verified with a probe script written before this fix existed (not authored alongside it) — all cases now match streaming/non-streaming, including a parallel-tool-calls case.
  • Full existing suite: tests/parser/engine/ — 3699 passed, 0 regressions (includes all converter-present deepseek/qwen/gemma streaming tests, confirming this change doesn't touch that path's behavior).
  • tests/parser/test_parse.py, tests/parser/test_streaming.py, tests/reasoning/test_kimi_k2_reasoning_parser.py: pass, unchanged.
  • tests/entrypoints/unit_tests/test_chat_utils.py: 12 failed / 7 errored, identically with and without this change applied (confirmed via stash-diff) — pre-existing environment gap (missing Torchvision), not a regression.
  • ruff check + ruff format: clean.

Not yet verified

Real-tokenizer reasoning/tool-parser suites requiring moonshotai/* tokenizer download couldn't be run in this environment (no network access to HuggingFace from the dev machine). Logic exercised is the same code path covered by the mock-tokenizer tests above, but a maintainer with the model cached should run these before merge.

Not a duplicate of

…chema type coercion

Streaming previously skipped schema type coercion for kimi_k2 since it
has no arg_converter; non-streaming always coerced via _fix_arg_types.
Same model output, different typed values depending on client mode.

Fixes silent type mismatch by giving kimi_k2 a converter that
normalizes to canonical JSON, routing it through the same
converter-present machinery PR vllm-project#48706 already hardened.

Co-authored-by: Mahad Rehman <mahadrehmann@users.noreply.github.com>
Signed-off-by: muhammadfawaz1 <135441198+muhammadfawaz1@users.noreply.github.com>
@mergify mergify Bot added tool-calling bug Something isn't working labels Jul 21, 2026
Comment thread vllm/parser/kimi_k2.py
@muhammadfawaz1
muhammadfawaz1 marked this pull request as ready for review July 23, 2026 07:24

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added the kimi label Jul 27, 2026
@mergify

mergify Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @professorsab.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants