fix(tool_parser): preserve Qwen XML streamed arguments

[ai-jz]

Description

Problem

QwenXmlParser can emit invalid or cross-contaminated tool arguments depending on the argument content and chunk boundaries:

• A literal } inside a parameter string is counted as a structural JSON brace. For <tool_call><function=get_weather><parameter=city>echo '}'</parameter></function></tool_call>, the streamed arguments omit the root closing brace.
• At end of stream, <tool_call><function=get_weather><parameter=city>Tokyo</parameter> leaves an open argument object. The generic recovery helper compares compact serialized JSON with the parser's spaced fragments and finds no matching prefix.
• When adjacent calls arrive together, parameter scanning includes subsequent calls. A Paris call can acquire the next call's fields or have its city overwritten by Tokyo.

Minimal reproducer

On a clean SMG checkout at 9c87ef5d499604089b25bd1900ea7f6804cbf1db, save the following temporary file as crates/tool_parser/tests/qwen_xml_minimal_repro.rs. It uses one tool, one string parameter containing }, and one complete chunk; no model server or GPU is needed.

use serde_json::{json, Value};
use tool_parser::{parsers::QwenXmlParser, ToolParser};

#[tokio::test]
async fn qwen_xml_minimal_repro() {
    let tools = vec![serde_json::from_value(json!({
        "type": "function",
        "function": {
            "name": "f",
            "parameters": {"type": "object", "properties": {"x": {"type": "string"}}}
        }
    }))
    .unwrap()];
    let mut parser = QwenXmlParser::new();
    let mut result = parser
        .parse_incremental(
            "<tool_call><function=f><parameter=x>}</parameter></function></tool_call>",
            &tools,
        )
        .await
        .unwrap();
    result
        .calls
        .extend(parser.get_unstreamed_tool_args().unwrap_or_default());
    let args: String = result
        .calls
        .iter()
        .map(|call| call.parameters.as_str())
        .collect();
    assert_eq!(
        serde_json::from_str::<Value>(&args).ok(),
        Some(json!({"x": "}"})),
        "arguments: {args}"
    );
}

Run from the repository root:

cargo test --locked -p tool-parser --test qwen_xml_minimal_repro -- --nocapture

Before the fix: the assertion fails with arguments: {"x": "}"; the root closing brace is missing. Result: 0 passed / 1 failed (exit 101), including the terminal argument getter.

With this fix: the same snippet passes and the collected arguments parse to {"x":"}"}. Result: 1 passed / 0 failed (exit 0). Both runs were executed against the same base, changing only the parser implementation. This temporary reproducer is documentation material; the committed patch still adds six regression tests.

Solution

Limit parameter extraction to the current </tool_call> boundary and close each argument object when that boundary is consumed, emitting {} for an empty call. At EOS, append only the root } when the resulting JSON equals exactly the already parsed argument object. Unfinished parameter values remain absent.

The repair stays within QwenXmlParser; the shared recovery helper, argument coercion, and caller finish-reason policies are unchanged. Existing tests retain coverage for the schema coercion from #1841 and literal argument values from #1899. The existing qwen_coder and nemotron aliases use the same parser implementation.

Prior work

This change follows earlier work on final argument validity, per-call isolation, and streaming completion:

Prior PR / review	Earlier contribution	How this PR follows it
Original Qwen XML parser, SGLang #12909	CatherineSue's review asked about final concatenated argument JSON and nested objects.	Reconstruct arguments per tool index and assert exact parsed JSON after streaming and terminal flush.
MiniMax-M2 parallel calls, SMG #1824	Fixed missing calls. A separate automated review by chatgpt-codex-connector identified parameter leakage when adjacent calls are scanned together.	Bound Qwen XML scanning to one call and assert exact arguments per index, covering foreign-field leakage and same-name overwrites.
Cohere framing, SMG #1941	Distinguished literal markers inside JSON strings from framing; review follow-through removed unsafe recovery fallbacks.	Treat braces inside parameter values as data. Qwen's emitted root object is already known to be open, so close it at the tool boundary without adding a string scanner; keep EOS recovery conservative.
Shared JSON streaming, SMG #2271	Repaired buffered-content loss and missing arguments when a call completed in one parser invocation, with regression coverage for shared-helper consumers.	Cover coalesced input and terminal getters in the separate XML implementation, which does not use that shared streaming helper.

The three Qwen mechanisms were present in the initial XML implementation. Following its mainline history through renaming and value-coercion changes found no intervening repair-and-revert sequence.

Changes

• Find the current tool boundary once, reuse it for parameter scanning and closure, and retain independent tool indices.
• Replace raw brace counting with closure at the tool boundary, including empty calls.
• Add a Qwen XML EOS recovery check using parsed JSON equality.
• Add six focused regression tests to the existing Qwen XML integration suite, covering argument closure, conservative EOS recovery, last-tool indexing, and per-call isolation.

Test Plan

Rebase validation

