mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
This commit is contained in:
@@ -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);
|
||||
|
||||
@@ -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<f64> {
|
||||
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}")))
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user