From 73e85c90244a8b3647db7ab2ba7d1fed65da4bd6 Mon Sep 17 00:00:00 2001 From: Matej Kosec Date: Mon, 29 Jun 2026 18:07:04 +0200 Subject: [PATCH 01/11] fix(toolcalling): synthesize tool_calls finish_reason when stream lacks one When the engine emits a complete tool call but the SSE stream ends without any finish_reason chunk (e.g. speculative decoding folded EOS into the content chunks, or the terminal signal was dropped), strict OpenAI clients wait for a non-null finish_reason before considering the tool call complete and hang until their client-side timeout (Codex's 30s limit). This is DGH-967. Both tool-call streaming paths had the gap: - v2 parser path (tool_parser_v2::apply_stream): the end-of-stream backstop flushed pending tool calls with finish_reason: None. Now synthesizes ToolCalls for a choice that emitted tool calls; text-only output stays None. - v1 jail path (JailedStream::fix_finish_reason): only rewrote Stop->ToolCalls when a finish_reason chunk arrived; if none arrived, finalize() emitted the tool call with the (absent) upstream reason and no ToolCalls was ever set. Added an end-of-stream backstop that emits a trailing ToolCalls chunk for choices that emitted tool calls but never received a finish_reason. Adds regression tests on both paths: a tool-call stream ending without a finish_reason must yield a terminal ToolCalls; a text-only stream must not get a synthesized finish_reason. Signed-off-by: Matej Kosec --- .../protocols/openai/chat_completions/jail.rs | 126 +++++++++++++++++- .../openai/chat_completions/tool_parser_v2.rs | 70 +++++++++- 2 files changed, 194 insertions(+), 2 deletions(-) diff --git a/lib/llm/src/protocols/openai/chat_completions/jail.rs b/lib/llm/src/protocols/openai/chat_completions/jail.rs index 543c19c694c7..3466fb428f9c 100644 --- a/lib/llm/src/protocols/openai/chat_completions/jail.rs +++ b/lib/llm/src/protocols/openai/chat_completions/jail.rs @@ -1461,9 +1461,17 @@ impl JailedStream { where S: Stream> + Send + 'static, { + let _ = named_tool_active; + let _ = &jail_mode; stream! { tokio::pin!(input_stream); let mut has_tool_calls_per_choice: HashMap = HashMap::new(); + // Choices that already received a finish_reason during the stream — used by + // the end-of-stream backstop below to avoid synthesizing a duplicate. + let mut terminated: std::collections::HashSet = std::collections::HashSet::new(); + // Last response, kept (with choices cleared) as a template for a synthesized + // trailing finish_reason chunk when the stream ended without one. + let mut template: Option = None; while let Some(mut response) = input_stream.next().await { // Track if any choice emitted tool calls @@ -1472,6 +1480,14 @@ impl JailedStream { if choice.delta.tool_calls.is_some() { has_tool_calls_per_choice.insert(choice.index, true); } + if choice.finish_reason.is_some() { + terminated.insert(choice.index); + } + } + { + let mut t = data.clone(); + t.inner.choices.clear(); + template = Some(t); } } @@ -1487,7 +1503,6 @@ impl JailedStream { // choice, finish_reason MUST be "tool_calls" — regardless of // whether tool_choice was "auto", "required", or a named // function. - let _ = named_tool_active; match &jail_mode { JailMode::MarkerBased => { if has_tool_calls { @@ -1508,6 +1523,45 @@ impl JailedStream { yield response; } + + // Backstop: the stream ended without a finish_reason for some choice that + // emitted tool calls (e.g. speculative decoding folded EOS into content, or + // the engine dropped the terminal signal). Strict OpenAI clients wait for a + // non-null finish_reason before considering a tool call complete; without a + // synthesized `ToolCalls` here they hang until their client-side timeout + // (DGH-967). Emit one trailing chunk per such choice carrying the missing + // reason. Choices that never emitted tool calls are left alone — there is no + // signal to invent a finish_reason from for text-only output. + if let Some(template) = template { + for (index, _) in has_tool_calls_per_choice.iter().filter(|(_, &has)| has) { + if terminated.contains(index) { + continue; + } + let mut response = template.clone(); + #[allow(deprecated)] + let choice = dynamo_protocols::types::ChatChoiceStream { + index: *index, + delta: dynamo_protocols::types::ChatCompletionStreamResponseDelta { + role: None, + content: None, + tool_calls: None, + function_call: None, + refusal: None, + reasoning_content: None, + }, + finish_reason: Some(FinishReason::ToolCalls), + logprobs: None, + }; + response.inner.choices = vec![choice]; + yield Annotated { + data: Some(response), + id: None, + event: None, + comment: None, + error: None, + }; + } + } } } } @@ -2079,4 +2133,74 @@ mod tests { all_text ); } + + /// The last `finish_reason` a client sees across the stream. `None` if the + /// stream never carried one (the DGH-967 hang condition). + fn final_finish_reason( + responses: &[Annotated], + ) -> Option { + responses + .iter() + .filter_map(|r| r.data.as_ref()) + .flat_map(|d| d.inner.choices.iter()) + .filter_map(|c| c.finish_reason) + .next_back() + } + + // DGH-967: when the engine emits a complete tool call but the stream ends + // WITHOUT any finish_reason chunk (speculative decoding folded EOS into + // content, or the terminal signal was dropped), a strict OpenAI client + // waits for a non-null finish_reason and hangs until its timeout. The jail + // path's finalize() emits the tool call with the (absent) upstream + // finish_reason; fix_finish_reason's end-of-stream backstop must synthesize + // `ToolCalls` so the client gets a terminal signal. + #[tokio::test] + async fn jail_synthesizes_tool_calls_finish_reason_when_stream_lacks_one() { + let jail = JailedStream::builder().tool_call_parser("hermes").build(); + + let chunks = vec![text_chunk("``` +{"name": "get_weather", "arguments": {"location": "SF"}} +```")]; + + let input_stream = Box::pin(stream::iter(chunks)); + let output_stream = jail.apply_with_finish_reason(input_stream); + + let responses: Vec<_> = output_stream.collect().await; + let tool_calls = collect_tool_calls(&responses); + assert!( + !tool_calls.is_empty(), + "expected the hermes tool call to be parsed: {tool_calls:?}" + ); + assert_eq!(tool_calls[0].0, "get_weather"); + assert_eq!( + final_finish_reason(&responses), + Some(FinishReason::ToolCalls), + "backstop must synthesize ToolCalls when the stream ended without a finish_reason" + ); + } + + // DGH-967 corollary: a text-only stream that ends without a finish_reason + // must NOT get a synthesized one — there is no signal to invent a + // finish_reason from when no tool call was emitted. + #[tokio::test] + async fn jail_does_not_synthesize_finish_reason_for_text_only_stream() { + let jail = JailedStream::builder().tool_call_parser("hermes").build(); + + let chunks = vec![text_chunk("hello world"), text_chunk("")]; + + let input_stream = Box::pin(stream::iter(chunks)); + let output_stream = jail.apply_with_finish_reason(input_stream); + + let responses: Vec<_> = output_stream.collect().await; + let tool_calls = collect_tool_calls(&responses); + assert!( + tool_calls.is_empty(), + "no tool calls expected: {tool_calls:?}" + ); + assert_eq!( + final_finish_reason(&responses), + None, + "text-only stream with no upstream finish_reason must not get a synthetic one" + ); + } } diff --git a/lib/llm/src/protocols/openai/chat_completions/tool_parser_v2.rs b/lib/llm/src/protocols/openai/chat_completions/tool_parser_v2.rs index 9ce7c12a02ff..61109f008f6d 100644 --- a/lib/llm/src/protocols/openai/chat_completions/tool_parser_v2.rs +++ b/lib/llm/src/protocols/openai/chat_completions/tool_parser_v2.rs @@ -299,6 +299,19 @@ where if result.normal_text.is_empty() && tool_calls.is_none() { continue; } + // A choice that produced tool calls during the stream must terminate + // with `ToolCalls` even when the backend never sent a finish_reason + // chunk (e.g. speculative decoding folded EOS into content, or the + // engine dropped the terminal signal). Strict OpenAI clients wait for + // a non-null finish_reason before considering a tool call complete; + // emitting `None` here left them hanging until their client-side + // timeout (DGH-967). Text-only output without an upstream finish + // reason stays `None` — we have no signal to invent one from. + let finish_reason = if tool_emitted.contains(index) { + Some(FinishReason::ToolCalls) + } else { + None + }; let mut response = template.clone(); #[allow(deprecated)] let choice = dynamo_protocols::types::ChatChoiceStream { @@ -312,7 +325,7 @@ where refusal: None, reasoning_content: None, }, - finish_reason: None, + finish_reason, logprobs: None, }; response.inner.choices = vec![choice]; @@ -535,4 +548,59 @@ mod tests { "finish_reason must flip Stop->ToolCalls when tool calls are emitted" ); } + + // DGH-967: the stream emits a complete tool call but ends WITHOUT any + // finish_reason chunk (e.g. speculative decoding folded EOS into content, or + // the engine dropped the terminal signal). A strict OpenAI client waits for a + // non-null finish_reason before considering the tool call complete; the + // end-of-stream backstop must synthesize `ToolCalls` so the client doesn't + // hang until its timeout. + #[tokio::test] + async fn qwen3_bypass_synthesizes_tool_calls_when_stream_lacks_finish_reason() { + // Same call as the incremental test, but the final chunk carries NO + // finish_reason — the stream simply ends after the tool markup. + let mut chunks: Vec<_> = QWEN3_GET_WEATHER + .as_bytes() + .chunks(8) + .map(|b| chunk(std::str::from_utf8(b).unwrap(), false)) + .collect(); + // Trailing empty chunk with finish=false — no finish_reason at all. + chunks.push(chunk("", false)); + + let out: Vec<_> = apply_stream(stream::iter(chunks), None, "qwen3_coder".to_string()) + .collect::>() + .await; + + let (calls, _content) = reassemble(&out); + assert_eq!(calls.len(), 1, "expected exactly one tool call: {calls:?}"); + assert_eq!(calls[0].0, "get_weather"); + let args: serde_json::Value = serde_json::from_str(&calls[0].1).unwrap(); + assert_eq!(args["location"], "Paris"); + assert_eq!( + final_finish_reason(&out), + Some(FinishReason::ToolCalls), + "backstop must synthesize ToolCalls when the stream ended without a finish_reason" + ); + } + + // DGH-967 corollary: when the stream ends without a finish_reason AND no tool + // call was emitted (text-only output), the backstop must NOT invent a + // finish_reason — there is no signal to synthesize one from. A trailing + // content chunk may be emitted, but its finish_reason stays None. + #[tokio::test] + async fn qwen3_bypass_does_not_synthesize_finish_reason_for_text_only_stream() { + let chunks = vec![chunk("hello world", false), chunk("", false)]; + + let out: Vec<_> = apply_stream(stream::iter(chunks), None, "qwen3_coder".to_string()) + .collect::>() + .await; + + let (calls, _content) = reassemble(&out); + assert!(calls.is_empty(), "no tool calls expected: {calls:?}"); + assert_eq!( + final_finish_reason(&out), + None, + "text-only stream with no upstream finish_reason must not get a synthetic one" + ); + } } From c93869348ffa37ce7723cdf28061b43a7b27f034 Mon Sep 17 00:00:00 2001 From: Matej Kosec Date: Mon, 29 Jun 2026 18:28:05 +0200 Subject: [PATCH 02/11] =?UTF-8?q?fix:=20edition=202024=20pattern=20?= =?UTF-8?q?=E2=80=94=20drop=20explicit=20deref=20in=20filter=20closure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `filter(|(_, &has)| has)` is rejected under Rust 2024 binding modes (cannot explicitly dereference within an implicitly-borrowing pattern). Rewrite as `filter(|(_, has)| *has)`. Signed-off-by: Matej Kosec --- lib/llm/src/protocols/openai/chat_completions/jail.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/llm/src/protocols/openai/chat_completions/jail.rs b/lib/llm/src/protocols/openai/chat_completions/jail.rs index 3466fb428f9c..22bf53df6be5 100644 --- a/lib/llm/src/protocols/openai/chat_completions/jail.rs +++ b/lib/llm/src/protocols/openai/chat_completions/jail.rs @@ -1533,7 +1533,7 @@ impl JailedStream { // reason. Choices that never emitted tool calls are left alone — there is no // signal to invent a finish_reason from for text-only output. if let Some(template) = template { - for (index, _) in has_tool_calls_per_choice.iter().filter(|(_, &has)| has) { + for (index, _) in has_tool_calls_per_choice.iter().filter(|(_, has)| *has) { if terminated.contains(index) { continue; } From 6d8ff0837847cc98903d28fedbc641360abd7a1c Mon Sep 17 00:00:00 2001 From: Matej Kosec Date: Mon, 29 Jun 2026 18:32:55 +0200 Subject: [PATCH 03/11] fix(toolcalling): emit synthesized ToolCalls before the usage-only chunk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DGH-967 requires the synthesized terminal finish_reason chunk to precede the usage-only chunk (OpenAI stream ordering — the terminal finish_reason comes before usage). The prior fix emitted it at stream end, after the usage chunk had already passed through, violating that ordering on the jail path. Restructure fix_finish_reason to detect the empty-choices/usage chunk as it arrives and synthesize the missing ToolCalls chunks *before* yielding it; keep an end-of-stream backstop for streams that never emit a usage chunk. Extract a synthesize_tool_calls_chunk helper. Add a regression test mirroring the ticket repro (tool call + usage-only chunk, no finish_reason) asserting the ToolCalls chunk precedes the usage chunk. Signed-off-by: Matej Kosec --- .../protocols/openai/chat_completions/jail.rs | 200 ++++++++++++++---- 1 file changed, 162 insertions(+), 38 deletions(-) diff --git a/lib/llm/src/protocols/openai/chat_completions/jail.rs b/lib/llm/src/protocols/openai/chat_completions/jail.rs index 22bf53df6be5..980f6b3c7d73 100644 --- a/lib/llm/src/protocols/openai/chat_completions/jail.rs +++ b/lib/llm/src/protocols/openai/chat_completions/jail.rs @@ -1451,6 +1451,41 @@ impl JailedStream { false } + /// Build a synthetic terminal chunk carrying `finish_reason: ToolCalls` for a + /// choice that emitted tool calls but never received a finish_reason from the + /// engine. Used by `fix_finish_reason` to satisfy the OpenAI stream ordering + /// requirement (the terminal chunk must carry a non-null `finish_reason`) + /// when the stream ends without one — DGH-967. The delta is empty; only the + /// `finish_reason` carries information. + fn synthesize_tool_calls_chunk( + template: &NvCreateChatCompletionStreamResponse, + index: u32, + ) -> Annotated { + let mut response = template.clone(); + #[allow(deprecated)] + let choice = dynamo_protocols::types::ChatChoiceStream { + index, + delta: dynamo_protocols::types::ChatCompletionStreamResponseDelta { + role: None, + content: None, + tool_calls: None, + function_call: None, + refusal: None, + reasoning_content: None, + }, + finish_reason: Some(FinishReason::ToolCalls), + logprobs: None, + }; + response.inner.choices = vec![choice]; + Annotated { + data: Some(response), + id: None, + event: None, + comment: None, + error: None, + } + } + /// Post-processor that sets finish_reason to ToolCalls when tool calls were emitted /// This should be called after apply() to fix the finish_reason for tool call chunks fn fix_finish_reason( @@ -1467,14 +1502,17 @@ impl JailedStream { tokio::pin!(input_stream); let mut has_tool_calls_per_choice: HashMap = HashMap::new(); // Choices that already received a finish_reason during the stream — used by - // the end-of-stream backstop below to avoid synthesizing a duplicate. + // the backstop below to avoid synthesizing a duplicate. let mut terminated: std::collections::HashSet = std::collections::HashSet::new(); // Last response, kept (with choices cleared) as a template for a synthesized - // trailing finish_reason chunk when the stream ended without one. + // finish_reason chunk when the stream ended without one. let mut template: Option = None; + // Whether we have already emitted the synthesized terminal chunks (either + // before the usage-only chunk or at stream end), so we don't emit twice. + let mut synthesized = false; while let Some(mut response) = input_stream.next().await { - // Track if any choice emitted tool calls + // Track if any choice emitted tool calls, and which already terminated. if let Some(ref data) = response.data { for choice in &data.inner.choices { if choice.delta.tool_calls.is_some() { @@ -1521,45 +1559,48 @@ impl JailedStream { } } + // OpenAI stream ordering: the terminal finish_reason chunk must precede + // the usage-only chunk. When a chunk with no choices arrives (the + // frontend's compliance usage chunk, or any other empty-choices chunk) + // and tool-call choices are still missing a finish_reason, synthesize + // their terminal `ToolCalls` chunks *before* yielding this one. + let is_empty_choices = response + .data + .as_ref() + .map(|d| d.inner.choices.is_empty()) + .unwrap_or(true); + if is_empty_choices && !synthesized { + if let Some(template) = &template { + for (index, _) in has_tool_calls_per_choice.iter().filter(|(_, has)| *has) { + if terminated.contains(index) { + continue; + } + yield synthesize_tool_calls_chunk(template, *index); + } + } + synthesized = true; + } + yield response; } - // Backstop: the stream ended without a finish_reason for some choice that - // emitted tool calls (e.g. speculative decoding folded EOS into content, or - // the engine dropped the terminal signal). Strict OpenAI clients wait for a - // non-null finish_reason before considering a tool call complete; without a - // synthesized `ToolCalls` here they hang until their client-side timeout - // (DGH-967). Emit one trailing chunk per such choice carrying the missing - // reason. Choices that never emitted tool calls are left alone — there is no - // signal to invent a finish_reason from for text-only output. - if let Some(template) = template { - for (index, _) in has_tool_calls_per_choice.iter().filter(|(_, has)| *has) { - if terminated.contains(index) { - continue; + // Backstop: the stream ended without a finish_reason AND without an + // empty-choices/usage chunk to anchor the synthesized terminal chunks + // before (e.g. the engine dropped the terminal signal and the frontend + // never emitted a usage chunk). Emit one trailing `ToolCalls` chunk per + // tool-call choice that never received a finish_reason. Strict OpenAI + // clients wait for a non-null finish_reason before considering a tool call + // complete; without this they hang until their client-side timeout + // (DGH-967). Choices that never emitted tool calls are left alone — there + // is no signal to invent a finish_reason from for text-only output. + if !synthesized { + if let Some(template) = template { + for (index, _) in has_tool_calls_per_choice.iter().filter(|(_, has)| *has) { + if terminated.contains(index) { + continue; + } + yield synthesize_tool_calls_chunk(template, *index); } - let mut response = template.clone(); - #[allow(deprecated)] - let choice = dynamo_protocols::types::ChatChoiceStream { - index: *index, - delta: dynamo_protocols::types::ChatCompletionStreamResponseDelta { - role: None, - content: None, - tool_calls: None, - function_call: None, - refusal: None, - reasoning_content: None, - }, - finish_reason: Some(FinishReason::ToolCalls), - logprobs: None, - }; - response.inner.choices = vec![choice]; - yield Annotated { - data: Some(response), - id: None, - event: None, - comment: None, - error: None, - }; } } } @@ -1829,6 +1870,38 @@ mod tests { } } + /// A usage-only chunk (empty `choices`, a usage object) — the frontend's + /// OpenAI-compliance terminal chunk. Used to test that the synthesized + /// `ToolCalls` finish_reason chunk is emitted *before* this one. + fn usage_only_chunk() -> Annotated { + Annotated { + data: Some(NvCreateChatCompletionStreamResponse { + inner: CreateChatCompletionStreamResponse { + id: "id-42".to_string(), + object: "chat.completion.chunk".to_string(), + created: 0, + model: "test-model".to_string(), + choices: vec![], + usage: Some(dynamo_protocols::types::CompletionUsage { + prompt_tokens: 10, + completion_tokens: 5, + total_tokens: 15, + prompt_tokens_details: None, + completion_tokens_details: None, + }), + service_tier: None, + system_fingerprint: None, + }, + nvext: None, + llm_metrics: None, + }), + id: None, + event: None, + comment: None, + error: None, + } + } + /// Collect all emitted tool calls from the jailed stream output fn collect_tool_calls( responses: &[Annotated], @@ -2203,4 +2276,55 @@ mod tests { "text-only stream with no upstream finish_reason must not get a synthetic one" ); } + + // DGH-967 ordering: the ticket's repro is a tool call followed by a + // usage-only chunk, with NO finish_reason chunk from the engine. The + // synthesized `ToolCalls` terminal chunk must be emitted *before* the + // usage-only chunk (OpenAI stream ordering — the terminal finish_reason + // precedes usage). This mirrors the production repro in the ticket. + #[tokio::test] + async fn jail_synthesizes_tool_calls_before_usage_only_chunk() { + let jail = JailedStream::builder().tool_call_parser("hermes").build(); + + let chunks = vec![text_chunk("``` +{"name": "get_weather", "arguments": {"location": "SF"}} +```"), usage_only_chunk()]; + + let input_stream = Box::pin(stream::iter(chunks)); + let output_stream = jail.apply_with_finish_reason(input_stream); + + let responses: Vec<_> = output_stream.collect().await; + let tool_calls = collect_tool_calls(&responses); + assert!( + !tool_calls.is_empty(), + "expected the hermes tool call: {tool_calls:?}" + ); + assert_eq!(tool_calls[0].0, "get_weather"); + assert_eq!( + final_finish_reason(&responses), + Some(FinishReason::ToolCalls), + "a synthesized ToolCalls terminal chunk must be present" + ); + + // The ToolCalls terminal chunk must precede the usage-only chunk. + let finish_pos = responses.iter().position(|r| { + r.data.as_ref().is_some_and(|d| { + d.inner + .choices + .iter() + .any(|c| c.finish_reason == Some(FinishReason::ToolCalls)) + }) + }); + let usage_pos = responses.iter().position(|r| { + r.data + .as_ref() + .is_some_and(|d| d.inner.usage.is_some() && d.inner.choices.is_empty()) + }); + let finish_pos = finish_pos.expect("no ToolCalls chunk emitted"); + let usage_pos = usage_pos.expect("no usage-only chunk in output"); + assert!( + finish_pos < usage_pos, + "ToolCalls chunk at {finish_pos} must precede the usage chunk at {usage_pos}" + ); + } } From c231745c2be11f6756974afa659124cfd63e44e8 Mon Sep 17 00:00:00 2001 From: Matej Kosec Date: Mon, 29 Jun 2026 22:02:38 +0200 Subject: [PATCH 04/11] fix(toolcalling): complete missing finish reason backstops Compile the jail-path synthesis helper and use valid Hermes payloads in its regression tests. Emit a terminal ToolCalls chunk on the v2 path when the parser already streamed the call and finish() has no additional output. Signed-off-by: Matej Kosec --- .../protocols/openai/chat_completions/jail.rs | 31 ++++++++++--------- .../openai/chat_completions/tool_parser_v2.rs | 14 +++++++-- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/lib/llm/src/protocols/openai/chat_completions/jail.rs b/lib/llm/src/protocols/openai/chat_completions/jail.rs index 980f6b3c7d73..057957f9d7ba 100644 --- a/lib/llm/src/protocols/openai/chat_completions/jail.rs +++ b/lib/llm/src/protocols/openai/chat_completions/jail.rs @@ -1571,11 +1571,11 @@ impl JailedStream { .unwrap_or(true); if is_empty_choices && !synthesized { if let Some(template) = &template { - for (index, _) in has_tool_calls_per_choice.iter().filter(|(_, has)| *has) { + for (index, _) in has_tool_calls_per_choice.iter().filter(|(_, has)| **has) { if terminated.contains(index) { continue; } - yield synthesize_tool_calls_chunk(template, *index); + yield Self::synthesize_tool_calls_chunk(template, *index); } } synthesized = true; @@ -1593,14 +1593,12 @@ impl JailedStream { // complete; without this they hang until their client-side timeout // (DGH-967). Choices that never emitted tool calls are left alone — there // is no signal to invent a finish_reason from for text-only output. - if !synthesized { - if let Some(template) = template { - for (index, _) in has_tool_calls_per_choice.iter().filter(|(_, has)| *has) { - if terminated.contains(index) { - continue; - } - yield synthesize_tool_calls_chunk(template, *index); + if !synthesized && let Some(template) = template { + for (index, _) in has_tool_calls_per_choice.iter().filter(|(_, has)| **has) { + if terminated.contains(index) { + continue; } + yield Self::synthesize_tool_calls_chunk(&template, *index); } } } @@ -2231,9 +2229,9 @@ mod tests { async fn jail_synthesizes_tool_calls_finish_reason_when_stream_lacks_one() { let jail = JailedStream::builder().tool_call_parser("hermes").build(); - let chunks = vec![text_chunk("``` -{"name": "get_weather", "arguments": {"location": "SF"}} -```")]; + let chunks = vec![text_chunk( + "\n{\"name\": \"get_weather\", \"arguments\": {\"location\": \"SF\"}}\n", + )]; let input_stream = Box::pin(stream::iter(chunks)); let output_stream = jail.apply_with_finish_reason(input_stream); @@ -2286,9 +2284,12 @@ mod tests { async fn jail_synthesizes_tool_calls_before_usage_only_chunk() { let jail = JailedStream::builder().tool_call_parser("hermes").build(); - let chunks = vec![text_chunk("``` -{"name": "get_weather", "arguments": {"location": "SF"}} -```"), usage_only_chunk()]; + let chunks = vec![ + text_chunk( + "\n{\"name\": \"get_weather\", \"arguments\": {\"location\": \"SF\"}}\n", + ), + usage_only_chunk(), + ]; let input_stream = Box::pin(stream::iter(chunks)); let output_stream = jail.apply_with_finish_reason(input_stream); diff --git a/lib/llm/src/protocols/openai/chat_completions/tool_parser_v2.rs b/lib/llm/src/protocols/openai/chat_completions/tool_parser_v2.rs index 61109f008f6d..9919ab5327ee 100644 --- a/lib/llm/src/protocols/openai/chat_completions/tool_parser_v2.rs +++ b/lib/llm/src/protocols/openai/chat_completions/tool_parser_v2.rs @@ -286,7 +286,9 @@ where } // Backstop: the stream ended without a finish_reason for some choice. Flush - // each unfinished parser; emit a trailing chunk only when it yields output. + // each unfinished parser; emit a trailing chunk when the flush yields output + // or when the choice already emitted tool calls and still needs a terminal + // `ToolCalls` reason. if let Some(template) = template { for (index, state) in states.iter_mut() { if finished.contains(index) { @@ -296,8 +298,8 @@ where continue; }; let tool_calls = state.emit_chunks(result.calls); - if result.normal_text.is_empty() && tool_calls.is_none() { - continue; + if tool_calls.is_some() { + tool_emitted.insert(*index); } // A choice that produced tool calls during the stream must terminate // with `ToolCalls` even when the backend never sent a finish_reason @@ -312,6 +314,12 @@ where } else { None }; + if result.normal_text.is_empty() + && tool_calls.is_none() + && finish_reason.is_none() + { + continue; + } let mut response = template.clone(); #[allow(deprecated)] let choice = dynamo_protocols::types::ChatChoiceStream { From 9cb6f6a223970fbea97d8f2248d00eb26acccc8f Mon Sep 17 00:00:00 2001 From: Matej Kosec Date: Tue, 30 Jun 2026 08:51:38 +0200 Subject: [PATCH 05/11] test(toolcalling): expect synthesized finish chunks Update jail integration tests that omit an upstream finish_reason to assert the additional empty ToolCalls terminal chunk required by DGH-967. Signed-off-by: Matej Kosec --- lib/llm/tests/test_jail.rs | 179 ++++++++++++++++--------------------- 1 file changed, 76 insertions(+), 103 deletions(-) diff --git a/lib/llm/tests/test_jail.rs b/lib/llm/tests/test_jail.rs index 4624603463e5..695b96a3ccd7 100644 --- a/lib/llm/tests/test_jail.rs +++ b/lib/llm/tests/test_jail.rs @@ -321,6 +321,40 @@ mod tests { } } + /// Assert that a stream with tool calls but no upstream finish chunk ends + /// with the synthesized `ToolCalls` terminal chunk required by DGH-967. + pub fn assert_synthesized_tool_calls_finish( + results: &[Annotated], + expected_prefix_chunks: usize, + index: u32, + ) { + assert_eq!( + results.len(), + expected_prefix_chunks + 1, + "expected {expected_prefix_chunks} content/tool-call chunks followed by one synthesized terminal chunk" + ); + + let choice = results + .last() + .and_then(|result| result.data.as_ref()) + .and_then(|data| data.inner.choices.first()) + .expect("expected a synthesized terminal choice"); + assert_eq!(choice.index, index, "terminal choice index mismatch"); + assert_eq!( + choice.finish_reason, + Some(FinishReason::ToolCalls), + "missing synthesized ToolCalls finish reason" + ); + assert!( + choice.delta.content.is_none(), + "synthesized terminal chunk must not contain content" + ); + assert!( + choice.delta.tool_calls.is_none(), + "synthesized terminal chunk must not repeat tool calls" + ); + } + /// Helper to assert no content or tool calls (for accumulated chunks) #[allow(dead_code)] pub fn assert_empty_emission(result: &Annotated) { @@ -549,7 +583,7 @@ mod tests { async fn test_jailed_stream_early_exit() { // Tests detection of complete tool call with unjail in same chunk as the end marker // Input: "" + "[{\"name\": \"test\", " + "\"arguments\": {}}]" + "More text" - // Expected output: 2 chunks [ToolCall(), Content()] + // Expected output: 3 chunks [ToolCall(), Content(), Finish(ToolCalls)] let chunks = vec![ create_mock_response_chunk("".to_string(), 0), create_mock_response_chunk("[{\"name\": \"test\", ".to_string(), 0), @@ -565,14 +599,9 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - // Should have exactly 2 chunks: tool call + trailing content - assert_eq!( - results.len(), - 2, - "Should have tool call and trailing content" - ); + assert_synthesized_tool_calls_finish(&results, 2, 0); - // Verify exact output structure: [ToolCall(), Content()] + // Verify the non-terminal prefix: [ToolCall(), Content()] test_utils::assert_tool_call(&results[0], "test", serde_json::json!({})); test_utils::assert_content(&results[1], "More text"); @@ -640,7 +669,7 @@ mod tests { async fn test_jailed_stream_hermes_parser() { // Tests Hermes format tool call parsing with markers // Input: "I'll help you with that. " + "{\"name\": \"search_web\", \"arguments\": {\"query\": \"weather today\"}}" + " Let me search for that." - // Expected output: 3 chunks [Content(), ToolCall(), Content()] + // Expected output: 4 chunks [Content(), ToolCall(), Content(), Finish(ToolCalls)] let chunks = vec![ create_mock_response_chunk("I'll help you with that. ".to_string(), 0), create_mock_response_chunk("".to_string(), 0), @@ -660,14 +689,9 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - // Should have exactly 3 chunks: content + tool call + content - assert_eq!( - results.len(), - 3, - "Should have content, tool call, and trailing content" - ); + assert_synthesized_tool_calls_finish(&results, 3, 0); - // Verify exact output structure: [Content(), ToolCall(), Content()] + // Verify the non-terminal prefix: [Content(), ToolCall(), Content()] test_utils::assert_content(&results[0], "I'll help you with that. "); test_utils::assert_tool_call( &results[1], @@ -688,7 +712,7 @@ mod tests { async fn test_jailed_stream_mistral_parser() { // Tests Mistral format tool call parsing with [{ pattern // Input: "Sure, I can help. " + "[{\"name\": \"calculate\", \"arguments\": {\"expression\": \"2+2\"}}]" + " The calculation is done." - // Expected output: 3 chunks [Content(), ToolCall(), Content()] + // Expected output: 4 chunks [Content(), ToolCall(), Content(), Finish(ToolCalls)] let chunks = vec![ create_mock_response_chunk("Sure, I can help. ".to_string(), 0), create_mock_response_chunk("[{".to_string(), 0), @@ -705,14 +729,9 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - // Should have exactly 3 chunks: content + tool call + content - assert_eq!( - results.len(), - 3, - "Should have content, tool call, and trailing content" - ); + assert_synthesized_tool_calls_finish(&results, 3, 0); - // Verify exact output structure: [Content(), ToolCall(), Content()] + // Verify the non-terminal prefix: [Content(), ToolCall(), Content()] test_utils::assert_content(&results[0], "Sure, I can help. "); test_utils::assert_tool_call( &results[1], @@ -730,7 +749,7 @@ mod tests { async fn test_jailed_stream_mistral_parser_with_tool_calls_marker() { // Tests Mistral format tool call parsing with explicit [TOOL_CALLS] marker // Input: "Let me check that for you. " + "[TOOL_CALLS][{\"name\": \"get_time\", \"arguments\": {\"timezone\": \"UTC\"}}]" + " Here's the time." - // Expected output: 3 chunks [Content(), ToolCall(), Content()] + // Expected output: 4 chunks [Content(), ToolCall(), Content(), Finish(ToolCalls)] let chunks = vec![ create_mock_response_chunk("Let me check that for you. ".to_string(), 0), create_mock_response_chunk("[TOOL_CALLS]".to_string(), 0), @@ -746,14 +765,9 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - // Should have exactly 3 chunks: content + tool call + content - assert_eq!( - results.len(), - 3, - "Should have content, tool call, and trailing content" - ); + assert_synthesized_tool_calls_finish(&results, 3, 0); - // Verify exact output structure: [Content(), ToolCall(), Content()] + // Verify the non-terminal prefix: [Content(), ToolCall(), Content()] test_utils::assert_content(&results[0], "Let me check that for you. "); test_utils::assert_tool_call( &results[1], @@ -824,7 +838,7 @@ mod tests { async fn test_jailed_stream_phi4_parser() { // Tests Phi4 format tool call parsing with functools[ pattern // Input: "I'll analyze this data. " + "functools[{\"name\": \"analyze_data\", \"arguments\": {\"dataset\": \"sales_data\"}}]" + " Analysis complete." - // Expected output: 3 chunks [Content(), ToolCall(), Content()] + // Expected output: 4 chunks [Content(), ToolCall(), Content(), Finish(ToolCalls)] let chunks = vec![ create_mock_response_chunk("I'll analyze this data. ".to_string(), 0), create_mock_response_chunk("functools[".to_string(), 0), @@ -844,14 +858,9 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - // Should have exactly 3 chunks: content + tool call + content - assert_eq!( - results.len(), - 3, - "Should have content, tool call, and trailing content" - ); + assert_synthesized_tool_calls_finish(&results, 3, 0); - // Verify exact output structure: [Content(), ToolCall(), Content()] + // Verify the non-terminal prefix: [Content(), ToolCall(), Content()] test_utils::assert_content(&results[0], "I'll analyze this data. "); test_utils::assert_tool_call( &results[1], @@ -869,7 +878,7 @@ mod tests { async fn test_jailed_stream_llama3_json_parser() { // Tests Llama3 JSON format tool call parsing with <|python_tag|> pattern // Input: "Let me run some code. " + "<|python_tag|>{\"name\": \"execute_code\", \"arguments\": {\"code\": \"print('Hello')\"}}" + " Done executing." - // Expected output: 3 chunks [Content(), ToolCall(), Content()] + // Expected output: 4 chunks [Content(), ToolCall(), Content(), Finish(ToolCalls)] let chunks = vec![ create_mock_response_chunk("Let me run some code. ".to_string(), 0), create_mock_response_chunk("<|python_tag|>".to_string(), 0), @@ -890,14 +899,9 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - // Should have exactly 3 chunks: content + tool call + content - assert_eq!( - results.len(), - 3, - "Should have content, tool call, and trailing content" - ); + assert_synthesized_tool_calls_finish(&results, 3, 0); - // Verify exact output structure: [Content(), ToolCall(), Content()] + // Verify the non-terminal prefix: [Content(), ToolCall(), Content()] test_utils::assert_content(&results[0], "Let me run some code. "); test_utils::assert_tool_call( &results[1], @@ -1003,7 +1007,8 @@ mod tests { // end-token to actually arrive, so this only fires at stream end. // // Input: "Starting function call. " + "[{\"name\": \"incomplete_func\", \"arguments\": {" (no end marker) - // Expected output: 2 chunks [Content(), ToolCall(incomplete_func, {})] + // Expected output: 3 chunks + // [Content(), ToolCall(incomplete_func, {}), Finish(ToolCalls)] let chunks = vec![ create_mock_response_chunk("Starting function call. ".to_string(), 0), create_mock_response_chunk("".to_string(), 0), @@ -1021,11 +1026,7 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - assert_eq!( - results.len(), - 2, - "Should emit prefix content + recovered tool call" - ); + assert_synthesized_tool_calls_finish(&results, 2, 0); test_utils::assert_content(&results[0], "Starting function call. "); test_utils::assert_tool_call(&results[1], "incomplete_func", serde_json::json!({})); @@ -1073,6 +1074,7 @@ mod tests { // [2] Content(" Now let me get the time. ") // [3] ToolCall("get_time", {"timezone": "EST"}) // [4] Content(" Both tasks completed!") + // [5] Finish(ToolCalls) let chunks = vec![ create_mock_response_chunk("I'll help with multiple tasks. ".to_string(), 0), // First tool call @@ -1102,12 +1104,7 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - // === Verify chunk count === - assert_eq!( - results.len(), - 5, - "Should emit exactly 5 chunks as documented above" - ); + assert_synthesized_tool_calls_finish(&results, 5, 0); // === Verify individual chunks === assert_content(&results[0], "I'll help with multiple tasks. "); @@ -1130,7 +1127,7 @@ mod tests { async fn test_jailed_stream_tool_call_across_many_chunks() { // Tests extreme fragmentation: tool call split across 65 individual character chunks // Input: "I'll process your request. " + "[{"name": "process_data", "arguments": {}}]" + " Processing complete!" - // Expected output: 3 chunks [Content(), ToolCall(), Content()] + // Expected output: 4 chunks [Content(), ToolCall(), Content(), Finish(ToolCalls)] let chunks = vec![ create_mock_response_chunk("I'll process your request. ".to_string(), 0), create_mock_response_chunk("<".to_string(), 0), @@ -1211,14 +1208,10 @@ mod tests { // Should consolidate extreme fragmentation into 3 clean chunks // Input: "I'll process your request. " + 54-char tool call + " Processing complete!" - // Expected output: [Content(), ToolCall(), Content()] - assert_eq!( - results.len(), - 3, - "Should consolidate fragments into 3 chunks" - ); + // Expected output: [Content(), ToolCall(), Content(), Finish(ToolCalls)] + assert_synthesized_tool_calls_finish(&results, 3, 0); - // Verify exact output structure + // Verify the non-terminal prefix. test_utils::assert_content(&results[0], "I'll process your request. "); test_utils::assert_tool_call(&results[1], "process_data", serde_json::json!({})); test_utils::assert_content(&results[2], " Processing complete!"); @@ -1353,8 +1346,9 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - // Should get 2 chunks: first chunk passes through, stream end releases accumulated - assert_eq!(results.len(), 2, "Should have exactly 2 chunks"); + // The first chunk passes through, stream end releases the accumulated tool + // call, and the finish-reason postprocessor adds its terminal chunk. + assert_synthesized_tool_calls_finish(&results, 2, 0); // The second chunk is the accumulated content released when stream ended let accumulated_chunk = &results[1]; @@ -1481,6 +1475,7 @@ mod tests { // [0] Content("I'll help you. ") // [1] ToolCall("search", {}) // [2] Content("trailing text that should not be lost") + // [3] Finish(ToolCalls) let chunks = vec![ create_mock_response_chunk("I'll help you. ".to_string(), 0), create_mock_response_chunk("".to_string(), 0), @@ -1498,12 +1493,7 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - // === Verify chunk count === - assert_eq!( - results.len(), - 3, - "Should emit exactly 3 chunks as documented above" - ); + assert_synthesized_tool_calls_finish(&results, 3, 0); // === Verify individual chunks === assert_content(&results[0], "I'll help you. "); @@ -1523,7 +1513,7 @@ mod tests { async fn test_jailed_stream_early_exit_with_trailing() { // Tests early exit when complete tool call is detected in chunk that also contains trailing content // Input: "Starting task: " + "{\"name\": \"complete_task\", \"arguments\": {}}" + " Task completed successfully." - // Expected output: 3 chunks [Content(), ToolCall(), Content()] + // Expected output: 4 chunks [Content(), ToolCall(), Content(), Finish(ToolCalls)] let chunks = vec![ create_mock_response_chunk("Starting task: ".to_string(), 0), create_mock_response_chunk( @@ -1540,14 +1530,9 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - // Should have exactly 3 chunks: content + tool call + trailing - assert_eq!( - results.len(), - 3, - "Should have content, tool call, and trailing content" - ); + assert_synthesized_tool_calls_finish(&results, 3, 0); - // Verify exact output structure: [Content(), ToolCall(), Content()] + // Verify the non-terminal prefix: [Content(), ToolCall(), Content()] test_utils::assert_content(&results[0], "Starting task: "); test_utils::assert_tool_call(&results[1], "complete_task", serde_json::json!({})); test_utils::assert_content(&results[2], " Task completed successfully."); @@ -1896,6 +1881,7 @@ mod tests { // Expected output: // [0] Content("text") // " = jail.apply_with_finish_reason(input_stream).collect().await; - // === Verify chunk count === - assert_eq!( - results.len(), - 2, - "Should emit exactly 2 chunks: [0] 'text' content, [1] tool call" - ); + assert_synthesized_tool_calls_finish(&results, 2, 0); // === Verify individual chunks === assert_content(&results[0], "text"); @@ -2220,7 +2201,7 @@ mod tests { // "I'll call a function. " // + "San Franciscocelsius" // + " Done." - // Expected output: 3 chunks [Content(), ToolCall(), Content()] + // Expected output: 4 chunks [Content(), ToolCall(), Content(), Finish(ToolCalls)] let chunks = vec![ create_mock_response_chunk("I'll call a function. ".to_string(), 0), create_mock_response_chunk("".to_string(), 0), @@ -2243,13 +2224,9 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - assert_eq!( - results.len(), - 3, - "Should have content, tool call, and trailing content" - ); + assert_synthesized_tool_calls_finish(&results, 3, 0); - // Verify exact output structure: [Content(), ToolCall(), Content()]. + // Verify the non-terminal prefix: [Content(), ToolCall(), Content()]. test_utils::assert_content(&results[0], "I'll call a function. "); test_utils::assert_tool_call( &results[1], @@ -2298,7 +2275,7 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - assert_eq!(results.len(), 3, "Should have 3 chunks"); + assert_synthesized_tool_calls_finish(&results, 3, 0); test_utils::assert_content(&results[0], "Let me search for that. "); test_utils::assert_tool_call( @@ -2338,11 +2315,7 @@ mod tests { let results: Vec<_> = jail.apply_with_finish_reason(input_stream).collect().await; - assert_eq!( - results.len(), - 3, - "Should have content, tool call, and trailing content" - ); + assert_synthesized_tool_calls_finish(&results, 3, 0); test_utils::assert_content(&results[0], "Before tool call. "); test_utils::assert_tool_call( From b1db135ee78eeed407479ae3163e9b4416e77c9c Mon Sep 17 00:00:00 2001 From: Matej Kosec Date: Tue, 30 Jun 2026 09:02:25 +0200 Subject: [PATCH 06/11] fix(toolcalling): finalize terminal stream handling Clear usage and LLM metrics copied from usage-only templates, stop replay fixtures before decoding [DONE], and describe the regression without internal ticket references. Signed-off-by: Matej Kosec --- .../protocols/openai/chat_completions/jail.rs | 62 ++++++++++++++----- .../openai/chat_completions/tool_parser_v2.rs | 22 +++---- lib/llm/tests/common/http_harness.rs | 1 + lib/llm/tests/test_jail.rs | 2 +- 4 files changed, 59 insertions(+), 28 deletions(-) diff --git a/lib/llm/src/protocols/openai/chat_completions/jail.rs b/lib/llm/src/protocols/openai/chat_completions/jail.rs index 057957f9d7ba..a06a15e3ef72 100644 --- a/lib/llm/src/protocols/openai/chat_completions/jail.rs +++ b/lib/llm/src/protocols/openai/chat_completions/jail.rs @@ -1455,13 +1455,17 @@ impl JailedStream { /// choice that emitted tool calls but never received a finish_reason from the /// engine. Used by `fix_finish_reason` to satisfy the OpenAI stream ordering /// requirement (the terminal chunk must carry a non-null `finish_reason`) - /// when the stream ends without one — DGH-967. The delta is empty; only the + /// when the stream ends without one. The delta is empty; only the /// `finish_reason` carries information. fn synthesize_tool_calls_chunk( template: &NvCreateChatCompletionStreamResponse, index: u32, ) -> Annotated { let mut response = template.clone(); + // A terminal finish chunk must not repeat accounting data copied from a + // usage-only template. + response.inner.usage = None; + response.llm_metrics = None; #[allow(deprecated)] let choice = dynamo_protocols::types::ChatChoiceStream { index, @@ -1590,8 +1594,8 @@ impl JailedStream { // never emitted a usage chunk). Emit one trailing `ToolCalls` chunk per // tool-call choice that never received a finish_reason. Strict OpenAI // clients wait for a non-null finish_reason before considering a tool call - // complete; without this they hang until their client-side timeout - // (DGH-967). Choices that never emitted tool calls are left alone — there + // complete; without this they hang until their client-side timeout. + // Choices that never emitted tool calls are left alone — there // is no signal to invent a finish_reason from for text-only output. if !synthesized && let Some(template) = template { for (index, _) in has_tool_calls_per_choice.iter().filter(|(_, has)| **has) { @@ -1891,7 +1895,21 @@ mod tests { system_fingerprint: None, }, nvext: None, - llm_metrics: None, + llm_metrics: Some(crate::protocols::common::metrics::LLMMetricAnnotation { + input_tokens: 10, + output_tokens: 5, + chunk_tokens: 0, + cached_tokens: None, + prefill_worker_id: None, + prefill_dp_rank: None, + prefill_worker_type: None, + decode_worker_id: None, + decode_dp_rank: None, + decode_worker_type: None, + tokenize_latency: None, + detokenize_total_latency: None, + detokenize_count: None, + }), }), id: None, event: None, @@ -2206,7 +2224,7 @@ mod tests { } /// The last `finish_reason` a client sees across the stream. `None` if the - /// stream never carried one (the DGH-967 hang condition). + /// stream never carried one (the missing-finish-reason hang condition). fn final_finish_reason( responses: &[Annotated], ) -> Option { @@ -2218,12 +2236,12 @@ mod tests { .next_back() } - // DGH-967: when the engine emits a complete tool call but the stream ends - // WITHOUT any finish_reason chunk (speculative decoding folded EOS into - // content, or the terminal signal was dropped), a strict OpenAI client - // waits for a non-null finish_reason and hangs until its timeout. The jail - // path's finalize() emits the tool call with the (absent) upstream - // finish_reason; fix_finish_reason's end-of-stream backstop must synthesize + // Missing-finish-reason regression: when the engine emits a complete tool call + // but the stream ends without any finish_reason chunk (speculative decoding + // folded EOS into content, or the terminal signal was dropped), a strict + // OpenAI client waits for a non-null finish_reason and hangs until its timeout. + // The jail path's finalize() emits the tool call with the absent upstream + // finish_reason; fix_finish_reason's end-of-stream path must synthesize // `ToolCalls` so the client gets a terminal signal. #[tokio::test] async fn jail_synthesizes_tool_calls_finish_reason_when_stream_lacks_one() { @@ -2250,8 +2268,8 @@ mod tests { ); } - // DGH-967 corollary: a text-only stream that ends without a finish_reason - // must NOT get a synthesized one — there is no signal to invent a + // Text-only corollary: a text-only stream that ends without a finish_reason + // must not get a synthesized one. There is no signal to invent a // finish_reason from when no tool call was emitted. #[tokio::test] async fn jail_does_not_synthesize_finish_reason_for_text_only_stream() { @@ -2275,11 +2293,11 @@ mod tests { ); } - // DGH-967 ordering: the ticket's repro is a tool call followed by a - // usage-only chunk, with NO finish_reason chunk from the engine. The + // Usage-ordering regression: a tool call is followed by a usage-only chunk, + // with no finish_reason chunk from the engine. The // synthesized `ToolCalls` terminal chunk must be emitted *before* the // usage-only chunk (OpenAI stream ordering — the terminal finish_reason - // precedes usage). This mirrors the production repro in the ticket. + // precedes usage). This mirrors the production stream ordering. #[tokio::test] async fn jail_synthesizes_tool_calls_before_usage_only_chunk() { let jail = JailedStream::builder().tool_call_parser("hermes").build(); @@ -2327,5 +2345,17 @@ mod tests { finish_pos < usage_pos, "ToolCalls chunk at {finish_pos} must precede the usage chunk at {usage_pos}" ); + let finish_data = responses[finish_pos] + .data + .as_ref() + .expect("ToolCalls chunk has no response data"); + assert!( + finish_data.inner.usage.is_none(), + "synthesized ToolCalls chunk must not repeat usage data" + ); + assert!( + finish_data.llm_metrics.is_none(), + "synthesized ToolCalls chunk must not repeat LLM metrics" + ); } } diff --git a/lib/llm/src/protocols/openai/chat_completions/tool_parser_v2.rs b/lib/llm/src/protocols/openai/chat_completions/tool_parser_v2.rs index 9919ab5327ee..0984ad3b5118 100644 --- a/lib/llm/src/protocols/openai/chat_completions/tool_parser_v2.rs +++ b/lib/llm/src/protocols/openai/chat_completions/tool_parser_v2.rs @@ -307,7 +307,7 @@ where // engine dropped the terminal signal). Strict OpenAI clients wait for // a non-null finish_reason before considering a tool call complete; // emitting `None` here left them hanging until their client-side - // timeout (DGH-967). Text-only output without an upstream finish + // timeout. Text-only output without an upstream finish // reason stays `None` — we have no signal to invent one from. let finish_reason = if tool_emitted.contains(index) { Some(FinishReason::ToolCalls) @@ -557,12 +557,12 @@ mod tests { ); } - // DGH-967: the stream emits a complete tool call but ends WITHOUT any - // finish_reason chunk (e.g. speculative decoding folded EOS into content, or - // the engine dropped the terminal signal). A strict OpenAI client waits for a - // non-null finish_reason before considering the tool call complete; the - // end-of-stream backstop must synthesize `ToolCalls` so the client doesn't - // hang until its timeout. + // Missing-finish-reason regression: the stream emits a complete tool call but + // ends without any finish_reason chunk (e.g. speculative decoding folded EOS + // into content, or the engine dropped the terminal signal). A strict OpenAI + // client waits for a non-null finish_reason before considering the tool call + // complete; the end-of-stream backstop must synthesize `ToolCalls` so the + // client doesn't hang until its timeout. #[tokio::test] async fn qwen3_bypass_synthesizes_tool_calls_when_stream_lacks_finish_reason() { // Same call as the incremental test, but the final chunk carries NO @@ -591,10 +591,10 @@ mod tests { ); } - // DGH-967 corollary: when the stream ends without a finish_reason AND no tool - // call was emitted (text-only output), the backstop must NOT invent a - // finish_reason — there is no signal to synthesize one from. A trailing - // content chunk may be emitted, but its finish_reason stays None. + // Text-only corollary: when the stream ends without a finish_reason and no + // tool call was emitted, the backstop must not invent a finish_reason. There + // is no signal to synthesize one from. A trailing content chunk may be + // emitted, but its finish_reason stays None. #[tokio::test] async fn qwen3_bypass_does_not_synthesize_finish_reason_for_text_only_stream() { let chunks = vec![chunk("hello world", false), chunk("", false)]; diff --git a/lib/llm/tests/common/http_harness.rs b/lib/llm/tests/common/http_harness.rs index 9c187b616d49..76f3eb2f4a5c 100644 --- a/lib/llm/tests/common/http_harness.rs +++ b/lib/llm/tests/common/http_harness.rs @@ -139,6 +139,7 @@ pub async fn load_sse_fixture(path: impl AsRef) -> Result