Rebased onto main a8dc4f4088974925ee6e05f8a4c8aae65b011a23 as 1a3ff347dc3ed12dbc94b64d1dec21a50f99146f. git range-diff confirms the original parser/test patch is unchanged: two files, +186/-20. On this new commit, the CPU run of cargo test --locked -p tool-parser passed 501 tests / 0 failures / 0 ignored across 26 completed harnesses. cargo clippy --locked -p tool-parser --all-targets --all-features -- -D warnings, nightly whole-repository formatting, and the base-to-head whitespace check all returned exit 0. The initial full-workspace result below is historical; new-head hosted CI, including the GPU lanes, remains required.

The previous CI run had two independent failures plus the aggregate finish failure:

• TensorRT-LLM failed during engine startup because cuda-bindings 13.4.1 removed cudaIpcMemHandle_t.reserved. The rebase includes the exact dependency pin and canary from merged #2494.
• vLLM failed during the first no-tool warmup request with UCX/RoCE connection errors, before the multimodal assertions. The same runner host has a prior diagnosis and successful unchanged-code rerun on another host. This supports a runner transport cause; the exact device configuration was not inspected.
• finish failed because those two prerequisite jobs failed. No parser or CI-gate changes were added for these failures.

Original red/green evidence

Reproduction baseline: a5901cb5905eb929ec60448f39b3c082d5940b91. The initial publication base was 9c87ef5d499604089b25bd1900ea7f6804cbf1db; neither Qwen XML file changed upstream, and the reviewed patch is byte-for-byte unchanged. Applying only the final test-file changes to the unchanged production baseline gives 39 passed / 5 failed; adding the production fix gives 44 passed / 0 failed. All 38 pre-existing tests pass in both runs. Of the six new tests, five expose defects in the baseline and one protects already-correct handling of an unfinished parameter. All fixtures are synthetic.

New regression tests: baseline versus fix

All test names below have the prefix test_qwen_xml_ and live in crates/tool_parser/tests/tool_parser_qwen_xml.rs. The baseline column records the first observed failing assertion in each test; later subcases in that test are not claimed as independently reproduced failures.

New test	Unchanged baseline	With this fix
streaming_braces_inside_string_do_not_close_object	FAIL: the echo '}' value leaves streamed JSON without its root closing brace.	PASS: retain the literal brace string and emit valid JSON.
eos_closes_only_complete_parameter_values	FAIL: a completed city=Tokyo parameter followed by EOS leaves the argument object open.	PASS: close the object after a completed parameter, including when an unfinished next parameter follows; preserve only completed values.
eos_does_not_invent_unfinished_parameter	PASS: an unfinished first parameter, city=Tok, retains the existing {} fallback.	PASS: recovery still does not invent a value for the unfinished parameter.
eos_closes_last_of_multiple_calls	FAIL: the second call's Tokyo value overwrites the first call's Paris value.	PASS: keep calls independent and emit the EOS closure for the last call at index 1. This checks an index that the standalone EOS case cannot exercise.
coalesced_calls_do_not_share_parameters	FAIL: the first Paris call becomes Tokyo and acquires units=celsius from the second call.	PASS: one fixture checks both value overwriting and foreign-field leakage, with exact arguments per index.
empty_calls_and_nested_values_are_closed_once	FAIL: the first empty call receives malformed arguments containing the following call's nested data.	PASS: the earlier empty call emits {}; the following nested value remains exact; completed calls need no additional argument flush or duplicate closure.

The six tests cover whole input, character-by-character input, and every valid two-chunk split. Assertions reconstruct exact semantic JSON per tool index, verify names and terminal getters, and check reset behavior. Completed fixtures require no pending closure. Each test protects a distinct boundary: payload braces, single-call EOS, no completed value, last-call EOS, adjacent-call isolation, or an earlier empty call. Existing suites supply nonstream and factory-mapping coverage.

Original-base commands and broader compatibility checks

Run from the repository root with Rust 1.95 and nightly rustfmt:

cargo test --locked -p tool-parser --test tool_parser_qwen_xml
cargo test --locked -p tool-parser --lib \
  --test tool_parser_qwen_xml --test tool_parser_qwen_dotted \
  --test tool_parser_nemotron --test tool_parser_streaming_flush
cargo clippy --locked -p tool-parser --all-targets --all-features -- -D warnings
cargo +nightly fmt --all -- --check
git diff --check

The original focused CPU run passed 166 tests with zero failures:

Suite	Passed	Checked surface
Library (--lib)	113	Existing tool-parser unit coverage.
tool_parser_qwen_xml	44	38 existing tests plus six new regressions above.
tool_parser_qwen_dotted	3	Qwen dotted-family mappings, earlier-family compatibility, and a representative Qwen3.8 nonstream parse.
tool_parser_nemotron	3	Nemotron mappings, named alias, earlier-family exclusion, and a representative Nemotron-3.5 nonstream parse.
tool_parser_streaming_flush	3	Buffered text recovery, streamed arguments, and reset behavior in shared-helper consumers.

The family suites verify parser selection and representative nonstream parsing; they do not establish model-serving qualification.

