mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
This commit is contained in:
@@ -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:<model>"` 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:…"`, `"<byok-slug>:…"`, `"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:…"`, `"<byok-slug>:…"`, 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 <id>` 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.
|
||||
@@ -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<dyn Provider> = 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<dyn Provider>, String, String) =
|
||||
match resolve_chat_workload(&config) {
|
||||
ChatWorkloadResolution::Cloud => {
|
||||
let p: Arc<dyn Provider> =
|
||||
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<dyn Memory> = 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<String, Arc<dyn Provider>> = 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::*;
|
||||
|
||||
@@ -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"),
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user