From c204e1b561c4445c9c862d0ddd018fbc67dbee01 Mon Sep 17 00:00:00 2001 From: Cyrus Gray <144336577+graycyrus@users.noreply.github.com> Date: Fri, 29 May 2026 14:41:26 +0530 Subject: [PATCH] =?UTF-8?q?fix(inference):=20disable=20Responses=20API=20f?= =?UTF-8?q?allback=20for=20ollama/lmstudio=20=E2=80=94=20stops=20404=20Sen?= =?UTF-8?q?try=20noise=20(TAURI-RUST-59Y)=20(#2902)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/openhuman/inference/provider/factory.rs | 45 +++++- .../inference/provider/factory_test.rs | 152 ++++++++++++++++++ 2 files changed, 195 insertions(+), 2 deletions(-) diff --git a/src/openhuman/inference/provider/factory.rs b/src/openhuman/inference/provider/factory.rs index 4a08122ac..d47eb46ba 100644 --- a/src/openhuman/inference/provider/factory.rs +++ b/src/openhuman/inference/provider/factory.rs @@ -694,6 +694,9 @@ fn make_ollama_provider( redact_endpoint(&endpoint), temperature_override ); + // Ollama does not expose the Responses API (/v1/responses) — passing + // `false` prevents a guaranteed-404 fallback attempt and the Sentry + // noise it would generate (TAURI-RUST-59Y). let p = make_openai_compatible_provider_with_config( "ollama", &endpoint, @@ -701,6 +704,7 @@ fn make_ollama_provider( CompatAuthStyle::None, &config.temperature_unsupported_models, temperature_override, + false, )?; Ok((p, model.to_string())) } @@ -719,6 +723,7 @@ fn make_lm_studio_provider( redact_endpoint(&endpoint), temperature_override ); + // LM Studio does not expose the Responses API — same rationale as Ollama. let p = make_openai_compatible_provider_with_config( "lmstudio", &endpoint, @@ -730,6 +735,7 @@ fn make_lm_studio_provider( }, &config.temperature_unsupported_models, temperature_override, + false, )?; Ok((p, model.to_string())) } @@ -843,6 +849,7 @@ fn make_cloud_provider_by_slug( CompatAuthStyle::Anthropic, unsupported, temperature_override, + true, )?; Ok((p, effective_model)) } @@ -863,6 +870,7 @@ fn make_cloud_provider_by_slug( CompatAuthStyle::None, unsupported, temperature_override, + true, )?; Ok((p, effective_model)) } @@ -874,6 +882,7 @@ fn make_cloud_provider_by_slug( CompatAuthStyle::Bearer, unsupported, temperature_override, + true, )?; Ok((p, effective_model)) } @@ -953,12 +962,26 @@ fn make_openai_compatible_provider( api_key: &str, auth_style: CompatAuthStyle, ) -> anyhow::Result> { - make_openai_compatible_provider_with_config("cloud", endpoint, api_key, auth_style, &[], None) + make_openai_compatible_provider_with_config( + "cloud", + endpoint, + api_key, + auth_style, + &[], + None, + true, + ) } /// Build an `OpenAiCompatibleProvider` with auth style, temperature /// suppression list from config, and an optional per-workload temperature /// override (extracted from the provider string's `@` suffix). +/// +/// `supports_responses_fallback` controls whether a 404 on the chat +/// completions endpoint triggers an automatic retry against `/v1/responses`. +/// Local providers (Ollama, LM Studio) do not expose the Responses API, so +/// passing `false` for them prevents a guaranteed-404 secondary request and +/// the Sentry noise it would generate (TAURI-RUST-59Y). fn make_openai_compatible_provider_with_config( provider_name: &str, endpoint: &str, @@ -966,14 +989,32 @@ fn make_openai_compatible_provider_with_config( auth_style: CompatAuthStyle, temperature_unsupported_models: &[String], temperature_override: Option, + supports_responses_fallback: bool, ) -> anyhow::Result> { let key = if api_key.trim().is_empty() { None } else { Some(api_key) }; - Ok(Box::new( + log::debug!( + "[providers][chat-factory] building compatible provider name={} endpoint_host={} responses_fallback={} temp_override={:?}", + provider_name, + redact_endpoint(endpoint), + supports_responses_fallback, + temperature_override + ); + let provider = if supports_responses_fallback { OpenAiCompatibleProvider::new(provider_name, endpoint, key, auth_style) + } else { + OpenAiCompatibleProvider::new_no_responses_fallback( + provider_name, + endpoint, + key, + auth_style, + ) + }; + Ok(Box::new( + provider .with_temperature_unsupported_models(temperature_unsupported_models.to_vec()) .with_temperature_override(temperature_override), )) diff --git a/src/openhuman/inference/provider/factory_test.rs b/src/openhuman/inference/provider/factory_test.rs index 7366954ac..091a6b7d7 100644 --- a/src/openhuman/inference/provider/factory_test.rs +++ b/src/openhuman/inference/provider/factory_test.rs @@ -4,6 +4,8 @@ use crate::openhuman::config::Config; use crate::openhuman::credentials::AuthService; use crate::openhuman::inference::provider::traits::{ChatMessage, ChatRequest, ProviderDelta}; use tempfile::TempDir; +use wiremock::matchers::{method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; fn config_with_providers(providers: Vec) -> Config { let mut c = Config::default(); @@ -1251,6 +1253,156 @@ fn byok_fallback_background_workloads_never_inherit() { } } +/// Regression guard for TAURI-RUST-59Y: when Ollama returns 404 on +/// `/chat/completions` (e.g. model not found), the provider must NOT +/// attempt a fallback request to `/responses`. The Ollama API has no +/// Responses endpoint, so the fallback produces a second guaranteed-404 +/// that previously generated Sentry noise at scale (1,598 events). +/// +/// This test mounts a mock server that returns 404 for chat/completions +/// and an empty 200 for the responses endpoint (so we can detect if it +/// was called). After the provider call fails, we assert the responses +/// endpoint received zero requests. +#[tokio::test] +async fn ollama_provider_does_not_fall_back_to_responses_on_404() { + let mock_server = MockServer::start().await; + + // chat/completions always returns 404 (model not found). + Mock::given(method("POST")) + .and(path("/v1/chat/completions")) + .respond_with(ResponseTemplate::new(404).set_body_string( + r#"{"error":{"message":"model 'gemma3:1b-it-qat' not found","code":404}}"#, + )) + .expect(1) // exactly one attempt — no retry + .mount(&mock_server) + .await; + + // /v1/responses should NOT be called — mount with expect(0). + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with( + ResponseTemplate::new(200) + .set_body_string(r#"{"output_text":"should not reach here"}"#), + ) + .expect(0) // must not be called + .mount(&mock_server) + .await; + + let mut config = Config::default(); + // Point the Ollama base URL at the mock server. + config.local_ai.base_url = Some(mock_server.uri()); + let (provider, model) = + create_chat_provider_from_string("chat", "ollama:gemma3:1b-it-qat", &config) + .expect("ollama provider must build"); + + // The call should fail (404), but must not trigger the /v1/responses path. + let result = provider.chat_with_system(None, "hello", &model, 0.0).await; + assert!( + result.is_err(), + "provider should fail with 404, got success" + ); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("404") || err_msg.contains("not found"), + "error should reference 404/not-found, got: {err_msg}" + ); + + // wiremock verifies expect(0) on the responses mock when the server is dropped. +} + +/// Same regression guard as above but for LM Studio — it also lacks the +/// Responses API and must not trigger the fallback on 404. +#[tokio::test] +async fn lmstudio_provider_does_not_fall_back_to_responses_on_404() { + let mock_server = MockServer::start().await; + + Mock::given(method("POST")) + .and(path("/v1/chat/completions")) + .respond_with(ResponseTemplate::new(404).set_body_string(r#"{"error":"model not found"}"#)) + .expect(1) + .mount(&mock_server) + .await; + + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with( + ResponseTemplate::new(200) + .set_body_string(r#"{"output_text":"should not reach here"}"#), + ) + .expect(0) + .mount(&mock_server) + .await; + + let mut config = Config::default(); + config.local_ai.base_url = Some(mock_server.uri()); + let (provider, model) = + create_chat_provider_from_string("chat", "lmstudio:google/gemma-4-e4b", &config) + .expect("lmstudio provider must build"); + + let result = provider.chat_with_system(None, "hello", &model, 0.0).await; + assert!( + result.is_err(), + "provider should fail with 404, got success" + ); +} + +/// Counterpart to the no-fallback tests: a cloud provider (responses_fallback=true) +/// MUST retry against `/v1/responses` when chat/completions returns 404. +/// This guards against an accidental inversion of the supports_responses_fallback flag. +#[tokio::test] +async fn cloud_provider_falls_back_to_responses_on_404() { + let mock_server = MockServer::start().await; + + // chat/completions returns 404 → should trigger fallback. + Mock::given(method("POST")) + .and(path("/v1/chat/completions")) + .respond_with( + ResponseTemplate::new(404) + .set_body_string(r#"{"error":{"message":"model not found","code":404}}"#), + ) + .expect(1) // exactly one attempt + .mount(&mock_server) + .await; + + // /v1/responses MUST be called — the provider should fall back to it. + Mock::given(method("POST")) + .and(path("/v1/responses")) + .respond_with( + ResponseTemplate::new(200).set_body_string( + r#"{"output":[{"content":[{"type":"output_text","text":"ok"}]}]}"#, + ), + ) + .expect(1) // must be called exactly once + .mount(&mock_server) + .await; + + // Use AuthStyle::None so no API key lookup is needed. + // The endpoint must include /v1 so that chat_completions_url() resolves to + // /v1/chat/completions and responses_url() resolves to /v1/responses. + let config = config_with_providers(vec![CloudProviderCreds { + id: "p_test".to_string(), + slug: "test-cloud".to_string(), + label: "Test Cloud".to_string(), + endpoint: format!("{}/v1", mock_server.uri()), + auth_style: AuthStyle::None, + default_model: Some("test-model".to_string()), + ..Default::default() + }]); + + let (provider, model) = + create_chat_provider_from_string("chat", "test-cloud:test-model", &config) + .expect("cloud provider must build"); + + // The call should succeed via the responses fallback. + let result = provider.chat_with_system(None, "hello", &model, 0.0).await; + + // wiremock verifies expect(1) on the responses mock when the server is dropped. + // We don't assert Ok here because the provider may return an error even after a + // successful fallback call (e.g. if the response body doesn't fully satisfy parsing). + // The important invariant is that /v1/responses was called — verified by wiremock. + drop(result); +} + #[tokio::test] #[ignore = "requires live LM Studio on localhost:1234"] async fn live_lmstudio_provider_streams_thinking_and_text() {