Skip to content

fix(agent): suppress empty-provider-response from Sentry (TAURI-RUST-4JX) - #2790

Merged
graycyrus merged 1 commit into
tinyhumansai:mainfrom
CodeGhost21:fix/sentry-tauri-rust-4jx-empty-response
May 28, 2026
Merged

graycyrus merged 1 commit into
tinyhumansai:mainfrom
CodeGhost21:fix/sentry-tauri-rust-4jx-empty-response

Conversation

@CodeGhost21

@CodeGhost21 CodeGhost21 commented May 27, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Add AgentError::EmptyProviderResponse { iteration } and route agent::harness::session::turn's empty-final-response bail through it, so the existing user-facing string is preserved while the typed variant flows up to run_single.
  • Centralise the "skip Sentry" decision behind a new AgentError::skips_sentry() method covering both MaxIterationsExceeded and the new variant; run_single calls it in place of the inline MaxIterationsExceeded-only check.
  • Keeps Sentry TAURI-RUST-4JX off the radar (~33 events, escalating on 0.56.0) without changing the user-visible error or the AgentError / recoverable semantics.

Why

Latest event payload: Windows user, LM Studio at http://localhost:1234, custom community fine-tune (qwen3.6-27b-heretic-uncensored-finetune-neo-code-di-imatrix-max). Provider response is text_chars=0 thinking_chars=0 tool_calls=0 in 60 ms — completely empty. agent/harness/session/turn.rs:805 then returned:

return Err(anyhow::anyhow!(
    "The model returned an empty response. Please try again."
));

That bubbled to run_single (agent/harness/session/runtime.rs:540), which routes anything other than AgentError::MaxIterationsExceeded through report_error_or_expected("agent", "run_single", …) → Sentry. The result is the same shape as OPENHUMAN-TAURI-99 / -98 (max-iter cap) before that fix: user-state outcome shipped to Sentry as a code-bug.

It's not actionable from Sentry (the user picked a flaky local model), the UI already surfaces the user-facing message, and the deeper "fix" lives in the user's model / provider config. Same call for suppression as the max-iter cap.

What changed

  1. src/openhuman/agent/error.rs — new variant EmptyProviderResponse { iteration: usize } with a Display arm that emits the verbatim user-facing string (so UI surfaces and any external grep/fingerprint contracts hold). New skips_sentry(&self) -> bool method as single source of truth for the suppressed set: today { MaxIterationsExceeded, EmptyProviderResponse }.
  2. src/openhuman/agent/harness/session/turn.rs:805 — replace anonymous anyhow::anyhow! with AgentError::EmptyProviderResponse { iteration: iteration + 1 }.into(). Warn-level breadcrumb that recorded the surfacing decision is preserved.
  3. src/openhuman/agent/harness/session/runtime.rs:540 — replace let is_max_iter = matches!(err.downcast_ref::<AgentError>(), Some(AgentError::MaxIterationsExceeded { .. })) with err.downcast_ref::<AgentError>().is_some_and(AgentError::skips_sentry). Log message generalised to "user-state agent error". Comment updated.
  4. src/openhuman/agent/harness/session/runtime.rs:388 (sanitize_event_error_message) — new arm returning "empty_provider_response" for the Sentry error_kind tag (required by the non-exhaustive match).
  5. src/openhuman/cron/scheduler.rs:37 (agent_error_to_user_message) — new arm returning actionable canned copy: "The model returned an empty response. Try a different model or check your local provider in Settings → AI → LLM." Required by the same non-exhaustive contract; gives cron job failures a useful message instead of the generic fallback.

Tests added (agent::error::tests)

  • empty_provider_response_display_matches_user_facing_string — locks the wire shape against accidental message changes (also a Sentry-fingerprint stability guarantee).
  • skips_sentry_returns_true_for_known_user_state_variants — MaxIterationsExceeded + EmptyProviderResponse.
  • skips_sentry_returns_false_for_real_failures — covers all 7 other variants (ProviderError, ContextLimitExceeded, ToolExecutionError, CostBudgetExceeded, CompactionFailed, PermissionDenied, Other); any future variant that should also suppress must be added to skips_sentry() and this test, both at once.

Test plan

  • cargo test openhuman::agent::error::tests — 9 tests pass (4 new)
  • cargo test openhuman::agent — 751 tests pass, 0 regressions
  • cargo test openhuman::cron::scheduler — 50 tests pass, 0 regressions
  • cargo check --manifest-path Cargo.toml --bin openhuman-core — passes
  • cargo fmt --check on touched files — clean

Post-merge observation: TAURI-RUST-4JX should drop to ~0 events on the next release. The variant still produces structured log::info! lines locally for diagnosability, so a real spike will still be visible in shipped logs (just not in Sentry).

