diff --git a/src-tauri/src/services/ai/providers.rs b/src-tauri/src/services/ai/providers.rs index 8bd90b09..594a034f 100644 --- a/src-tauri/src/services/ai/providers.rs +++ b/src-tauri/src/services/ai/providers.rs @@ -5,10 +5,163 @@ use std::time::Duration; const DEFAULT_AI_TIMEOUT_SECONDS: u64 = 120; const MIN_AI_TIMEOUT_SECONDS: u64 = 30; const MAX_AI_TIMEOUT_SECONDS: u64 = 60 * 60; +/// The most times we will auto-correct rejected sampling parameters before +/// giving up. Only `temperature` and `max_tokens` can be adjusted, so two +/// corrections is already the practical ceiling; the extra margin is a cheap +/// safety net. +/// +/// This counts *corrections*, not total requests: each correction is followed +/// by another attempt, so the worst case is `MAX_OPENAI_PARAM_ADJUSTMENTS + 1` +/// requests (the initial attempt plus one send per correction). +const MAX_OPENAI_PARAM_ADJUSTMENTS: usize = 3; + fn normalized_summary_max_tokens(custom_max_tokens: Option) -> Option { custom_max_tokens.filter(|value| *value > 0) } +/// OpenAI model families that still use the legacy Chat Completions sampling +/// parameters (`temperature` + `max_tokens`). +/// +/// Newer reasoning-style families (o1/o3/o4/gpt-5 and later) instead use +/// `max_completion_tokens` and do not allow overriding `temperature`. +/// +/// We intentionally maintain a short allowlist of legacy families because they +/// are effectively frozen, while newer OpenAI models have consistently moved to +/// the new parameter style. +const OPENAI_LEGACY_MODEL_PREFIXES: &[&str] = &[ + "gpt-3.5", + "gpt-4-", + "gpt-4o", + "gpt-4.1", + "chatgpt-4o", +]; + +fn openai_is_legacy_model(model: &str) -> bool { + let model = model.to_lowercase(); + OPENAI_LEGACY_MODEL_PREFIXES + .iter() + .any(|prefix| model.starts_with(prefix)) + || model == "gpt-4" +} + +/// Adds the appropriate sampling parameters for the selected OpenAI model. +/// +/// Legacy chat models receive: +/// - `temperature` +/// - `max_tokens` +/// +/// Newer reasoning-style models receive: +/// - `max_completion_tokens` +/// +/// This is only our default guess. If OpenAI rejects the chosen parameters, +/// `post_openai_chat()` automatically retries with the corrected ones. +fn apply_openai_sampling_params( + body: &mut serde_json::Value, + model: &str, + temperature: f64, + max_tokens: Option, +) { + if openai_is_legacy_model(model) { + body["temperature"] = serde_json::json!(temperature); + if let Some(max_tokens) = max_tokens { + body["max_tokens"] = serde_json::json!(max_tokens); + } + } else if let Some(max_tokens) = max_tokens { + body["max_completion_tokens"] = serde_json::json!(max_tokens); + } +} + +/// Inspects an OpenAI "unsupported parameter/value" error and corrects the +/// offending field in `body` in place. Returns whether anything was changed. +/// +/// Prefers the structured `error.param` field when OpenAI provides one; falls +/// back to matching the field name in the human-readable message otherwise. +/// +/// The token-limit field is swapped in whichever direction is needed: OpenAI +/// reasoning models reject `max_tokens` (swap to `max_completion_tokens`), +/// while some OpenAI-compatible proxies reject `max_completion_tokens` (swap +/// back to `max_tokens`). Matching either field name covers both cases. +fn adjust_openai_request(body: &mut serde_json::Value, error: &OpenAIErrorDetail) -> bool { + let Some(fields) = body.as_object_mut() else { + return false; + }; + + let names_temperature = |s: &str| s.contains("temperature"); + // Substring match, so this also fires for "max_completion_tokens". + let names_max_tokens = |s: &str| s.contains("max_tokens") || s.contains("max_completion_tokens"); + + let targets_temperature = error + .param + .as_deref() + .map(names_temperature) + .unwrap_or_else(|| names_temperature(&error.message)); + let targets_max_tokens = error + .param + .as_deref() + .map(names_max_tokens) + .unwrap_or_else(|| names_max_tokens(&error.message)); + + let mut adjusted = false; + if targets_temperature && fields.remove("temperature").is_some() { + adjusted = true; + } + if targets_max_tokens { + // Swap the token-limit field to whichever name we're not currently + // using, preserving its value. Only one of the two is ever present. + if let Some(value) = fields.remove("max_tokens") { + fields.insert("max_completion_tokens".to_string(), value); + adjusted = true; + } else if let Some(value) = fields.remove("max_completion_tokens") { + fields.insert("max_tokens".to_string(), value); + adjusted = true; + } + } + adjusted +} + +/// Sends a chat completion request to OpenAI. +/// +/// The request is built using our best-known parameter set for the model family. +/// If OpenAI responds that a parameter (such as `temperature` or `max_tokens`) +/// is unsupported, we adjust just that parameter and retry until no further +/// automatic correction is possible. +async fn post_openai_chat( + client: &Client, + url: &str, + api_key: &str, + mut body: serde_json::Value, +) -> Result<(reqwest::StatusCode, String), AIError> { + let send = |body: &serde_json::Value| { + client + .post(url) + .header("Content-Type", "application/json") + .header("Authorization", format!("Bearer {}", api_key)) + .json(body) + .send() + }; + + // Initial attempt plus one retry per successful parameter correction. + for adjustments_left in (0..=MAX_OPENAI_PARAM_ADJUSTMENTS).rev() { + let response = send(&body).await.map_err(|e| AIError::NetworkError(e.to_string()))?; + let status = response.status(); + let response_text = response.text().await.unwrap_or_default(); + + if status != reqwest::StatusCode::BAD_REQUEST || adjustments_left == 0 { + return Ok((status, response_text)); + } + + let Some(error) = extract_openai_compatible_error_detail(&response_text) else { + return Ok((status, response_text)); + }; + + if !adjust_openai_request(&mut body, &error) { + return Ok((status, response_text)); + } + } + + unreachable!("loop always returns on its final iteration") +} + #[cfg(test)] fn summary_max_tokens_for_config(config: &AIConfig) -> Option { normalized_summary_max_tokens(config.summary_max_tokens) @@ -59,6 +212,29 @@ fn extract_openai_compatible_error(response_text: &str) -> Option { .map(str::to_string) } +/// An OpenAI API error, with the structured `param` field (when present) alongside +/// the human-readable message. Used by `adjust_openai_request` to reliably tell which +/// request field OpenAI rejected instead of guessing from the message text. +struct OpenAIErrorDetail { + message: String, + param: Option, +} + +fn extract_openai_compatible_error_detail(response_text: &str) -> Option { + let json = serde_json::from_str::(response_text).ok()?; + let error = json.get("error")?; + let message = error + .get("message") + .and_then(|m| m.as_str()) + .or_else(|| error.as_str())? + .to_string(); + let param = error + .get("param") + .and_then(|p| p.as_str()) + .map(str::to_string); + Some(OpenAIErrorDetail { message, param }) +} + fn extract_openai_compatible_text(json: &serde_json::Value) -> Option { let choice = json.get("choices")?.get(0)?; let message = choice.get("message"); @@ -305,24 +481,17 @@ pub async fn generate_with_openai( let mut body = serde_json::json!({ "model": model, - "messages": [{ "role": "user", "content": prompt }], - "temperature": 0.7 + "messages": [{ "role": "user", "content": prompt }] }); - if let Some(max_tokens) = max_tokens { - body["max_tokens"] = serde_json::json!(max_tokens); - } - - let response = client - .post("https://api.openai.com/v1/chat/completions") - .header("Content-Type", "application/json") - .header("Authorization", format!("Bearer {}", api_key)) - .json(&body) - .send() - .await - .map_err(|e| AIError::NetworkError(e.to_string()))?; - - let status = response.status(); - let response_text = response.text().await.unwrap_or_default(); + apply_openai_sampling_params(&mut body, model, 0.7, max_tokens); + + let (status, response_text) = post_openai_chat( + &client, + "https://api.openai.com/v1/chat/completions", + api_key, + body, + ) + .await?; let summary = parse_openai_compatible_response("OpenAI", status, &response_text)?; Ok(SummaryResult { @@ -500,31 +669,15 @@ pub async fn generate_with_proxy( let url = chat_completions_url(proxy_url); + // Proxy is OpenAI-compatible and defaults to api.openai.com with a GPT-5 + // model, so it shares OpenAI's sampling-parameter handling and retry path. let mut body = serde_json::json!({ "model": model, - "messages": [{ "role": "user", "content": prompt }], - "temperature": 0.7 + "messages": [{ "role": "user", "content": prompt }] }); - if let Some(max_tokens) = max_tokens { - body["max_tokens"] = serde_json::json!(max_tokens); - } - - let response = client - .post(&url) - .header("Content-Type", "application/json") - .header("Authorization", format!("Bearer {}", api_key)) - .json(&body) - .send() - .await - .map_err(|e| { - AIError::NetworkError(format!( - "Failed to connect to proxy at {}: {}", - proxy_url, e - )) - })?; + apply_openai_sampling_params(&mut body, model, 0.7, max_tokens); - let status = response.status(); - let response_text = response.text().await.unwrap_or_default(); + let (status, response_text) = post_openai_chat(&client, &url, api_key, body).await?; let summary = parse_openai_compatible_response("Proxy", status, &response_text)?; @@ -680,24 +833,22 @@ async fn generate_raw_with_openai( let client = ai_client(timeout_seconds)?; let mut body = serde_json::json!({ "model": model, - "messages": [{ "role": "user", "content": prompt }], - "temperature": 0.3 + "messages": [{ "role": "user", "content": prompt }] }); - if let Some(max_tokens) = normalized_summary_max_tokens(summary_max_tokens) { - body["max_tokens"] = serde_json::json!(max_tokens); - } - - let response = client - .post("https://api.openai.com/v1/chat/completions") - .header("Authorization", format!("Bearer {}", api_key)) - .header("Content-Type", "application/json") - .json(&body) - .send() - .await - .map_err(|e| AIError::NetworkError(e.to_string()))?; + apply_openai_sampling_params( + &mut body, + model, + 0.3, + normalized_summary_max_tokens(summary_max_tokens), + ); - let status = response.status(); - let response_text = response.text().await.unwrap_or_default(); + let (status, response_text) = post_openai_chat( + &client, + "https://api.openai.com/v1/chat/completions", + api_key, + body, + ) + .await?; let text = parse_openai_compatible_response("OpenAI", status, &response_text)?; Ok(SummaryResult { @@ -891,26 +1042,19 @@ async fn generate_raw_with_proxy( let client = ai_client(timeout_seconds)?; let url = chat_completions_url(proxy_url); + // Proxy is OpenAI-compatible; share OpenAI's sampling params + retry path. let mut body = serde_json::json!({ "model": model, - "messages": [{ "role": "user", "content": prompt }], - "temperature": 0.3 + "messages": [{ "role": "user", "content": prompt }] }); - if let Some(max_tokens) = normalized_summary_max_tokens(summary_max_tokens) { - body["max_tokens"] = serde_json::json!(max_tokens); - } - - let response = client - .post(&url) - .header("Authorization", format!("Bearer {}", api_key)) - .header("Content-Type", "application/json") - .json(&body) - .send() - .await - .map_err(|e| AIError::NetworkError(e.to_string()))?; + apply_openai_sampling_params( + &mut body, + model, + 0.3, + normalized_summary_max_tokens(summary_max_tokens), + ); - let status = response.status(); - let response_text = response.text().await.unwrap_or_default(); + let (status, response_text) = post_openai_chat(&client, &url, api_key, body).await?; let text = parse_openai_compatible_response("Proxy", status, &response_text)?; Ok(SummaryResult { @@ -1061,4 +1205,147 @@ mod tests { assert!(matches!(error, AIError::ApiError(message) if message.contains("cut off"))); } + + // --- apply_openai_sampling_params --- + + #[test] + fn legacy_model_uses_temperature_and_max_tokens() { + let mut body = serde_json::json!({}); + apply_openai_sampling_params(&mut body, "gpt-4o", 0.7, Some(1024)); + + assert_eq!(body["temperature"], serde_json::json!(0.7)); + assert_eq!(body["max_tokens"], serde_json::json!(1024)); + assert!(body.get("max_completion_tokens").is_none()); + } + + #[test] + fn reasoning_model_uses_max_completion_tokens_without_temperature() { + let mut body = serde_json::json!({}); + apply_openai_sampling_params(&mut body, "gpt-5.5", 0.7, Some(1024)); + + assert_eq!(body["max_completion_tokens"], serde_json::json!(1024)); + assert!(body.get("temperature").is_none()); + assert!(body.get("max_tokens").is_none()); + } + + #[test] + fn reasoning_model_omits_token_limit_when_unset() { + let mut body = serde_json::json!({}); + apply_openai_sampling_params(&mut body, "o3", 0.7, None); + + assert!(body.get("max_completion_tokens").is_none()); + assert!(body.get("temperature").is_none()); + assert!(body.get("max_tokens").is_none()); + } + + #[test] + fn legacy_model_detection_is_case_insensitive() { + assert!(openai_is_legacy_model("GPT-4o")); + assert!(openai_is_legacy_model("gpt-4")); + assert!(openai_is_legacy_model("gpt-3.5-turbo")); + assert!(!openai_is_legacy_model("gpt-5.5")); + assert!(!openai_is_legacy_model("o3-mini")); + } + + // --- adjust_openai_request --- + + fn error_with_param(message: &str, param: Option<&str>) -> OpenAIErrorDetail { + OpenAIErrorDetail { + message: message.to_string(), + param: param.map(str::to_string), + } + } + + #[test] + fn adjust_removes_rejected_temperature() { + let mut body = serde_json::json!({ + "temperature": 0.7, + "max_tokens": 1024 + }); + let error = error_with_param( + "Unsupported value: 'temperature' does not support 0.7", + Some("temperature"), + ); + + assert!(adjust_openai_request(&mut body, &error)); + assert!(body.get("temperature").is_none()); + // Untargeted fields are left alone. + assert_eq!(body["max_tokens"], serde_json::json!(1024)); + } + + #[test] + fn adjust_converts_rejected_max_tokens_to_max_completion_tokens() { + let mut body = serde_json::json!({ + "max_tokens": 1024 + }); + let error = error_with_param( + "Unsupported parameter: 'max_tokens' is not supported with this model. Use 'max_completion_tokens' instead.", + Some("max_tokens"), + ); + + assert!(adjust_openai_request(&mut body, &error)); + assert!(body.get("max_tokens").is_none()); + assert_eq!(body["max_completion_tokens"], serde_json::json!(1024)); + } + + #[test] + fn adjust_converts_rejected_max_completion_tokens_back_to_max_tokens() { + // An OpenAI-compatible proxy that only understands the legacy field. + let mut body = serde_json::json!({ + "max_completion_tokens": 1024 + }); + let error = error_with_param( + "Unrecognized request argument supplied: max_completion_tokens", + Some("max_completion_tokens"), + ); + + assert!(adjust_openai_request(&mut body, &error)); + assert!(body.get("max_completion_tokens").is_none()); + assert_eq!(body["max_tokens"], serde_json::json!(1024)); + } + + #[test] + fn adjust_falls_back_to_message_when_param_absent() { + let mut body = serde_json::json!({ + "temperature": 0.7 + }); + let error = error_with_param( + "This model does not support setting temperature.", + None, + ); + + assert!(adjust_openai_request(&mut body, &error)); + assert!(body.get("temperature").is_none()); + } + + #[test] + fn adjust_reports_no_change_when_field_already_absent() { + // OpenAI names max_tokens, but the body has neither token field. + let mut body = serde_json::json!({ + "temperature": 0.7 + }); + let error = error_with_param("max_tokens is not supported", Some("max_tokens")); + + assert!(!adjust_openai_request(&mut body, &error)); + // Nothing was touched. + assert_eq!(body["temperature"], serde_json::json!(0.7)); + } + + #[test] + fn adjust_removes_temperature_and_swaps_tokens_together() { + let mut body = serde_json::json!({ + "temperature": 0.7, + "max_tokens": 512 + }); + // Some models complain about both fields in one message. + let error = error_with_param( + "temperature is unsupported and max_tokens must be max_completion_tokens", + None, + ); + + assert!(adjust_openai_request(&mut body, &error)); + assert!(body.get("temperature").is_none()); + assert!(body.get("max_tokens").is_none()); + assert_eq!(body["max_completion_tokens"], serde_json::json!(512)); + } }