diff --git a/src/openhuman/tinyagents/model.rs b/src/openhuman/tinyagents/model.rs index 56b037f6f4..dc8807c505 100644 --- a/src/openhuman/tinyagents/model.rs +++ b/src/openhuman/tinyagents/model.rs @@ -61,11 +61,11 @@ fn build_chat_inputs( /// /// The text-mode fallback parse needs each tool's positional parameter layout /// to reconstruct named JSON arguments from a P-Format `name[a|b]` body. The -/// harness always populates `request.tools` (schemas are rendered into the -/// prompt for prompt-guided providers, or advertised natively otherwise), so -/// the registry is available in both modes. An empty registry (no tools -/// advertised) makes the P-Format-aware parser short-circuit to the canonical -/// grammar, so this is behaviour-neutral when there are no tools. +/// harness populates `request.tools` when tools are available (schemas are +/// rendered into the prompt for prompt-guided providers, or advertised natively +/// otherwise), so the registry is available in both modes. Tool-less requests +/// skip fallback parsing entirely; this empty registry is therefore consulted +/// only alongside a non-empty advertised tool list. fn pformat_registry_from_request( request: &ModelRequest, ) -> crate::openhuman::agent::pformat::PFormatRegistry { @@ -84,22 +84,25 @@ fn pformat_registry_from_request( /// Translate an openhuman [`ChatResponse`] into a harness [`ModelResponse`] /// (visible text + tool calls + token usage). /// -/// Native `tool_calls` take precedence; when absent, the response text is parsed -/// for prompt-guided (`…` / p-format) calls — matching the legacy -/// dispatcher — so text-mode models drive the tinyagents loop too. The visible -/// text is the prose with any tool-call markup stripped. +/// Native `tool_calls` take precedence; when absent and the request advertised +/// tools, the response text is parsed for prompt-guided (`…` / +/// p-format) calls — matching the legacy dispatcher — so text-mode models drive +/// the tinyagents loop too. Tool-less requests preserve response text verbatim, +/// including literal tool-call examples. When parsing is enabled, visible text +/// is the prose with any parsed tool-call markup stripped. /// /// `pformat_registry` carries the advertised tools' positional layouts so the /// text-mode fallback can recover P-Format (`name[a|b]`) calls that ~10 builtin /// prompts still teach — the migrated parse path had dropped that grammar and /// silently lost those calls (issue #4465). It is empty for the native-tool -/// path (where `response.tool_calls` is used directly) and for tool-less turns. +/// path, where `response.tool_calls` is used directly. /// /// Unknown-tool recovery is handled by `RunPolicy::unknown_tool`, so the model /// adapter preserves the provider-requested tool name. fn response_to_model_response( response: &ChatResponse, pformat_registry: &crate::openhuman::agent::pformat::PFormatRegistry, + parse_text_tool_calls: bool, ) -> ModelResponse { let (visible_text, tool_calls): (String, Vec) = if !response.tool_calls.is_empty() { let calls = response @@ -113,7 +116,8 @@ fn response_to_model_response( }) .collect(); (response.text.clone().unwrap_or_default(), calls) - } else if let Some(text) = response.text.as_deref() { + } else if parse_text_tool_calls { + let text = response.text.as_deref().unwrap_or_default(); let (prose, parsed) = crate::openhuman::agent::harness::parse_tool_calls_with_pformat(text, pformat_registry); if parsed.is_empty() { @@ -134,7 +138,7 @@ fn response_to_model_response( (prose, calls) } } else { - (String::new(), Vec::new()) + (response.text.clone().unwrap_or_default(), Vec::new()) }; let mut content = Vec::new(); @@ -632,7 +636,11 @@ impl ChatModel<()> for ProviderModel { // Provider usage (charged USD / context window / cache-creation-reasoning) // now reaches the event bridge via `UsageCarryMiddleware`, which reads it // off the returned `ModelResponse` (G1) — the adapter no longer carries it. - Ok(response_to_model_response(&response, &pformat_registry)) + Ok(response_to_model_response( + &response, + &pformat_registry, + !request.tools.is_empty(), + )) } /// Stream the model response, forwarding openhuman's `ProviderDelta` events @@ -650,6 +658,7 @@ impl ChatModel<()> for ProviderModel { // Positional layouts for the text-mode P-Format fallback (issue #4465); // built here so it can move into the `'static` producer task below. let pformat_registry = pformat_registry_from_request(&request); + let parse_text_tool_calls = !request.tools.is_empty(); let provider = self.provider.clone(); let model = self.model.clone(); // Per-request temperature when set (see `invoke`), else the pinned value; @@ -749,7 +758,11 @@ impl ChatModel<()> for ProviderModel { // Provider usage rides the `Completed` response's crate `Usage` // + raw (G1); `UsageCarryMiddleware` reads it off the folded // response for the bridge, so the adapter no longer pushes here. - ModelStreamItem::Completed(response_to_model_response(&resp, &pformat_registry)) + ModelStreamItem::Completed(response_to_model_response( + &resp, + &pformat_registry, + parse_text_tool_calls, + )) } Err(e) => { // Streaming failures ride `ModelStreamItem::Failed(String)`, which @@ -827,7 +840,7 @@ mod g1_usage_tests { }), reasoning_content: None, }; - let model_response = response_to_model_response(&chat, &empty_registry()); + let model_response = response_to_model_response(&chat, &empty_registry(), false); // Crate Usage carries every token breakdown natively. let usage = model_response.usage.expect("usage present"); @@ -860,7 +873,7 @@ mod g1_usage_tests { }), reasoning_content: None, }; - let model_response = response_to_model_response(&chat, &empty_registry()); + let model_response = response_to_model_response(&chat, &empty_registry(), false); assert!( model_response.raw.is_none(), "no charged USD / window ⇒ raw stays None" @@ -879,9 +892,37 @@ mod g1_usage_tests { usage: None, reasoning_content: None, }; - let model_response = response_to_model_response(&chat, &empty_registry()); + let model_response = response_to_model_response(&chat, &empty_registry(), false); assert!(usage_info_from_response(&model_response).is_none()); } + + #[test] + fn tool_less_response_preserves_literal_tool_call_markup() { + let text = r#"Example: {"name":"lookup","arguments":{}}"#; + let chat = ChatResponse { + text: Some(text.to_string()), + ..Default::default() + }; + + let response = response_to_model_response(&chat, &empty_registry(), false); + + assert_eq!(response.text(), text); + assert!(response.message.tool_calls.is_empty()); + } + + #[test] + fn tool_enabled_response_still_extracts_tool_call_markup() { + let chat = ChatResponse { + text: Some(r#"{"name":"lookup","arguments":{}}"#.to_string()), + ..Default::default() + }; + + let response = response_to_model_response(&chat, &empty_registry(), true); + + assert_eq!(response.text(), ""); + assert_eq!(response.message.tool_calls.len(), 1); + assert_eq!(response.message.tool_calls[0].name, "lookup"); + } } #[cfg(test)]