…4JX)

`agent::harness::session::turn` returned an anonymous `anyhow::anyhow!(
"The model returned an empty response. Please try again.")` when the
provider's chat completion contained no text, no thinking, and no tool
calls. That bubbled to `run_single`'s catch-all `report_error_or_expected`
arm and shipped to Sentry as TAURI-RUST-4JX.

The latest event shows the typical trigger: a Windows user running LM
Studio locally with a community fine-tune
(`qwen3.6-27b-heretic-uncensored-finetune-neo-code-di-imatrix-max`) that
returned an empty stream. That's a model / user-config outcome, not an
OpenHuman bug — the UI already surfaces the user-facing string, and
there is no developer remediation path through Sentry.

Mirror the existing `MaxIterationsExceeded` pattern:

1. Add `AgentError::EmptyProviderResponse { iteration }` with a `Display`
   impl that emits the verbatim user-facing string (so UI / fingerprint
   contract is preserved).
2. Replace the anonymous `anyhow!` at `turn.rs:805` with the typed
   variant, retaining the warn-level breadcrumb that records the
   surfacing decision.
3. Introduce `AgentError::skips_sentry()` as the single source of truth
   for which variants get suppressed (`MaxIterationsExceeded` +
   `EmptyProviderResponse`), and call it from `run_single` in place of
   the inline `MaxIterationsExceeded`-only check.
4. Extend `sanitize_event_error_message` (Sentry error_kind tag) and
   `agent_error_to_user_message` (cron job user-facing copy) with arms
   for the new variant — required by the non-exhaustive match contract,
   and gives cron job failures actionable canned copy.

Tests added in `agent::error`:
- `Display` returns the canonical user-facing string (locks the wire
  shape against regressions).
- `skips_sentry()` returns true for both suppressed variants and false
  for every other AgentError variant (positive + negative coverage).

The user still sees the same error, the `Err` still propagates, and
the `AgentError` domain event / `recoverable` semantics are unchanged.
Sentry just stops getting paged for it.

## Test plan
- [x] `cargo test openhuman::agent::error::tests` — 9 tests pass (4 new)
- [x] `cargo test openhuman::agent` — 751 tests pass, 0 regressions
- [x] `cargo test openhuman::cron::scheduler` — 50 tests pass, 0 regressions
- [x] `cargo check --bin openhuman-core` — passes
- [x] `cargo fmt --check` on touched files — clean
@CodeGhost21
CodeGhost21 requested a review from a team May 27, 2026 20:33
@coderabbitai

coderabbitai Bot commented May 27, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This change introduces typed error classification for degenerate provider responses (no text, no thinking, no tool calls). A new AgentError::EmptyProviderResponse variant with iteration metadata is detected in turn execution, routed through telemetry via a skips_sentry() helper to separate it from true failures, and mapped to user-facing messages in scheduler notifications.

Changes

Empty Provider Response Error Handling

Layer / File(s) Summary
Error Type and Observability Contract
src/openhuman/agent/error.rs
AgentError::EmptyProviderResponse { iteration } variant is added with documentation, Display formatting outputs "The model returned an empty response. Please try again.", and a new public method skips_sentry() returns true for this variant and MaxIterationsExceeded, false otherwise. Tests verify the exact display string and skips_sentry() behavior for suppressed and real-failure variants.
Turn-Level Detection and Error Creation
src/openhuman/agent/harness/session/turn.rs
The empty provider-response failure path returns a typed AgentError::EmptyProviderResponse { iteration: iteration + 1 } instead of a generic anyhow error, enabling type-safe observability downstream.
Telemetry and Event Routing
src/openhuman/agent/harness/session/runtime.rs
sanitize_event_error_message adds a match arm to classify EmptyProviderResponse as "empty_provider_response". run_single generalizes Sentry suppression logic by calling skips_sentry() on the downcast error; when true, it emits structured log::info! instead of reporting through Sentry.
User-Facing Message Mapping
src/openhuman/cron/scheduler.rs
agent_error_to_user_message adds a case for AgentError::EmptyProviderResponse, returning a static notification message indicating the model returned an empty response.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

agent, bug, working

Suggested reviewers

  • M3gA-Mind
  • graycyrus
  • senamakel

Poem

🐰 A response that's empty brings tears to the try,
So we typed it, we routed it, gave users a why.
No Sentry alarms for the model's misstep,
Just logged info softly, with empathy's rep! 🌙

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding a new error variant and suppressing it from Sentry reporting, which is the core objective of the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot added agent Built-in agents, prompts, orchestration, and agent runtime in src/openhuman/agent/. working A PR that is being worked on by the team. bug labels May 27, 2026

