From 873a74eaee698b3e8905a8828f19d9d2d3267820 Mon Sep 17 00:00:00 2001 From: CodeGhost21 <164498022+CodeGhost21@users.noreply.github.com> Date: Tue, 2 Jun 2026 20:01:00 +0530 Subject: [PATCH] fix(channels): honor chat_provider workload routing in channel runtime (#3098 sub-issue 1) (#3217) --- ...-02-telegram-honor-chat-provider-design.md | 53 ++++++++ src/openhuman/channels/runtime/startup.rs | 82 +++++++++++-- .../channels/runtime/startup_tests.rs | 114 ++++++++++++++++++ 3 files changed, 237 insertions(+), 12 deletions(-) create mode 100644 docs/superpowers/specs/2026-06-02-telegram-honor-chat-provider-design.md create mode 100644 src/openhuman/channels/runtime/startup_tests.rs diff --git a/docs/superpowers/specs/2026-06-02-telegram-honor-chat-provider-design.md b/docs/superpowers/specs/2026-06-02-telegram-honor-chat-provider-design.md new file mode 100644 index 000000000..17b006c19 --- /dev/null +++ b/docs/superpowers/specs/2026-06-02-telegram-honor-chat-provider-design.md @@ -0,0 +1,53 @@ +# Telegram (and all channels) honor `chat_provider` workload routing + +**Issue:** [#3098](https://github.com/tinyhumansai/openhuman/issues/3098), sub-issue 1. +**Scope:** Sub-issue 1 only. Sub-issues 2 (Telegram file commits — by-design question), 3 (skills not running), and 4 (Brave Search) are out of scope. + +## Problem + +A user picks Ollama in **Settings → AI** (which writes `chat_provider = "ollama:"` to config), but the Telegram bot still routes through the managed OpenHuman cloud backend and the user's `config.default_model`. The Ollama selection is silently ignored by every channel (Telegram, Discord, Slack, iMessage, Mattermost). + +## Root cause + +`src/openhuman/channels/runtime/startup.rs:171-183, 267-270, 682-695` builds the channels runtime provider via `create_intelligent_routing_provider(…)` — an exclusively cloud-backed chain (`OpenHumanBackendProvider` wrapped in `ReliableProvider` and the legacy hint-based `IntelligentRoutingProvider`). It pairs that provider with `ctx.model = config.default_model.unwrap_or(DEFAULT_MODEL)`. + +It never consults `provider_for_role("chat", &config)` or `Config::workload_local_model("chat")` — the documented single source of truth for "is this workload local?" (`src/openhuman/config/schema/types.rs:511-544`). The unified workload factory `inference::provider::create_chat_provider` (`src/openhuman/inference/provider/factory.rs:206-217`) already knows how to build the right `(provider, model_id)` for any chat-workload string (`"ollama:…"`, `"lmstudio:…"`, `":…"`, `"openhuman"`, `"cloud"`). The channel runtime simply doesn't use it. + +## Fix + +In `runtime/startup.rs`, branch on `provider_for_role("chat", &config)` once during runtime setup: + +- **Workload override path** (`chat_provider` is `"ollama:…"`, `"lmstudio:…"`, `":…"`, or `"claude_agent_sdk[:…]"`): build the provider via `inference::provider::create_chat_provider("chat", &config)`. Use the returned `model_id` as `ctx.model`. The cache key (`ctx.default_provider`) becomes the slug portion of the provider string (e.g. `"ollama"`). +- **Default path** (unset, `"cloud"`, or `"openhuman"`): keep the existing `create_intelligent_routing_provider` chain verbatim. `ctx.model` continues to come from `config.default_model`. Zero behavior change for cloud users. + +The branch happens once during channel-runtime startup. All channels share `ChannelRuntimeContext`, so the fix uniformly covers Telegram, Discord, Slack, iMessage, Mattermost (and Matrix when feature-gated on). + +## Behavior matrix + +| `chat_provider` | Today | After fix | +|---|---|---| +| unset / `"cloud"` / `"openhuman"` | Cloud + `default_model` | Cloud + `default_model` (unchanged) | +| `"ollama:llama3.2"` | Cloud + `default_model` (bug) | Ollama + `llama3.2` | +| `"openai:gpt-4o"` (BYOK) | Cloud + `default_model` (bug) | OpenAI + `gpt-4o` | + +## Tests + +Unit-level coverage in the existing channels runtime test surface: + +1. `chat_provider` unset → `ctx.model == config.default_model` (regression guard for cloud users). +2. `chat_provider = "ollama:llama3.2"` → `ctx.model == "llama3.2"` and `ctx.default_provider == "ollama"`. +3. `chat_provider = "cloud"` → identical to (1). + +Tests use the existing `test_support.rs` harness and `Config` builders. No new mocks required; the workload factory is already exercised by `factory_tests.rs`. + +## Non-goals (deliberate) + +- **`/model ` per-conversation override** — `routes.rs:169-211`'s `get_or_create_provider` ignores the provider name and always rebuilds a cloud provider. This is a pre-existing limitation unrelated to this fix; addressing it would expand scope significantly. Once the default is correct, the `/model` command becomes a model-name-only override against whichever provider the channel runtime was constructed with, which is a reasonable interim state. +- Sub-issues 2, 3, 4 of #3098 — separate root causes, will each get their own PR if/when triaged. +- Any change to `local_ai.usage.*` (the deprecated legacy hint-based routing) — explicitly out of scope per the comment in `config/schema/types.rs:520-523`. + +## Blast radius + +- One file edited: `src/openhuman/channels/runtime/startup.rs` (~15 lines changed). +- Cloud-only users: zero behavior change (the default branch is the existing code path). +- Local-model users: gain a working Telegram/Discord/Slack/etc. experience with their selected Ollama (or BYOK) provider. diff --git a/src/openhuman/channels/runtime/startup.rs b/src/openhuman/channels/runtime/startup.rs index c5561228a..a0d1f05c6 100644 --- a/src/openhuman/channels/runtime/startup.rs +++ b/src/openhuman/channels/runtime/startup.rs @@ -41,6 +41,42 @@ use anyhow::Result; use std::collections::HashMap; use std::sync::{Arc, Mutex}; +/// How the channels runtime should construct its default chat provider. +/// +/// Issue #3098 sub-issue 1: the runtime used to ignore the per-workload +/// `chat_provider` routing and unconditionally build a cloud chain, so +/// Telegram (and other channels) never honored a user's local-Ollama / +/// BYOK selection. `resolve_chat_workload` inspects the resolved chat +/// workload string and chooses between preserving the legacy +/// `create_intelligent_routing_provider` chain (Cloud) and dispatching +/// to the unified workload factory (Workload). +pub(super) enum ChatWorkloadResolution { + /// Preserve the existing cloud chain (`ReliableProvider` + + /// `IntelligentRoutingProvider`) and `config.default_model`. + Cloud, + /// Build the channel provider via `create_chat_provider("chat", config)`. + Workload { + provider_string: String, + slug: String, + }, +} + +pub(super) fn resolve_chat_workload(config: &Config) -> ChatWorkloadResolution { + let resolved = provider::provider_for_role("chat", config); + let trimmed = resolved.trim(); + if trimmed.is_empty() || trimmed == "cloud" || trimmed == provider::INFERENCE_BACKEND_ID { + return ChatWorkloadResolution::Cloud; + } + let slug = trimmed + .split_once(':') + .map(|(s, _)| s.to_string()) + .unwrap_or_else(|| trimmed.to_string()); + ChatWorkloadResolution::Workload { + provider_string: trimmed.to_string(), + slug, + } +} + pub async fn start_channels(mut config: Config) -> Result<()> { // Initialize the global event bus singleton and register the tracing // subscriber for debug logging of all domain events. @@ -178,13 +214,36 @@ pub async fn start_channels(mut config: Config) -> Result<()> { secrets_encrypt: config.secrets.encrypt, reasoning_enabled: config.runtime.reasoning_enabled, }; - let provider: Arc = Arc::from(provider::create_intelligent_routing_provider( - config.inference_url.as_deref(), - config.api_url.as_deref(), - config.api_key.as_deref(), - &config, - &provider_runtime_options, - )?); + let (provider, model, provider_name): (Arc, String, String) = + match resolve_chat_workload(&config) { + ChatWorkloadResolution::Cloud => { + let p: Arc = + Arc::from(provider::create_intelligent_routing_provider( + config.inference_url.as_deref(), + config.api_url.as_deref(), + config.api_key.as_deref(), + &config, + &provider_runtime_options, + )?); + let m = config + .default_model + .clone() + .unwrap_or_else(|| crate::openhuman::config::DEFAULT_MODEL.into()); + (p, m, provider::INFERENCE_BACKEND_ID.to_string()) + } + ChatWorkloadResolution::Workload { + provider_string, + slug, + } => { + tracing::info!( + chat_provider = %provider_string, + slug = %slug, + "[channels][startup] chat workload routed to per-workload provider — building dedicated channel provider" + ); + let (boxed, model_id) = provider::create_chat_provider("chat", &config)?; + (Arc::from(boxed), model_id, slug) + } + }; // Warm up the provider connection pool (TLS handshake, DNS, HTTP/2 setup) // so the first real message doesn't hit a cold-start timeout. @@ -268,10 +327,6 @@ pub async fn start_channels(mut config: Config) -> Result<()> { crate::openhuman::config::AuditConfig::default(), config.workspace_dir.clone(), )?; - let model = config - .default_model - .clone() - .unwrap_or_else(|| crate::openhuman::config::DEFAULT_MODEL.into()); let temperature = config.default_temperature; let local_embedding = config.workload_local_model("embeddings"); let mem: Arc = Arc::from(memory_store::create_memory_with_local_ai( @@ -683,7 +738,6 @@ pub async fn start_channels(mut config: Config) -> Result<()> { println!(" 🚦 In-flight message limit: {max_in_flight_messages}"); - let provider_name = provider::INFERENCE_BACKEND_ID.to_string(); let mut provider_cache_seed: HashMap> = HashMap::new(); provider_cache_seed.insert(provider_name.clone(), Arc::clone(&provider)); let message_timeout_secs = @@ -833,6 +887,10 @@ pub mod test_support { } } +#[cfg(test)] +#[path = "startup_tests.rs"] +mod tests; + #[cfg(test)] mod yuanbao_secret_tests { use super::*; diff --git a/src/openhuman/channels/runtime/startup_tests.rs b/src/openhuman/channels/runtime/startup_tests.rs new file mode 100644 index 000000000..9d83b6d59 --- /dev/null +++ b/src/openhuman/channels/runtime/startup_tests.rs @@ -0,0 +1,114 @@ +//! Tests for the chat-workload resolver wired into channel runtime startup. +//! +//! Issue #3098 sub-issue 1: prior to this fix, channel runtime startup +//! always built a cloud-only provider chain and used +//! `config.default_model`, ignoring the per-workload `chat_provider` +//! routing string. These tests pin the resolver behavior so the default +//! (cloud) path is preserved for users who haven't picked a local / +//! BYOK model, and the override path activates for those who have. + +use super::{resolve_chat_workload, ChatWorkloadResolution}; +use crate::openhuman::config::Config; + +fn config_with_chat_provider(s: Option<&str>) -> Config { + let mut config = Config::default(); + config.chat_provider = s.map(str::to_string); + config +} + +#[test] +fn chat_provider_unset_resolves_to_cloud() { + let config = config_with_chat_provider(None); + assert!(matches!( + resolve_chat_workload(&config), + ChatWorkloadResolution::Cloud + )); +} + +#[test] +fn chat_provider_blank_resolves_to_cloud() { + let config = config_with_chat_provider(Some("")); + assert!(matches!( + resolve_chat_workload(&config), + ChatWorkloadResolution::Cloud + )); +} + +#[test] +fn chat_provider_cloud_sentinel_resolves_to_cloud() { + let config = config_with_chat_provider(Some("cloud")); + assert!(matches!( + resolve_chat_workload(&config), + ChatWorkloadResolution::Cloud + )); +} + +#[test] +fn chat_provider_openhuman_sentinel_resolves_to_cloud() { + let config = config_with_chat_provider(Some("openhuman")); + assert!(matches!( + resolve_chat_workload(&config), + ChatWorkloadResolution::Cloud + )); +} + +#[test] +fn chat_provider_ollama_resolves_to_workload() { + let config = config_with_chat_provider(Some("ollama:llama3.2")); + match resolve_chat_workload(&config) { + ChatWorkloadResolution::Workload { + provider_string, + slug, + } => { + assert_eq!(provider_string, "ollama:llama3.2"); + assert_eq!(slug, "ollama"); + } + ChatWorkloadResolution::Cloud => panic!("expected Workload for ollama, got Cloud"), + } +} + +#[test] +fn chat_provider_lmstudio_resolves_to_workload() { + let config = config_with_chat_provider(Some("lmstudio:qwen2.5:0.5b")); + match resolve_chat_workload(&config) { + ChatWorkloadResolution::Workload { + provider_string, + slug, + } => { + assert_eq!(provider_string, "lmstudio:qwen2.5:0.5b"); + assert_eq!(slug, "lmstudio"); + } + ChatWorkloadResolution::Cloud => panic!("expected Workload for lmstudio"), + } +} + +#[test] +fn chat_provider_byok_slug_resolves_to_workload() { + let config = config_with_chat_provider(Some("openai:gpt-4o")); + match resolve_chat_workload(&config) { + ChatWorkloadResolution::Workload { + provider_string, + slug, + } => { + assert_eq!(provider_string, "openai:gpt-4o"); + assert_eq!(slug, "openai"); + } + ChatWorkloadResolution::Cloud => panic!("expected Workload for byok slug"), + } +} + +#[test] +fn chat_provider_claude_agent_sdk_resolves_to_workload() { + // Bare sentinel (no colon) — slug is the full string. + let config = config_with_chat_provider(Some("claude_agent_sdk")); + match resolve_chat_workload(&config) { + ChatWorkloadResolution::Workload { + provider_string, + slug, + } => { + assert_eq!(provider_string, "claude_agent_sdk"); + assert_eq!(slug, "claude_agent_sdk"); + } + ChatWorkloadResolution::Cloud => panic!("expected Workload for claude_agent_sdk"), + } +}