From 16ae6964949d6f26c772dd352d487fbb7ba7ff5e Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Thu, 9 Jul 2026 08:23:21 -0700 Subject: [PATCH 1/3] refactor(flows): migrate agent-node completion to crate ChatModel (Motion B) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4727 straggler: tinyflows/caps.rs llm.complete now drives a crate ChatModel (create_chat_model_with_model_id + a create_chat_model_pinned rebuild for the node raw/BYOK override, #4598) via ModelRequest/invoke, instead of create_chat_provider + provider.chat. Adds two reusable seam converters — chat_messages_to_model_messages (host history -> crate messages) and model_response_to_chat_response (crate response -> host {text, tool_calls, usage, reasoning_content}) — so the flows node's JSON output envelope is byte-identical. Core lib green; convert/factory/caps test modules clean. Claude-Session: https://claude.ai/code/session_018MnMVgnzxchtcs1DDUxTfF --- src/openhuman/inference/provider/factory.rs | 16 +++++++ src/openhuman/tinyagents/convert.rs | 27 +++++++++++ src/openhuman/tinyagents/mod.rs | 3 ++ src/openhuman/tinyflows/caps.rs | 50 ++++++++++++++------- 4 files changed, 79 insertions(+), 17 deletions(-) diff --git a/src/openhuman/inference/provider/factory.rs b/src/openhuman/inference/provider/factory.rs index 15f2d67ff6..b1016b5712 100644 --- a/src/openhuman/inference/provider/factory.rs +++ b/src/openhuman/inference/provider/factory.rs @@ -978,6 +978,22 @@ pub(crate) fn chat_model_from_provider( ) } +/// Build an `Arc` for `role`, pinned to an explicit `model` id +/// rather than the role's default (issue #4249, Motion B). +/// +/// Used by completion callers (the flows `agent` node) that resolve the role's +/// provider but pin a node-specified raw/BYOK model id verbatim (issue #4598). +/// The provider is the role's provider; only the baked model differs. +pub fn create_chat_model_pinned( + role: &str, + config: &Config, + model: &str, + temperature: f64, +) -> anyhow::Result>> { + let (provider, _default_model) = create_chat_provider(role, config)?; + Ok(chat_model_from_provider(provider, model.to_string(), temperature)) +} + /// Build a local-runtime provider without applying the custom-provider session gate. /// /// Used by setup/probe flows that need to validate an endpoint before the diff --git a/src/openhuman/tinyagents/convert.rs b/src/openhuman/tinyagents/convert.rs index ff16365dce..48d31aeb9e 100644 --- a/src/openhuman/tinyagents/convert.rs +++ b/src/openhuman/tinyagents/convert.rs @@ -417,6 +417,33 @@ pub(super) fn ta_call_to_oh_call( } } +/// Convert openhuman history into harness messages for a one-shot `ChatModel` +/// request. Exposed (issue #4249, Motion B) for callers outside the seam that +/// build a crate model request from openhuman [`ChatMessage`]s (e.g. the flows +/// `agent` node). +pub(crate) fn chat_messages_to_model_messages( + history: &[ChatMessage], +) -> Vec { + history_to_messages(history) +} + +/// Convert a harness [`ModelResponse`](tinyagents::harness::model::ModelResponse) +/// back into an openhuman [`ChatResponse`] — text + tool calls + reasoning + +/// usage — for one-shot callers that consume the full response envelope (e.g. the +/// flows `agent` node round-tripping into its JSON output). Issue #4249, Motion B. +pub(crate) fn model_response_to_chat_response( + response: &tinyagents::harness::model::ModelResponse, +) -> crate::openhuman::inference::provider::ChatResponse { + let assistant = &response.message; + let text = response.text(); + crate::openhuman::inference::provider::ChatResponse { + text: (!text.is_empty()).then_some(text), + tool_calls: assistant.tool_calls.iter().map(ta_call_to_oh_call).collect(), + usage: super::model::usage_info_from_response(response), + reasoning_content: reasoning_from_content(&assistant.content), + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/openhuman/tinyagents/mod.rs b/src/openhuman/tinyagents/mod.rs index 2afd5da8c6..ee6e9e8e44 100644 --- a/src/openhuman/tinyagents/mod.rs +++ b/src/openhuman/tinyagents/mod.rs @@ -21,6 +21,9 @@ mod abort_guard; mod convert; +// One-shot response/message converters for callers outside the seam that drive a +// crate `ChatModel` directly (issue #4249, Motion B — e.g. the flows agent node). +pub(crate) use convert::{chat_messages_to_model_messages, model_response_to_chat_response}; pub(crate) mod delegation; mod embeddings; pub(crate) mod journal; diff --git a/src/openhuman/tinyflows/caps.rs b/src/openhuman/tinyflows/caps.rs index 30877158b0..fc288a3a36 100644 --- a/src/openhuman/tinyflows/caps.rs +++ b/src/openhuman/tinyflows/caps.rs @@ -31,8 +31,7 @@ use crate::openhuman::config::{Config, HttpRequestConfig}; use crate::openhuman::credentials::{HttpCredential, HttpCredentialsStore}; use crate::openhuman::flows; use crate::openhuman::inference::provider::{ - create_chat_provider, is_raw_passthrough_model, role_for_model_tier, ChatMessage, ChatRequest, - UsageInfo, + is_raw_passthrough_model, role_for_model_tier, ChatMessage, UsageInfo, }; use crate::openhuman::sandbox::{execute_in_sandbox, resolve_sandbox_policy}; use crate::openhuman::security::{ @@ -516,25 +515,42 @@ impl LlmProvider for OpenHumanLlm { "[flows] llm.complete: dispatching agent-node completion" ); - let (provider, model) = create_chat_provider(role, &self.config) + // Build the completion model on the crate `ChatModel` interface (issue + // #4249, Motion B — replaces the raw `provider.chat`). `create_chat_model_*` + // resolves the role's provider + default model; if the node pinned a + // raw/BYOK id, rebuild the model pinned to it verbatim (issue #4598). + let (chat, factory_model) = + crate::openhuman::inference::provider::create_chat_model_with_model_id( + role, + &self.config, + temperature, + ) .map_err(|e| EngineError::Capability(e.to_string()))?; - // `create_chat_provider` handed back the role's default model. If the node - // pinned a raw/BYOK id, forward it verbatim instead (issue #4598). - let model = resolve_completion_model(node_model, model); - - let response = provider - .chat( - ChatRequest { - messages: &messages, - tools: None, - stream: None, - max_tokens, - }, + let model = resolve_completion_model(node_model, factory_model.clone()); + let chat = if model == factory_model { + chat + } else { + crate::openhuman::inference::provider::factory::create_chat_model_pinned( + role, + &self.config, &model, temperature, ) - .await - .map_err(|e| EngineError::Capability(e.to_string()))?; + .map_err(|e| EngineError::Capability(e.to_string()))? + }; + + let mut model_request = tinyagents::harness::model::ModelRequest::new( + crate::openhuman::tinyagents::chat_messages_to_model_messages(&messages), + ); + if let Some(cap) = max_tokens { + model_request = model_request.with_max_tokens(cap); + } + let response = crate::openhuman::tinyagents::model_response_to_chat_response( + &chat + .invoke(&(), model_request) + .await + .map_err(|e| EngineError::Capability(e.to_string()))?, + ); // Structured mode: surface the parsed object itself so downstream // `=item.` / `=nodes..item.` bindings work. The From 880d8335522f9bb640acc0e00edaec8072d5c9d1 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 14 Jul 2026 02:12:13 +0000 Subject: [PATCH 2/3] refactor(inference): drop superseded flow compatibility helpers --- src/openhuman/inference/provider/factory.rs | 16 ------------ src/openhuman/tinyagents/convert.rs | 27 --------------------- src/openhuman/tinyagents/mod.rs | 3 --- 3 files changed, 46 deletions(-) diff --git a/src/openhuman/inference/provider/factory.rs b/src/openhuman/inference/provider/factory.rs index 62140fac60..8152c0a29c 100644 --- a/src/openhuman/inference/provider/factory.rs +++ b/src/openhuman/inference/provider/factory.rs @@ -1069,22 +1069,6 @@ pub(crate) fn chat_model_from_provider( ) } -/// Build an `Arc` for `role`, pinned to an explicit `model` id -/// rather than the role's default (issue #4249, Motion B). -/// -/// Used by completion callers (the flows `agent` node) that resolve the role's -/// provider but pin a node-specified raw/BYOK model id verbatim (issue #4598). -/// The provider is the role's provider; only the baked model differs. -pub fn create_chat_model_pinned( - role: &str, - config: &Config, - model: &str, - temperature: f64, -) -> anyhow::Result>> { - let (provider, _default_model) = create_chat_provider(role, config)?; - Ok(chat_model_from_provider(provider, model.to_string(), temperature)) -} - /// Build a local-runtime provider without applying the custom-provider session gate. /// /// Used by setup/probe flows that need to validate an endpoint before the diff --git a/src/openhuman/tinyagents/convert.rs b/src/openhuman/tinyagents/convert.rs index a0d4186040..29ccfe53d0 100644 --- a/src/openhuman/tinyagents/convert.rs +++ b/src/openhuman/tinyagents/convert.rs @@ -418,33 +418,6 @@ pub(crate) fn ta_call_to_oh_call( } } -/// Convert openhuman history into harness messages for a one-shot `ChatModel` -/// request. Exposed (issue #4249, Motion B) for callers outside the seam that -/// build a crate model request from openhuman [`ChatMessage`]s (e.g. the flows -/// `agent` node). -pub(crate) fn chat_messages_to_model_messages( - history: &[ChatMessage], -) -> Vec { - history_to_messages(history) -} - -/// Convert a harness [`ModelResponse`](tinyagents::harness::model::ModelResponse) -/// back into an openhuman [`ChatResponse`] — text + tool calls + reasoning + -/// usage — for one-shot callers that consume the full response envelope (e.g. the -/// flows `agent` node round-tripping into its JSON output). Issue #4249, Motion B. -pub(crate) fn model_response_to_chat_response( - response: &tinyagents::harness::model::ModelResponse, -) -> crate::openhuman::inference::provider::ChatResponse { - let assistant = &response.message; - let text = response.text(); - crate::openhuman::inference::provider::ChatResponse { - text: (!text.is_empty()).then_some(text), - tool_calls: assistant.tool_calls.iter().map(ta_call_to_oh_call).collect(), - usage: super::model::usage_info_from_response(response), - reasoning_content: reasoning_from_content(&assistant.content), - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/openhuman/tinyagents/mod.rs b/src/openhuman/tinyagents/mod.rs index 993eb324e9..ec50571a01 100644 --- a/src/openhuman/tinyagents/mod.rs +++ b/src/openhuman/tinyagents/mod.rs @@ -21,9 +21,6 @@ mod abort_guard; mod convert; -// One-shot response/message converters for callers outside the seam that drive a -// crate `ChatModel` directly (issue #4249, Motion B — e.g. the flows agent node). -pub(crate) use convert::{chat_messages_to_model_messages, model_response_to_chat_response}; pub(crate) mod delegation; mod embeddings; pub(crate) mod journal; From dd2dee76467bd21a6d4bdba9b085b8352d833a8b Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Tue, 14 Jul 2026 02:12:13 +0000 Subject: [PATCH 3/3] fix(flows): preserve literal tool markup without tools (addresses @chatgpt-codex-connector on src/openhuman/tinyflows/caps.rs:550) --- src/openhuman/tinyagents/model.rs | 75 ++++++++++++++++++++++++------- 1 file changed, 58 insertions(+), 17 deletions(-) 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)]