fix(trtllm): cherry-pick rc22 disagg request ID compat into feat/aa_glm5.2 - #12205
Conversation
…preservation Fix 2: parse assistant tool_calls[*].function.arguments from JSON string to serde_json object before passing messages to MiniJinja. GLM-5.2 Jinja template iterates arguments with for k, v in _args.items() which requires a dict; the OpenAI wire schema stores arguments as a JSON-object string. This fixes malformed or missing tool-call argument rendering in multi-turn GLM-5.2 prompt history. Fix 3: when glm47 parser drops a truncated tool_call block (no end fence, finish_reason=length), preserve the raw text as content instead of returning an empty assistant turn. This matches TRT-LLM behavior where the partial XML is returned as content, preventing silent conversation-trajectory divergence on max_tokens truncation. Refs: DYN-3082
…ool_call recovery Fix 2 (correct path): Move tool_calls[*].function.arguments JSON-string->dict normalization to a shared pub(crate) helper normalize_tool_call_arguments() in preprocessor/prompt.rs and call it from both: - NvCreateChatCompletionRequest::messages() in prompt.rs <-- main /v1/chat path - UnifiedRequest::messages() in unified.rs <-- secondary path The previous fix was only in unified.rs which is not the path RWLT uses. Fix 3 (streaming path): Add truncated-tool_call content-recovery to the streaming jail path in preprocessor.rs::apply_tool_calling_jail(). Buffers input content text; on finish_reason=length with no tool_calls in the jail output, computes the dropped bytes and emits them as a synthetic content chunk before the finish chunk if they contain <tool_call>. The previous fix was only in aggregator.rs (non-streaming batch path), but RWLT uses stream=true so the jail path is what fires. Together with the AA reasoning_content fix (peili/dynamo_conv_routing@767073e), these address the ISL gap (~3K tokens) and OSL/trajectory divergence observed between Dynamo and TRT-LLM on GLM-5.2 RWLT benchmarks. Refs: DYN-3082
Signed-off-by: krishung5 <krish@nvidia.com>
Signed-off-by: krishung5 <krish@nvidia.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| let dropped = if emitted < input_text.len() { | ||
| &input_text[emitted..] | ||
| } else { | ||
| "" | ||
| }; |
There was a problem hiding this comment.
🟡 Recovering truncated tool-call output can crash the response when the byte offset lands mid-character
The dropped text is extracted by byte-slicing the buffered input (&input_text[emitted..] at lib/llm/src/preprocessor.rs:2830) using a byte count of previously emitted content, so if that offset is not a UTF-8 character boundary the request stream panics instead of returning the partial output.
Impact: A truncated tool-call response containing multi-byte (e.g. non-ASCII) text can abort the request with a panic rather than delivering the recovered content.
Why the byte offset may not be a char boundary
output_content_len accumulates content.len() (byte lengths) of content the jail emits (lib/llm/src/preprocessor.rs:2811-2814), while input_text accumulates the raw input content deltas (lib/llm/src/preprocessor.rs:2748-2752). The recovery code assumes emitted is a byte prefix boundary of input_text and slices input_text[emitted..]. Rust's str indexing panics if emitted falls inside a multi-byte UTF-8 sequence, which can happen if the jail's emitted content does not byte-align with the input prefix (e.g. content transformation, multiple choices sharing the global counters, or partial multi-byte content). Using input_text.get(emitted..) avoids the panic.
| let dropped = if emitted < input_text.len() { | |
| &input_text[emitted..] | |
| } else { | |
| "" | |
| }; | |
| let dropped = if emitted < input_text.len() { | |
| input_text.get(emitted..).unwrap_or("") | |
| } else { | |
| "" | |
| }; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| let is_terminal = first_output | ||
| .data | ||
| .as_ref() | ||
| .and_then(|output| output.finish_reason.as_ref()) | ||
| .is_some_and(|reason| !matches!(reason, FinishReason::Length)); | ||
| if is_terminal { | ||
| return Ok(PrefillCompletion::Terminal { | ||
| output: first_output, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🔍 Terminal prefill treats Error/Cancelled/ContentFilter as complete responses
is_terminal in consume_prefill_stream (lib/llm/src/kv_router/prefill_router/admission.rs:117-121) is true for any finish reason except Length, which includes FinishReason::Error, Cancelled, and ContentFilter. Previously such a prefill output lacking disaggregated_params would raise NoDisaggregatedParams; now it is returned directly to the caller as a terminal completion. For genuine EOS/Stop this is the intended fix, but an errored/cancelled one-token prefill would now surface as a normal terminal response rather than an error. This appears acceptable (the finish reason is preserved) but worth confirming against desired error-propagation semantics.
Was this helpful? React with 👍 or 👎 to provide feedback.
github-actions
Bot
added
backend::trtllm
2 participants
Summary
Cherry-picks both commits from #12203 into the
feat/aa_glm5.2branch to enable GLM-5.2 benchmarking against TRT-LLM rc22 (d593b0cd).Why: The new TRT-LLM
d593b0cdsubstrate changedDISAGG_NODE_ID_BITSfrom 10 to 8, reducingNODE_ID_SPACEfrom 1024 to 256. Dynamo computesdisagg_machine_id = int(endpoint.connection_id()) % 1021, which produces values up to 1020 — exceeding rc22's 8-bit range and causingValueError: node_id must be in range [0, 256)on every request (100% error rate).Commits cherry-picked from #12203:
ebbd4ccfix(trtllm): adapt disagg request IDs across API versions — detects rc21/rc22 API signature and adapts accordingly461ee0defix(kv-router): return terminal prefill responses directly — handles CTX requests that finish during one-token prefill (EOS/stop token)Test plan
trtllm_d593b0cdsubstrate🤖 Generated with Claude Code