diff --git a/src/openhuman/agent/harness/subagent_runner/ops_tests.rs b/src/openhuman/agent/harness/subagent_runner/ops_tests.rs index 49683926f..9674cb595 100644 --- a/src/openhuman/agent/harness/subagent_runner/ops_tests.rs +++ b/src/openhuman/agent/harness/subagent_runner/ops_tests.rs @@ -1283,16 +1283,21 @@ fn resolve_subagent_provider_hint_with_config_routes_via_factory() { // We don't assert the *resulting* provider identity here (the // factory may return a fresh OpenHuman backend or whatever // primary_cloud resolves to), but we DO assert the resolved model - // matches the workload's configured exact id — not the legacy - // `{workload}-v1` synthesis. + // is the workload's canonical managed tier — NOT `default_model`, + // and NOT the parent's model. + // + // Regression (#hint-routing): the managed backend used to ignore the + // workload role and return `default_model`, so `hint = "agentic"` + // silently ran on whatever `default_model` was (here `chat-v1`). + // `make_openhuman_backend` now pins specialised roles to their tier, + // so `agentic` resolves to `agentic-v1` regardless of `default_model`. use crate::openhuman::config::Config; let mut config = Config::default(); - // Route `agentic` to OpenHuman backend explicitly. The backend returns - // the configured default_model. Use `coding-v1` — a recognized tier - // that the factory validation accepts and that differs from the old - // `agentic-v1` synthesis, making the assertion meaningful. + // Route `agentic` to the OpenHuman backend explicitly, and set a + // distinct `default_model` so the assertion proves the role — not the + // global default — drives the resolved tier. config.agentic_provider = Some("openhuman".to_string()); - config.default_model = Some("coding-v1".to_string()); + config.default_model = Some("chat-v1".to_string()); let parent: Arc = ScriptedProvider::new(vec![]); let (_resolved_provider, resolved_model) = super::resolve_subagent_provider( @@ -1305,9 +1310,9 @@ fn resolve_subagent_provider_hint_with_config_routes_via_factory() { None, ); assert_eq!( - resolved_model, "coding-v1", - "Hint must use the factory-resolved exact model, not synthesise `agentic-v1` \ - and not fall back to parent's model" + resolved_model, "agentic-v1", + "Hint must resolve to the workload's managed tier (agentic-v1), not \ + fall back to default_model (chat-v1) or the parent's model" ); } diff --git a/src/openhuman/inference/provider/factory.rs b/src/openhuman/inference/provider/factory.rs index 80dcc9ae2..ea718bbd4 100644 --- a/src/openhuman/inference/provider/factory.rs +++ b/src/openhuman/inference/provider/factory.rs @@ -815,23 +815,68 @@ pub(crate) fn create_local_chat_provider_from_string( // ── Internal helpers ────────────────────────────────────────────────────────── +/// Canonical managed-backend tier for a specialised workload role. +/// +/// The managed backend otherwise derives its model from `config.default_model` +/// (which defaults to the `chat-v1` tier), so a tier-specific workload whose +/// per-workload provider is unset would silently inherit the global default — +/// e.g. the `code_executor` sub-agent (`hint = "coding"`) would run on `chat-v1` +/// instead of the dedicated `coding-v1` tier, defeating the whole point of the +/// hint. The `hint:` translation in [`make_openhuman_backend`] only fires +/// when the *model string itself* is `hint:coding`; here the model originates +/// from `default_model`, so the workload role is the only signal left and must +/// be mapped explicitly. +/// +/// Returns `Some(tier)` for the specialised roles that map 1:1 to a managed +/// tier (`reasoning`, `agentic`, `coding`, `vision`). Returns `None` for: +/// +/// - the generic `chat` role (and any background/unknown role), which keeps +/// inheriting `default_model`: the front-line chat turn and legacy +/// `default_model = "reasoning-v1"` installs deliberately fall through to the +/// `chat` role (see the session builder) and rely on `default_model` driving +/// the model — pinning `chat` here would regress them. +/// - `summarization`, which is intentionally NOT pinned: the memory subsystem +/// ([`crate::openhuman::memory::chat::build_chat_runtime`]) routes the +/// summarization model through `routed.default_model`, sourced from the +/// user-configurable `memory_tree.cloud_llm_model`. Pinning `summarization` +/// to a fixed tier would silently ignore that override. +/// +/// For `vision` the default-inheritance mismatch is not just suboptimal but +/// fatal: an unset `vision_provider` would resolve to `chat-v1`, +/// `model_supports_vision` would report `false`, and the turn engine would strip +/// every attached image — leaving the managed vision sub-agent blind. +fn managed_tier_for_role(role: &str) -> Option<&'static str> { + use crate::openhuman::config::{ + MODEL_AGENTIC_V1, MODEL_CODING_V1, MODEL_REASONING_V1, MODEL_VISION_V1, + }; + match role { + "reasoning" => Some(MODEL_REASONING_V1), + "agentic" => Some(MODEL_AGENTIC_V1), + "coding" => Some(MODEL_CODING_V1), + "vision" => Some(MODEL_VISION_V1), + _ => None, + } +} + /// Build the OpenHuman backend provider (session-JWT auth). /// -/// `role` is the workload name (e.g. `"chat"`, `"vision"`). The managed backend -/// otherwise derives its model from `config.default_model` (which defaults to the -/// non-vision `chat-v1` tier), so a tier-specific workload whose per-workload -/// provider is unset would silently inherit the global default. For the `vision` -/// workload that mismatch is fatal: an unset `vision_provider` would resolve to -/// `chat-v1`, `model_supports_vision` would report `false`, and the turn engine -/// would strip every attached image — leaving the managed vision sub-agent blind. -/// Pin `vision` to the dedicated multimodal `vision-v1` tier so the managed -/// default path keeps working without requiring the user to set `vision_provider`. +/// `role` is the workload name (e.g. `"chat"`, `"coding"`, `"vision"`). A +/// specialised workload role is pinned to its canonical managed tier via +/// [`managed_tier_for_role`] so the `hint = "..."` a sub-agent declares actually +/// reaches the matching backend tier instead of collapsing to `default_model`. +/// The generic `chat` role (and background roles) keep inheriting +/// `config.default_model`. fn make_openhuman_backend( role: &str, config: &Config, ) -> anyhow::Result<(Box, String)> { - let model = if role == "vision" { - crate::openhuman::config::MODEL_VISION_V1.to_string() + let model = if let Some(tier) = managed_tier_for_role(role) { + log::debug!( + "[providers][chat-factory] role={} pinned to managed tier model={}", + role, + tier + ); + tier.to_string() } else { config .default_model diff --git a/src/openhuman/inference/provider/factory_tests.rs b/src/openhuman/inference/provider/factory_tests.rs index e85b90d81..043191ce7 100644 --- a/src/openhuman/inference/provider/factory_tests.rs +++ b/src/openhuman/inference/provider/factory_tests.rs @@ -390,6 +390,119 @@ fn create_chat_provider_uses_role() { assert_eq!(model, "gpt-4o-mini"); } +// Regression (#hint-routing): on the managed OpenHuman backend, a specialised +// workload role must resolve to its dedicated tier — NOT collapse to +// `default_model` (which defaults to `chat-v1`). Before the fix, +// `make_openhuman_backend` only special-cased `vision`, so `hint = "coding"` +// sub-agents (code_executor, skill_creator, tool_maker) silently ran on +// `chat-v1` instead of `coding-v1`, and likewise for `agentic`/`reasoning`. +// (`summarization` is intentionally excluded — see +// `managed_backend_summarization_role_inherits_default_model`.) This drives +// `make_openhuman_backend` directly via the explicit `"openhuman"` provider +// string. +#[test] +fn managed_backend_pins_specialised_role_to_tier() { + use crate::openhuman::config::{ + MODEL_AGENTIC_V1, MODEL_CODING_V1, MODEL_REASONING_V1, MODEL_VISION_V1, + }; + // default_model is chat-v1 — the value the buggy path would have leaked. + let config = Config::default(); + assert_eq!(config.default_model.as_deref(), Some("chat-v1")); + + for (role, expected_tier) in &[ + ("reasoning", MODEL_REASONING_V1), + ("agentic", MODEL_AGENTIC_V1), + ("coding", MODEL_CODING_V1), + ("vision", MODEL_VISION_V1), + ] { + let (_, model) = create_chat_provider_from_string(role, "openhuman", &config) + .expect("managed backend must build"); + assert_eq!( + model, *expected_tier, + "role={role} must pin to {expected_tier} on the managed backend, got {model}" + ); + } +} + +// `summarization` is deliberately NOT pinned: the memory subsystem +// (`memory::chat::build_chat_runtime`) routes the summarization model through +// `default_model` (sourced from the user-configurable +// `memory_tree.cloud_llm_model`), so it must keep inheriting `default_model`. +#[test] +fn managed_backend_summarization_role_inherits_default_model() { + let mut config = Config::default(); + config.default_model = Some("summarization-v1".to_string()); + let (_, model) = create_chat_provider_from_string("summarization", "openhuman", &config) + .expect("managed backend must build"); + assert_eq!(model, "summarization-v1"); + + // A non-tier custom override is not pinned away — it follows the existing + // known-tier validation (unknown → platform default reasoning-v1). + config.default_model = Some("custom-summary-model".to_string()); + let (_, model) = create_chat_provider_from_string("summarization", "openhuman", &config) + .expect("managed backend must build"); + assert_eq!(model, "reasoning-v1"); +} + +// End-to-end of the sub-agent path: the subagent runner resolves a +// `ModelSpec::Hint(workload)` by calling `create_chat_provider(workload, cfg)`. +// With a default config (every per-workload provider unset → managed backend), +// each shipped hint must still reach its tier. This is the exact call the +// `code_executor` agent (`hint = "coding"`) makes when it spawns. +#[test] +fn subagent_hint_resolves_to_tier_on_managed_backend() { + use crate::openhuman::config::{MODEL_AGENTIC_V1, MODEL_CODING_V1, MODEL_REASONING_V1}; + let config = Config::default(); + for (hint, expected_tier) in &[ + ("coding", MODEL_CODING_V1), + ("agentic", MODEL_AGENTIC_V1), + ("reasoning", MODEL_REASONING_V1), + ] { + let (_, model) = + create_chat_provider(hint, &config).expect("create_chat_provider must succeed"); + assert_eq!( + model, *expected_tier, + "hint={hint} sub-agent must run on {expected_tier}, got {model}" + ); + } +} + +// The generic `chat` role must keep inheriting `default_model` — the front-line +// chat turn and legacy `default_model = "reasoning-v1"` installs deliberately +// fall through to the `chat` role (see the session builder), so pinning `chat` +// would regress them. +#[test] +fn managed_backend_chat_role_inherits_default_model() { + // Default (chat-v1). + let config = Config::default(); + let (_, model) = create_chat_provider_from_string("chat", "openhuman", &config) + .expect("managed backend must build"); + assert_eq!(model, "chat-v1"); + + // Legacy literal: a user pinned `default_model = "reasoning-v1"` must still + // get reasoning-v1 for the chat-role front-line turn. + let mut config = Config::default(); + config.default_model = Some("reasoning-v1".to_string()); + let (_, model) = create_chat_provider_from_string("chat", "openhuman", &config) + .expect("managed backend must build"); + assert_eq!(model, "reasoning-v1"); +} + +// The tier pin only governs the *managed* backend. A user who routes the coding +// workload to their own BYOK provider must still get that provider's model — +// `provider_for_role` resolves the per-workload route before the managed path. +#[test] +fn coding_workload_byok_route_wins_over_managed_pin() { + let mut config = Config::default(); + config + .cloud_providers + .push(anthropic_entry("p_ant", "anthropic")); + config.coding_provider = Some("anthropic:claude-sonnet-4-6".to_string()); + let (_, model) = + create_chat_provider("coding", &config).expect("create_chat_provider must succeed"); + assert_eq!(model, "claude-sonnet-4-6"); +} + #[test] fn unknown_slug_rejected() { let config = Config::default();