From 9796bbc40407185f268e0204dfa7f4d671141afd Mon Sep 17 00:00:00 2001 From: Ghost Scripter Date: Thu, 28 May 2026 10:06:08 +0530 Subject: [PATCH 1/2] fix(observability): classify web-channel empty-provider-response re-report as expected (Sentry TAURI-RUST-4Z1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `ExpectedErrorKind::EmptyProviderResponse` + `is_empty_provider_response_message` predicate anchored on the user-facing string `"model returned an empty response"`. Routes the `web_channel.run_chat_task` re-report through the demote path (`tracing::warn!`, breadcrumb only) instead of a Sentry error event. Root cause: when the provider/model returns a completely empty body (`text_chars=0 thinking_chars=0 tool_calls=0`), the agent harness (`agent::harness::session::turn`) bails with the user-facing `"The model returned an empty response. Please try again."` string. PR #2790 (TAURI-RUST-4JX) suppresses the AGENT-layer Sentry event via the typed `AgentError::EmptyProviderResponse` + `skips_sentry()`. But `channels::providers::web::run_chat_task` RE-REPORTS the same failure under `domain=web_channel operation=run_chat_task` after the typed error was flattened to a `String` at the native-bus boundary — so the typed suppression can't reach it and it escapes as a fresh Sentry event (TAURI-RUST-4Z1). Same two-emit-site pattern as the embedding 4P0/4K5 session-expired fix; mirrors how `MaxIterationsExceeded` is suppressed at both the agent layer and the web_channel `report_error_or_expected` funnel. Anchored on `"model returned an empty response"` (not the looser `"empty response"`) so the internal fall-through paths stay actionable: `payload_summarizer` (`"summarizer returned empty response, falling through"`) and `subagent_runner::extract_tool` (`"provider returned an empty response; returning empty extraction"`) use different subjects and are NOT silenced — pinned by the rejection test. 3 new tests: web-channel wire shape (verbatim TAURI-RUST-4Z1 body) + bare user-facing string, and a 3-case rejection contract (summarizer fall-through, extract-tool graceful empty, generic health-probe empty body). cargo test --lib core::observability::tests -> 94 passed. Sentry-Issue: TAURI-RUST-4Z1 --- src/core/observability.rs | 128 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/src/core/observability.rs b/src/core/observability.rs index 42ea76a2fc..88e876fd20 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -198,6 +198,28 @@ pub enum ExpectedErrorKind { /// - `"kv key cannot contain personal identifiers"` /// - `"kv namespace/key cannot contain personal identifiers"` MemoryStorePiiRejection, + /// The provider/model completed a turn with a completely empty body + /// (`text_chars=0 thinking_chars=0 tool_calls=0`), so the agent harness + /// bailed with the user-facing `"The model returned an empty response. + /// Please try again."` string + /// (`agent::harness::session::turn`). This is a model/user-config + /// condition — a quirky or broken local fine-tune that returns nothing, + /// a provider that dropped the stream — not a code bug. The UI already + /// surfaces the typed error and the user can retry; Sentry has no + /// remediation path. + /// + /// `agent::run_single` already suppresses the **agent-layer** Sentry + /// event for this condition via the typed + /// `AgentError::EmptyProviderResponse` + `AgentError::skips_sentry()` + /// (PR #2790, TAURI-RUST-4JX). But `channels::providers::web:: + /// run_chat_task` **re-reports** the same failure under + /// `domain=web_channel operation=run_chat_task` after the typed error + /// has been flattened to a `String` at the native-bus boundary — so the + /// typed suppression can't reach it and it escapes as a fresh Sentry + /// event (TAURI-RUST-4Z1). This string classifier closes that second + /// emit site, mirroring how `MaxIterationsExceeded` is handled at both + /// layers. See [`is_empty_provider_response_message`]. + EmptyProviderResponse, } pub fn expected_error_kind(message: &str) -> Option { @@ -290,6 +312,15 @@ pub fn expected_error_kind(message: &str) -> Option { if is_memory_store_pii_rejection(&lower) { return Some(ExpectedErrorKind::MemoryStorePiiRejection); } + // Empty-provider-response re-report from the web-channel layer. Runs + // last so an earlier, more specific matcher always wins. See the + // variant doc-comment and [`is_empty_provider_response_message`] for + // the two-emit-site rationale (agent layer is handled by the typed + // `AgentError::skips_sentry()` in PR #2790; this covers the + // web_channel re-report where the type was flattened to a String). + if is_empty_provider_response_message(&lower) { + return Some(ExpectedErrorKind::EmptyProviderResponse); + } None } @@ -885,6 +916,34 @@ fn is_memory_store_pii_rejection(lower: &str) -> bool { lower.contains("cannot contain personal identifiers") } +/// Detect the agent harness's empty-provider-response bail. +/// +/// Anchored on the literal user-facing string emitted at +/// `agent::harness::session::turn` — +/// `"The model returned an empty response. Please try again."` — which is +/// preserved verbatim as the provider/model returns a body with +/// `text_chars=0 thinking_chars=0 tool_calls=0`. +/// +/// This catches the **web-channel re-report** (Sentry TAURI-RUST-4Z1): +/// `channels::providers::web::run_chat_task` wraps the failure as +/// `"run_chat_task failed client_id=… error=The model returned an empty +/// response. Please try again."` and routes it through +/// `report_error_or_expected` after the typed +/// `AgentError::EmptyProviderResponse` was flattened to a `String` at the +/// native-bus boundary (so the agent-layer `skips_sentry()` suppression +/// from PR #2790 can't reach it). +/// +/// Anchored on `"model returned an empty response"` (not the looser +/// `"empty response"`) so the sibling phrases stay actionable: +/// `"summarizer returned empty response, falling through"` +/// (`payload_summarizer`) and `"provider returned an empty response; +/// returning empty extraction"` (`subagent_runner::extract_tool`) are +/// internal fall-through paths with different wording and are NOT +/// silenced. +fn is_empty_provider_response_message(lower: &str) -> bool { + lower.contains("model returned an empty response") +} + /// Capture an error to Sentry with structured tags. /// /// `domain` and `operation` are required and become tags `domain:<…>` and @@ -1161,6 +1220,24 @@ fn report_expected_message(kind: ExpectedErrorKind, message: &str, domain: &str, "[observability] {domain}.{operation} skipped expected memory-store PII rejection" ); } + ExpectedErrorKind::EmptyProviderResponse => { + // Model/user-config condition — the provider returned a + // completely empty body and the agent harness bailed with the + // user-facing retry message. The agent layer already suppresses + // this via the typed `AgentError::skips_sentry()` (PR #2790); + // this arm covers the `web_channel.run_chat_task` re-report + // where the type was flattened to a String. Demote to `warn!` + // (breadcrumb only) — same tier as `MaxIterationsExceeded`, + // the other deterministic agent-state outcome surfaced to the + // user via the `chat_error` event. + tracing::warn!( + domain = domain, + operation = operation, + kind = "empty_provider_response", + error = %message, + "[observability] {domain}.{operation} skipped expected empty-provider-response error: {message}" + ); + } } } @@ -1877,6 +1954,57 @@ mod tests { } } + // ── EmptyProviderResponse (TAURI-RUST-4Z1) ───────────────────────────── + + #[test] + fn classifies_empty_provider_response_web_channel_rereport() { + // TAURI-RUST-4Z1: the web-channel re-report of the agent harness's + // empty-provider-response bail. `run_chat_task` wraps the flattened + // string and routes it through `report_error_or_expected` — the + // agent-layer typed suppression (PR #2790) can't reach it, so this + // string classifier must. + assert_eq!( + expected_error_kind( + "run_chat_task failed client_id=l1uxaLd20_1mAdhp \ + thread_id=thread-8f03e7f7-3477-42cd-9283-f0bacd4bfbca \ + request_id=a73716a3-a85a-4045-984b-315772c5b3b8 \ + error=The model returned an empty response. Please try again." + ), + Some(ExpectedErrorKind::EmptyProviderResponse) + ); + + // Bare user-facing string (the verbatim `turn.rs` emission), in case + // a different call site re-reports it without the run_chat_task wrap. + assert_eq!( + expected_error_kind("The model returned an empty response. Please try again."), + Some(ExpectedErrorKind::EmptyProviderResponse) + ); + } + + #[test] + fn does_not_classify_unrelated_empty_response_phrases() { + // Polarity contract: the anchor is `"model returned an empty + // response"`, NOT the looser `"empty response"`. The two internal + // fall-through paths use different subjects ("summarizer" / + // "provider") and are not user-facing failures — they must stay + // out of this bucket so a real regression in those paths still + // reaches Sentry. + for raw in [ + // payload_summarizer.rs:261 — internal fall-through, not a failure. + "[payload_summarizer] summarizer returned empty response, falling through", + // subagent_runner/extract_tool.rs:379 — graceful empty extraction. + "[extract_from_result] provider returned an empty response; returning empty extraction", + // Generic mention without the model-subject anchor. + "warning: empty response body from health probe", + ] { + assert_eq!( + expected_error_kind(raw), + None, + "must NOT classify as EmptyProviderResponse: {raw}" + ); + } + } + #[test] fn classifies_memory_store_pii_rejection_errors() { // TAURI-RUST-54T: ~915 events from one user where the PII guard From e33102ad19a2ac814eba2699f803c99dcd06317e Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Fri, 29 May 2026 00:41:32 +0530 Subject: [PATCH 2/2] fix(observability): expand EmptyProviderResponse rejection test and doc-comment - Add 4 more sibling phrases to the rejection test (channel-inbound graceful fallback, memory walk, reflection skip, and turn.rs "provider returned an empty final response") so any future anchor loosening can't silently stop those paths reaching Sentry. - Expand the EmptyProviderResponse variant doc to note that the classifier runs in the central dispatcher and covers all channel providers, not just web_channel.run_chat_task. Addresses nitpicks from code review. --- src/core/observability.rs | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/core/observability.rs b/src/core/observability.rs index 88e876fd20..24417f5338 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -219,6 +219,14 @@ pub enum ExpectedErrorKind { /// event (TAURI-RUST-4Z1). This string classifier closes that second /// emit site, mirroring how `MaxIterationsExceeded` is handled at both /// layers. See [`is_empty_provider_response_message`]. + /// + /// Although the immediate trigger is the `web_channel.run_chat_task` + /// re-report, this classifier runs in the central `expected_error_kind` + /// dispatcher, so any caller of `report_error_or_expected` + /// (`channels/runtime/dispatch.rs`, `channels/runtime/supervision.rs`, + /// any future channel provider) whose error chain contains `"model + /// returned an empty response"` is also demoted — no per-channel typed + /// suppression needed. EmptyProviderResponse, } @@ -1984,11 +1992,10 @@ mod tests { #[test] fn does_not_classify_unrelated_empty_response_phrases() { // Polarity contract: the anchor is `"model returned an empty - // response"`, NOT the looser `"empty response"`. The two internal - // fall-through paths use different subjects ("summarizer" / - // "provider") and are not user-facing failures — they must stay - // out of this bucket so a real regression in those paths still - // reaches Sentry. + // response"`, NOT the looser `"empty response"`. The sibling paths + // below use different subjects or phrasings and are not user-facing + // failures — they must stay out of this bucket so a real regression + // in those paths still reaches Sentry. for raw in [ // payload_summarizer.rs:261 — internal fall-through, not a failure. "[payload_summarizer] summarizer returned empty response, falling through", @@ -1996,6 +2003,16 @@ mod tests { "[extract_from_result] provider returned an empty response; returning empty extraction", // Generic mention without the model-subject anchor. "warning: empty response body from health probe", + // channels/bus.rs:185 — channel-inbound graceful fallback (routes + // through report_error_or_expected; subject is "agent", not "model"). + "[channel-inbound] agent returned empty response — finalizing draft with fallback", + // memory/query/walk.rs:292 — debug-level memory walk, not a failure. + "[memory_tree_walk] turn=3 LLM gave up (empty response)", + // learning/reflection.rs:576 — reflection skip, not a failure. + "[learning] reflection skipped (empty response — gate off or local AI unavailable)", + // agent/harness/session/turn.rs:811 — "provider returned an empty + // final response" uses subject "provider", not "model"; must not match. + "[agent_loop] provider returned an empty final response (i=2, no text, no tool calls)", ] { assert_eq!( expected_error_kind(raw),