From a6fd156131507dc3843dca88aea09cc137dca802 Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Fri, 29 May 2026 04:12:29 +0530 Subject: [PATCH] fix(inference): default empty model to reasoning-v1 on OpenHuman backend (#2837) --- src/openhuman/inference/local/ops.rs | 31 ++++++- src/openhuman/inference/local/ops_tests.rs | 34 +++++++ .../inference/provider/openhuman_backend.rs | 91 ++++++++++++++++++- 3 files changed, 151 insertions(+), 5 deletions(-) diff --git a/src/openhuman/inference/local/ops.rs b/src/openhuman/inference/local/ops.rs index 22fec3bec..efecec060 100644 --- a/src/openhuman/inference/local/ops.rs +++ b/src/openhuman/inference/local/ops.rs @@ -32,6 +32,29 @@ fn prompt_guard_user_message(action: PromptEnforcementAction) -> &'static str { } } +/// Normalize a `model_override` string into the `Option` form the +/// downstream config-resolution path expects. +/// +/// `None` → `None`. `Some(non-empty-after-trim)` → `Some(trimmed)`. Anything +/// else (`Some("")`, `Some(" ")`, `Some("\t\n")`) collapses to `None` so +/// the existing default-model fallback applies instead of overwriting +/// `config.default_model` with a blank string that the OpenHuman backend +/// would reject with `400 model is required` (Sentry TAURI-RUST-RS). +/// +/// Extracted to keep `agent_chat` and `agent_chat_simple` in lockstep — +/// future tweaks (additional log lines, tightening the trim rules) live in +/// exactly one place. +fn normalize_model_override(opt: Option) -> Option { + opt.and_then(|m| { + let t = m.trim(); + if t.is_empty() { + None + } else { + Some(t.to_string()) + } + }) +} + fn enforce_user_prompt_or_reject(prompt: &str, source: &'static str) -> Result<(), String> { let decision = enforce_prompt_input( prompt, @@ -69,7 +92,10 @@ pub async fn agent_chat( ) -> Result, String> { enforce_user_prompt_or_reject(message, "local_ai.ops.agent_chat")?; - if let Some(model) = model_override { + // TAURI-RUST-RS: an upstream caller (frontend, JSON-RPC client) can pass + // `model_override: Some("")`. See `normalize_model_override` for the + // rationale — an empty / whitespace-only override collapses to `None`. + if let Some(model) = normalize_model_override(model_override) { config.default_model = Some(model); } if let Some(temp) = temperature { @@ -90,7 +116,8 @@ pub async fn agent_chat_simple( enforce_user_prompt_or_reject(message, "local_ai.ops.agent_chat_simple")?; let mut effective = config.clone(); - if let Some(model) = model_override { + // TAURI-RUST-RS: see `normalize_model_override` for the rationale. + if let Some(model) = normalize_model_override(model_override) { effective.default_model = Some(model); } if let Some(temp) = temperature { diff --git a/src/openhuman/inference/local/ops_tests.rs b/src/openhuman/inference/local/ops_tests.rs index 08bc65efa..566d9e405 100644 --- a/src/openhuman/inference/local/ops_tests.rs +++ b/src/openhuman/inference/local/ops_tests.rs @@ -146,3 +146,37 @@ async fn local_ai_assets_status_returns_without_panic() { let config = test_config(&tmp); let _ = local_ai_assets_status(&config).await; } + +// ── normalize_model_override (TAURI-RUST-RS) ─────────────────────────── + +#[test] +fn normalize_model_override_passthrough_none() { + assert_eq!(normalize_model_override(None), None); +} + +#[test] +fn normalize_model_override_blank_collapses_to_none() { + assert_eq!(normalize_model_override(Some(String::new())), None); + assert_eq!(normalize_model_override(Some(" ".to_string())), None); + assert_eq!(normalize_model_override(Some("\t\n".to_string())), None); +} + +#[test] +fn normalize_model_override_trims_surrounding_whitespace() { + assert_eq!( + normalize_model_override(Some(" reasoning-v1 ".to_string())), + Some("reasoning-v1".to_string()) + ); +} + +#[test] +fn normalize_model_override_passes_non_empty_verbatim() { + assert_eq!( + normalize_model_override(Some("agentic-v1".to_string())), + Some("agentic-v1".to_string()) + ); + assert_eq!( + normalize_model_override(Some("hint:reasoning".to_string())), + Some("hint:reasoning".to_string()) + ); +} diff --git a/src/openhuman/inference/provider/openhuman_backend.rs b/src/openhuman/inference/provider/openhuman_backend.rs index f145b5eb7..40b1d7363 100644 --- a/src/openhuman/inference/provider/openhuman_backend.rs +++ b/src/openhuman/inference/provider/openhuman_backend.rs @@ -15,6 +15,40 @@ use std::path::PathBuf; pub const PROVIDER_LABEL: &str = "OpenHuman"; +/// Normalize an inbound `model` argument before forwarding to the OpenHuman backend. +/// +/// The backend rejects a blank `model` field with +/// `400 {"success":false,"error":"model is required"}` (Sentry **TAURI-RUST-RS**, +/// 163 events / 14d). Empty values reach this layer when a workload routes to +/// `:` with no model after the colon (see the `[config][migrate]` +/// rewrites in `src/openhuman/config/schema/load.rs:967`) or when an upstream +/// caller passes `model_override: Some("")`. +/// +/// Substitute the canonical default tier so the call succeeds instead of +/// failing the wire round-trip. Mirrors the same fallback `make_openhuman_backend` +/// already applies when `default_model` is missing +/// (`src/openhuman/inference/provider/factory.rs:404`), so behavior stays +/// consistent across both entry paths. +fn resolve_model(model: &str) -> String { + let trimmed = model.trim(); + if trimmed.is_empty() { + // Debug-tier on purpose: the routing-migration path + // (`config/schema/load.rs:967`) can hit this on every chat turn for + // an affected user (~163 events / 14d on Sentry pre-fix). Warn-tier + // here would just move the noise from Sentry to local log dashboards. + // Per-process throttling via `Once` was considered — debug is simpler + // and gives the same diagnostic when needed (set RUST_LOG=debug). + log::debug!( + "[providers][openhuman-backend] empty model passed to OpenHuman backend; \ + substituting default `{}` (TAURI-RUST-RS)", + crate::openhuman::config::MODEL_REASONING_V1 + ); + crate::openhuman::config::MODEL_REASONING_V1.to_string() + } else { + trimmed.to_string() + } +} + /// Routes chat to `config.api_url` + `/openai` with `Authorization: Bearer` from the `app-session` profile. pub struct OpenHumanBackendProvider { options: ProviderRuntimeOptions, @@ -109,8 +143,9 @@ impl Provider for OpenHumanBackendProvider { ) -> anyhow::Result { let token = self.resolve_bearer()?; let inner = self.inner(&token)?; + let model = resolve_model(model); inner - .chat_with_system(system_prompt, message, model, temperature) + .chat_with_system(system_prompt, message, &model, temperature) .await } @@ -122,7 +157,8 @@ impl Provider for OpenHumanBackendProvider { ) -> anyhow::Result { let token = self.resolve_bearer()?; let inner = self.inner(&token)?; - inner.chat_with_history(messages, model, temperature).await + let model = resolve_model(model); + inner.chat_with_history(messages, &model, temperature).await } async fn chat( @@ -133,7 +169,8 @@ impl Provider for OpenHumanBackendProvider { ) -> anyhow::Result { let token = self.resolve_bearer()?; let inner = self.inner(&token)?; - inner.chat(request, model, temperature).await + let model = resolve_model(model); + inner.chat(request, &model, temperature).await } async fn warmup(&self) -> anyhow::Result<()> { @@ -154,6 +191,9 @@ impl Provider for OpenHumanBackendProvider { _temperature: f64, _options: StreamOptions, ) -> futures_util::stream::BoxStream<'static, StreamResult> { + // TODO(stream-support): when streaming is enabled here, route + // `_model` through `resolve_model` before forwarding — same blank + // model guard as the non-streaming methods (TAURI-RUST-RS). stream::once(async move { Ok(StreamChunk::error( "streaming is not supported for OpenHuman backend provider", @@ -162,3 +202,48 @@ impl Provider for OpenHumanBackendProvider { .boxed() } } + +#[cfg(test)] +mod tests { + use super::*; + + // TAURI-RUST-RS regression coverage: the OpenHuman backend rejects an + // empty `model` field with 400 `model is required`. `resolve_model` must + // intercept blank / whitespace-only values before they hit the wire and + // substitute the canonical default tier. + + #[test] + fn resolve_model_substitutes_default_for_empty() { + assert_eq!( + resolve_model(""), + crate::openhuman::config::MODEL_REASONING_V1 + ); + } + + #[test] + fn resolve_model_substitutes_default_for_whitespace_only() { + assert_eq!( + resolve_model(" "), + crate::openhuman::config::MODEL_REASONING_V1 + ); + assert_eq!( + resolve_model("\t\n"), + crate::openhuman::config::MODEL_REASONING_V1 + ); + } + + #[test] + fn resolve_model_trims_surrounding_whitespace() { + assert_eq!(resolve_model(" reasoning-v1 "), "reasoning-v1"); + } + + #[test] + fn resolve_model_preserves_non_empty_value_verbatim() { + // Non-empty values are passed through unchanged (after trim) — no + // canonicalisation, no remapping. The backend is authoritative over + // which model strings it accepts. + assert_eq!(resolve_model("agentic-v1"), "agentic-v1"); + assert_eq!(resolve_model("hint:reasoning"), "hint:reasoning"); + assert_eq!(resolve_model("some-custom-model"), "some-custom-model"); + } +}