@graycyrus graycyrus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodeGhost21 hey! the code looks good to me — clean approach to centralising the Sentry suppression policy behind skips_sentry(), the typed variant is the right call over the anonymous anyhow!, and the test coverage is solid (locking the wire string, both suppressed variants, and all 7 real-failure variants). but there are CI failures on this PR (PR Submission Checklist is failing and most checks are still pending), so i'll hold off on approving until those are green. once CI is clean, i'll come back and approve. let me know if you need any help!

@graycyrus graycyrus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodeGhost21 CI is green across the board now. The approach is the right one — typed variant instead of anonymous anyhow!, centralised suppression policy behind skips_sentry(), wire string locked by test, and all 7 real-failure variants explicitly excluded from suppression. Good work.

@oxoxDev oxoxDev assigned oxoxDev and unassigned oxoxDev May 28, 2026
@graycyrus
graycyrus merged commit 4adf6b6 into tinyhumansai:main May 28, 2026
40 of 49 checks passed
M3gA-Mind pushed a commit to CodeGhost21/openhuman that referenced this pull request May 28, 2026
…eport as expected (Sentry TAURI-RUST-4Z1)

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 tinyhumansai#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
M3gA-Mind added a commit that referenced this pull request Jun 22, 2026
… log empty 2xx streams (#3335) (#3415)

## Summary

- Replace the misleading **"Try a different model or check your local provider in Settings → AI → LLM"** copy on the empty-response classifier — that sent **Managed-route users toward a remedy that doesn't exist for them**, because there is no local provider on the Managed route. The new copy explicitly names the credits / billing remedy alongside the model / provider-config remedies (chat surface), and a tighter ≤120-char form for the cron notification drawer.
- Add a forensic `warn`-level log in `stream_native_chat` for the exact upstream shape that triggers `AgentError::EmptyProviderResponse` — a streaming chat call that returns **HTTP 2xx with zero text, zero thinking, and zero tool calls**. Captures `elapsed_ms`, `sse_chunks_parsed`, `raw_bytes_received`, `has_usage`, `has_openhuman_meta` so production data can disambiguate the three plausible upstream root causes (credit exhaustion served as 200 + empty body, upstream provider stall, real degenerate model output).
- Strictly additive — no behavioural change to request paths, no new Sentry suppression. The diagnostic log fires only on the empty-2xx case (otherwise the existing aggregated log carries the same fields at `info`).

## Problem

Issue #3335 (`iamrhn`): on Managed settings, every chat turn surfaces:

> *"The model returned an empty response. Try a different model or check your local provider in Settings → AI → LLM."*

The "local provider" remedy is nonsensical for Managed users. The Sentry data confirms this is real and broad:

- **TAURI-RUST-4JX**: 2,633 events, 55 distinct clients in 14 days. Releases include current `0.57.13` (PR #2790's Sentry suppression silenced the events but the underlying user-visible bug remains).
- Latest event breadcrumbs (Managed route, `reasoning-v1`, web channel):

  ```
  [provider:OpenHuman] outbound chat/completions -> https://api.tinyhumans.ai/openai/v1/chat/completions
  [stream]   OpenHuman POST … (stream=true, tools=21)
  …~7 s gap, no [stream] error breadcrumb, no [llm_provider] api_error breadcrumb…
  [agent]    provider responded — parsed tool_calls=0 text_chars=0
  [agent_loop] provider returned an empty final response (i=1, no text, no tool calls) — surfacing as error
  ```

  HTTP status was 2xx (no `streaming API error` line), so none of the existing classifiers (`is_budget_exhausted_http_400`, `is_provider_access_policy_denied_http_403`, …) at `compatible.rs:1028-1060` fire. The streaming aggregator builds a `ChatResponse` with `content: None`, the agent loop returns `AgentError::EmptyProviderResponse`, and the chat surface renders the wrong-remediation copy.

This is exactly the shape #3386 (team-filed) identifies as path **(b)**: the OpenHuman managed backend returns 200 + empty SSE under credit exhaustion / upstream stall instead of a typed 400 + `Insufficient budget`. The deeper fix lives in `tinyhumansai/backend`; this PR addresses the **two client-side wins that don't require the server change**.

## Solution

**(1) Copy fix — chat surface (`web_errors.rs::classify_inference_error` empty_response arm).** Replace the `"Try a different model or check your local provider"` text with a three-remedy form that's accurate for **all** providers: *"This usually means your inference credits are exhausted (Settings → Billing), the upstream model is temporarily unhealthy, or your provider configuration is rejecting the request (Settings → AI → LLM). Try one of those, or pick a different model."* New regression test `classify_inference_error_empty_response_copy_names_billing_remedy_and_drops_local_provider_misdirect` locks in: contains `Settings → Billing`, contains `Settings → AI → LLM`, contains `different model`, does NOT contain `local provider`, and `provider` stays `None`. Existing test `classify_inference_error_empty_response_is_actionable_and_retryable` continues to pass.

**(2) Copy fix — cron drawer (`cron/scheduler.rs::agent_error_to_user_message`).** Shorter form for the drawer's ≤120-char convention: *"Empty model response. Out of credits (Settings → Billing) or try a different model in Settings → AI → LLM."* New regression test `agent_error_to_user_message_classifies_empty_provider_response_for_3335` mirrors the chat-side assertions; `EmptyProviderResponse` is added to the existing `agent_error_to_user_message_canned_strings_are_short` variants list to lock in the ≤120-char contract (was previously absent — fit incidentally, but nothing enforced it).

**(3) Diagnostic log — `compatible.rs::stream_native_chat`.** Right after the existing aggregated `info` log, emit a `warn` when `text_accum.is_empty() && thinking_accum.is_empty() && tool_call_count == 0`:

```
[stream] {name} empty 2xx stream — model={..} elapsed_ms={..} sse_chunks={..} raw_bytes={..} has_usage={..} has_openhuman_meta={..}
```

`stream_started_at: Instant`, `sse_chunks_parsed: usize`, and `raw_bytes_received: usize` are added as append-only counters (never read by the request path). The signal disambiguates:

- `elapsed_ms` small + `sse_chunks=0` + `raw_bytes` small → backend closed the SSE near-immediately (credit reject or auth)
- `elapsed_ms` large + `sse_chunks=0` + `raw_bytes=0` → upstream stall / timeout
- `sse_chunks>0` + `has_usage=true` + accumulators empty → backend streamed metadata-only chunks (tokens counted, no content delivered)

**(4) Deliberately deferred — the in-session budget correlator from #3386.** `EmptyProviderResponse`'s flattened Display carries no `" API error"` infix for `extract_provider_name` to anchor on, so the classifier here cannot tell which provider was used without plumbing the typed `provider: Option<String>` through the AgentError. That plumbing affects ~10 string-match test/observability sites and balloons the scope. The diagnostic log is the cheapest path to the production data that decides whether the plumbing is worth doing — once we have evidence on which path is dominant (200+empty under credit exhaustion vs. upstream stall vs. true degenerate output), the next iteration of the empty-response arm can be provider-aware.

## Submission Checklist

> If a section does not apply to this change, mark the item as `N/A` with a one-line reason. Do not delete items.

- [x] Tests added or updated (happy path + at least one failure / edge case) per [Testing Strategy](../gitbooks/developing/testing-strategy.md#failure-path-requirement) — three new tests: `classify_inference_error_empty_response_copy_names_billing_remedy_and_drops_local_provider_misdirect` (web chat copy), `agent_error_to_user_message_classifies_empty_provider_response_for_3335` (cron drawer copy), plus `EmptyProviderResponse` added to `agent_error_to_user_message_canned_strings_are_short` variants (locks in the ≤120-char contract that was previously unenforced).
- [x] **Diff coverage ≥ 80%** — every new/changed line in `web_errors.rs` and `cron/scheduler.rs` is exercised by the three new regression tests; the `compatible.rs` diagnostic log is observability-only (no behavioural branching) and covered by the existing streaming-chat test surface that drives it. Run `pnpm test:rust` locally to verify.
- [x] Coverage matrix updated — `N/A: behaviour-only change` (error-message copy + observability log, no new feature ID).
- [x] All affected feature IDs from the matrix are listed in the PR description under `## Related` — `N/A: behaviour-only change, no matrix rows touched`.
- [x] No new external network dependencies introduced (mock backend used per [Testing Strategy](../gitbooks/developing/testing-strategy.md#mock-policy)) — `N/A: tests are pure unit-level over the classifier and the AgentError variant; no provider HTTP exercised`.
- [x] Manual smoke checklist updated if this touches release-cut surfaces ([`docs/RELEASE-MANUAL-SMOKE.md`](../docs/RELEASE-MANUAL-SMOKE.md)) — `N/A: error-copy + log only, no UI flow or release-cut surface affected`.
- [x] Linked issue closed via `Closes #NNN` in the `## Related` section.

## Impact

- **Desktop (chat surface)**: Managed-route users hitting empty-response failures now see an actionable, non-misleading message that names `Settings → Billing` as the most likely fix (per Sentry / #3386 evidence) without losing the existing `Settings → AI → LLM` and model-switch remedies. Self-hosted users see the same three-remedy framing — strictly more accurate than the prior "local provider" claim for users on cloud providers.
- **Desktop (cron notifications)**: Same behavioural change, shorter copy to fit the ≤120-char drawer-render budget. Now enforced by the existing length test.
- **Observability**: Net +1 `warn` log per streaming chat turn that returns 2xx-with-empty-body. This is the failure case being investigated; non-empty streams are unaffected. Lands in Sentry breadcrumbs even after `AgentError::skips_sentry()` (#2790) silences the parent event, unblocking the data needed to decide whether the in-session budget correlator from #3386 is the right next move or whether the fix has to be server-side.
- **Performance / security**: No request-path behaviour change. The added counters are stack-local `usize` increments. The diagnostic log redacts nothing new — provider name, model name, and counts are all already in the existing aggregated `info` log; only the byte total and elapsed ms are net-new and both are content-free.
- **Migration / compatibility**: None. Pure string + observability change.

## Related

- Closes: #3335
- Related: #3386 (the team-filed deep-dive that this PR addresses without requiring the per-session budget correlator), #3104 (cascade-error reports likely sharing the same empty-stream root cause), #2790 (the Sentry suppression that silenced TAURI-RUST-4JX without fixing the user-visible bug), #3199 (the empty-response arm being modified), #3121 (the budget-exhausted copy whose remedy this copy now points users toward), #3092 (closed parent issue for the chat-error cluster).
- Follow-up PR(s)/TODOs: Once the diagnostic log produces 24–48 h of production data, decide between (a) opening a backend issue against `tinyhumansai/backend` to start returning `400 + Insufficient budget` on streaming under credit exhaustion (the right server-side fix), and (b) plumbing `provider: Option<String>` through `AgentError::EmptyProviderResponse` so the classifier can be provider-aware (the #3386 in-session correlator approach).

---

## AI Authored PR Metadata (required for Codex/Linear PRs)

> Keep this section for AI-authored PRs. For human-only PRs, mark each field `N/A`.

### Linear Issue

- Key: N/A — GitHub issue #3335, not Linear-tracked.
- URL: N/A

### Commit & Branch

- Branch: `fix/3335-managed-empty-response-diagnostic-and-copy`
- Commit SHA: `7064cbfee`

### Validation Run

- [x] `pnpm --filter openhuman-app format:check` — `N/A: no frontend / TS files changed`
- [x] `pnpm typecheck` — `N/A: no frontend / TS files changed`
- [x] Focused tests: `cargo test --lib empty_response` (10 pass), `cargo test --lib empty_provider` (5 pass), `cargo test --lib cron::scheduler::tests::agent_error` (11 pass), `cargo test --lib compatible::tests` (153 pass).
- [x] Rust fmt/check (if changed): `cargo fmt --manifest-path Cargo.toml` + `cargo check --manifest-path Cargo.toml --lib` — both clean (only pre-existing unrelated warnings).
- [x] Tauri fmt/check (if changed): `N/A: no app/src-tauri files changed`

### Validation Blocked

- `command:` N/A
- `error:` N/A
- `impact:` N/A

### Behavior Changes

- Intended behavior change: User-facing error copy for `AgentError::EmptyProviderResponse` no longer claims a "local provider" exists; new copy names `Settings → Billing` (credits) alongside `Settings → AI → LLM` (provider config) and the model-switch fallback.
- User-visible effect: Managed-route users who hit empty-response failures now receive remedy text that matches the most likely actual cause (credit exhaustion, per #3386 evidence). Self-hosted users see the same three-remedy framing, which remains accurate for their configurations.

### Parity Contract

- Legacy behavior preserved: `AgentError::EmptyProviderResponse` Display, `classify_inference_error` error_type / source / retryable / provider fields, `agent_error_to_user_message` dispatch matrix, the `Settings → AI → LLM` deep-link reference. Existing test `classify_inference_error_empty_response_is_actionable_and_retryable` continues to pass unchanged.
- Guard/fallback/dispatch parity checks: The empty-response arm in `web_errors.rs` keeps the same `error_type: "empty_response"`, `source: "agent_loop"`, `retryable: true`, `retry_after_ms: None`, `provider: None`, `fallback_available: None` — only the message string changes. The cron `agent_error_to_user_message` dispatch matrix is unchanged; only the `EmptyProviderResponse` branch's returned string changes.

### Duplicate / Superseded PR Handling

- Duplicate PR(s): None — no other open PR references #3335.
- Canonical PR: This one.
- Resolution: N/A (no prior PR to supersede).


<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

## Release Notes

* **Bug Fixes**
  * Improved error messaging when model returns an empty response, now guiding users to check inference credits (Settings → Billing), try a different model, or verify LLM provider configuration (Settings → AI → LLM).

* **Improvements**
  * Added enhanced diagnostic logging for empty response scenarios to better support troubleshooting.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Closes #3335

Co-authored-by: M3gA-Mind <megamind@mahadao.com>
senamakel pushed a commit to nocstah/openhuman that referenced this pull request Sep 11, 2026
senamakel pushed a commit to nocstah/openhuman that referenced this pull request Sep 11, 2026
… log empty 2xx streams (tinyhumansai#3335) (tinyhumansai#3415)\n\n## Summary\n\n- Replace the misleading **"Try a different model or check your local provider in Settings → AI → LLM"** copy on the empty-response classifier — that sent **Managed-route users toward a remedy that doesn't exist for them**, because there is no local provider on the Managed route. The new copy explicitly names the credits / billing remedy alongside the model / provider-config remedies (chat surface), and a tighter ≤120-char form for the cron notification drawer.\n- Add a forensic `warn`-level log in `stream_native_chat` for the exact upstream shape that triggers `AgentError::EmptyProviderResponse` — a streaming chat call that returns **HTTP 2xx with zero text, zero thinking, and zero tool calls**. Captures `elapsed_ms`, `sse_chunks_parsed`, `raw_bytes_received`, `has_usage`, `has_openhuman_meta` so production data can disambiguate the three plausible upstream root causes (credit exhaustion served as 200 + empty body, upstream provider stall, real degenerate model output).\n- Strictly additive — no behavioural change to request paths, no new Sentry suppression. The diagnostic log fires only on the empty-2xx case (otherwise the existing aggregated log carries the same fields at `info`).\n\n## Problem\n\nIssue tinyhumansai#3335 (`iamrhn`): on Managed settings, every chat turn surfaces:\n\n> *"The model returned an empty response. Try a different model or check your local provider in Settings → AI → LLM."*\n\nThe "local provider" remedy is nonsensical for Managed users. The Sentry data confirms this is real and broad:\n\n- **TAURI-RUST-4JX**: 2,633 events, 55 distinct clients in 14 days. Releases include current `0.57.13` (PR tinyhumansai#2790's Sentry suppression silenced the events but the underlying user-visible bug remains).\n- Latest event breadcrumbs (Managed route, `reasoning-v1`, web channel):\n\n  ```\n  [provider:OpenHuman] outbound chat/completions -> https://api.tinyhumans.ai/openai/v1/chat/completions\n  [stream]   OpenHuman POST … (stream=true, tools=21)\n  …~7 s gap, no [stream] error breadcrumb, no [llm_provider] api_error breadcrumb…\n  [agent]    provider responded — parsed tool_calls=0 text_chars=0\n  [agent_loop] provider returned an empty final response (i=1, no text, no tool calls) — surfacing as error\n  ```\n\n  HTTP status was 2xx (no `streaming API error` line), so none of the existing classifiers (`is_budget_exhausted_http_400`, `is_provider_access_policy_denied_http_403`, …) at `compatible.rs:1028-1060` fire. The streaming aggregator builds a `ChatResponse` with `content: None`, the agent loop returns `AgentError::EmptyProviderResponse`, and the chat surface renders the wrong-remediation copy.\n\nThis is exactly the shape tinyhumansai#3386 (team-filed) identifies as path **(b)**: the OpenHuman managed backend returns 200 + empty SSE under credit exhaustion / upstream stall instead of a typed 400 + `Insufficient budget`. The deeper fix lives in `tinyhumansai/backend`; this PR addresses the **two client-side wins that don't require the server change**.\n\n## Solution\n\n**(1) Copy fix — chat surface (`web_errors.rs::classify_inference_error` empty_response arm).** Replace the `"Try a different model or check your local provider"` text with a three-remedy form that's accurate for **all** providers: *"This usually means your inference credits are exhausted (Settings → Billing), the upstream model is temporarily unhealthy, or your provider configuration is rejecting the request (Settings → AI → LLM). Try one of those, or pick a different model."* New regression test `classify_inference_error_empty_response_copy_names_billing_remedy_and_drops_local_provider_misdirect` locks in: contains `Settings → Billing`, contains `Settings → AI → LLM`, contains `different model`, does NOT contain `local provider`, and `provider` stays `None`. Existing test `classify_inference_error_empty_response_is_actionable_and_retryable` continues to pass.\n\n**(2) Copy fix — cron drawer (`cron/scheduler.rs::agent_error_to_user_message`).** Shorter form for the drawer's ≤120-char convention: *"Empty model response. Out of credits (Settings → Billing) or try a different model in Settings → AI → LLM."* New regression test `agent_error_to_user_message_classifies_empty_provider_response_for_3335` mirrors the chat-side assertions; `EmptyProviderResponse` is added to the existing `agent_error_to_user_message_canned_strings_are_short` variants list to lock in the ≤120-char contract (was previously absent — fit incidentally, but nothing enforced it).\n\n**(3) Diagnostic log — `compatible.rs::stream_native_chat`.** Right after the existing aggregated `info` log, emit a `warn` when `text_accum.is_empty() && thinking_accum.is_empty() && tool_call_count == 0`:\n\n```\n[stream] {name} empty 2xx stream — model={..} elapsed_ms={..} sse_chunks={..} raw_bytes={..} has_usage={..} has_openhuman_meta={..}\n```\n\n`stream_started_at: Instant`, `sse_chunks_parsed: usize`, and `raw_bytes_received: usize` are added as append-only counters (never read by the request path). The signal disambiguates:\n\n- `elapsed_ms` small + `sse_chunks=0` + `raw_bytes` small → backend closed the SSE near-immediately (credit reject or auth)\n- `elapsed_ms` large + `sse_chunks=0` + `raw_bytes=0` → upstream stall / timeout\n- `sse_chunks>0` + `has_usage=true` + accumulators empty → backend streamed metadata-only chunks (tokens counted, no content delivered)\n\n**(4) Deliberately deferred — the in-session budget correlator from tinyhumansai#3386.** `EmptyProviderResponse`'s flattened Display carries no `" API error"` infix for `extract_provider_name` to anchor on, so the classifier here cannot tell which provider was used without plumbing the typed `provider: Option<String>` through the AgentError. That plumbing affects ~10 string-match test/observability sites and balloons the scope. The diagnostic log is the cheapest path to the production data that decides whether the plumbing is worth doing — once we have evidence on which path is dominant (200+empty under credit exhaustion vs. upstream stall vs. true degenerate output), the next iteration of the empty-response arm can be provider-aware.\n\n## Submission Checklist\n\n> If a section does not apply to this change, mark the item as `N/A` with a one-line reason. Do not delete items.\n\n- [x] Tests added or updated (happy path + at least one failure / edge case) per [Testing Strategy](../gitbooks/developing/testing-strategy.md#failure-path-requirement) — three new tests: `classify_inference_error_empty_response_copy_names_billing_remedy_and_drops_local_provider_misdirect` (web chat copy), `agent_error_to_user_message_classifies_empty_provider_response_for_3335` (cron drawer copy), plus `EmptyProviderResponse` added to `agent_error_to_user_message_canned_strings_are_short` variants (locks in the ≤120-char contract that was previously unenforced).\n- [x] **Diff coverage ≥ 80%** — every new/changed line in `web_errors.rs` and `cron/scheduler.rs` is exercised by the three new regression tests; the `compatible.rs` diagnostic log is observability-only (no behavioural branching) and covered by the existing streaming-chat test surface that drives it. Run `pnpm test:rust` locally to verify.\n- [x] Coverage matrix updated — `N/A: behaviour-only change` (error-message copy + observability log, no new feature ID).\n- [x] All affected feature IDs from the matrix are listed in the PR description under `## Related` — `N/A: behaviour-only change, no matrix rows touched`.\n- [x] No new external network dependencies introduced (mock backend used per [Testing Strategy](../gitbooks/developing/testing-strategy.md#mock-policy)) — `N/A: tests are pure unit-level over the classifier and the AgentError variant; no provider HTTP exercised`.\n- [x] Manual smoke checklist updated if this touches release-cut surfaces ([`docs/RELEASE-MANUAL-SMOKE.md`](../docs/RELEASE-MANUAL-SMOKE.md)) — `N/A: error-copy + log only, no UI flow or release-cut surface affected`.\n- [x] Linked issue closed via `Closes #NNN` in the `## Related` section.\n\n## Impact\n\n- **Desktop (chat surface)**: Managed-route users hitting empty-response failures now see an actionable, non-misleading message that names `Settings → Billing` as the most likely fix (per Sentry / tinyhumansai#3386 evidence) without losing the existing `Settings → AI → LLM` and model-switch remedies. Self-hosted users see the same three-remedy framing — strictly more accurate than the prior "local provider" claim for users on cloud providers.\n- **Desktop (cron notifications)**: Same behavioural change, shorter copy to fit the ≤120-char drawer-render budget. Now enforced by the existing length test.\n- **Observability**: Net +1 `warn` log per streaming chat turn that returns 2xx-with-empty-body. This is the failure case being investigated; non-empty streams are unaffected. Lands in Sentry breadcrumbs even after `AgentError::skips_sentry()` (tinyhumansai#2790) silences the parent event, unblocking the data needed to decide whether the in-session budget correlator from tinyhumansai#3386 is the right next move or whether the fix has to be server-side.\n- **Performance / security**: No request-path behaviour change. The added counters are stack-local `usize` increments. The diagnostic log redacts nothing new — provider name, model name, and counts are all already in the existing aggregated `info` log; only the byte total and elapsed ms are net-new and both are content-free.\n- **Migration / compatibility**: None. Pure string + observability change.\n\n## Related\n\n- Closes: tinyhumansai#3335\n- Related: tinyhumansai#3386 (the team-filed deep-dive that this PR addresses without requiring the per-session budget correlator), tinyhumansai#3104 (cascade-error reports likely sharing the same empty-stream root cause), tinyhumansai#2790 (the Sentry suppression that silenced TAURI-RUST-4JX without fixing the user-visible bug), tinyhumansai#3199 (the empty-response arm being modified), tinyhumansai#3121 (the budget-exhausted copy whose remedy this copy now points users toward), tinyhumansai#3092 (closed parent issue for the chat-error cluster).\n- Follow-up PR(s)/TODOs: Once the diagnostic log produces 24–48 h of production data, decide between (a) opening a backend issue against `tinyhumansai/backend` to start returning `400 + Insufficient budget` on streaming under credit exhaustion (the right server-side fix), and (b) plumbing `provider: Option<String>` through `AgentError::EmptyProviderResponse` so the classifier can be provider-aware (the tinyhumansai#3386 in-session correlator approach).\n\n---\n\n## AI Authored PR Metadata (required for Codex/Linear PRs)\n\n> Keep this section for AI-authored PRs. For human-only PRs, mark each field `N/A`.\n\n### Linear Issue\n\n- Key: N/A — GitHub issue tinyhumansai#3335, not Linear-tracked.\n- URL: N/A\n\n### Commit & Branch\n\n- Branch: `fix/3335-managed-empty-response-diagnostic-and-copy`\n- Commit SHA: `7064cbfee`\n\n### Validation Run\n\n- [x] `pnpm --filter openhuman-app format:check` — `N/A: no frontend / TS files changed`\n- [x] `pnpm typecheck` — `N/A: no frontend / TS files changed`\n- [x] Focused tests: `cargo test --lib empty_response` (10 pass), `cargo test --lib empty_provider` (5 pass), `cargo test --lib cron::scheduler::tests::agent_error` (11 pass), `cargo test --lib compatible::tests` (153 pass).\n- [x] Rust fmt/check (if changed): `cargo fmt --manifest-path Cargo.toml` + `cargo check --manifest-path Cargo.toml --lib` — both clean (only pre-existing unrelated warnings).\n- [x] Tauri fmt/check (if changed): `N/A: no app/src-tauri files changed`\n\n### Validation Blocked\n\n- `command:` N/A\n- `error:` N/A\n- `impact:` N/A\n\n### Behavior Changes\n\n- Intended behavior change: User-facing error copy for `AgentError::EmptyProviderResponse` no longer claims a "local provider" exists; new copy names `Settings → Billing` (credits) alongside `Settings → AI → LLM` (provider config) and the model-switch fallback.\n- User-visible effect: Managed-route users who hit empty-response failures now receive remedy text that matches the most likely actual cause (credit exhaustion, per tinyhumansai#3386 evidence). Self-hosted users see the same three-remedy framing, which remains accurate for their configurations.\n\n### Parity Contract\n\n- Legacy behavior preserved: `AgentError::EmptyProviderResponse` Display, `classify_inference_error` error_type / source / retryable / provider fields, `agent_error_to_user_message` dispatch matrix, the `Settings → AI → LLM` deep-link reference. Existing test `classify_inference_error_empty_response_is_actionable_and_retryable` continues to pass unchanged.\n- Guard/fallback/dispatch parity checks: The empty-response arm in `web_errors.rs` keeps the same `error_type: "empty_response"`, `source: "agent_loop"`, `retryable: true`, `retry_after_ms: None`, `provider: None`, `fallback_available: None` — only the message string changes. The cron `agent_error_to_user_message` dispatch matrix is unchanged; only the `EmptyProviderResponse` branch's returned string changes.\n\n### Duplicate / Superseded PR Handling\n\n- Duplicate PR(s): None — no other open PR references tinyhumansai#3335.\n- Canonical PR: This one.\n- Resolution: N/A (no prior PR to supersede).\n\n\n<!-- This is an auto-generated comment: release notes by coderabbit.ai -->\n\n## Summary by CodeRabbit\n\n## Release Notes\n\n* **Bug Fixes**\n  * Improved error messaging when model returns an empty response, now guiding users to check inference credits (Settings → Billing), try a different model, or verify LLM provider configuration (Settings → AI → LLM).\n\n* **Improvements**\n  * Added enhanced diagnostic logging for empty response scenarios to better support troubleshooting.\n\n<!-- end of auto-generated comment: release notes by coderabbit.ai -->\n\nCloses tinyhumansai#3335\n\n
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Built-in agents, prompts, orchestration, and agent runtime in src/openhuman/agent/. bug working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants