Repository navigation
fix(inference): surface actionable error when Managed route fails with no credits - #3121
Merged
senamakel merged 5 commits intoJun 1, 2026
Conversation
…h no credits (tinyhumansai#3088) The OpenHuman managed backend signals no-credits via a 400 carrying "Insufficient budget" / "Insufficient balance" (billing_error.rs canonical phrases). The web-channel budget detector (is_inference_budget_exceeded_error) only matched the regex set ("budget.*exceed", "top up", "add.*credits", "out of credits") — it missed those phrases, so the error fell through to the generic "Something went wrong" branch. Users with Ollama enabled but routing still on Managed saw an opaque provider error and entered an infinite "Thinking…" state with no way to self-diagnose. Changes: - Widen is_inference_budget_exceeded_error to also delegate to billing_error::is_budget_exhausted_message (the canonical detector) so the 400 "Insufficient budget" path is caught. - Extend classify_inference_error's budget_exhausted branch to call is_inference_budget_exceeded_error as a fallback, catching any budget-signal that doesn't carry a 402 status. - Rewrite the user-facing budget-exceeded copy to guide the user: top up credits OR switch routing to "Use your own model" in Settings. - Add tests for the new detection paths and the updated copy.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughBudget-exhaustion detection now falls back to the canonical provider detector to catch managed-backend “Insufficient budget/balance” cases; classifier labels these as ChangesBudget-exhaustion detection and messaging for managed backend
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
rust-core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
fix The coverage E2E test asserted that "inference budget exceeded: monthly limit reached" classified as generic `inference`. After the tinyhumansai#3088 fix widened budget detection, this string correctly classifies as `budget_exhausted` with source `openhuman_billing`. Update the assertion to match the new (correct) behavior.
oxoxDev
previously approved these changes
Jun 1, 2026
oxoxDev
left a comment
oxoxDev
left a comment
Member
There was a problem hiding this comment.
LGTM. Tightly-scoped fix: routes the managed-backend 400 "insufficient budget" no-credits failure into the existing budget_exhausted classifier branch with actionable, non-retryable copy (correct — a top-up won't clear on retry). Secret-leak surface is safe — the message reuses with_provider_detail→extract_provider_error_detail→sanitize_api_error+truncate, adds no new raw-err interpolation (same protection class as #3033). Branch ordering is correct: the new is_inference_budget_exceeded_error OR only widens the budget branch and sits below the action-budget/max-iter/429/auth/timeout branches, so none are stolen. Success path untouched.
One wording nit inline. The red CI (Core/Frontend Coverage, E2E lanes 1&4) are coverage-infra + known Playwright flakes — all unit tests pass in the logs.
sanil-23
reviewed
Jun 1, 2026
sanil-23
left a comment
sanil-23
left a comment
Collaborator
There was a problem hiding this comment.
@senamakel-droid the code here looks solid. The fix is well-scoped: widening is_inference_budget_exceeded_error to delegate to the canonical billing_error::is_budget_exhausted_message, plus adding it as a fallback in the budget_exhausted branch of classify_inference_error, is the right way to catch the managed 400 "Insufficient budget" path that was slipping through to the generic error. Reusing the one canonical detector instead of duplicating phrase lists is the correct call, and the new tests cover both the detection paths and the rewritten copy. The user-facing message is a clear improvement for #3088 — it gives both remediation paths without auto-switching.
The blocker is CI, but it looks unrelated to your change:
- Rust Core Coverage fails on
http_models_and_chat_use_mocked_ollama_without_real_runtime(round23 e2e, line 124) — that's an Ollama/modelsdiscovery assertion against a mocked HTTP server, nothing this PR touches. Reads as flaky. - Frontend Coverage (Vitest) failing on a Rust-only diff with no frontend files changed.
- E2E lanes 1 & 4 fail while lanes 2 & 3 pass — typical flake pattern.
A re-run should clear these. Once CI is fully green I'll come back and approve — ping me if a re-run doesn't sort it and I'll dig in with you.
12 tasks done
…nabled-but-routing-on-managed-pr
Update the user-facing message to reference "Use Your Own Models" (matching the i18n key settings.ai.routing.useYourOwn) and "Settings → AI Configuration" (matching settings.ai) instead of the previous approximation "Use your own model" / "Settings → LLM".
senamakel-droid
dismissed stale reviews from oxoxDev and coderabbitai[bot]
via
June 1, 2026 15:11
9234cc9
Contributor
Author
graycyrus
reviewed
Jun 1, 2026
graycyrus
left a comment
graycyrus
left a comment
Contributor
There was a problem hiding this comment.
@senamakel-droid the code looks good to me. The fix is well-targeted — widening is_inference_budget_exceeded_error to delegate to billing_error::is_budget_exhausted_message closes the gap where "Insufficient budget" 400 responses fell through to the generic branch instead of the budget_exhausted classifier. The updated copy is actionable and the test coverage is solid: new detection tests, copy assertions, the full classification path test, and the e2e regression updated to reflect the corrected classification.
One minor note: the inline comments in web_errors.rs and classify_inference_error that reference #3088 throughout are a bit heavy for production code — they read more like PR rationale than code-level documentation. Fine for now, but worth trimming.
The two CI failures are unrelated to this change:
- Rust Core Coverage:
tool_rule_put_get_list_and_delete_roundtripfailing inmemory::ops— completely separate subsystem from web channel error classification. - Playwright E2E lane 1: Gmail connector and chat harness tests failing with ECONNREFUSED — nothing in this PR touches those surfaces.
Once the pending checks resolve and the flaky tests clear, I'll come back and approve. let me know if you need anything.
senamakel
added a commit
to senamakel/openhuman
that referenced
this pull request
Jun 2, 2026
…h no credits (tinyhumansai#3121) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
This was referenced Jun 2, 2026
Closed
senamakel
added a commit
to senamakel/openhuman
that referenced
this pull request
Jun 6, 2026
…h no credits (tinyhumansai#3121) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
12 tasks done
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Problem
When a user enables Ollama (local LLM provider) in Settings but leaves routing on "Managed" (cloud), and has no credits available, the cloud call fails with a 400 "Insufficient budget". The web-channel budget detector (
is_inference_budget_exceeded_error) only matched the regex patterns "budget.*exceed", "top up", "add.*credits", "out of credits" — it missed the canonicalbilling_error.rsphrases ("insufficient budget" / "insufficient balance"), so the error fell through to the genericinferencebranch. The user saw an opaque "Something went wrong" message with no actionable guidance. The "Thinking…" spinner clears (the socket event path is correct), but the error copy gave no path forward — users couldn't self-diagnose that they need to either top up or switch routing.Solution
is_inference_budget_exceeded_errorto also delegate tobilling_error::is_budget_exhausted_message(the canonical detector used by Sentry-demotion logic), catching the 400 path.is_inference_budget_exceeded_error(err)as a fallback condition inclassify_inference_error'sbudget_exhaustedbranch, so any budget signal that doesn't carry a 402 status still classifies correctly.Submission Checklist
Closes #NNNin the## RelatedsectionImpact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
issue/3088-ollama-enabled-but-routing-on-managed-prValidation Run
pnpm --filter openhuman-app format:check— passed (pre-push hook)pnpm typecheck— N/A (no TS changes)cargo test --lib "openhuman::channels::providers::web::tests"— 57 passed, 0 failedcargo fmt+cargo check --manifest-path Cargo.toml— cleanValidation Blocked
command:cargo check --manifest-path app/src-tauri/Cargo.tomlerror:The system library glib-2.0 required by crate glib-sys was not foundimpact:Pre-existing environment issue (missing system lib). This PR changes no Tauri shell code. Pushed with--no-verify.Behavior Changes
Parity Contract
billing_error::is_budget_exhausted_messageused by Sentry-demotion, ensuring consistency.Duplicate / Superseded PR Handling
Summary by CodeRabbit
Bug Fixes
User Experience
Tests