From c257a04e76d2b5ce38a359949d3e125b8bcbec8f Mon Sep 17 00:00:00 2001 From: Taimoor Date: Thu, 28 May 2026 09:49:00 +0500 Subject: [PATCH 1/4] fix(inference): fail-fast on empty model for cloud providers, demote nvidia-nim Sentry noise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #2784. `make_cloud_provider_by_slug` now rejects a zero-length effective model (provider string `nvidia-nim:` with no inline model AND no `default_model` entry) at factory construction time instead of silently forwarding `model=""` to the upstream API. Providers such as nvidia-nim respond with `{"error":{"message":"model field is required",...}}` (400) and Sentry accumulates one event per agent turn (TAURI-RUST-4NM). Changes: - `factory.rs`: bail with a clear, actionable message when `effective_model.trim().is_empty()` after resolution. Actionable path: either supply the model in the provider string (`nvidia-nim:`) or set `default_model` in the `[[cloud_providers]]` config entry. - `config_rejection.rs`: add "model field is required" to the `PHRASES` list as defense-in-depth so any body that still reaches the HTTP layer is demoted to `log::info!` rather than Sentry. - `factory_test.rs`: two new unit tests — `cloud_provider_with_no_model_and_no_default_rejected` asserts the early-error path fires with a message naming the slug, and `cloud_provider_default_model_used_when_model_part_is_empty` asserts the happy-path fallback to `default_model` is preserved. - `config_rejection.rs` tests: `detects_nvidia_nim_missing_model_body` pins the "model field is required" classifier. --- .../inference/provider/config_rejection.rs | 21 ++++++++ src/openhuman/inference/provider/factory.rs | 15 ++++++ .../inference/provider/factory_test.rs | 53 +++++++++++++++++++ 3 files changed, 89 insertions(+) diff --git a/src/openhuman/inference/provider/config_rejection.rs b/src/openhuman/inference/provider/config_rejection.rs index 125edc4cc6..be17f3618b 100644 --- a/src/openhuman/inference/provider/config_rejection.rs +++ b/src/openhuman/inference/provider/config_rejection.rs @@ -148,6 +148,14 @@ pub fn is_provider_config_rejection_message(body: &str) -> bool { // this is the `type` field used by litellm/Anthropic-style // envelopes for the same class of user-state error. "not_found_error", + // TAURI-RUST-4NM — nvidia-nim (and some other providers) return + // `{"error":{"message":"model field is required","type":"invalid_request_error","code":"missing_required_field"}}` + // when the `model` key is absent or empty in the request body. + // This is a user-configuration error (provider string has no model + // component, e.g. `nvidia-nim:` with empty model), not a product + // regression. Demote from Sentry; the factory now validates this + // up-front so in practice this phrase should no longer appear. + "model field is required", ]; let lower = body.to_ascii_lowercase(); @@ -316,6 +324,19 @@ mod tests { } } + #[test] + fn detects_nvidia_nim_missing_model_body() { + // TAURI-RUST-4NM — nvidia-nim rejects requests with model="" with + // `{"error":{"message":"model field is required",...}}`. + let body = r#"nvidia-nim API error (400 Bad Request): {"error":{"message":"model field is required","type":"invalid_request_error","code":"missing_required_field"}}"#; + assert!( + is_provider_config_rejection_message(body), + "TAURI-RUST-4NM body must classify as provider config-rejection: {body:?}" + ); + // Also verify the bare phrase on its own (defense-in-depth path). + assert!(is_provider_config_rejection_message("model field is required")); + } + #[test] fn unknown_model_helper_rejects_other_config_rejection_phrases() { // Polarity exception must stay narrow: other config-rejection diff --git a/src/openhuman/inference/provider/factory.rs b/src/openhuman/inference/provider/factory.rs index 2fd5bfe7f2..1c6bd3b287 100644 --- a/src/openhuman/inference/provider/factory.rs +++ b/src/openhuman/inference/provider/factory.rs @@ -768,6 +768,21 @@ fn make_cloud_provider_by_slug( model.to_string() }; + // Fail fast when there is still no model after resolution rather than sending + // an API request with model="" which some providers (e.g. nvidia-nim) reject + // with "model field is required" (TAURI-RUST-4NM). This surfaces a clear, + // actionable error at factory build time instead of a confusing provider 400. + if effective_model.trim().is_empty() { + anyhow::bail!( + "[chat-factory] cloud provider slug '{}' for role '{}' has no model configured. \ + Provide a model in the provider string (e.g. '{}:') or set \ + default_model in the [[cloud_providers]] config entry for this slug.", + slug, + role, + slug + ); + } + if entry.auth_style != AuthStyle::OpenhumanJwt && is_abstract_tier_model(&effective_model) { if let Some(default_model) = entry .default_model diff --git a/src/openhuman/inference/provider/factory_test.rs b/src/openhuman/inference/provider/factory_test.rs index 9bb7712c53..ea3115db66 100644 --- a/src/openhuman/inference/provider/factory_test.rs +++ b/src/openhuman/inference/provider/factory_test.rs @@ -369,6 +369,59 @@ fn empty_model_in_ollama_rejected() { assert!(err.to_string().contains("empty model"), "{err}"); } +#[test] +fn cloud_provider_with_no_model_and_no_default_rejected() { + // TAURI-RUST-4NM — nvidia-nim (and others) reject `model=""` with + // "model field is required". The factory must catch this up-front with + // a clear, actionable message instead of leaking an empty model to the API. + let mut config = Config::default(); + config.cloud_providers.push(CloudProviderCreds { + id: "p_nim".to_string(), + slug: "nvidia-nim".to_string(), + label: "NVIDIA NIM".to_string(), + endpoint: "https://integrate.api.nvidia.com/v1".to_string(), + auth_style: AuthStyle::Bearer, + default_model: None, // no fallback model configured + ..Default::default() + }); + + let err = + match create_chat_provider_from_string("reasoning", "nvidia-nim:", &config) { + Ok(_) => panic!("empty model must fail"), + Err(e) => e, + }; + let msg = err.to_string(); + assert!( + msg.contains("no model configured"), + "expected 'no model configured' in error, got: {msg}" + ); + assert!( + msg.contains("nvidia-nim"), + "error must name the slug; got: {msg}" + ); +} + +#[test] +fn cloud_provider_default_model_used_when_model_part_is_empty() { + // When provider string is "nvidia-nim:" (empty model) but the entry + // has a default_model, the factory must use the default — not error. + let mut config = Config::default(); + config.cloud_providers.push(CloudProviderCreds { + id: "p_nim".to_string(), + slug: "nvidia-nim".to_string(), + label: "NVIDIA NIM".to_string(), + endpoint: "https://integrate.api.nvidia.com/v1".to_string(), + auth_style: AuthStyle::Bearer, + default_model: Some("meta/llama-3.1-8b-instruct".to_string()), + ..Default::default() + }); + + let (_, model) = + create_chat_provider_from_string("reasoning", "nvidia-nim:", &config) + .expect("empty model with default_model must succeed"); + assert_eq!(model, "meta/llama-3.1-8b-instruct"); +} + #[test] fn missing_slug_for_openai_gives_clear_error() { let config = Config::default(); From 6168e1c9aebb8e14966afc75ccbfc92316c2edfa Mon Sep 17 00:00:00 2001 From: Taimoor Date: Thu, 28 May 2026 09:59:31 +0500 Subject: [PATCH 2/4] style: fix cargo fmt violations in nvidia-nim test code Three lines were formatted in a way cargo fmt rejects: - config_rejection.rs: wrap bare assert! arg onto its own line - factory_test.rs: inline `let err = match` (no line-break before match) - factory_test.rs: inline `let (_, model) = call()` method chain --- .../inference/provider/config_rejection.rs | 4 +++- src/openhuman/inference/provider/factory_test.rs | 14 ++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/openhuman/inference/provider/config_rejection.rs b/src/openhuman/inference/provider/config_rejection.rs index be17f3618b..1ec881abdd 100644 --- a/src/openhuman/inference/provider/config_rejection.rs +++ b/src/openhuman/inference/provider/config_rejection.rs @@ -334,7 +334,9 @@ mod tests { "TAURI-RUST-4NM body must classify as provider config-rejection: {body:?}" ); // Also verify the bare phrase on its own (defense-in-depth path). - assert!(is_provider_config_rejection_message("model field is required")); + assert!(is_provider_config_rejection_message( + "model field is required" + )); } #[test] diff --git a/src/openhuman/inference/provider/factory_test.rs b/src/openhuman/inference/provider/factory_test.rs index ea3115db66..230ad3e9b3 100644 --- a/src/openhuman/inference/provider/factory_test.rs +++ b/src/openhuman/inference/provider/factory_test.rs @@ -385,11 +385,10 @@ fn cloud_provider_with_no_model_and_no_default_rejected() { ..Default::default() }); - let err = - match create_chat_provider_from_string("reasoning", "nvidia-nim:", &config) { - Ok(_) => panic!("empty model must fail"), - Err(e) => e, - }; + let err = match create_chat_provider_from_string("reasoning", "nvidia-nim:", &config) { + Ok(_) => panic!("empty model must fail"), + Err(e) => e, + }; let msg = err.to_string(); assert!( msg.contains("no model configured"), @@ -416,9 +415,8 @@ fn cloud_provider_default_model_used_when_model_part_is_empty() { ..Default::default() }); - let (_, model) = - create_chat_provider_from_string("reasoning", "nvidia-nim:", &config) - .expect("empty model with default_model must succeed"); + let (_, model) = create_chat_provider_from_string("reasoning", "nvidia-nim:", &config) + .expect("empty model with default_model must succeed"); assert_eq!(model, "meta/llama-3.1-8b-instruct"); } From 20a51ecb10caea06e093ada339279715e5a1cdb5 Mon Sep 17 00:00:00 2001 From: Taimoor Date: Thu, 28 May 2026 11:36:11 +0500 Subject: [PATCH 3/4] fix(inference): exempt OpenhumanJwt slugs from empty-model guard The empty-model bail! added in TAURI-RUST-4NM fired for openhuman: (auth_style=OpenhumanJwt) slugs, breaking openhuman_slug_routes_to_backend. OpenhumanJwt entries route to the OpenHuman backend and never forward the effective_model to an upstream API, so an empty model is valid for that auth style. Guard the check with auth_style != OpenhumanJwt. --- src/openhuman/inference/provider/factory.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/openhuman/inference/provider/factory.rs b/src/openhuman/inference/provider/factory.rs index 1c6bd3b287..afdf5249b6 100644 --- a/src/openhuman/inference/provider/factory.rs +++ b/src/openhuman/inference/provider/factory.rs @@ -772,7 +772,11 @@ fn make_cloud_provider_by_slug( // an API request with model="" which some providers (e.g. nvidia-nim) reject // with "model field is required" (TAURI-RUST-4NM). This surfaces a clear, // actionable error at factory build time instead of a confusing provider 400. - if effective_model.trim().is_empty() { + // + // Exception: OpenhumanJwt entries route to the OpenHuman backend and never + // forward `effective_model` to an upstream API (see the OpenhumanJwt arm in + // the match below), so an empty model is valid for that auth style. + if effective_model.trim().is_empty() && entry.auth_style != AuthStyle::OpenhumanJwt { anyhow::bail!( "[chat-factory] cloud provider slug '{}' for role '{}' has no model configured. \ Provide a model in the provider string (e.g. '{}:') or set \ From eae48b0c3b63a2bd56495fbf11b059bc924414d4 Mon Sep 17 00:00:00 2001 From: Taimoor Date: Fri, 29 May 2026 04:24:03 +0500 Subject: [PATCH 4/4] fix(inference): add missing phrases to config_rejection and 'no model configured' to factory error --- .../inference/provider/config_rejection.rs | 31 +++++++++++++++++++ src/openhuman/inference/provider/factory.rs | 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/openhuman/inference/provider/config_rejection.rs b/src/openhuman/inference/provider/config_rejection.rs index 9f7f5fdcb4..a27d27f21a 100644 --- a/src/openhuman/inference/provider/config_rejection.rs +++ b/src/openhuman/inference/provider/config_rejection.rs @@ -179,6 +179,37 @@ pub fn is_provider_config_rejection_message(body: &str) -> bool { // regression. Demote from Sentry; the factory now validates this // up-front so in practice this phrase should no longer appear. "model field is required", + // TAURI-RUST-4XK — Ollama 403 when the requested model requires a + // paid Ollama subscription. Body carries the upgrade URL. User must + // switch to a free model or upgrade their Ollama account. + "requires a subscription", + // TAURI-RUST-2G / TAURI-RUST-2F — DeepSeek / compatible providers + // that use extended thinking reject tool-call turns when the + // `reasoning_content` block from a prior assistant turn is not + // threaded back. This is user-config state (model requires the + // caller to replay the thinking block; the frontend replay logic in + // `turn.rs` handles it for subsequent turns, so the first-turn 400 + // is expected capability-discovery, not a regression). + "in the thinking mode must be passed back", + // TAURI-RUST-35 / TAURI-RUST-4K7 / TAURI-RUST-4Z0 — Ollama models + // (e.g. gemma3, phi3, deepseek-r1) that do not support function + // calling return HTTP 400 with this phrase. The compatible provider + // retries without tools on 400, so the initial rejection is expected + // capability-discovery. Sentry noise suppressed here. + "does not support tools", + // TAURI-RUST-4K7-d — alternative phrasing used by some Ollama model + // versions for the same tool-unsupported condition. + "function calling is not supported", + // TAURI-RUST-4K7-e — litellm / OpenAI-compatible proxies reject the + // `tools` key in the request body when the backing model does not + // support tool use (e.g. local Ollama via LiteLLM gateway). + "unknown parameter: tools", + // TAURI-RUST-4K7-f — Ollama native API surface rejects the field + // outright when the model has no function-calling capability. + "unrecognized field `tools`", + // TAURI-RUST-4K7-g — another litellm / proxy variant of the same + // tool-unsupported condition. + "unsupported parameter: tools", ]; let lower = body.to_ascii_lowercase(); diff --git a/src/openhuman/inference/provider/factory.rs b/src/openhuman/inference/provider/factory.rs index 55268f8d63..4a08122ac2 100644 --- a/src/openhuman/inference/provider/factory.rs +++ b/src/openhuman/inference/provider/factory.rs @@ -788,7 +788,7 @@ fn make_cloud_provider_by_slug( slug, ); anyhow::bail!( - "[chat-factory] role '{}' resolved to an empty model id for slug '{}'. \ + "[chat-factory] no model configured: role '{}' resolved to an empty model id for slug '{}'. \ Include a model in the provider string (e.g. '{slug}:') or \ set default_model on the cloud_providers entry for slug '{slug}'.", role,