mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(routing): pin specialised sub-agent hints to their managed backend tier (#4081)
This commit is contained in:
@@ -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<dyn Provider> = 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"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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:<tier>` 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<dyn Provider>, 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
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user