diff --git a/src/openhuman/channels/providers/web_errors.rs b/src/openhuman/channels/providers/web_errors.rs index c23c000ba..d1e04b417 100644 --- a/src/openhuman/channels/providers/web_errors.rs +++ b/src/openhuman/channels/providers/web_errors.rs @@ -305,7 +305,7 @@ pub(crate) fn classify_inference_error(err: &str) -> ClassifiedError { // before the generic provider-429 branch — otherwise users see // a confusing "your AI provider is rate-limiting you" message // for limits OpenHuman itself enforced (issue #2364). - if is_action_budget_exhausted(&lower) { + let classified = if is_action_budget_exhausted(&lower) { ClassifiedError { error_type: "action_budget_exceeded", message: with_provider_detail( @@ -341,6 +341,30 @@ pub(crate) fn classify_inference_error(err: &str) -> ClassifiedError { provider, fallback_available: None, } + } else if is_empty_provider_response_text(&lower) { + // The agent harness bailed because the provider/model completed a + // turn with a completely empty body (text_chars=0 thinking_chars=0 + // tool_calls=0) — `AgentError::EmptyProviderResponse`, flattened to + // a `String` at the native-bus boundary. Without this arm the + // message falls through to the generic catch-all and the user sees a + // bare "Something went wrong" with no remedy (Sentry TAURI-RUST-4JW, + // the single largest source of the #3092 / #3119 chat-error + // cluster). Placed early next to max_iterations: both are + // deterministic agent-state outcomes with a specific anchor, so + // neither can be shadowed by the broad provider-429 / 5xx arms below. + // No `with_provider_detail` — an empty response carries no JSON body + // to quote. + ClassifiedError { + error_type: "empty_response", + message: "The model returned an empty response. Try a different model or check \ + your local provider in Settings → AI → LLM." + .to_string(), + source: "agent_loop", + retryable: true, + retry_after_ms: None, + provider: None, + fallback_available: None, + } } else if lower.contains("rate limit") || lower.contains("429") { let retry_secs = parse_retry_after_secs_from_str(err); // Non-retryable business 429s ("plan does not include", balance @@ -500,6 +524,48 @@ pub(crate) fn classify_inference_error(err: &str) -> ClassifiedError { provider, fallback_available: None, } + } else if lower.contains("does not support vision") || lower.contains("capability=vision") { + // A multimodal turn sent image markers to a text-only model + // (`provider_capability_error … capability=vision … does not support + // vision input`, raised in agent/harness/engine/core.rs). Without + // this arm it dead-ends on the generic catch-all. Retrying the same + // image against the same model can't help — the user must drop the + // attachment or pick a vision-capable model, so this is non-retryable. + ClassifiedError { + error_type: "capability_unsupported", + message: "This model can't process images. Remove the attachment or switch to a \ + vision-capable model in Settings → AI → LLM." + .to_string(), + source: "config", + retryable: false, + retry_after_ms: None, + provider: None, + fallback_available: None, + } + } else if is_provider_request_rejected_text(&lower) { + // A provider rejected the request with a 4xx that none of the + // specific arms above claimed (generic 400 Bad Request, 404, 422). + // The DeepSeek thinking-mode `reasoning_content` round-trip 400 + // (deeper fix tracked separately in #3197) and other model/parameter + // incompatibilities land here. MUST stay below the + // provider-config-rejection (invalid temperature, stale model pin) + // and model-unavailable arms so their more specific 4xx verdicts win + // first. 4xx is a client/request problem — identical retry fails, so + // non-retryable. The real provider reason is already secret-scrubbed + // and length-capped by `with_provider_detail` and quoted to the user. + ClassifiedError { + error_type: "provider_request_rejected", + message: with_provider_detail( + "The AI provider rejected the request — this is usually a model or \ + parameter incompatibility. Try a different model in Settings → AI → LLM.", + err, + ), + source: "provider", + retryable: false, + retry_after_ms: None, + provider, + fallback_available, + } } else { ClassifiedError { error_type: "inference", @@ -510,7 +576,67 @@ pub(crate) fn classify_inference_error(err: &str) -> ClassifiedError { provider, fallback_available, } - } + }; + + // Verbose diagnostics on the classification flow (per CLAUDE.md). Stable + // grep-friendly prefix + low-cardinality fields only — the raw `err` (which + // may carry provider payload / PII) is intentionally NOT logged here; the + // caller (`web.rs::run_chat_task`) already records it at warn level and + // routes it through `report_error_or_expected`. + log::debug!( + "[chat-error][classify] error_type={} source={} retryable={} provider={:?}", + classified.error_type, + classified.source, + classified.retryable, + classified.provider, + ); + + classified +} + +/// String-flat mirror of +/// `crate::core::observability::is_empty_provider_response_message`. +/// +/// The typed `AgentError::EmptyProviderResponse` is collapsed to a `String` +/// at the native-bus boundary before reaching this layer, so we re-detect +/// the same canonical phrase the agent harness emits. Anchored on +/// `"model returned an empty response"` (the verbatim user-facing string from +/// `AgentError::EmptyProviderResponse`) — NOT the looser `"empty response"`, +/// so internal fall-through phrases (`"summarizer returned empty response"`, +/// `"provider returned an empty response; returning empty extraction"`) are +/// not misclassified. Keep the anchor in lockstep with the observability +/// mirror. +/// +/// Caller passes the already-lowercased error string. +pub(crate) fn is_empty_provider_response_text(lower: &str) -> bool { + lower.contains("model returned an empty response") +} + +/// Detect an un-claimed provider 4xx (generic client-side request rejection). +/// +/// Mirrors the status tokens emitted by `inference::provider::ops::api_error` +/// (`" API error (400 Bad Request): …"`). Ordered AFTER the +/// provider-config-rejection and model-unavailable arms in +/// [`classify_inference_error`], so only 4xx shapes those arms did not claim +/// reach this predicate. +/// +/// Caller passes the already-lowercased error string. +pub(crate) fn is_provider_request_rejected_text(lower: &str) -> bool { + // Match only when the 4xx status appears inside a provider error envelope + // (` API error (4xx …)`, emitted by + // `inference::provider::ops::api_error`). Matching a bare "400"/"404" + // anywhere would misclassify unrelated errors that merely contain those + // digits (token counts, byte offsets, timestamps). Per CodeRabbit review + // on PR #3199. + const PROVIDER_4XX_MARKERS: &[&str] = &[ + "api error (400", + "api error (404", + "api error (409", + "api error (422", + ]; + PROVIDER_4XX_MARKERS + .iter() + .any(|marker| lower.contains(marker)) } /// String-flat mirror of diff --git a/src/openhuman/channels/providers/web_tests.rs b/src/openhuman/channels/providers/web_tests.rs index 7400c84dd..18eb3a136 100644 --- a/src/openhuman/channels/providers/web_tests.rs +++ b/src/openhuman/channels/providers/web_tests.rs @@ -894,6 +894,125 @@ fn generic_error_copy_is_sanitized_and_has_discord_report_action() { )); } +#[test] +fn classify_inference_error_empty_response_is_actionable_and_retryable() { + // #3092 / #3119: the dominant chat-error cause (Sentry TAURI-RUST-4JW, + // 986+ events). An empty provider completion must get an actionable, + // retryable message — NOT the generic "Something went wrong" dead-end. + let raw = "run_chat_task failed client_id=abc thread_id=t-1 request_id=r-1 \ + error=The model returned an empty response. Please try again."; + let classified = classify_inference_error(raw); + assert_eq!(classified.error_type, "empty_response"); + assert_ne!( + classified.error_type, "inference", + "empty response must NOT fall through to the generic catch-all" + ); + assert!( + classified.retryable, + "empty response is transiently retryable" + ); + assert_eq!(classified.source, "agent_loop"); + assert!( + classified.message.contains("Settings → AI → LLM"), + "must give the actionable model-switch remedy: {}", + classified.message + ); + assert!( + !classified.message.contains("Something went wrong"), + "must not be the generic apology: {}", + classified.message + ); +} + +#[test] +fn classify_inference_error_vision_capability_is_non_retryable() { + // A multimodal turn sent an image to a text-only model. Retrying the + // same image+model can't help, so non-retryable with a switch-model hint. + let raw = "provider_capability_error provider=web_channel capability=vision \ + message=received 1 image marker(s), but this provider does not support vision input"; + let classified = classify_inference_error(raw); + assert_eq!(classified.error_type, "capability_unsupported"); + assert!( + !classified.retryable, + "same image + text-only model always fails" + ); + assert!( + classified.message.contains("vision-capable model"), + "must point the user at a vision model: {}", + classified.message + ); +} + +#[test] +fn classify_inference_error_generic_4xx_surfaces_provider_detail() { + // A provider 400 none of the specific arms claimed: the real reason must + // be quoted (via with_provider_detail) under a friendly, non-retryable + // summary instead of the generic dead-end. + let raw = r#"cloud API error (400 Bad Request): {"error":{"message":"tool_calls.id and tool_calls.type are required","type":"input_invalid"}}"#; + let classified = classify_inference_error(raw); + assert_eq!(classified.error_type, "provider_request_rejected"); + assert!( + !classified.retryable, + "4xx request rejection is not retryable" + ); + assert!( + classified.message.contains("Try a different model"), + "friendly summary present: {}", + classified.message + ); + assert!( + classified.message.contains("tool_calls.id"), + "must quote the real provider reason: {}", + classified.message + ); +} + +#[test] +fn classify_inference_error_deepseek_reasoning_400_stays_config_rejection() { + // ORDERING LOCK: the DeepSeek / Moonshot thinking-mode reasoning_content + // round-trip 400 is ALREADY claimed by the provider-config-rejection arm + // (the "thinking mode must be passed back" phrase, Sentry TAURI-RUST-2G / + // -2F), which is ordered BEFORE the generic 4xx arm. So it must keep its + // specific, actionable `model_unavailable` + Settings → LLM verdict and + // NOT be downgraded to the generic provider_request_rejected copy. The + // deeper round-trip fix (so the turn actually succeeds) is tracked in + // #3197; this only asserts the user-facing classification stays specific. + let raw = r#"cloud API error (400 Bad Request): {"error":{"message":"The reasoning_content in the thinking mode must be passed back","type":"invalid_request_error"}}"#; + let classified = classify_inference_error(raw); + assert_eq!( + classified.error_type, "model_unavailable", + "DeepSeek reasoning_content 400 must stay config-rejection, not generic 4xx" + ); + assert_ne!(classified.error_type, "inference"); +} + +#[test] +fn classify_inference_error_invalid_temperature_400_stays_config_rejection() { + // ORDERING LOCK: a 400 carrying the #2076 "invalid temperature" body must + // keep its specific provider-config-rejection verdict (model_unavailable + + // Settings → LLM remediation) and NOT be stolen by the generic 4xx arm, + // which is ordered after it. + let raw = r#"custom_openai API error (400 Bad Request): {"error":{"message":"invalid temperature: only 1 is allowed for this model","type":"invalid_request_error"}}"#; + let classified = classify_inference_error(raw); + assert_eq!( + classified.error_type, "model_unavailable", + "invalid-temperature 400 must stay config-rejection, not generic 4xx" + ); + assert!(classified.message.contains("Settings → LLM")); +} + +#[test] +fn classify_inference_error_model_not_found_404_stays_model_unavailable() { + // ORDERING LOCK: a 404 "model does not exist" must keep its specific + // model_unavailable verdict and NOT be stolen by the generic 4xx arm. + let raw = r#"custom_openai API error (404 Not Found): {"error":{"message":"The model `gpt-5.5` does not exist or you do not have access to it.","code":"model_not_found"}}"#; + let classified = classify_inference_error(raw); + assert_eq!( + classified.error_type, "model_unavailable", + "model-not-found 404 must stay model_unavailable, not generic 4xx" + ); +} + // ── Schema catalog ──────────────────────────────────────────── #[test]