fix(channels): classify empty-response, 4xx and vision chat errors with actionable copy (#3119) (#3199)

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`
+/// (`"<provider> 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
+    // (`<provider> 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

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]