refactor: stream tool parameters incrementally - #4802
Conversation
* feat: stream tool arguments incrementally * refactor: simplify XML tool parser streaming * fix: restore empty stream delta fallback * fix: suppress empty streaming deltas * perf: stream xml tool parser incrementally * refactor: stream json tool parser incrementally * refactor: simplify XML tool parser consumers * refactor: rename XML tool parser stream state * refactor: extract JSON tool parser base Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors tool-call streaming so tool parameters/arguments can be emitted incrementally (instead of waiting for the full payload), spanning JSON, XML-like, and DeepSeek DSML tool formats, and adds/updates tests to validate incremental emission and bounded buffering. It also updates OpenAI streaming to buffer token metadata across parser steps that produce no visible deltas.
Changes:
- Introduce
JsonToolParser(incremental JSON argument streaming) and refactor Qwen/Llama3/InternLM parsers to reuse it. - Refactor
XmlToolParserinto a phase-driven incremental engine and update Qwen3Coder/GLM47 parsers to implement adapter-style consumption. - Update streaming plumbing/tests: incremental argument assertions, metadata buffering across “no visible delta” steps, and new regression coverage for bounded buffers.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_lmdeploy/test_deepseek_v4_encoding.py | Update DSML tests to join streamed argument fragments before JSON parsing. |
| tests/test_lmdeploy/test_deepseek_v32_encoding.py | Same as above for DeepSeek v3.2. |
| tests/test_lmdeploy/serve/parsers/test_tool_parser_incremental.py | New tests covering incremental argument emission + bounded buffering across parsers. |
| tests/test_lmdeploy/serve/parsers/test_qwen3_parser.py | Update streaming reference assertions for incremental tool argument fragments; add regressions. |
| tests/test_lmdeploy/serve/parsers/test_qwen3_5_parser.py | Restructure streaming expectations into flattened events; add many incremental/complete-parse equivalence tests. |
| tests/test_lmdeploy/serve/parsers/test_llama3_parser.py | Add regression ensuring arguments are emitted before JSON completes. |
| tests/test_lmdeploy/serve/parsers/test_glm47_parser.py | Restructure streaming assertions for incremental fragments; add incremental/complete-parse equivalence tests. |
| tests/test_lmdeploy/serve/openai/chat_completions/test_streaming_metadata.py | New tests for buffering token_ids/logprobs when parser returns no visible delta. |
| tests/test_lmdeploy/serve/openai/chat_completions/test_delta_tool_call_id.py | Update/extend incremental JSON streaming tests via JsonToolParser. |
| tests/test_lmdeploy/serve/openai/chat_completions/test_arguments_validation.py | Switch validation tests to JsonToolParser.parse_tool_call_complete. |
| lmdeploy/serve/parsers/tool_parser/xml_tool_parser.py | Major refactor: shared incremental XML-like parse state + streaming JSON emission/coercion. |
| lmdeploy/serve/parsers/tool_parser/tool_parser.py | Remove embedded JSON parsing helpers; simplify base lifecycle state. |
| lmdeploy/serve/parsers/tool_parser/qwen3coder_tool_parser.py | Rework incremental parsing using XmlToolParser adapter methods. |
| lmdeploy/serve/parsers/tool_parser/glm47_tool_parser.py | Rework incremental parsing using XmlToolParser adapter methods. |
| lmdeploy/serve/parsers/tool_parser/json_tool_parser.py | New incremental JSON tool payload parser with early argument streaming. |
| lmdeploy/serve/parsers/tool_parser/qwen3_tool_parser.py | Refactor to inherit JsonToolParser. |
| lmdeploy/serve/parsers/tool_parser/qwen2d5_tool_parser.py | Refactor to inherit JsonToolParser. |
| lmdeploy/serve/parsers/tool_parser/llama3_tool_parser.py | Refactor to inherit JsonToolParser (no close tag). |
| lmdeploy/serve/parsers/tool_parser/internlm2_tool_parser.py | Refactor to inherit JsonToolParser. |
| lmdeploy/serve/parsers/tool_parser/deepseek_v32_tool_parser.py | Implement incremental DSML emission across invocations/parameters. |
| lmdeploy/serve/parsers/tool_parser/deepseek_v4_tool_parser.py | Point V4 parser at the refactored V3.2 incremental implementation. |
| lmdeploy/serve/parsers/tool_parser/init.py | Export JsonToolParser. |
| lmdeploy/serve/parsers/response_parser.py | Update tool streaming to use _payload_closed instead of JSONDecoder completeness checks. |
| lmdeploy/serve/openai/api_server.py | Buffer token metadata across parser steps that emit no visible deltas. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _consume_arg_delta(self, raw: str, json_fragments: list[str]) -> None: | ||
| arg_name = self._state.arg_name | ||
| if arg_name is None: | ||
| return | ||
|
|
| out = self.tool_parser.decode_tool_incremental(added_text=emit, final=False) | ||
| if (self.profile.tool_payload_format == 'json' | ||
| and self._is_complete_json_object(self.tool_parser._tool_payload)): | ||
| if self.profile.tool_payload_format == 'json' and self.tool_parser._payload_closed: | ||
| out.extend(self.tool_parser.decode_tool_incremental(added_text='', final=True)) | ||
| self.tool_parser.finish_tool_call() | ||
| self._mode = self.MODE_PLAIN |
ErenAta16
left a comment
There was a problem hiding this comment.
Scoped this to json_tool_parser.py, since that's the new state machine everything else in the PR now routes through, and an incremental parser is the piece where chunk boundaries can hide problems that a whole-payload test never sees.
I stubbed out ToolParser and the protocol dataclasses so I could drive decode_tool_incremental directly, and replayed each payload at several chunk sizes down to one character.
In-contract behaviour is solid. Every case below produced the same name and the same concatenated argument string at chunk sizes of whole payload, 7, 3 and 1:
simple {"a":1} escaped quote {"s":"a\"b"}
"parameters" spelling backslash {"s":"a\\b"}
\u escape nested {"a":{"b":[1,2]}}
braces inside a string {"s":"}{"} empty {}
"arguments" before "name" extra leading field
The brace-inside-a-string and escaped-quote cases are the ones I expected to break a hand-rolled scanner, and the separate _string_open_in_container / _value_escaped tracking handles both. The "arguments"-before-"name" ordering works too, even though the class docstring says the protocol puts name first — worth knowing the implementation is more tolerant than the comment claims.
Two edges fall outside that contract, and in both the streaming path and parse_tool_call_complete disagree about the same bytes.
Two argument fields concatenate into invalid JSON.
{"name":"f","arguments":{"a":1},"parameters":{"b":2}}
streaming deltas -> {"a":1}{"b":2}
parse_tool_call_complete -> {"a":1}
parse_tool_call_complete resolves it with obj.get('arguments', obj.get('parameters', {})), so it picks one. The stream emits both, back to back, and a client accumulating deltas and calling json.loads at the end gets a decode error rather than either object. Out of contract, but the failure is silent and lands on the client rather than here.
A repeated name makes the emitted name depend on chunk size.
{"name":"f","name":"g","arguments":{"a":1}}
one chunk -> name = "g"
5-char chunks -> name = "f"
1-char chunks -> name = "f"
parse_tool_call_complete -> "g" (json.loads keeps the last)
This one bothers me more than the first. In a single _consume_payload pass func_name is overwritten before anything is emitted, so the last key wins; split across calls, _name_emitted latches after the first and the rest are dropped. That means the same model output yields a different tool name depending only on how the transport happened to segment it, which is the sort of thing that turns into an unreproducible bug report. Duplicate keys are malformed input either way, so the fix is probably to pick one rule and state it rather than to support it — but right now the rule is "whatever the chunking did".
Neither is a blocker and both need a malformed payload to reach. Flagging them because the class currently documents an assumption (name first, exactly one argument field) that the code partly enforces, partly exceeds, and partly resolves differently from its own non-streaming sibling — and a short note in the docstring about what happens off the contract would save the next person the harness.
No description provided.