diff --git a/src/openhuman/inference/provider/factory.rs b/src/openhuman/inference/provider/factory.rs index abf7f99fe..cd57c8bb2 100644 --- a/src/openhuman/inference/provider/factory.rs +++ b/src/openhuman/inference/provider/factory.rs @@ -258,6 +258,22 @@ pub(crate) fn is_known_openhuman_tier(model: &str) -> bool { ) } +/// Return whether `model` is a raw BYOK/custom model id that must be forwarded +/// **verbatim** to provider construction rather than mapped onto a managed tier. +/// +/// A raw passthrough id is any **non-empty** string that is neither a `hint:*` +/// alias nor a known managed tier ([`is_known_openhuman_tier`]) — i.e. the model +/// ids a user pins directly on an agent/node (e.g. `"claude-opus-4"`). The +/// OpenHuman backend preserves such ids verbatim +/// ([`super::openhuman_backend`]'s `resolve_model`) and is authoritative over +/// their validity, so the core must **not** silently collapse them onto +/// `reasoning-v1` (issue #4598). Managed tiers and every `hint:*` string return +/// `false` so their existing resolution is untouched. +pub(crate) fn is_raw_passthrough_model(model: &str) -> bool { + let trimmed = model.trim(); + !trimmed.is_empty() && !trimmed.starts_with("hint:") && !is_known_openhuman_tier(trimmed) +} + /// Per-tier vision (image-input) capability for the managed OpenHuman backend. /// /// The remote managed backend (`api.tinyhumans.ai`) does not advertise per-tier @@ -1213,17 +1229,28 @@ fn make_openhuman_backend( model } None => { + // `model` is guaranteed non-empty here: an empty/whitespace + // `default_model` was already normalised to `reasoning-v1` above, and + // the managed-tier / summarization branches yield non-empty tier + // constants. So a non-`hint:` id is either a known canonical tier or a + // raw/BYOK id the user pinned — both forward verbatim; only the log + // line differs. if is_known_openhuman_tier(&model) { model } else { - log::warn!( - "[providers][chat-factory] model '{}' is not a recognized OpenHuman \ - backend tier (valid: reasoning-v1, chat-v1, agentic-v1, burst-v1, coding-v1, \ - reasoning-quick-v1, summarization-v1, vision-v1); falling back to '{}'", - model, - crate::openhuman::config::MODEL_REASONING_V1, + // Unrecognised NON-empty model id — a raw/BYOK model the user + // pinned (e.g. `claude-opus-4`, written into `default_model` or + // a per-agent model pin). Forward it verbatim so the selected + // model actually reaches provider construction instead of the + // core silently collapsing it onto `reasoning-v1`. The managed + // backend is authoritative over validity and returns a clear + // error for a genuinely bad id (issue #4598). + log::debug!( + "[providers][chat-factory] forwarding raw/BYOK model '{}' verbatim to the \ + OpenHuman backend (not a managed tier); the backend validates it", + model ); - crate::openhuman::config::MODEL_REASONING_V1.to_string() + model } } }; diff --git a/src/openhuman/inference/provider/factory_tests.rs b/src/openhuman/inference/provider/factory_tests.rs index e388e430e..c8477737e 100644 --- a/src/openhuman/inference/provider/factory_tests.rs +++ b/src/openhuman/inference/provider/factory_tests.rs @@ -1047,6 +1047,57 @@ fn invalid_models_fail() { assert!(!is_known_openhuman_tier("hint:")); } +// ── is_raw_passthrough_model ───────────────────────────────────────────────── +// Raw/BYOK model ids (non-empty, non-tier, non-`hint:*`) must be recognized as +// passthrough so they reach provider construction verbatim (issue #4598). +#[test] +fn raw_passthrough_model_detects_byok_ids() { + assert!(is_raw_passthrough_model("claude-opus-4")); + assert!(is_raw_passthrough_model("deepseek-v4-pro")); + assert!(is_raw_passthrough_model("gpt-4o")); + assert!(is_raw_passthrough_model("reasoning-v2")); + // Surrounding whitespace is trimmed before the classification. + assert!(is_raw_passthrough_model(" claude-opus-4 ")); +} + +#[test] +fn raw_passthrough_model_excludes_tiers_hints_and_empty() { + // Managed tiers are resolved, never forwarded raw. + assert!(!is_raw_passthrough_model("reasoning-v1")); + assert!(!is_raw_passthrough_model("chat-v1")); + assert!(!is_raw_passthrough_model("summarization-v1")); + // Every `hint:*` alias (known or not) stays on the hint-resolution path. + assert!(!is_raw_passthrough_model("hint:reasoning")); + assert!(!is_raw_passthrough_model("hint:garbage")); + // Empty / whitespace-only ids are not passthrough — they fall back to default. + assert!(!is_raw_passthrough_model("")); + assert!(!is_raw_passthrough_model(" ")); +} + +// End-to-end through the public factory string entry: a raw BYOK model pinned in +// `default_model` reaches provider construction verbatim on the managed backend, +// while a known tier and a `hint:*` alias keep their existing resolution. +#[test] +fn managed_backend_passthrough_via_create_chat_provider_from_string() { + let mut config = Config::default(); + config.default_model = Some("claude-opus-4".to_string()); + let (_, model) = create_chat_provider_from_string("chat", "openhuman", &config) + .expect("managed backend must build"); + assert_eq!(model, "claude-opus-4"); + + // Managed tier resolution is unchanged. + config.default_model = Some("chat-v1".to_string()); + let (_, model) = create_chat_provider_from_string("chat", "openhuman", &config) + .expect("managed backend must build"); + assert_eq!(model, "chat-v1"); + + // `hint:*` resolution is unchanged. + config.default_model = Some("hint:reasoning".to_string()); + let (_, model) = create_chat_provider_from_string("chat", "openhuman", &config) + .expect("managed backend must build"); + assert_eq!(model, crate::openhuman::config::MODEL_REASONING_V1); +} + // ── oh_tier_supports_vision ────────────────────────────────────────────────────── #[test] @@ -1192,17 +1243,24 @@ fn make_openhuman_backend_reports_vision_capability() { } #[test] -fn make_openhuman_backend_falls_back_for_invalid_model() { - // An invalid default_model must not be forwarded to the backend. - // The factory must silently fall back to reasoning-v1 (the platform default). +fn make_openhuman_backend_forwards_raw_byok_model_verbatim() { + // Issue #4598: a raw/BYOK model id pinned into `default_model` (e.g. a user + // selecting "claude-opus-4" for an agent) must reach provider construction + // verbatim rather than the core silently collapsing it onto reasoning-v1. + // The managed backend preserves non-empty ids and is authoritative over + // their validity. let mut config = Config::default(); - config.default_model = Some("deepseek-v4-pro".to_string()); + config.default_model = Some("claude-opus-4".to_string()); let (_, model) = make_openhuman_backend("chat", &config).expect("factory should succeed"); assert_eq!( - model, - crate::openhuman::config::MODEL_REASONING_V1, - "invalid default_model should fall back to MODEL_REASONING_V1" + model, "claude-opus-4", + "a raw/BYOK default_model must be forwarded verbatim, not collapsed" ); + + // Another stale/custom id shape from the wild — still forwarded verbatim. + config.default_model = Some("deepseek-v4-pro".to_string()); + let (_, model) = make_openhuman_backend("chat", &config).expect("factory should succeed"); + assert_eq!(model, "deepseek-v4-pro"); } #[test] diff --git a/src/openhuman/inference/provider/mod.rs b/src/openhuman/inference/provider/mod.rs index faa3e0f3a..39f30d426 100644 --- a/src/openhuman/inference/provider/mod.rs +++ b/src/openhuman/inference/provider/mod.rs @@ -46,6 +46,7 @@ pub use error_code::{ BackendErrorCode, }; pub(crate) use factory::chat_model_from_provider; +pub(crate) use factory::is_raw_passthrough_model; pub use factory::{ create_chat_model, create_chat_model_from_string, create_chat_model_with_model_id, create_chat_provider, provider_for_role, role_for_model_tier, BYOK_INCOMPLETE_SENTINEL, diff --git a/src/openhuman/tinyflows/caps.rs b/src/openhuman/tinyflows/caps.rs index d5e70bd9d..d975f2a85 100644 --- a/src/openhuman/tinyflows/caps.rs +++ b/src/openhuman/tinyflows/caps.rs @@ -31,7 +31,8 @@ use crate::openhuman::config::{Config, HttpRequestConfig}; use crate::openhuman::credentials::{HttpCredential, HttpCredentialsStore}; use crate::openhuman::flows; use crate::openhuman::inference::provider::{ - create_chat_provider, role_for_model_tier, ChatMessage, ChatRequest, UsageInfo, + create_chat_provider, is_raw_passthrough_model, role_for_model_tier, ChatMessage, ChatRequest, + UsageInfo, }; use crate::openhuman::sandbox::{execute_in_sandbox, resolve_sandbox_policy}; use crate::openhuman::security::{ @@ -412,6 +413,28 @@ pub(crate) fn parse_llm_json(text: &str) -> Option { matches!(parsed, Value::Object(_) | Value::Array(_)).then_some(parsed) } +/// Select the model an `agent` node completion actually runs on. +/// +/// `resolved_model` is what [`create_chat_provider`] returned for the node's +/// mapped workload role. A node may instead pin a **raw/BYOK** model id +/// (e.g. `claude-opus-4`) that [`role_for_model_tier`] collapsed to the `chat` +/// role — in that case the pinned id, not the role default, is the model the +/// user selected, so it is forwarded verbatim (issue #4598). Managed tiers and +/// every `hint:*` alias fall through to `resolved_model` unchanged. +fn resolve_completion_model(node_model: Option<&str>, resolved_model: String) -> String { + match node_model { + Some(pinned) if is_raw_passthrough_model(pinned) => { + tracing::debug!( + target: "flows", + raw_model = pinned, + "[flows] llm.complete: forwarding raw/BYOK node model verbatim (not a managed tier)" + ); + pinned.to_string() + } + _ => resolved_model, + } +} + /// [`LlmProvider`] adapter over OpenHuman's inference stack /// (`src/openhuman/inference/provider/`). /// @@ -495,6 +518,9 @@ impl LlmProvider for OpenHumanLlm { let (provider, model) = create_chat_provider(role, &self.config) .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( @@ -675,11 +701,19 @@ pub(crate) fn resolve_node_model(request: &Value, entry_model: Option<&str>) -> /// to the workload serving that tier. The session builder's `provider_role_for` /// only routes the `hint:` form to a specialised workload, so a bare tier /// name (`reasoning-v1`) must be normalised to `hint:reasoning` here — otherwise -/// it would silently fall through to the chat workload. Mirrors the per-node -/// routing [`OpenHumanLlm::complete`] applies via -/// [`role_for_model_tier`](crate::openhuman::inference::provider::role_for_model_tier); -/// an unrecognised string maps to the chat workload, same as there. +/// it would silently fall through to the chat workload. +/// +/// A **raw/BYOK** model id (e.g. `claude-opus-4`) is instead forwarded verbatim: +/// wrapping it in `hint:chat` would collapse the user's explicit per-node model +/// onto the managed `chat-v1` tier (issue #4598). Left verbatim, it flows through +/// the session builder's generic `chat` role — which inherits +/// `config.default_model` — to `make_openhuman_backend`, which forwards non-tier +/// ids to the backend unchanged. Mirrors the per-node routing +/// [`OpenHumanLlm::complete`] applies via [`resolve_completion_model`]. pub(crate) fn harness_model_default_override(node_model: &str) -> String { + if is_raw_passthrough_model(node_model) { + return node_model.to_string(); + } format!("hint:{}", role_for_model_tier(node_model)) } @@ -4129,4 +4163,46 @@ mod tests { "an action with no published output schema must be None, not Some(vec![])" ); } + + // ── resolve_completion_model raw/BYOK passthrough (issue #4598) ─────────── + #[test] + fn resolve_completion_model_forwards_raw_byok_node_model_verbatim() { + // A raw/BYOK id maps to the `chat` role, so the role resolves to the + // default model — but the pinned id is what the user selected and must + // be the model the completion runs on. + assert_eq!( + resolve_completion_model(Some("claude-opus-4"), "chat-v1".to_string()), + "claude-opus-4" + ); + assert_eq!( + resolve_completion_model(Some("deepseek-v4-pro"), "chat-v1".to_string()), + "deepseek-v4-pro" + ); + } + + #[test] + fn resolve_completion_model_leaves_managed_tier_and_hint_node_models_untouched() { + // Managed tiers and every `hint:*` alias keep the role-resolved model. + assert_eq!( + resolve_completion_model(Some("chat-v1"), "chat-v1".to_string()), + "chat-v1" + ); + assert_eq!( + resolve_completion_model(Some("hint:reasoning"), "reasoning-v1".to_string()), + "reasoning-v1" + ); + assert_eq!( + resolve_completion_model(Some("hint:garbage"), "reasoning-v1".to_string()), + "reasoning-v1" + ); + // No pinned model, or a whitespace-only pin, keeps the resolved default. + assert_eq!( + resolve_completion_model(None, "chat-v1".to_string()), + "chat-v1" + ); + assert_eq!( + resolve_completion_model(Some(" "), "chat-v1".to_string()), + "chat-v1" + ); + } } diff --git a/src/openhuman/tinyflows/tests.rs b/src/openhuman/tinyflows/tests.rs index 285aae2e1..6b7b4f5b2 100644 --- a/src/openhuman/tinyflows/tests.rs +++ b/src/openhuman/tinyflows/tests.rs @@ -604,8 +604,25 @@ fn harness_model_default_override_normalises_tiers_to_hint_roles() { harness_model_default_override("hint:reasoning"), "hint:reasoning" ); - // Unrecognised strings map to the chat workload (matches OpenHumanLlm). - assert_eq!(harness_model_default_override("openai:gpt-4o"), "hint:chat"); +} + +#[test] +fn harness_model_default_override_forwards_raw_byok_models_verbatim() { + // Raw/BYOK ids a user pins on an agent node are forwarded verbatim (issue + // #4598) — normalising them to `hint:chat` would collapse the explicit + // per-node model onto the managed chat tier. They reach the harness `chat` + // role, which inherits `config.default_model`, and `make_openhuman_backend` + // forwards the non-tier id to the backend unchanged. + assert_eq!( + harness_model_default_override("claude-opus-4"), + "claude-opus-4" + ); + assert_eq!( + harness_model_default_override("openai:gpt-4o"), + "openai:gpt-4o" + ); + // Empty / whitespace is not a raw id — falls back to the chat workload. + assert_eq!(harness_model_default_override(" "), "hint:chat"); } #[test]