Skip to content

fix(inference): explain background-role provider fallback and verify BYOK keys (#5146 §2.1, §2.4) - #5254

Merged
senamakel merged 4 commits into
tinyhumansai:mainfrom
M3gA-Mind:fix/5146-stream-b-provider-routing
Jul 29, 2026
Merged

fix(inference): explain background-role provider fallback and verify BYOK keys (#5146 §2.1, §2.4)#5254
senamakel merged 4 commits into
tinyhumansai:mainfrom
M3gA-Mind:fix/5146-stream-b-provider-routing

Conversation

@M3gA-Mind

@M3gA-Mind M3gA-Mind commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Stream B of #5146 — provider detection and routing bugs.

Covered here: §2.1 Spurious API key errors for unrelated providers and §2.4 Codex / GPT BYOK — connected but unusable.
Deferred: §2.3 (DeepSeek) — see Deferred work below.

Summary

  • §2.1 — a local-Ollama user who attaches an image gets failed to read API key for slug 'anthropic', naming a provider they never configured. Fixed by explaining the fallback, not by changing where requests route.
  • The background-role → cloud fallback is deliberate and is preserved exactly; a hard failure there would break local-chat + managed-subscription users.
  • New fallback_diagnostics module carries the user-facing wording as pure, unit-testable functions, wired at the two sites named in triage plus a vision pre-flight.
  • §2.4 — adds a real inference check (verifyCloudProviderConnection) over the existing inference_test_provider_model RPC, and a classifier that maps opaque upstream errors onto concrete next steps.
  • Fixes a latent papercut found on the way: the old fallback hint told burst users to set burst_provider, a setting that does not exist.

Problem

§2.1 — why an unconfigured provider is asked for a key

provider_for_role (src/openhuman/inference/provider/factory.rs) routes the background roles — vision, embeddings, memory, summarization, heartbeat, learning, subconscious, agentic, burst — to the primary cloud provider when their own route is unset. Only chat, reasoning and coding inherit a configured BYOK slug.

So: chat is ollama:gemma3:1b, the user attaches an image, the vision sub-agent runs with role="vision", vision_provider is unset → resolves to the primary cloud slug → lookup_key_for_slug fails → a bare slug-level auth error naming Anthropic.

The routing is correct and is kept. Background workloads run tier-specific models (vision-v1, summarization-v1, …) that local runtimes do not serve, and a user with a local chat model and a managed subscription genuinely wants those workloads on the cloud they are paying for. Hard-failing would break that configuration. Users who want zero cloud egress already have PrivacyMode::LocalOnly, enforced at the inference chokepoint by enforce_local_only_inference — this PR deliberately does not duplicate that gate.

What was actually broken is that the fallback was unexplained: nothing told the user that this was a background role, that it fell back because the local model cannot serve that capability, or what to change.

§2.4 — "connected" only ever meant "the key was written to disk"

setCloudProviderKey stores a credential. The save flow's existing probe calls listProviderModels (/models), which proves the key is valid and the endpoint reachable — but not that inference works. A provider with a wrong default model id, an exhausted quota, or a base URL missing /v1 lists models happily and then fails on the user's first real message.

Solution

§2.1 (Option A — explain, don't reroute)

New src/openhuman/inference/provider/fallback_diagnostics.rs. Every function is a pure string/predicate so the wording and the role rules are testable without a live provider. Wired at three sites:

Site Change
provider_for_role The pre-existing [providers][local-fallback] log now emits a full sentence: which capability, which local chat model caused the fallback, which provider it landed on, and the knob that overrides it.
resolve_cloud_slug The key-lookup failure is wrapped with role + local-model context, so the error explains itself and names a remedy. The underlying provider error is preserved in the message.
session/builder/factory.rs Vision pre-flight logs the actionable "switch to a vision-capable model such as llava:7b" message at the moment the builder decides a model cannot take images — where the turn engine would otherwise strip the image and let the model answer confidently about something it never saw.

override_knob_for_role maps burst → agentic and summarization → memory, so the message never points at a setting that does not exist. The old code emitted set burst_provider explicitly to override; there is no such field.

§2.4

  • verifyCloudProviderConnection(slug, workload) — runs one real minimal completion through openhuman.inference_test_provider_model. Never throws: a provider that cannot serve a request is a result, so the caller can surface it without losing the save. An empty reply counts as unusable.
  • describeProviderVerificationFailure(slug, raw) — maps the common upstream shapes onto concrete next steps: auth rejection → check the key; model_not_found → fix the model id; quota/429 → check balance; bare 404 → check the base URL for the /v1 suffix; timeout → check endpoint/network. Anything unrecognised passes through verbatim rather than inventing a diagnosis.
  • The manual provider Test action now shows the classified message instead of a raw upstream string.

Impact

  • No routing behaviour changes. Every provider resolution returns exactly what it returned before; four provider_for_role regression tests pin this so a future "never fall back" refactor cannot silently break local-chat + managed-subscription users.
  • Log volume: one extra info line per background-role resolution, only when the chat provider is local.
  • Frontend: one changed error string in the Test action; the new API functions are additive.

Deferred work

  • §2.3 (DeepSeek garbage output) is not addressed here. DeepSeek has no dedicated handling anywhere in the inference path — it is a cloud_providers.rs preset (https://api.deepseek.com/v1) routed through the generic OpenAI-compatible client, so its known quirks (reasoning_content on R-series, tool-call streaming deltas) would be unhandled. That is a hypothesis, not a diagnosis: confirming it needs a real DeepSeek key or a captured failing response body. Writing a speculative response adapter against a guess would silently reshape live provider output, which is worse than shipping nothing. Deferred pending a key or a captured response.
  • §2.4 automatic post-save probe. The classifier and the verification call ship here, but they are not yet hooked into the save flow. That flow's existing /models probe rolls back the key and the provider entry on failure, and hooking an inference call into the same validation needs a fully-qualified slug:model provider string. The frontend CloudProvider shape does not carry a default model, so deriving one would risk a false negative that destroys a working provider config — a worse bug than the one being fixed. Left as follow-up rather than guessed at.

Related

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) — 13 Rust unit tests over the diagnostics wording, knob mapping and vision pre-flight; 4 provider_for_role regressions (including a drift guard tying the diagnostics role list to the real fallback set); 11 Vitest cases covering every branch of the failure classifier and the verification result shape, including the non-Error rejection path.
  • Diff coverage ≥ 80% — the new Rust module and the new TS functions are covered by the tests above. Not measured locally: per the task constraint no cargo command was run, and Vitest needs a pnpm install in a fresh worktree. CI's frontend-coverage / rust-core-coverage / coverage-gate jobs are the arbiter.
  • Coverage matrix updated — N/A: diagnostics/error-message change, no new feature row.
  • All affected feature IDs from the matrix are listed under ## RelatedN/A: no feature IDs affected.
  • No new external network dependencies introduced — verifyCloudProviderConnection reuses the existing inference_test_provider_model RPC; no new endpoints.
  • Manual smoke checklist updated if this touches release-cut surfaces — N/A: not a release-cut surface.
  • Linked issue closed via Closes #NNNN/A: #5146 stays open, see Related.

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

Linear Issue

Commit & Branch

  • Branch: fix/5146-stream-b-provider-routing
  • Commit SHA: d5669906c

Validation Run

  • pnpm --filter openhuman-app format:check — not run locally; see Validation Blocked.
  • pnpm typecheck — not run locally; see Validation Blocked.
  • Focused tests: added, not executed locally — factory_tests.rs, fallback_diagnostics.rs inline tests, aiSettingsApi.test.ts.
  • Rust fmt/check (if changed): not run — explicitly prohibited for this task (rely on CI).
  • Tauri fmt/check (if changed): N/A: app/src-tauri unchanged.

Validation Blocked

  • command: cargo check / cargo test / pnpm test
  • error: cargo commands were prohibited by the task directive for this change; Vitest needs a pnpm install in a fresh git worktree.
  • impact: no compiler or test feedback was available locally. One path bug was found and fixed by inspection — factory_tests is a child module of factory, so super::fallback_diagnostics does not resolve there and the crate-absolute path is used instead. vision_preflight was wired to a real call site rather than left test-only so it cannot trip a dead-code deny. Reviewers may want to look hardest at the Option::filter closure in the map_err at resolve_cloud_slug (relies on &&str → &str deref coercion) and the format-string argument counts in the new module.

Behavior Changes

  • Intended behavior change: none in routing. Error and log text for background-role provider fallback becomes actionable; the manual provider Test action reports a classified failure.
  • User-visible effect: instead of failed to read API key for slug 'anthropic', a local-model user is told that a background workload fell back to the cloud, why, and which setting to change.

Parity Contract

  • Legacy behavior preserved: provider_for_role returns identical strings for every role and config; the chat-tier BYOK inheritance, the LocalOnly privacy gate, and the save-flow /models probe and its rollback are untouched.
  • Guard/fallback/dispatch parity checks: local_chat_still_routes_background_roles_to_the_managed_backend, local_chat_role_is_returned_verbatim_and_never_falls_back, explicit_background_route_overrides_the_cloud_fallback, and cloud_fallback_roles_match_the_roles_provider_for_role_actually_falls_back.

Duplicate / Superseded PR Handling

  • Duplicate PR(s): none
  • Canonical PR: this one
  • Resolution: N/A

Summary by CodeRabbit

  • New Features
    • Added post-save verification for cloud AI providers, with structured success/empty/failure results and localized, actionable explanations.
    • Improved provider test failure classification (auth rejection, unknown model, quota/billing limits, endpoint 404, timeouts, unknown errors with details).
    • Added vision-model preflight checks and clearer guidance when background workloads fall back to cloud or credentials are missing.
    • Extended translations for the AI provider test flow.
  • Bug Fixes
    • Provider testing no longer surfaces confusing raw errors; messages are now mapped to user-friendly outcomes.
  • Tests
    • Expanded unit coverage for provider verification and fallback/diagnostic messaging.

…BYOK keys

Addresses tinyhumansai#5146 §2.1 and §2.4.

§2.1 — spurious cross-provider key errors

A user on a local Ollama chat model who attaches an image gets
"failed to read API key for slug 'anthropic'", naming a provider they
never configured. The cause is `provider_for_role`: the background roles
(vision, embeddings, memory, heartbeat, learning, subconscious, agentic,
burst) fall through to the primary cloud provider when their own route is
unset, and the resulting auth failure surfaces as a bare slug error.

That routing is deliberate and stays. Background workloads run
tier-specific models (vision-v1, summarization-v1, ...) that local
runtimes do not serve, and a local-chat + managed-subscription user
genuinely wants them on the cloud. Hard-failing here would break that
configuration; users who want no cloud egress at all already have
PrivacyMode::LocalOnly, enforced at the inference chokepoint.

What was missing is the explanation. New `fallback_diagnostics` module
holds the wording as pure, unit-testable functions, wired at two sites:

- `provider_for_role` now logs why a background role went to the cloud,
  naming the local chat model and the config knob that overrides it
  (`burst` correctly points at `agentic_provider`, which exists, rather
  than a `burst_provider` that does not).
- `resolve_cloud_slug` wraps the key-lookup failure with that context, so
  the error explains that a background role fell back, why, and what to
  change — instead of naming an unconfigured provider with no reason.

A vision pre-flight logs the actionable "switch to llava:7b" message at
the point the session builder decides a model cannot take images, where
the turn engine would otherwise strip the image silently.

§2.4 — connected but unusable

Storing a key only proves it was written to disk, and the existing save
probe calls /models, which proves the key and endpoint are valid but not
that inference works. Adds `verifyCloudProviderConnection`, which runs a
real minimal completion through the existing
`openhuman.inference_test_provider_model` RPC and never throws, plus
`describeProviderVerificationFailure`, which maps the common upstream
shapes (401, model_not_found, quota/429, bare 404, timeout) onto concrete
next steps and passes anything unrecognised through verbatim. The manual
provider Test action now reports the classified message instead of a raw
upstream string.

The automatic post-save probe is deliberately not wired yet: the RPC
takes a fully-qualified `slug:model` provider string, and guessing one
from the provider entry risks a false negative that would roll back a
working provider. Tracked in the PR body as follow-up.

§2.3 (DeepSeek) is not in this change — see the PR body.

Tests: Rust unit tests pin the diagnostics wording, the knob mapping, and
the vision pre-flight, plus `provider_for_role` regressions asserting the
background-role routing is unchanged and that the drift between the
diagnostics role list and the real fallback set is caught. Vitest covers
every branch of the failure classifier and the verification result shape.
@M3gA-Mind
M3gA-Mind requested a review from a team July 28, 2026 21:13
@M3gA-Mind

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 19 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c557741a-9bf3-42a4-8de2-b6ef04b444e6

📥 Commits

Reviewing files that changed from the base of the PR and between c5789e1 and 4b0f266.

📒 Files selected for processing (20)
  • app/src/components/settings/panels/AIPanel.tsx
  • app/src/components/settings/panels/__tests__/AIPanel.test.tsx
  • app/src/lib/i18n/ar.ts
  • app/src/lib/i18n/bn.ts
  • app/src/lib/i18n/de.ts
  • app/src/lib/i18n/en.ts
  • app/src/lib/i18n/es.ts
  • app/src/lib/i18n/fr.ts
  • app/src/lib/i18n/hi.ts
  • app/src/lib/i18n/id.ts
  • app/src/lib/i18n/it.ts
  • app/src/lib/i18n/ko.ts
  • app/src/lib/i18n/pl.ts
  • app/src/lib/i18n/pt.ts
  • app/src/lib/i18n/ru.ts
  • app/src/lib/i18n/zh-CN.ts
  • app/src/services/api/__tests__/aiSettingsApi.test.ts
  • app/src/services/api/aiSettingsApi.ts
  • src/openhuman/inference/provider/factory.rs
  • src/openhuman/inference/provider/factory_tests.rs
📝 Walkthrough

Walkthrough

Adds structured cloud-provider verification and localized failure classification in the settings UI, while centralizing Rust provider fallback diagnostics, credential guidance, routing behavior, regression tests, and vision capability preflight logging.

Changes

Cloud provider verification

Layer / File(s) Summary
Verification contract and API
app/src/services/api/aiSettingsApi.ts, app/src/services/api/__tests__/aiSettingsApi.test.ts
Adds structured verification results, failure classification, non-throwing test inference, and coverage for replies, failures, translations, and raw details.
Verification error rendering and translations
app/src/components/settings/panels/AIPanel.tsx, app/src/lib/i18n/*
Displays classified provider-test failures in the custom routing dialog and adds localized messages for provider-test outcomes.

Inference fallback diagnostics

Layer / File(s) Summary
Fallback diagnostics module
src/openhuman/inference/provider/fallback_diagnostics.rs, src/openhuman/inference/provider/mod.rs
Defines fallback roles, capability and override wording, credential guidance, vision preflight checks, and unit tests.
Provider routing integration
src/openhuman/inference/provider/factory.rs, src/openhuman/inference/provider/factory_tests.rs
Uses centralized fallback notices, wraps cloud credential failures with guidance, and tests implicit and explicit routing behavior.
Session vision preflight
src/openhuman/agent/harness/session/builder/factory.rs
Logs diagnostics when the selected session model does not support image input.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AIPanel
  participant aiSettingsApi
  participant testProviderModel
  AIPanel->>aiSettingsApi: request provider verification
  aiSettingsApi->>testProviderModel: send ping inference
  testProviderModel-->>aiSettingsApi: return reply or rejection
  aiSettingsApi-->>AIPanel: return localized verification result
Loading

Suggested labels: feature, rust-core

Suggested reviewers: senamakel, sanil-23, graycyrus

Poem

A rabbit hops where providers meet,
Sorting errors neat and sweet.
Cloud routes glow, credentials chime,
Vision checks arrive on time.
“No mystery hops!” the bunny sings.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the two main changes: background-role fallback diagnostics and BYOK provider verification.
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.

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

@coderabbitai coderabbitai Bot added bug rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. labels Jul 28, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d5669906cb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/openhuman/inference/provider/factory.rs Outdated
Comment thread app/src/services/api/aiSettingsApi.ts Outdated
Comment thread app/src/services/api/aiSettingsApi.ts Outdated

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 6

🧹 Nitpick comments (1)
src/openhuman/agent/harness/session/builder/factory.rs (1)

493-505: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Log the successful vision-preflight branch.

This new flow only logs failures, so grep-friendly diagnostics cannot distinguish a successful check from a skipped or unexecuted check. Add an entry and success log containing the resolved model, while retaining the actionable error message.

As per coding guidelines, changed Rust flows must include verbose, grep-friendly diagnostics for relevant branches and errors.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/agent/harness/session/builder/factory.rs` around lines 493 -
505, Update the vision preflight flow around
fallback_diagnostics::vision_preflight to log a grep-friendly success entry
containing the resolved model_name when validation succeeds, while preserving
the existing actionable failure log and its reason.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/src/components/settings/panels/AIPanel.tsx`:
- Around line 2154-2160: The error handler passes the composite
currentProviderString to describeProviderVerificationFailure instead of the
plain provider slug. Update this call to use the existing registrySlug value,
while preserving the raw error extraction and stale-request guard.

In `@app/src/services/api/aiSettingsApi.ts`:
- Around line 489-527: Update describeProviderVerificationFailure to return
translated messages through the established useT/I18nContext pattern instead of
hardcoded English, preserving all six matching branches and fallback behavior.
Add corresponding translation keys with {slug} and other dynamic details to
en.ts and every locale file, then interpolate slug and error detail via the
existing translation-key convention before AIPanel renders the banner.
- Around line 538-561: Add verbose, grep-friendly diagnostics to
verifyCloudProviderConnection covering function entry, the testProviderModel
external call, successful non-empty response, empty-reply branch, and caught
errors. Use the existing logging conventions from nearby flows such as
listProviderModels, and avoid logging secrets or raw credentials while including
safe provider/workload and error context.

In `@src/openhuman/inference/provider/factory_tests.rs`:
- Around line 1488-1497: Update both all-roles matrices in
src/openhuman/inference/provider/factory_tests.rs: add "summarization" to the
managed-backend fallback assertions at lines 1488-1497 and the
diagnostics/routing consistency assertions at lines 1542-1551, preserving the
existing ordering and assertion behavior.

In `@src/openhuman/inference/provider/factory.rs`:
- Around line 2211-2219: Update the lookup_key_for_slug error branch in the
provider factory to emit a verbose, grep-friendly diagnostic before returning
the guidance error. Include the role, slug, and whether routing used an implicit
fallback, while excluding the underlying error details and all credential values
from logs; preserve the existing returned error message.
- Around line 2212-2218: Update the error construction around
missing_provider_credentials_message so local_chat is passed only for
cloud-fallback roles whose route is unset or explicitly "cloud"; when the role
has an explicitly configured non-cloud route such as vision_provider, pass None.
Preserve the existing guidance generation and underlying error reporting.

---

Nitpick comments:
In `@src/openhuman/agent/harness/session/builder/factory.rs`:
- Around line 493-505: Update the vision preflight flow around
fallback_diagnostics::vision_preflight to log a grep-friendly success entry
containing the resolved model_name when validation succeeds, while preserving
the existing actionable failure log and its reason.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 91249e08-7354-4b34-b383-da2316215bf4

📥 Commits

Reviewing files that changed from the base of the PR and between 9750b80 and d566990.

📒 Files selected for processing (8)
  • app/src/components/settings/panels/AIPanel.tsx
  • app/src/services/api/__tests__/aiSettingsApi.test.ts
  • app/src/services/api/aiSettingsApi.ts
  • src/openhuman/agent/harness/session/builder/factory.rs
  • src/openhuman/inference/provider/factory.rs
  • src/openhuman/inference/provider/factory_tests.rs
  • src/openhuman/inference/provider/fallback_diagnostics.rs
  • src/openhuman/inference/provider/mod.rs

Comment thread app/src/components/settings/panels/AIPanel.tsx Outdated
Comment thread app/src/services/api/aiSettingsApi.ts Outdated
Comment thread app/src/services/api/aiSettingsApi.ts
Comment thread src/openhuman/inference/provider/factory_tests.rs
Comment thread src/openhuman/inference/provider/factory.rs Outdated
Comment thread src/openhuman/inference/provider/factory.rs Outdated
…sai#5146)

Frontend Checks was red on Prettier; that is fixed, and the review items on
top of it:

i18n (codex P1, CodeRabbit major) — `describeProviderVerificationFailure`
returned six hard-coded English sentences that `AIPanel` renders verbatim, so
non-English builds showed English and the i18n tooling could not see the copy.
Classification is now split from wording: `classifyProviderVerificationFailure`
returns a reason code, and the message resolves
`settings.ai.providerTest.*` through the translator, following the existing
`presentProviderSetupError` idiom. Eight keys added to en.ts and all 13 locale
files with real translations, placeholders preserved, no em dashes.

Wrong identifier (CodeRabbit major) — AIPanel passed `currentProviderString`
(the composite `provider:model[@temp]`) where a bare slug was expected, so the
banner read "'openai:gpt-4o' rejected it". It now passes `registrySlug`.

Model-qualified verification (codex P2) — the core only builds a configured
cloud provider from `<slug>:<model>`, so `verifyCloudProviderConnection` would
have failed a valid key when handed a bare slug. It now forwards the composite
verbatim and derives the bare slug for the message.

Diagnostics (CodeRabbit major/minor) — `verifyCloudProviderConnection` logs
entry/empty-reply/ok/failure, and the credential-lookup failure logs role,
slug, auth style and whether the route was an implicit fallback. Never the
underlying error or the key.

Empty BYOK key (codex P2) — a readable auth profile with no key for the slug
returns `Ok("")`, which built a client with an empty bearer and surfaced as a
raw 401 later: exactly the error this diagnostic replaces. An empty key is now
treated as missing for the credentialed auth styles (`OpenhumanJwt` and `None`
are legitimately blank).

Fallback attribution (CodeRabbit major) — an explicitly configured cloud route
that fails key lookup was described as a local-chat fallback. The role→route
mapping is factored out of `provider_for_role` into
`configured_route_for_role`, and `role_uses_implicit_cloud_fallback` gates the
explanation so only a genuine implicit fallback names the local chat model.

Tests: `summarization` added to both fallback matrices (CodeRabbit), plus
coverage for the empty-key path, the implicit-fallback predicate, the i18n key
resolution, and model-qualified verification.
@M3gA-Mind

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot added feature Net-new user-facing capability or product behavior. and removed bug labels Jul 28, 2026
Comment thread app/src/services/api/aiSettingsApi.ts
Comment thread app/src/services/api/aiSettingsApi.ts
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds actionable diagnostics for two provider-routing problems in #5146: background-role cloud fallback (§2.1) and BYOK keys that are saved but cannot serve inference (§2.4). Routing behaviour is unchanged — the explanation layer is entirely additive.

  • §2.1: Extracts configured_route_for_role from provider_for_role, adds role_uses_implicit_cloud_fallback, and wires fallback_diagnostics at the fallback log site and at a new empty-key early-exit guard in resolve_cloud_slug; a bare Ok(\"\") key for an implicit-fallback Bearer/Anthropic route now produces a named, actionable error instead of a raw 401 several layers later.
  • §2.4: Adds classifyProviderVerificationFailure, describeProviderVerificationFailure, and verifyCloudProviderConnection to aiSettingsApi.ts; the Test action in CustomRoutingDialog now shows the classified message instead of the raw upstream string; 13 Rust unit tests and 11 Vitest cases cover the new paths.

Confidence Score: 5/5

Safe to merge. No routing behaviour changes; all new code is additive diagnostics and error-message classification wired behind the existing Test action and fallback log site.

The core routing is pinned by four regression tests. The new Rust module is pure string functions with 13 inline unit tests. The TypeScript classifier and verify functions have 11 Vitest cases including edge paths. The one finding (the 'insufficient' substring potentially misclassifying scope errors from Groq-style providers as quota issues) is an uncommon edge case that still produces better output than the raw upstream error it replaces.

Files Needing Attention: app/src/services/api/aiSettingsApi.ts — classifyProviderVerificationFailure's quota branch uses 'insufficient' which can collide with permission-denied messages from some providers.

Important Files Changed

Filename Overview
app/src/services/api/aiSettingsApi.ts Adds CloudProviderVerification, classifyProviderVerificationFailure, describeProviderVerificationFailure, and verifyCloudProviderConnection. The classifier has one edge-case false positive: 'insufficient' can match permission-denied errors from providers like Groq.
src/openhuman/inference/provider/fallback_diagnostics.rs New pure-function module for actionable fallback diagnostics. Role list, capability labels, override knob mapping (burst→agentic, summarization→memory), and vision preflight are all correct.
src/openhuman/inference/provider/factory.rs Refactors the role→config-field match into configured_route_for_role (no behaviour change), adds role_uses_implicit_cloud_fallback, and wires the diagnostics at the fallback log site and at the empty-key guard.
src/openhuman/inference/provider/factory_tests.rs Adds four routing-regression tests plus a drift guard tying CLOUD_FALLBACK_ROLES to what provider_for_role actually falls back on.
app/src/components/settings/panels/AIPanel.tsx Single change: the raw upstream error in the Test-action catch block is replaced with describeProviderVerificationFailure. The bare slug — not the composite provider:model string — is passed to the classifier, which is correct.
src/openhuman/agent/harness/session/builder/factory.rs Adds vision_preflight call (log-only) after the existing model_supports_vision call. The extra call is intentional (logging at the decision point) and has no semantic impact.
app/src/services/api/tests/aiSettingsApi.test.ts Adds 11 test cases covering every branch of classifyProviderVerificationFailure and verifyCloudProviderConnection including the non-Error rejection path.

Reviews (3): Last reviewed commit: "fix(inference): scope the empty-key guar..." | Re-trigger Greptile

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/src/lib/i18n/en.ts`:
- Around line 5317-5318: Update the settings.ai.providerTest.unknown translation
usage so the {detail} placeholder receives only an allowlisted reason or fixed
unknown-error message, rather than the raw upstream provider/RPC error. Preserve
the existing key-saved and test-failed message behavior while sanitizing the
fallback branch.

In `@app/src/lib/i18n/hi.ts`:
- Around line 4729-4730: Update the `settings.ai.providerTest.unknown`
translation to remove the `{detail}` interpolation and use a generic
verification-failure message. Keep sanitized diagnostic information routed
exclusively through the existing diagnostics path.

In `@app/src/lib/i18n/pl.ts`:
- Around line 4800-4801: Update the settings.ai.providerTest.unknown translation
to use a redacted unknownError-style fallback instead of interpolating the raw
{detail} value, ensuring provider or RPC internals are not surfaced in the UI
while preserving the existing failure message context.

In `@app/src/lib/i18n/ru.ts`:
- Around line 4773-4774: Update the localized settings.ai.providerTest.unknown
message in the Russian translations so it no longer interpolates the raw
{detail} error text; use the localized unknownError fallback or another
sanitized reason while preserving the existing key-saved and test-failed
context.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6968b155-e41d-4191-b9ad-cc2a21e841d4

📥 Commits

Reviewing files that changed from the base of the PR and between d566990 and c5789e1.

📒 Files selected for processing (19)
  • app/src/components/settings/panels/AIPanel.tsx
  • app/src/lib/i18n/ar.ts
  • app/src/lib/i18n/bn.ts
  • app/src/lib/i18n/de.ts
  • app/src/lib/i18n/en.ts
  • app/src/lib/i18n/es.ts
  • app/src/lib/i18n/fr.ts
  • app/src/lib/i18n/hi.ts
  • app/src/lib/i18n/id.ts
  • app/src/lib/i18n/it.ts
  • app/src/lib/i18n/ko.ts
  • app/src/lib/i18n/pl.ts
  • app/src/lib/i18n/pt.ts
  • app/src/lib/i18n/ru.ts
  • app/src/lib/i18n/zh-CN.ts
  • app/src/services/api/__tests__/aiSettingsApi.test.ts
  • app/src/services/api/aiSettingsApi.ts
  • src/openhuman/inference/provider/factory.rs
  • src/openhuman/inference/provider/factory_tests.rs
🚧 Files skipped from review as they are similar to previous changes (3)
  • app/src/components/settings/panels/AIPanel.tsx
  • app/src/services/api/tests/aiSettingsApi.test.ts
  • src/openhuman/inference/provider/factory.rs

Comment thread app/src/lib/i18n/en.ts Outdated
Comment thread app/src/lib/i18n/hi.ts Outdated
Comment thread app/src/lib/i18n/pl.ts Outdated
Comment thread app/src/lib/i18n/ru.ts Outdated
…nyhumansai#5146)

Second review pass on the verification messages.

Raw error leakage (CodeRabbit, 4 threads) — the fallback branch interpolated
the upstream provider/RPC string straight into user-visible copy, which can
echo request material (headers, key fragments) into a screenshot-able banner
and contradicted this PR's own rule that diagnostics must not carry the
underlying error. The message is now generic; the raw text stays on
`CloudProviderVerification.detail` for the details surface, and both callers
log it (`console.error`) so it is still reachable for diagnosis.

`model not found` crossfire (greptile P1) — the endpoint branch matches a bare
`not found`, so providers phrasing a missing model as "model not found" or
"The model `x` was not found" were told to check their base URL and `/v1`
suffix instead of their model id. Classified as a model problem when the text
mentions both, which covers the contiguous and the interpolated phrasings.

Dead `empty` variant (greptile P2) — removed from `ProviderVerificationReason`:
the empty-reply path is handled inside `verifyCloudProviderConnection` and
never reaches the classifier, so the variant could only mislead.

Locales updated for the new generic wording; `providerTest.unknownError` is
gone with the `{detail}` interpolation it existed to fill.
@M3gA-Mind

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 28, 2026
…inyhumansai#5146)

CI caught the first pass being too broad: treating a blank key as missing for
every Bearer/Anthropic slug failed 8 pre-existing factory tests that build
cloud models without a stored key, plus the BYOK subconscious routing test.
Callers legitimately construct such models to probe or describe a provider
before a key is saved, so failing at construction time was a behaviour change
nobody asked for.

The guard now fires only on the implicit cloud-fallback path — the case the
diagnostic exists for, and the one codex described: a local-chat user whose
background role landed on a BYOK slug they never configured. Explicitly routed
providers keep their existing behaviour. The test pins both halves.

Also fixes the AIPanel test this PR had broken: the panel now imports
`describeProviderVerificationFailure`, which the suite's `vi.mock` factory did
not stub, so the dialog threw and rendered no alert at all. The mock now mirrors
the mapped copy, and the assertion checks the actionable message rather than the
raw upstream string the UI no longer shows. (Frontend Checks had been dying at
Prettier before reaching vitest, which is why this surfaced only now.)
@M3gA-Mind

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@senamakel
senamakel merged commit 73d3439 into tinyhumansai:main Jul 29, 2026
21 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in Team Openhuman Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Net-new user-facing capability or product behavior. rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure.

Projects

Status: Done

3 participants