diff --git a/src/openhuman/accessibility/automate.rs b/src/openhuman/accessibility/automate.rs index b7a16a938..ce20f2b3c 100644 --- a/src/openhuman/accessibility/automate.rs +++ b/src/openhuman/accessibility/automate.rs @@ -657,13 +657,17 @@ impl AutomateBackend for RealBackend { } async fn decide(&self, system: &str, user: &str) -> Result { - // Fast tier: the `memory` role maps to `memory_provider` — a cheap, - // quick model class. A dedicated `automation` provider knob is a - // follow-up (see plan §5); routing through `memory` keeps M1 free of - // Config-schema churn while still keeping the chat model out of the loop. - let (provider, model) = - crate::openhuman::inference::provider::create_chat_provider("memory", &self.config) - .map_err(|e| format!("fast-model provider unavailable: {e}"))?; + // Fast tier: the canonical `summarization` hint maps to `memory_provider` + // (the Settings "Memory" routing row) — a cheap, quick model class, and on + // the managed backend the dedicated `summarization-v1` tier. A dedicated + // `automation` provider knob is a follow-up (see plan §5); routing through + // `summarization` keeps M1 free of Config-schema churn while still keeping + // the chat model out of the loop. + let (provider, model) = crate::openhuman::inference::provider::create_chat_provider( + "summarization", + &self.config, + ) + .map_err(|e| format!("fast-model provider unavailable: {e}"))?; provider .chat_with_system(Some(system), user, &model, 0.0) .await diff --git a/src/openhuman/agent/harness/subagent_runner/extract_tool.rs b/src/openhuman/agent/harness/subagent_runner/extract_tool.rs index 65dbf94b4..e7f799895 100644 --- a/src/openhuman/agent/harness/subagent_runner/extract_tool.rs +++ b/src/openhuman/agent/harness/subagent_runner/extract_tool.rs @@ -6,9 +6,13 @@ //! That dragged along system-prompt scaffolding, a tool-loop, and an //! extra inference round for a workload that really only needs one //! completion call. So the tool now drives `provider.chat_with_system` -//! directly against the extraction model (`"summarization-v1"` — same -//! string [`super::definition::ModelSpec::Hint("summarization").resolve`] -//! would have produced, so router entries keyed on it still apply). +//! directly. Both the provider AND the model id are resolved by the runner +//! through the `summarization` role (`create_chat_provider("summarization")`) +//! and handed in, so this extraction follows the user's `memory_provider` +//! routing — managed (`summarization-v1`), BYOK, or local — exactly like every +//! other summarization path, instead of borrowing the parent agent's provider +//! with a hardcoded tier string (which 400'd on BYOK/local providers that don't +//! know the literal `summarization-v1`). //! //! Transcript discipline: the LLM call still costs tokens, so every //! extraction round-trip is persisted as its own `session_raw/` JSONL (+ @@ -33,27 +37,24 @@ use crate::openhuman::tools::{Tool, ToolCategory, ToolResult}; // ── Tunables ────────────────────────────────────────────────────────── -/// Model id used for `extract_from_result` LLM calls. Mirrors the -/// resolution `ModelSpec::Hint("summarization").resolve(...)` would have -/// produced for the retired summarizer sub-agent so routing table -/// entries that targeted the summarizer continue to apply. -const EXTRACT_MODEL_ID: &str = "summarization-v1"; - /// Temperature for extraction calls. Low but non-zero so the model can /// pick reasonable phrasings when rewriting identifiers into a compact /// answer, without straying into creative territory. const EXTRACT_TEMPERATURE: f64 = 0.2; -/// Char budget per extraction call, derived from the extraction model's -/// context window. A payload at or under this budget is extracted in a single -/// shot over its **entire** content — higher quality than the chunk+concat -/// fallback, which has no reduce stage and can miss facts that span a chunk -/// boundary or need global context. `summarization-v1` resolves to a -/// long-context flash model (~1M tokens), so this is large; only payloads that -/// exceed it fall back to parallel chunked extraction. Headroom is reserved for +/// Convert a context window (tokens) into the per-chunk char budget. A payload +/// at or under this budget is extracted in a single shot over its **entire** +/// content — higher quality than the chunk+concat fallback, which has no reduce +/// stage and can miss facts that span a chunk boundary. Headroom is reserved for /// the extraction contract, the query, and the response. -fn extract_chunk_char_budget() -> usize { - /// Fallback window (tokens) when the model id is unknown to the registry. +/// +/// `window_tokens = None` means neither the provider nor the static registry +/// could size the model — only reached for **cloud** models the registry doesn't +/// know (an unknown *local* model resolves to its small provider-profile window +/// via [`ExtractFromResultTool::extract_chunk_char_budget`], not here), so a +/// large window is a safe assumption. +fn chunk_char_budget_for_window(window_tokens: Option) -> usize { + /// Last-resort window (tokens) when the model is unsizable — see above. const FALLBACK_WINDOW_TOKENS: u64 = 128_000; /// Approximate chars per token used for budgeting. const CHARS_PER_TOKEN: u64 = 4; @@ -61,9 +62,8 @@ fn extract_chunk_char_budget() -> usize { /// the prompt scaffolding, query, and model response. const USABLE_PCT: u64 = 70; - let window_tokens = crate::openhuman::inference::context_window_for_model(EXTRACT_MODEL_ID) - .unwrap_or(FALLBACK_WINDOW_TOKENS); - (window_tokens * USABLE_PCT / 100 * CHARS_PER_TOKEN) as usize + let window = window_tokens.unwrap_or(FALLBACK_WINDOW_TOKENS); + (window * USABLE_PCT / 100 * CHARS_PER_TOKEN) as usize } /// System prompt fed to the provider on every `extract_from_result` @@ -86,6 +86,11 @@ empty string — do not invent information."; pub(super) struct ExtractFromResultTool { cache: Arc, provider: Arc, + /// Model id for the extraction `chat_with_system` calls. Resolved by the + /// runner through the `summarization` role (alongside `provider`), so it + /// tracks the user's `memory_provider` routing + `cloud_llm_model` override + /// instead of a hardcoded tier string. + model: String, /// Workspace root for transcript writes. workspace_dir: PathBuf, /// Parent session chain joined with `__`, e.g. @@ -104,6 +109,7 @@ impl ExtractFromResultTool { pub(super) fn new( cache: Arc, provider: Arc, + model: String, workspace_dir: PathBuf, parent_chain: String, owner_agent_id: String, @@ -111,6 +117,7 @@ impl ExtractFromResultTool { Self { cache, provider, + model, workspace_dir, parent_chain, owner_agent_id, @@ -118,6 +125,25 @@ impl ExtractFromResultTool { } } + /// Resolve the per-chunk char budget for `self.model` against the chosen + /// provider's context window. + /// + /// Asks the **provider** first: a local runtime (Ollama / LM Studio) reports + /// its real loaded / profile window here (~8k tokens for Ollama), so an + /// unknown *local* model is budgeted against its actual small context and the + /// payload is chunked — instead of assuming a 128k window and sending an + /// oversized single-shot prompt that overflows the local context (Codex P2). + /// Falls back to the static registry, then the cloud-safe default in + /// [`chunk_char_budget_for_window`]. + async fn extract_chunk_char_budget(&self) -> usize { + let window = self + .provider + .effective_context_window(&self.model) + .await + .or_else(|| crate::openhuman::inference::context_window_for_model(&self.model)); + chunk_char_budget_for_window(window) + } + fn next_call_seq(&self) -> u64 { let mut guard = self .call_seq @@ -187,10 +213,13 @@ impl Tool for ExtractFromResultTool { // Allow test harnesses to lower the chunk budget so multi-chunk // extraction can be exercised on compacted payloads. Never consulted // in production (env var absent). - let effective_chunk_budget = std::env::var("OPENHUMAN_TEST_EXTRACT_CHUNK_BUDGET") + let effective_chunk_budget = match std::env::var("OPENHUMAN_TEST_EXTRACT_CHUNK_BUDGET") .ok() .and_then(|v| v.parse::().ok()) - .unwrap_or_else(extract_chunk_char_budget); + { + Some(budget) => budget, + None => self.extract_chunk_char_budget().await, + }; // Fast path: payload fits in a single provider turn. if cached.content.len() <= effective_chunk_budget { @@ -244,6 +273,7 @@ impl Tool for ExtractFromResultTool { let workspace_dir = self.workspace_dir.clone(); let parent_chain = self.parent_chain.clone(); let owner_agent_id = self.owner_agent_id.clone(); + let model = self.model.clone(); // Consume `chunks` with `into_iter` so each async block owns // its `String` — `buffer_unordered` polls the stream lazily @@ -255,6 +285,7 @@ impl Tool for ExtractFromResultTool { let workspace_dir = workspace_dir.clone(); let parent_chain = parent_chain.clone(); let owner_agent_id = owner_agent_id.clone(); + let model = model.clone(); async move { let user_prompt = format!( "Tool name: {tool_name}\nChunk {idx} of {total}\n\n\ @@ -271,7 +302,7 @@ impl Tool for ExtractFromResultTool { .chat_with_system( Some(EXTRACT_SYSTEM_PROMPT), &user_prompt, - EXTRACT_MODEL_ID, + &model, EXTRACT_TEMPERATURE, ) .await; @@ -296,7 +327,7 @@ impl Tool for ExtractFromResultTool { Ok(s) => Ok(*s), Err(s) => Err(s.as_str()), }, - EXTRACT_MODEL_ID, + &model, ); (i, result) @@ -369,7 +400,7 @@ impl ExtractFromResultTool { .chat_with_system( Some(EXTRACT_SYSTEM_PROMPT), &user_prompt, - EXTRACT_MODEL_ID, + &self.model, EXTRACT_TEMPERATURE, ) .await; @@ -392,7 +423,7 @@ impl ExtractFromResultTool { Ok(s) => Ok(*s), Err(s) => Err(s.as_str()), }, - EXTRACT_MODEL_ID, + &self.model, ); match provider_result { @@ -526,3 +557,46 @@ fn write_extract_transcript( ); } } + +#[cfg(test)] +mod tests { + use super::*; + + // The chunk budget tracks the resolved context window, so a small local + // window yields a much smaller budget than a long-context cloud tier — this + // is what forces chunking instead of an oversized single-shot prompt. + #[test] + fn chunk_budget_tracks_context_window() { + let summarization_window = + crate::openhuman::inference::context_window_for_model("summarization-v1"); + let big = chunk_char_budget_for_window(summarization_window); + let small = chunk_char_budget_for_window(Some(8_192)); // Ollama local default + assert!( + big > small, + "long-context tier budget {big} must exceed an 8k local window budget {small}" + ); + } + + // Codex P2: an unknown LOCAL model resolves (via the provider) to its small + // ~8k profile window, NOT the 128k cloud fallback. The resulting budget must + // be well under a production handoff payload (~200k chars) so it chunks + // instead of single-shotting into a local context overflow. + #[test] + fn chunk_budget_for_small_local_window_forces_chunking() { + let budget = chunk_char_budget_for_window(Some(8_192)); + // 8192 * 70% * 4 = 22_937 chars. + assert_eq!(budget, (8_192u64 * 70 / 100 * 4) as usize); + assert!( + budget < 200_000, + "an 8k local window must budget below a typical handoff payload so it chunks" + ); + } + + // When neither provider nor registry can size the model (cloud-unknown), the + // cloud-safe 128k fallback applies. + #[test] + fn chunk_budget_uses_cloud_fallback_when_unsizable() { + let expected = (128_000u64 * 70 / 100 * 4) as usize; + assert_eq!(chunk_char_budget_for_window(None), expected); + } +} diff --git a/src/openhuman/agent/harness/subagent_runner/ops/runner.rs b/src/openhuman/agent/harness/subagent_runner/ops/runner.rs index 8404830ff..e72c3df57 100644 --- a/src/openhuman/agent/harness/subagent_runner/ops/runner.rs +++ b/src/openhuman/agent/harness/subagent_runner/ops/runner.rs @@ -473,9 +473,65 @@ async fn run_typed_mode( Some(prefix) => format!("{}__{}", prefix, parent.session_key), None => parent.session_key.clone(), }; + // Resolve the extraction provider + model through the `summarization` + // role so extraction follows the user's `memory_provider` routing. + // + // When summarization routes to the **managed** backend, the parent + // provider already speaks the managed tier names, so we reuse it with the + // fixed `summarization-v1` model — no redundant provider build, and (with + // no live backend) no network dependency. Only when summarization routes + // to a **concrete BYOK/local** provider — exactly where passing the + // parent agent's (agentic) provider the literal `summarization-v1` would + // 400/404 — do we build the dedicated summarization provider so the call + // lands on the right endpoint + model. + // + // A local parent never reuses (its runtime would 404 on the managed tier + // string): it falls through to building the managed summarization + // provider. Any config/factory glitch degrades to parent + the fixed tier + // id rather than dead-ending extraction. + let summarization_tier = + crate::openhuman::inference::provider::factory::summarization_tier_model().to_string(); + let (extract_provider, extract_model): ( + Arc, + String, + ) = match crate::openhuman::config::Config::load_or_init().await { + Ok(cfg) => { + let route = + crate::openhuman::inference::provider::provider_for_role("summarization", &cfg); + let r = route.trim(); + let route_is_managed = r.is_empty() || r == "cloud" || r == "openhuman"; + if route_is_managed && !parent.provider.is_local_provider() { + (parent.provider.clone(), summarization_tier.clone()) + } else { + match crate::openhuman::inference::provider::create_chat_provider( + "summarization", + &cfg, + ) { + Ok((p, m)) => (Arc::from(p), m), + Err(e) => { + tracing::warn!( + agent_id = %definition.id, + error = %e, + "[subagent_runner:typed] extract summarization provider build failed; falling back to parent provider" + ); + (parent.provider.clone(), summarization_tier.clone()) + } + } + } + } + Err(e) => { + tracing::warn!( + agent_id = %definition.id, + error = %e, + "[subagent_runner:typed] config load failed for extract provider; falling back to parent provider + summarization-v1" + ); + (parent.provider.clone(), summarization_tier.clone()) + } + }; dynamic_tools.push(Box::new(ExtractFromResultTool::new( cache.clone(), - parent.provider.clone(), + extract_provider, + extract_model, parent.workspace_dir.clone(), parent_chain, definition.id.clone(), diff --git a/src/openhuman/config/schema/storage_memory.rs b/src/openhuman/config/schema/storage_memory.rs index 7f9974e11..7d12f68d5 100644 --- a/src/openhuman/config/schema/storage_memory.rs +++ b/src/openhuman/config/schema/storage_memory.rs @@ -331,11 +331,18 @@ pub struct MemoryTreeConfig { #[serde(default = "default_llm_backend")] pub llm_backend: LlmBackend, - /// Model identifier used when `llm_backend = "cloud"`. Routed through the - /// OpenHuman backend's chat-completions surface. + /// **Deprecated / inert.** Formerly the model identifier for managed + /// (`llm_backend = "cloud"`) summarization. The managed summarization tier is + /// now fixed at `summarization-v1` + /// ([`crate::openhuman::inference::provider::factory::summarization_tier_model`]) + /// and this field is no longer consumed — the hosted backend serves exactly + /// one tier for this workload. Kept for config back-compat (existing + /// `config.toml` / `OPENHUMAN_MEMORY_TREE_CLOUD_LLM_MODEL` still parse without + /// error). To run summarization on a different model, point `memory_provider` + /// at a BYOK/local provider instead, where the model rides in the provider + /// string. /// /// Defaults to [`DEFAULT_CLOUD_LLM_MODEL`] (`summarization-v1`). - /// Env override: `OPENHUMAN_MEMORY_TREE_CLOUD_LLM_MODEL`. #[serde(default = "default_cloud_llm_model")] pub cloud_llm_model: Option, diff --git a/src/openhuman/inference/provider/factory.rs b/src/openhuman/inference/provider/factory.rs index b9ba47e1e..188e7ac2f 100644 --- a/src/openhuman/inference/provider/factory.rs +++ b/src/openhuman/inference/provider/factory.rs @@ -77,11 +77,8 @@ pub(crate) const NO_MODEL_CONFIGURED_ANCHOR: &str = "resolved to an empty model fn is_abstract_tier_model(model: &str) -> bool { use crate::openhuman::config::{ MODEL_AGENTIC_V1, MODEL_CHAT_V1, MODEL_CODING_V1, MODEL_REASONING_QUICK_V1, - MODEL_REASONING_V1, MODEL_VISION_V1, + MODEL_REASONING_V1, MODEL_SUMMARIZATION_V1, MODEL_VISION_V1, }; - // No dedicated constant for the summarization tier yet; keep the literal - // in sync with the tier name used by the summarizer sub-agent. - const MODEL_SUMMARIZATION_V1: &str = "summarization-v1"; let trimmed = model.trim(); trimmed == MODEL_REASONING_V1 || trimmed == MODEL_REASONING_QUICK_V1 @@ -112,7 +109,10 @@ pub fn resolve_model_for_hint(hint_or_tier: &str, config: &Config) -> String { ("agentic", crate::openhuman::config::MODEL_AGENTIC_V1), ("coding", crate::openhuman::config::MODEL_CODING_V1), ("vision", crate::openhuman::config::MODEL_VISION_V1), - ("summarization", "summarization-v1"), + ( + "summarization", + crate::openhuman::config::MODEL_SUMMARIZATION_V1, + ), // Background subconscious workload rides the lightweight chat tier on the // managed backend; its `subconscious` *role* (handled below) still selects // the provider via `subconscious_provider`. @@ -125,7 +125,10 @@ pub fn resolve_model_for_hint(hint_or_tier: &str, config: &Config) -> String { (crate::openhuman::config::MODEL_AGENTIC_V1, "agentic"), (crate::openhuman::config::MODEL_CODING_V1, "coding"), (crate::openhuman::config::MODEL_VISION_V1, "vision"), - ("summarization-v1", "summarization"), + ( + crate::openhuman::config::MODEL_SUMMARIZATION_V1, + "summarization", + ), ]; let (tier, role) = if let Some(hint_key) = hint_or_tier.strip_prefix("hint:") { @@ -846,11 +849,11 @@ pub(crate) fn create_local_chat_provider_from_string( /// `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. +/// - `summarization` / `memory`, which are pinned in a dedicated branch of +/// [`make_openhuman_backend`] via [`summarization_tier_model`] (fixed at +/// `summarization-v1`) rather than here, only so the `memory` alias and the +/// role string share one resolution site. They do **not** fall through to +/// `default_model`. /// /// `subconscious` IS pinned (to the lightweight `chat-v1` tier) even though it /// is a background workload: the cloud subconscious tick builds via the session @@ -881,14 +884,34 @@ fn managed_tier_for_role(role: &str) -> Option<&'static str> { } } +/// The **managed-backend** summarization tier model — fixed at +/// [`MODEL_SUMMARIZATION_V1`] (`summarization-v1`). +/// +/// Read **only** on the managed OpenHuman path (inside [`make_openhuman_backend`]), +/// so it is consumed iff the `summarization`/`memory` role actually resolves to +/// the managed backend — BYOK and local routes carry their own model in the +/// provider string and never reach here. +/// +/// The managed summarization tier is intentionally **not** user-overridable: the +/// hosted backend serves exactly one tier (`summarization-v1`) for this workload, +/// so there is nothing else valid to point it at. Users who want a different +/// model run summarization on a BYOK/local `memory_provider`, where the model +/// rides in the provider string. (`memory_tree.cloud_llm_model` is no longer +/// consumed — see its config doc.) +pub(crate) fn summarization_tier_model() -> &'static str { + crate::openhuman::config::MODEL_SUMMARIZATION_V1 +} + /// Build the OpenHuman backend provider (session-JWT auth). /// /// `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`. +/// The `summarization`/`memory` roles resolve their tier from +/// [`summarization_tier_model`] (fixed at `summarization-v1`) so they never +/// collapse to `default_model`. The generic `chat` role (and background roles) +/// keep inheriting `config.default_model`. fn make_openhuman_backend( role: &str, config: &Config, @@ -900,6 +923,21 @@ fn make_openhuman_backend( tier ); tier.to_string() + } else if matches!(role, "summarization" | "memory") { + // Managed summarization/memory tier — fixed at `summarization-v1` rather + // than inherited from `config.default_model`, so every managed + // summarization caller — the memory tree, the chat-turn payload + // summarizer, meeting summaries, and any `hint = "summarization"` + // sub-agent — reaches the dedicated tier instead of silently collapsing + // to `chat-v1`. BYOK/local routes never reach here — they build from the + // provider string. + let tier = summarization_tier_model().to_string(); + log::debug!( + "[providers][chat-factory] role={} resolved managed summarization tier model={}", + role, + tier + ); + tier } else { config .default_model diff --git a/src/openhuman/inference/provider/factory_tests.rs b/src/openhuman/inference/provider/factory_tests.rs index 8dee38d1d..659733942 100644 --- a/src/openhuman/inference/provider/factory_tests.rs +++ b/src/openhuman/inference/provider/factory_tests.rs @@ -396,8 +396,9 @@ fn create_chat_provider_uses_role() { // `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 +// (`summarization`/`memory` resolve their tier separately from +// `memory_tree.cloud_llm_model` — see +// `managed_backend_summarization_role_resolves_summarization_tier`.) This drives // `make_openhuman_backend` directly via the explicit `"openhuman"` provider // string. #[test] @@ -424,24 +425,53 @@ fn managed_backend_pins_specialised_role_to_tier() { } } -// `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`. +// The managed `summarization`/`memory` role is fixed at `summarization-v1` (via +// `summarization_tier_model`), independent of both `config.default_model` and +// `memory_tree.cloud_llm_model`. This is what makes EVERY managed summarization +// caller — memory tree, chat-turn payload summarizer, meeting summaries, and +// `hint = "summarization"` sub-agents — reach the dedicated `summarization-v1` +// tier without each caller pre-routing `default_model`. #[test] -fn managed_backend_summarization_role_inherits_default_model() { - let mut config = Config::default(); - config.default_model = Some("summarization-v1".to_string()); +fn managed_backend_summarization_role_resolves_summarization_tier() { + // Default config: cloud_llm_model defaults to summarization-v1. + let config = Config::default(); 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()); + // `memory` is an alias of `summarization` (both → memory_provider). + let (_, model) = create_chat_provider_from_string("memory", "openhuman", &config) + .expect("managed backend must build"); + assert_eq!(model, "summarization-v1"); +} + +// `default_model` does NOT drive the summarization tier any more — only +// `memory_tree.cloud_llm_model` does. A stray `default_model` must not leak in. +#[test] +fn managed_backend_summarization_ignores_default_model() { + let mut config = Config::default(); + config.default_model = Some("reasoning-v1".to_string()); let (_, model) = create_chat_provider_from_string("summarization", "openhuman", &config) .expect("managed backend must build"); - assert_eq!(model, "reasoning-v1"); + assert_eq!(model, "summarization-v1"); +} + +// The managed summarization tier is LOCKED to `summarization-v1` — the +// (deprecated, inert) `memory_tree.cloud_llm_model` must not change it, whether +// set to another known tier or a custom string. Users who want a different model +// run summarization on a BYOK/local `memory_provider` instead. +#[test] +fn managed_backend_summarization_ignores_cloud_llm_model_override() { + let mut config = Config::default(); + config.memory_tree.cloud_llm_model = Some("chat-v1".to_string()); + let (_, model) = create_chat_provider_from_string("summarization", "openhuman", &config) + .expect("managed backend must build"); + assert_eq!(model, "summarization-v1"); + + config.memory_tree.cloud_llm_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, "summarization-v1"); } // End-to-end of the sub-agent path: the subagent runner resolves a diff --git a/src/openhuman/memory/chat.rs b/src/openhuman/memory/chat.rs index f92bfee7e..834a0424e 100644 --- a/src/openhuman/memory/chat.rs +++ b/src/openhuman/memory/chat.rs @@ -166,20 +166,6 @@ impl ChatProvider for InferenceChatProvider { } } -fn routed_memory_config(config: &Config) -> Config { - let mut routed = config.clone(); - if !config.workload_uses_local("memory") { - routed.default_model = Some( - config - .memory_tree - .cloud_llm_model - .clone() - .unwrap_or_else(|| DEFAULT_CLOUD_LLM_MODEL.to_string()), - ); - } - routed -} - #[cfg(test)] fn test_override_runtime() -> Option<(Arc, String)> { test_override::current().map(|provider| (provider, "test:override".to_string())) @@ -196,9 +182,12 @@ pub fn build_chat_runtime(config: &Config) -> Result<(Arc, Str return Ok(runtime); } - let routed = routed_memory_config(config); - let resolved_provider = provider_for_role("summarization", &routed); - let (provider, model) = create_chat_provider("summarization", &routed)?; + // The managed summarization tier is fixed at `summarization-v1`, resolved + // inside `make_openhuman_backend` for the `summarization` role — so no + // per-caller `default_model` pre-routing is needed here. BYOK/local routes + // carry their own model in the provider string. + let resolved_provider = provider_for_role("summarization", config); + let (provider, model) = create_chat_provider("summarization", config)?; log::debug!( "[memory::chat] built provider route={} model={}", @@ -282,23 +271,26 @@ mod tests { fn build_chat_runtime_defaults_to_openhuman_resolved_model() { let cfg = Config::default(); let (_provider, model) = build_chat_runtime(&cfg).unwrap(); - assert_eq!(model, DEFAULT_CLOUD_LLM_MODEL); - // build_chat_runtime resolves the "summarization" workload role, - // which routes to the dedicated DEFAULT_CLOUD_LLM_MODEL - // (`summarization-v1`, PR #2690) rather than the generic - // `reasoning-v1` fallback. + // The managed "summarization" tier is fixed at `summarization-v1` + // inside `make_openhuman_backend`. DEFAULT_CLOUD_LLM_MODEL is that same + // constant — asserted here only as the expected value, not because + // `cloud_llm_model` is consumed (it isn't; see the test below). assert_eq!(model, DEFAULT_CLOUD_LLM_MODEL); } #[test] - fn build_chat_runtime_still_builds_when_cloud_memory_model_is_overridden() { + fn build_chat_runtime_ignores_cloud_llm_model_on_managed() { + // The managed summarization tier is locked to `summarization-v1`; + // `memory_tree.cloud_llm_model` is inert and must not change it (neither a + // known tier nor a custom string leaks through). let mut cfg = Config::default(); + cfg.memory_tree.cloud_llm_model = Some("chat-v1".into()); + let (_provider, model) = build_chat_runtime(&cfg).unwrap(); + assert_eq!(model, DEFAULT_CLOUD_LLM_MODEL); + cfg.memory_tree.cloud_llm_model = Some("custom-summary-model".into()); let (_provider, model) = build_chat_runtime(&cfg).unwrap(); - // Setting memory_tree.cloud_llm_model overrides the cloud-memory - // model path; the routing falls back to the platform default - // (`reasoning-v1`) rather than the `summarization-v1` tier. - assert_eq!(model, "reasoning-v1"); + assert_eq!(model, DEFAULT_CLOUD_LLM_MODEL); } #[test]