diff --git a/src/openhuman/inference/provider/compatible_provider_impl.rs b/src/openhuman/inference/provider/compatible_provider_impl.rs index c9c615474..ba11df5db 100644 --- a/src/openhuman/inference/provider/compatible_provider_impl.rs +++ b/src/openhuman/inference/provider/compatible_provider_impl.rs @@ -7,7 +7,6 @@ use futures_util::{stream, StreamExt}; use super::compatible_dump::{dump_prompt_if_enabled, dump_response_if_enabled, reserve_dump_seq}; use super::compatible_parse::normalize_function_arguments; -use super::compatible_repeat::CHAT_FREQUENCY_PENALTY; use super::compatible_stream::sse_bytes_to_chunks; use super::compatible_types::{ ApiChatRequest, ApiChatResponse, Message, MessageContent, NativeChatRequest, @@ -512,7 +511,10 @@ impl Provider for OpenAiCompatibleProvider { include_usage: true, }), options: self.build_ollama_options(), - frequency_penalty: Some(CHAT_FREQUENCY_PENALTY), + // Omitted for endpoints whose OpenAI-compat surface 400s on the + // field (Google Gemini shim — TAURI-RUST-4PJ); the reactive + // retry below stays as defense-in-depth for unknown providers. + frequency_penalty: self.effective_frequency_penalty(), }; let stream_dump_seq = reserve_dump_seq(); dump_prompt_if_enabled(&self.name, model, stream_dump_seq, &native_request); diff --git a/src/openhuman/inference/provider/compatible_request.rs b/src/openhuman/inference/provider/compatible_request.rs index 67d0d62b9..d1bf3b21c 100644 --- a/src/openhuman/inference/provider/compatible_request.rs +++ b/src/openhuman/inference/provider/compatible_request.rs @@ -42,6 +42,28 @@ impl OpenAiCompatibleProvider { } } + /// Resolve the `frequency_penalty` to send on streaming chat requests. + /// + /// Returns `None` — omitting the field entirely — for endpoints whose + /// OpenAI-compatible surface rejects the parameter with an HTTP 400 + /// (see [`endpoint_rejects_frequency_penalty`]). Google's Gemini shim + /// (`generativelanguage.googleapis.com/v1beta/openai`) is the known case: + /// it 400s on the unknown field, which previously forced every streaming + /// call into a wasted reject→retry round-trip and one Sentry report + /// (TAURI-RUST-4PJ). Omitting it up front removes the failing request at + /// the source. Every other provider keeps the configured penalty. + pub(super) fn effective_frequency_penalty(&self) -> Option { + if endpoint_rejects_frequency_penalty(&self.base_url) { + tracing::debug!( + "[provider:{}] endpoint rejects frequency_penalty — omitting it", + self.name + ); + None + } else { + Some(super::compatible_repeat::CHAT_FREQUENCY_PENALTY) + } + } + /// Read the ambient `thread_id` only when this provider has been /// opted in via [`with_openhuman_thread_id`]. Returns `None` for /// every third-party provider so the field is omitted by @@ -322,3 +344,31 @@ impl OpenAiCompatibleProvider { .then_some((OPENROUTER_REFERER, OPENROUTER_TITLE)) } } + +/// Endpoint hosts whose OpenAI-compatible surface rejects the +/// `frequency_penalty` sampling field with an HTTP 400. Matched against the +/// request host (exact or as a registrable-domain suffix), so a BYOK provider +/// pointed at the same upstream is covered too. Single source of truth — add a +/// host here if another strict endpoint surfaces the same rejection. +const FREQUENCY_PENALTY_UNSUPPORTED_HOSTS: &[&str] = &["generativelanguage.googleapis.com"]; + +/// Whether `base_url`'s host is known to reject `frequency_penalty`. +/// +/// Google's Gemini OpenAI-compat shim +/// (`https://generativelanguage.googleapis.com/v1beta/openai`) 400s on the +/// unknown field (`Unknown name "frequency_penalty": Cannot find field`), +/// which previously forced every streaming call into a wasted reject→retry +/// and one Sentry report per first attempt (TAURI-RUST-4PJ). Detecting it by +/// host — rather than provider slug — also covers BYOK providers configured +/// against the same endpoint. +pub(super) fn endpoint_rejects_frequency_penalty(base_url: &str) -> bool { + let host = reqwest::Url::parse(base_url) + .ok() + .and_then(|url| url.host_str().map(str::to_ascii_lowercase)); + let Some(host) = host else { + return false; + }; + FREQUENCY_PENALTY_UNSUPPORTED_HOSTS + .iter() + .any(|known| host == *known || host.ends_with(&format!(".{known}"))) +} diff --git a/src/openhuman/inference/provider/compatible_stream_native.rs b/src/openhuman/inference/provider/compatible_stream_native.rs index 0e4314c52..cce09b2d3 100644 --- a/src/openhuman/inference/provider/compatible_stream_native.rs +++ b/src/openhuman/inference/provider/compatible_stream_native.rs @@ -94,6 +94,18 @@ impl OpenAiCompatibleProvider { self.name, status, ); + } else if Self::err_indicates_frequency_penalty_unsupported(&body) { + // Endpoint rejects `frequency_penalty` (e.g. an unknown strict + // provider not yet covered by `effective_frequency_penalty`). + // The caller retries without the field and succeeds, so this is + // a self-healed recoverable condition — log, don't page + // (TAURI-RUST-4PJ). Defense-in-depth behind the prevent-at-source + // omission; the bail! below still drives the retry path. + log::info!( + "[stream] {} rejected frequency_penalty (status={}) — caller will retry without it", + self.name, + status, + ); } else if super::super::is_backend_error_code_owned(self.name.as_str(), &body) { // F4/F2: managed-backend errorCode (#870) — backend-owned, FE // must not double-report. Malformed BAD_REQUEST is excluded and diff --git a/src/openhuman/inference/provider/compatible_tests.rs b/src/openhuman/inference/provider/compatible_tests.rs index 260e7d417..1e123a333 100644 --- a/src/openhuman/inference/provider/compatible_tests.rs +++ b/src/openhuman/inference/provider/compatible_tests.rs @@ -146,6 +146,127 @@ fn detects_frequency_penalty_rejection_for_retry() { )); } +#[test] +fn endpoint_rejects_frequency_penalty_matches_google_gemini_host() { + use super::compatible_request::endpoint_rejects_frequency_penalty as rejects; + // The Google Gemini OpenAI-compat shim 400s on the field (TAURI-RUST-4PJ). + assert!(rejects( + "https://generativelanguage.googleapis.com/v1beta/openai" + )); + // Host match is case-insensitive and covers a registrable-domain suffix, + // so a BYOK provider pointed at a regional/sub-host is covered too. + assert!(rejects( + "https://GenerativeLanguage.GoogleAPIs.com/v1beta/openai" + )); + assert!(rejects( + "https://eu.generativelanguage.googleapis.com/v1beta/openai" + )); + // Every other provider keeps the penalty; an unparseable URL is a no-op. + assert!(!rejects("https://api.openai.com/v1")); + assert!(!rejects("https://api.venice.ai")); + // A look-alike host must NOT match (suffix check is dot-anchored). + assert!(!rejects( + "https://notgenerativelanguage.googleapis.com.evil.test/v1" + )); + assert!(!rejects("not a url")); +} + +#[test] +fn effective_frequency_penalty_omitted_for_google_kept_for_others() { + // Google Gemini endpoint → field omitted at the source (no rejected + // round-trip, no Sentry report). + let google = OpenAiCompatibleProvider::new( + "google", + "https://generativelanguage.googleapis.com/v1beta/openai", + None, + AuthStyle::Bearer, + ); + assert_eq!( + google.effective_frequency_penalty(), + None, + "Gemini shim rejects frequency_penalty — it must be omitted up front" + ); + + // Any other OpenAI-compatible provider keeps the repetition-damping value. + let other = make_provider("openai", "https://api.openai.com/v1", None); + assert_eq!( + other.effective_frequency_penalty(), + Some(super::compatible_repeat::CHAT_FREQUENCY_PENALTY), + "providers that accept the field must still receive it" + ); +} + +#[tokio::test] +async fn streaming_chat_frequency_penalty_rejection_not_reported_to_sentry() { + // Defense-in-depth (TAURI-RUST-4PJ): an unknown strict provider — one not + // covered by the host allow-list, so prevention did not omit the field — + // 400s on frequency_penalty. The caller retries without it and succeeds, so + // this self-healed condition must NOT page Sentry, while still propagating + // as an Err so the retry path fires. + let mock_server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/chat/completions")) + .respond_with(ResponseTemplate::new(400).set_body_string( + r#"{"error":{"code":400,"message":"Invalid JSON payload received. Unknown name \"frequency_penalty\": Cannot find field.","status":"INVALID_ARGUMENT"}}"#, + )) + .mount(&mock_server) + .await; + + let transport = TestTransport::new(); + let sentry_options = sentry::ClientOptions { + dsn: Some("https://public@sentry.invalid/1".parse().unwrap()), + transport: Some(Arc::new(transport.clone())), + ..Default::default() + }; + let sentry_hub = Arc::new(sentry::Hub::new( + Some(Arc::new(sentry_options.into())), + Arc::new(Default::default()), + )); + let _sentry_guard = sentry::HubSwitchGuard::new(sentry_hub); + + // Provider URL is the mock host (not the google allow-list host), so the + // request DOES carry frequency_penalty and exercises the stream-error + // classifier arm rather than the prevent-at-source omission. + let provider = + OpenAiCompatibleProvider::new("strict_byok", &mock_server.uri(), None, AuthStyle::None); + let request = NativeChatRequest { + model: "some-model".to_string(), + messages: vec![NativeMessage { + role: "user".to_string(), + content: Some("hello".into()), + tool_call_id: None, + tool_calls: None, + reasoning_content: None, + }], + temperature: Some(0.7), + stream: Some(true), + tools: None, + tool_choice: None, + thread_id: None, + stream_options: Some(super::compatible_types::OpenAiStreamOptions { + include_usage: true, + }), + options: None, + frequency_penalty: Some(super::compatible_repeat::CHAT_FREQUENCY_PENALTY), + }; + let (delta_tx, _delta_rx) = tokio::sync::mpsc::channel(8); + + let err = provider + .stream_native_chat(None, &request, &delta_tx, 0) + .await + .expect_err( + "400 frequency_penalty rejection must still propagate as Err to drive the retry", + ); + assert!( + err.to_string().contains("streaming API error"), + "err: {err}" + ); + assert!( + transport.fetch_and_clear_events().is_empty(), + "a self-healed frequency_penalty rejection must not be reported to Sentry" + ); +} + /// Streaming responses arrive without `usage` unless the request asks /// for `stream_options.include_usage = true` (OpenAI spec). Without it /// the OpenHuman backend's `openhuman.billing` block also never lands,