From 4adf6b6c86cce98d5182e4ea138427da56ee79dc Mon Sep 17 00:00:00 2001 From: CodeGhost21 <164498022+CodeGhost21@users.noreply.github.com> Date: Thu, 28 May 2026 17:44:08 +0530 Subject: [PATCH] fix(agent): suppress empty-provider-response from Sentry (TAURI-RUST-4JX) (#2790) --- src/openhuman/agent/error.rs | 114 ++++++++++++++++++ .../agent/harness/session/runtime.rs | 28 +++-- src/openhuman/agent/harness/session/turn.rs | 15 ++- src/openhuman/cron/scheduler.rs | 3 + 4 files changed, 145 insertions(+), 15 deletions(-) diff --git a/src/openhuman/agent/error.rs b/src/openhuman/agent/error.rs index 5d57747df..d77283fff 100644 --- a/src/openhuman/agent/error.rs +++ b/src/openhuman/agent/error.rs @@ -31,6 +31,17 @@ pub enum AgentError { /// Typically indicates an infinite loop in the model's reasoning. MaxIterationsExceeded { max: usize }, + /// The provider's chat completion contained no text, no thinking, and + /// no tool calls — a degenerate / poisoned response. Typically observed + /// with flaky local model fine-tunes (e.g. community quantizations of + /// Qwen/Llama via LM Studio or Ollama). Surfaced as a user-facing + /// error instead of a silent blank reply (defense-in-depth from + /// `agent/harness/session/turn.rs`) but suppressed from Sentry — it's + /// a provider/user-state outcome, not an OpenHuman bug, and a deeper + /// fix lives in the model / provider config the user chose. Targets + /// Sentry TAURI-RUST-4JX (~33 events, escalating on 0.56.0). + EmptyProviderResponse { iteration: usize }, + /// Automated history compaction (summarization) failed. CompactionFailed { message: String, @@ -78,6 +89,12 @@ impl fmt::Display for AgentError { Self::MaxIterationsExceeded { max } => { write!(f, "{MAX_ITERATIONS_ERROR_PREFIX} ({max})") } + Self::EmptyProviderResponse { .. } => { + // Verbatim user-facing string from the old + // `agent/harness/session/turn.rs` emit site — UI / tests + // grep for this exact byte sequence. + write!(f, "The model returned an empty response. Please try again.") + } Self::CompactionFailed { message, consecutive_failures, @@ -111,6 +128,31 @@ impl std::error::Error for AgentError { } } +impl AgentError { + /// User/provider-state outcomes that the UI already surfaces to the + /// user and that no developer can act on from Sentry — `run_single` + /// suppresses their Sentry emission (`log::info!` only) while still + /// returning the `Err` so the existing `AgentError` + `recoverable` + /// semantics are preserved. + /// + /// - `MaxIterationsExceeded`: deterministic tool-loop cap, drives + /// OPENHUMAN-TAURI-99 / -98 suppression. + /// - `EmptyProviderResponse`: degenerate/poisoned chat completion, + /// drives TAURI-RUST-4JX suppression. + /// + /// Other variants are real failures (`ProviderError` upstream HTTP / + /// network, `ToolExecutionError` callable bug, `ContextLimitExceeded` + /// compaction gap, `CostBudgetExceeded`, `CompactionFailed`, + /// `PermissionDenied` config bug, `Other` escape hatch) and must + /// continue to escalate. + pub fn skips_sentry(&self) -> bool { + matches!( + self, + Self::MaxIterationsExceeded { .. } | Self::EmptyProviderResponse { .. } + ) + } +} + impl From for AgentError { fn from(e: anyhow::Error) -> Self { // Attempt to recover a typed AgentError that was wrapped in anyhow. @@ -252,4 +294,76 @@ mod tests { assert!(matches!(other, AgentError::Other(_))); assert!(other.source().is_some()); } + + // ── AgentError::EmptyProviderResponse (TAURI-RUST-4JX) ────────────────── + // + // `agent::harness::session::turn` returns this variant when the provider's + // chat completion contains no text, no thinking, and no tool calls (a + // degenerate/poisoned response — typically a flaky local model). The + // variant was added so `run_single` can route it through `skips_sentry()` + // and demote like `MaxIterationsExceeded`, keeping TAURI-RUST-4JX off + // Sentry while preserving the user-visible error and the `Err` propagation + // contract. + + #[test] + fn empty_provider_response_display_matches_user_facing_string() { + // The exact wire string is anchored: the UI surfaces it verbatim to + // the user, and the emit-site comment at + // `agent/harness/session/turn.rs:801` (the warn breadcrumb) explicitly + // calls out the "surfacing as error instead of a silent blank reply" + // contract. Any change to this byte string is a user-visible message + // change and a Sentry-fingerprint change. + let err = AgentError::EmptyProviderResponse { iteration: 1 }; + assert_eq!( + err.to_string(), + "The model returned an empty response. Please try again." + ); + } + + #[test] + fn skips_sentry_returns_true_for_known_user_state_variants() { + // The two variants that represent user/provider state rather than a + // code bug — `run_single` suppresses both from Sentry while still + // returning `Err` so the user sees the failure. + assert!(AgentError::MaxIterationsExceeded { max: 10 }.skips_sentry()); + assert!(AgentError::EmptyProviderResponse { iteration: 1 }.skips_sentry()); + } + + #[test] + fn skips_sentry_returns_false_for_real_failures() { + // Every other variant represents either an actionable bug, an + // upstream provider/network failure that triage cares about, or a + // CompactionFailed that already has its own follow-up logic — none + // of them should silently disappear from Sentry. + let real_failures = [ + AgentError::ProviderError { + message: "boom".into(), + retryable: true, + }, + AgentError::ContextLimitExceeded { + utilization_pct: 98, + }, + AgentError::ToolExecutionError { + tool_name: "shell".into(), + message: "denied".into(), + }, + AgentError::CostBudgetExceeded { + spent_microdollars: 1_000, + budget_microdollars: 500, + }, + AgentError::CompactionFailed { + message: "summary failed".into(), + consecutive_failures: 2, + }, + AgentError::PermissionDenied { + tool_name: "shell".into(), + required_level: "Execute".into(), + channel_max_level: "ReadOnly".into(), + }, + AgentError::Other(anyhow::anyhow!("plain failure")), + ]; + for err in real_failures { + assert!(!err.skips_sentry(), "must NOT skip Sentry for: {err}"); + } + } } diff --git a/src/openhuman/agent/harness/session/runtime.rs b/src/openhuman/agent/harness/session/runtime.rs index 3515906fe..0a6ccd3fe 100644 --- a/src/openhuman/agent/harness/session/runtime.rs +++ b/src/openhuman/agent/harness/session/runtime.rs @@ -391,6 +391,7 @@ impl Agent { Some(AgentError::ToolExecutionError { .. }) => Some("tool_execution_error"), Some(AgentError::CostBudgetExceeded { .. }) => Some("cost_budget_exceeded"), Some(AgentError::MaxIterationsExceeded { .. }) => Some("max_iterations_exceeded"), + Some(AgentError::EmptyProviderResponse { .. }) => Some("empty_provider_response"), Some(AgentError::CompactionFailed { .. }) => Some("compaction_failed"), Some(AgentError::PermissionDenied { .. }) => Some("permission_denied"), Some(AgentError::Other(_)) | None => None, @@ -519,12 +520,16 @@ impl Agent { } Err(err) => { let sanitized_message = Self::sanitize_event_error_message(&err); - // The max-tool-iterations cap is a deterministic agent-state - // outcome, not a bug — the UI already surfaces the failure - // to the user via the chat-rendered "Error: Agent exceeded - // maximum tool iterations" message. Skip the Sentry funnel - // for this variant entirely and emit a structured - // `log::info!` (OPENHUMAN-TAURI-99 / -98). + // Some typed `AgentError` variants represent agent / user / + // provider state that the UI already surfaces — the + // max-tool-iterations cap (OPENHUMAN-TAURI-99 / -98, + // chat-rendered "Error: Agent exceeded maximum tool + // iterations") and the empty-provider-response degeneracy + // (TAURI-RUST-4JX, "The model returned an empty response. + // Please try again."). Skip the Sentry funnel for both + // and emit a structured `log::info!` instead. The + // suppressed set is owned by `AgentError::skips_sentry()` + // so the policy stays in one place. // // Other agent errors go through `report_error_or_expected` // so OPENHUMAN-TAURI-5Z and the budget-noise cluster — @@ -534,14 +539,13 @@ impl Agent { // warn/info-level breadcrumb without losing genuine bugs. // `Err` propagation, the `AgentError` domain event, and // downstream `recoverable=false` semantics are preserved. - let is_max_iter = matches!( - err.downcast_ref::(), - Some(AgentError::MaxIterationsExceeded { .. }) - ); - if is_max_iter { + let skips_sentry = err + .downcast_ref::() + .is_some_and(AgentError::skips_sentry); + if skips_sentry { log::info!( target: "agent", - "[agent.run_single] suppressed Sentry emission for max-iteration cap \ + "[agent.run_single] suppressed Sentry emission for user-state agent error \ session_id={} channel={} error_kind={} message={}", self.event_session_id(), self.event_channel(), diff --git a/src/openhuman/agent/harness/session/turn.rs b/src/openhuman/agent/harness/session/turn.rs index ac749065a..3aa8a0d07 100644 --- a/src/openhuman/agent/harness/session/turn.rs +++ b/src/openhuman/agent/harness/session/turn.rs @@ -21,6 +21,7 @@ use super::transcript; use super::types::Agent; use crate::core::event_bus::{publish_global, DomainEvent}; use crate::openhuman::agent::dispatcher::{ParsedToolCall, ToolExecutionResult}; +use crate::openhuman::agent::error::AgentError; use crate::openhuman::agent::harness; use crate::openhuman::agent::hooks::{self, ToolCallRecord, TurnContext}; use crate::openhuman::agent::memory_loader::collect_recall_citations; @@ -811,9 +812,17 @@ impl Agent { "[agent_loop] provider returned an empty final response (i={}, no text, no tool calls) — surfacing as error instead of a silent blank reply", iteration + 1 ); - return Err(anyhow::anyhow!( - "The model returned an empty response. Please try again." - )); + // Typed variant so `run_single` can route this + // through `AgentError::skips_sentry()` and demote + // to a `log::info!` instead of escalating to + // Sentry (TAURI-RUST-4JX). The `Display` impl + // still renders the canonical user-facing string + // for UI surfaces, so the user behaviour is + // unchanged. + return Err(AgentError::EmptyProviderResponse { + iteration: iteration + 1, + } + .into()); } log::info!( "[agent] no tool calls — returning final response after {} iteration(s)", diff --git a/src/openhuman/cron/scheduler.rs b/src/openhuman/cron/scheduler.rs index 52e15e74a..b4086bc4e 100644 --- a/src/openhuman/cron/scheduler.rs +++ b/src/openhuman/cron/scheduler.rs @@ -51,6 +51,9 @@ fn agent_error_to_user_message(err: &AgentError) -> &'static str { AgentError::MaxIterationsExceeded { .. } => { "The agent stopped after too many tool iterations. Raise the iteration cap in Settings \u{2192} AI \u{2192} LLM or simplify the task." } + AgentError::EmptyProviderResponse { .. } => { + "The model returned an empty response. Try a different model or check your local provider in Settings \u{2192} AI \u{2192} LLM." + } AgentError::CompactionFailed { .. } => { "Automatic history compaction failed. The next run will start with a fresh context." }