The initial full publication gate was run on 9c87ef5d499604089b25bd1900ea7f6804cbf1db plus the unchanged patch, on a CPU builder with Rust 1.95:

Command	Result
cargo test --locked	5,481 passed / 0 failed / 40 ignored, summed across 120 completed test harnesses; exit 0. Includes the 44-test Qwen XML suite. No additional test filters were applied.
cargo clippy --locked --workspace --all-targets -- -D warnings	Exit 0. Uses the documented fallback without OpenCV; workspace non-default features were not linted. The tool-parser-only all-features check above also passed.
cargo +nightly fmt --all -- --check	Silent success, exit 0.
git diff --check	Silent success, exit 0.

Representative output from the full workspace test run and final workspace Clippy run:

test result: ok. 44 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.22s
    Finished `dev` profile [unoptimized] target(s) in 4m 13s

The first line is the Qwen XML test harness, not the workspace total. The earlier 166-test focused run is overlapping evidence and is not added to the 5,481-test total. Cargo emitted existing workspace-manifest warnings; there were no Clippy lint failures.

The gRPC ChatCompletion streaming caller's terminal getters were checked in source and exercised by the parser tests. The workspace run includes mock transport/gRPC tests, but no live-model Qwen gRPC or GPU serving canary was run. Existing delimiter ambiguity and duplicate parameter names within a single call remain outside this change. There is no configuration migration. Deployment validation remains a separate step; reverting this change restores the previous parser behavior.

Checklist

• cargo +nightly fmt passes
• cargo clippy --workspace --all-targets -- -D warnings passes (documented alternative without OpenCV); tool-parser scoped --all-features also passes
• (Optional) Documentation updated
• (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: dd9268d5-e2cc-4528-aa66-29f13c524616

📥 Commits

Reviewing files that changed from the base of the PR and between f237e22 and 45eecf7.

📒 Files selected for processing (2)

• crates/tool_parser/src/parsers/qwen_xml.rs
• crates/tool_parser/tests/tool_parser_qwen_xml.rs

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Improved streamed tool-call argument handling for Qwen XML responses.
  • Correctly closes completed calls at the end of a response, including empty calls and multiple coalesced calls.
  • Prevents braces inside string values or nested data from prematurely ending arguments.
  • Avoids incorrectly inventing unfinished parameters and keeps arguments from separate calls isolated.
• Tests
  • Added coverage for incremental streaming, split responses, nested values, and end-of-stream behavior.

Walkthrough

The Qwen XML parser now isolates parameters for coalesced tool calls, closes completed streamed arguments consistently, and preserves incomplete parameters for end-of-stream recovery. Tests cover chunking patterns, nested values, empty calls, and incomplete arguments.

Changes

Qwen XML streaming

Layer / File(s)	Summary
Parameter boundaries and closure crates/tool_parser/src/parsers/qwen_xml.rs, crates/tool_parser/tests/tool_parser_qwen_xml.rs	Parameter parsing stops at the current end token. Completed calls emit one closing fragment, including empty calls. Tests verify coalesced calls, nested values, and braces inside strings.
End-of-stream recovery crates/tool_parser/src/parsers/qwen_xml.rs, crates/tool_parser/tests/tool_parser_qwen_xml.rs	End-of-stream handling returns a closing brace when streamed arguments match the expected JSON. Otherwise, incomplete parameters remain pending. Tests cover complete and unfinished values across chunk boundaries.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 1a3ff

Qwen XML streamed tool-call arguments now remain isolated per call and close consistently, including at end of stream. No actionable current-head merge risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely identifies the main change: preserving streamed arguments in the Qwen XML tool parser.
Description check	✅ Passed	The description is directly related to the changes. It explains the Qwen XML parsing defects, the implementation, regression tests, and validation results.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ai-jz/qwen-xml-local-fix

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

🟡 Nit: Truncating the scan at the first </tool_call> silently changes behavior for parameter values that contain the literal </tool_call>, and nothing pins the new behavior.

For <tool_call><function=process><parameter=snippet>use </tool_call> tag</parameter></function></tool_call>:

• Before: the regex ran over the whole buffer, captured snippet = "use </tool_call> tag", and the brace counter appended } — the client received the complete, valid {"snippet": "use </tool_call> tag"} (plus the leftover tag</parameter>… leaking as normal text).
• After: parameter_end lands mid-value, the slice is <parameter=snippet>use with no </parameter>, so the parameter never matches and the call closes as {} — the argument is dropped entirely.

This is arguably the right call: parse_complete_inner's non-greedy (?s)<tool_call>\s*(.*?)\s*</tool_call> extractor already truncates at the same point and yields {}, so streaming and non-streaming now agree, and vLLM/SGLang split the same way. But the PR description lists this under "existing delimiter ambiguity … remain outside this change", when the change actually flips it from "correct args" to "empty args". Worth a short regression test in tool_parser_qwen_xml.rs asserting json!({}) for that fixture, so the alignment with parse_complete is intentional and stays that way.