fix(settings): round-trip LLM provider model_routes and consolidate UI#1660
Merged
senamakel merged 13 commits intoMay 13, 2026
Conversation
The LLM Provider settings page saved model_routes but never loaded them back: ClientConfig didn't expose them, so per-role model inputs reset to empty on every visit and re-clicking the active preset silently wiped the saved customizations with preset defaults. - Return model_routes from config.get_client_config (schema + handler). - Add model_routes to the TS ClientConfig type. - BackendProviderPanel: populate roleModels from saved routes on load; detect Custom preset when routes exist but api_url is empty; make applyPreset a no-op when re-clicking the active preset. - Remove duplicate CustomModelSection from LocalModelDebugPanel (delete the file). Settings -> LLM Provider is now the single source of truth for custom OpenAI-compatible providers.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPersist per-role model routes and a separate optional ChangesConfig, RPC, migration, and persistence
Provider API, router, and wiring
Channel runtime & API helper
Frontend settings and UI
Sequence DiagramsequenceDiagram
actor User
participant FE as Frontend (BackendProviderPanel)
participant Tauri as Tauri Config API
participant BE as Backend (openhuman controller)
participant Provider as ProviderFactory
User->>FE: Open settings panel
FE->>Tauri: request client config
Tauri->>BE: get_client_config RPC
BE-->>Tauri: { api_url, inference_url, model_routes[], ... }
Tauri-->>FE: ClientConfig (includes model_routes & inference_url)
FE->>FE: roleModelsFromRoutes(model_routes)
FE->>FE: detectPreset(api_url, model_routes)
FE-->>User: Render presets + per-role models
User->>FE: Click preset (same)
FE->>FE: applyPreset (no-op)
User->>FE: Click different preset / Save
FE->>Tauri: update_model_settings (send inference_url, model_routes, ...)
Tauri->>BE: handle_update_model_settings
BE-->>Tauri: success
Tauri-->>FE: saved
FE-->>User: Updated UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/openhuman/config/schemas.rs`:
- Around line 779-783: Add tracing-based diagnostics around the
get_client_config model-routes flow: at the start of the block that builds the
model_routes Vec (referencing config.model_routes and the local variable
model_routes) emit a trace/debug entry log with a safe correlation field like
model_routes_count = config.model_routes.len(); on successful completion emit a
debug/trace exit log with model_routes_count and maybe a sample hint count; on
error paths where get_client_config can fail emit an error log including the
same safe fields; use the repo's tracing/log macros (e.g.,
tracing::debug!/tracing::error!) and avoid logging full model identifiers, only
counts and non-sensitive metadata.
🪄 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: 55191092-e7f8-48a6-97af-b0beb3e5a0ec
📒 Files selected for processing (5)
app/src/components/settings/panels/BackendProviderPanel.tsxapp/src/components/settings/panels/LocalModelDebugPanel.tsxapp/src/components/settings/panels/local-model/CustomModelSection.tsxapp/src/utils/tauriCommands/config.tssrc/openhuman/config/schemas.rs
💤 Files with no reviewable changes (2)
- app/src/components/settings/panels/LocalModelDebugPanel.tsx
- app/src/components/settings/panels/local-model/CustomModelSection.tsx
Per CodeRabbit review on tinyhumansai#1660: log entry, load failure, and ok exit with safe correlation fields (api_key_set bool, model_routes_count) so the LLM Provider settings round-trip is traceable end-to-end.
The OpenAI preset shipped `gpt-5.5-2026-04-23` for reasoning/agentic, which OpenAI's API rejects with "model not found" once that dated snapshot rolls. Switch to the unversioned `gpt-5.5` alias for the flagship roles and `gpt-5.4-mini` for summarization (the actual mini variant in the 5.5 family per OpenAI's published model catalog). Reported by user after picking the OpenAI preset and seeing "The selected model is not available. Please check your model settings." on the web provider.
config.api_url was double-purposed as both the OpenHuman product backend URL (auth/billing/voice/integrations/...) AND the LLM inference endpoint introduced in tinyhumansai#1342. Pointing api_url at a custom OpenAI-compat provider silently rerouted every other backend call to OpenAI, breaking GET /auth/me, GET /auth/google/login, voice transcribe, billing, etc. This commit cleanly separates the two concerns: - New optional `config.inference_url`. When set together with `api_key`, the inference provider talks directly to that URL; otherwise inference flows through the OpenHuman backend at `{api_url}/openai/v1/...`. - `api_url` always means the OpenHuman product backend URL (defaults to api.tinyhumans.ai). Auth/billing/voice/etc. callers are untouched. - New `effective_inference_url()` helper derives the URL via the safe `api_url()` joiner from tinyhumansai#1650. - Inference provider factories (`create_backend_inference_provider`, `create_resilient_provider*`, `create_routed_provider*`, `create_intelligent_routing_provider`) now take separate `inference_url` and `backend_url` params; all callers updated. - ChannelRuntimeContext gains an `inference_url` field plumbed from Config so per-channel provider creation honours the override. - Settings: `update_model_settings` accepts an `inference_url` patch field; `get_client_config` returns it. - Frontend: BackendProviderPanel reads/writes `inference_url` (never `api_url`). OpenHuman preset clears `inference_url` so inference flows back through the backend. Migration: on config load, any legacy `api_url` value ending in `/chat/completions` is moved into `inference_url` (and cleared if it pointed at the OpenHuman backend itself). Also: RouterProvider now maps OpenHuman's abstract tier names (`reasoning-v1`, `agentic-v1`, `coding-v1`, `summarization-v1`) through the user's `model_routes` so a custom provider receives the configured model id instead of the literal tier name, which would 404 on OpenAI/Anthropic/etc. Reported by user: chat hit "model 'reasoning-v1' does not exist" on OpenAI, and login redirected to api.openai.com/.../auth/google/login.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/channels/routes.rs (1)
174-186:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDead code: the
ifbranch is unreachable.At line 160-162, the function returns early when
provider_name == ctx.default_provider.as_str(). Therefore, at line 174, this condition can never be true — theifbranch is dead code and(inference_url, backend_url)will always be(None, None).If the intent is to pass URLs only for the default provider, the current behavior is correct (returning early for default, passing
Nonefor others). In that case, simplify by removing the unreachable branch:🧹 Suggested simplification
- let (inference_url, backend_url) = if provider_name == ctx.default_provider.as_str() { - (ctx.inference_url.as_deref(), ctx.api_url.as_deref()) - } else { - (None, None) - }; - let provider = providers::create_resilient_provider_with_options( - inference_url, - backend_url, + None, + None, None, &ctx.reliability, &ctx.provider_runtime_options, )?;🤖 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/channels/routes.rs` around lines 174 - 186, The conditional that sets (inference_url, backend_url) is unreachable because the function already returns when provider_name == ctx.default_provider.as_str(); simplify by removing the if/else and directly set inference_url and backend_url to None for non-default providers, then call providers::create_resilient_provider_with_options with those None values; update the block around the inference_url/backend_url binding and the call to create_resilient_provider_with_options to reflect the simplified logic (keep references to provider_name and ctx.default_provider for clarity).
🧹 Nitpick comments (2)
src/openhuman/config/schema/load.rs (1)
659-659: ⚡ Quick winApply the same legacy migration in
load_from_default_paths()for consistency.
load_or_init()runsmigrate_legacy_inference_url, butload_from_default_paths()currently skips it. Aligning both prevents divergent config behavior across load paths.🤖 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/config/schema/load.rs` at line 659, load_from_default_paths() currently omits the legacy migration that load_or_init() runs; to fix, call migrate_legacy_inference_url(&mut config) in load_from_default_paths() after the config is loaded (and before returning/saving) so both load paths apply the same migration logic; locate the config variable in load_from_default_paths() and invoke migrate_legacy_inference_url(&mut config) the same way load_or_init() does to ensure consistent behavior.src/openhuman/providers/router.rs (1)
106-119: ⚡ Quick winAdd a regression test for abstract-tier routing through
model_routes.This branch is new core behavior and should be explicitly covered (e.g.,
reasoning-v1resolving to the configuredreasoningroute) to prevent future regressions.Suggested test shape
+ #[tokio::test] + async fn openhuman_tier_name_routes_via_hint_table() { + let (router, mocks) = make_router( + vec![("fast", "fast-response"), ("smart", "smart-response")], + vec![("reasoning", "smart", "claude-opus")], + ); + + let result = router.simple_chat("hello", "reasoning-v1", 0.5).await.unwrap(); + assert_eq!(result, "smart-response"); + assert_eq!(mocks[1].call_count(), 1); + assert_eq!(mocks[1].last_model(), "claude-opus"); + }🤖 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/providers/router.rs` around lines 106 - 119, Add a regression test that exercises the OpenHuman abstract-tier → hint resolution path: call the router logic that uses openhuman_tier_to_hint and self.routes (the same code block shown) to ensure an abstract model like "reasoning-v1" resolves to the configured "reasoning" route and returns the expected (idx, resolved_model). Create a unit test that constructs a Router with a model_routes entry mapping "reasoning" → (index, "reasoning"), invoke the routing lookup (the method containing the snippet that references openhuman_tier_to_hint and self.routes), and assert the returned index and resolved_model match the configured route; include both a positive case (abstract tier resolves) and a negative case (unknown abstract tier does not resolve).
🤖 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 `@src/openhuman/config/schema/load.rs`:
- Around line 547-549: The log call that prints the migrated inference URL value
(the line using moved.as_deref().unwrap_or("<derived>") inside the migration log
in load.rs) exposes potential credentials; update the logging in the migration
branch to never emit the raw URL value — either omit the value or replace it
with a redacted placeholder (e.g. "<redacted>" or "<derived, redacted>"), or use
a utility that strips credentials before logging; ensure the change targets the
migration log statement that currently formats
moved.as_deref().unwrap_or("<derived>") so no secrets are written to logs.
---
Outside diff comments:
In `@src/openhuman/channels/routes.rs`:
- Around line 174-186: The conditional that sets (inference_url, backend_url) is
unreachable because the function already returns when provider_name ==
ctx.default_provider.as_str(); simplify by removing the if/else and directly set
inference_url and backend_url to None for non-default providers, then call
providers::create_resilient_provider_with_options with those None values; update
the block around the inference_url/backend_url binding and the call to
create_resilient_provider_with_options to reflect the simplified logic (keep
references to provider_name and ctx.default_provider for clarity).
---
Nitpick comments:
In `@src/openhuman/config/schema/load.rs`:
- Line 659: load_from_default_paths() currently omits the legacy migration that
load_or_init() runs; to fix, call migrate_legacy_inference_url(&mut config) in
load_from_default_paths() after the config is loaded (and before
returning/saving) so both load paths apply the same migration logic; locate the
config variable in load_from_default_paths() and invoke
migrate_legacy_inference_url(&mut config) the same way load_or_init() does to
ensure consistent behavior.
In `@src/openhuman/providers/router.rs`:
- Around line 106-119: Add a regression test that exercises the OpenHuman
abstract-tier → hint resolution path: call the router logic that uses
openhuman_tier_to_hint and self.routes (the same code block shown) to ensure an
abstract model like "reasoning-v1" resolves to the configured "reasoning" route
and returns the expected (idx, resolved_model). Create a unit test that
constructs a Router with a model_routes entry mapping "reasoning" → (index,
"reasoning"), invoke the routing lookup (the method containing the snippet that
references openhuman_tier_to_hint and self.routes), and assert the returned
index and resolved_model match the configured route; include both a positive
case (abstract tier resolves) and a negative case (unknown abstract tier does
not resolve).
🪄 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: 7821ecf4-13e8-4b4d-becd-e6bd8e8c23bb
📒 Files selected for processing (26)
app/src/components/settings/panels/BackendProviderPanel.tsxapp/src/utils/tauriCommands/config.tssrc/api/config.rssrc/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/triage/routing.rssrc/openhuman/channels/context.rssrc/openhuman/channels/routes.rssrc/openhuman/channels/routes_tests.rssrc/openhuman/channels/runtime/startup.rssrc/openhuman/channels/tests/context.rssrc/openhuman/channels/tests/discord_integration.rssrc/openhuman/channels/tests/memory.rssrc/openhuman/channels/tests/runtime_dispatch.rssrc/openhuman/channels/tests/runtime_tool_calls.rssrc/openhuman/channels/tests/telegram_integration.rssrc/openhuman/config/ops.rssrc/openhuman/config/ops_tests.rssrc/openhuman/config/schema/load.rssrc/openhuman/config/schema/types.rssrc/openhuman/config/schemas.rssrc/openhuman/learning/linkedin_enrichment.rssrc/openhuman/local_ai/ops.rssrc/openhuman/providers/ops.rssrc/openhuman/providers/router.rssrc/openhuman/threads/ops.rssrc/openhuman/tools/impl/agent/delegate.rs
✅ Files skipped from review due to trivial changes (5)
- src/openhuman/channels/context.rs
- src/openhuman/channels/tests/context.rs
- src/openhuman/channels/tests/discord_integration.rs
- src/openhuman/tools/impl/agent/delegate.rs
- src/openhuman/learning/linkedin_enrichment.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/openhuman/config/schemas.rs
- app/src/utils/tauriCommands/config.ts
create_intelligent_routing_provider (used by chat/channels/threads/harness) was passing the backend provider straight to IntelligentRoutingProvider without consulting model_routes. With a custom inference URL configured, the abstract tier name `reasoning-v1` reached `custom_openai` unchanged and OpenAI 404'd it. When `config.model_routes` is non-empty, wrap the backend in a RouterProvider first so the previously-added tier-name → hint mapping (reasoning-v1 → reasoning → user's configured model id) actually runs on the inference dispatch path. Empty model_routes still skip the wrap so the OpenHuman backend keeps dispatching tier names natively.
Logs at three spots so it's unambiguous in the log stream where chat actually goes: - [providers] inference target = custom_openai @ <url> vs = openhuman_backend (emitted once per provider construction, with key length / url presence) - [providers] intelligent routing: model_routes_count / default_model (emitted when the chat path wires the router around the backend) - [router] tier reasoning-v1 -> hint=reasoning -> model=gpt-5.5 (emitted on every request that goes through model_routes) - [provider:<name>] outbound chat/completions -> <url> (emitted on every actual HTTP call, so the destination host is in plain text) Helpful for debugging when a chat reply looks like it came from the OpenHuman backend even though the user picked a custom provider.
Generic "Something went wrong" was hiding actionable failures from custom OpenAI-compatible providers — e.g. "Project ... does not have access to model \`gpt-5.5\`" was getting swallowed and replaced with the Discord-report fallback. Now classify_inference_error pulls the JSON `error.message` field out of the upstream provider response and quotes it under the friendly category summary, so users can act on the real reason without digging through logs. - New extract_provider_error_detail() parses the first `"message"` JSON field, manually unescapes \" and \\, sanitizes via the existing scrub_secret_patterns, and truncates to 300 chars. - Returns None for transport-level failures (no JSON body present) — preserves the existing no-leak policy on internal-infra URLs, which start_chat_emits_sanitized_chat_error_on_inference_failure asserts. - classify_inference_error now returns String and appends the detail via `with_provider_detail`. Adds "does not exist" and "does not have access" to the model_unavailable bucket. - Tests cover the extraction path (OpenAI body), the transport-error no-leak path, and the end-to-end model_unavailable classification.
Match the api_url/inference_url split — mocks return both fields and model_routes, and assertions check that the LLM Provider panel only ever writes to inference_url (the OpenHuman product backend api_url must never be touched by this UI). Includes the new "OpenHuman preset clears inference_url to empty string" assertion.
Per CodeRabbit review on tinyhumansai#1660: logging the raw inference URL during the legacy api_url -> inference_url migration could leak basic-auth credentials (`https://user:token@host/...`) or sensitive query parameters (`?api_key=...`) embedded by callers into log files / Sentry breadcrumbs. Add a private `redact_url_for_log` helper that strips userinfo, query, and fragment via `url::Url`, with a coarse `<scheme>://***@host` fallback when the input can't be parsed. Five unit tests cover the masking path and the migrate_legacy_inference_url branches (external move, OpenHuman-backend clear, no-op when inference_url already set).
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
1 participant
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
model_routesbut never loading them back, so per-role model inputs reset to empty on every visit and re-clicking the active preset silently wiped the user's saved customizations with preset defaults.model_routesfromconfig.get_client_configso the UI can round-trip them; extend the TSClientConfigto match.BackendProviderPanelnow populatesroleModelsfrom saved routes on load, detects theCustompreset when routes exist with noapi_url, and makesapplyPreseta no-op when re-clicking the already-active preset.CustomModelSectionfromLocalModelDebugPanel(and deleted the file). Settings → LLM Provider is now the single source of truth for custom OpenAI-compatible providers.Problem
Custom-model values typed into Settings → LLM Provider weren't reaching the rest of the app on subsequent visits: the page reloaded with empty role inputs, masking the persisted state, and a stray click on the highlighted preset card overwrote saved routes. There was also a redundant
Custom Providerform buried inside Developer Options → Local Model Debug, creating two ways to set the same thing.Solution
src/openhuman/config/schemas.rs) — extendedconfig.get_client_configto also returnmodel_routes: [{hint, model}]. Schema and handler updated in lockstep.app/src/utils/tauriCommands/config.ts) — addedmodel_routes: ModelRoute[]toClientConfig.app/src/components/settings/panels/BackendProviderPanel.tsx):load()calls a newroleModelsFromRouteshelper and populates the role inputs from the persisted routes.detectPreset()takes routes into account: emptyapi_url+ non-empty routes →Custom, otherwise OpenHuman.applyPreset()short-circuits when the user re-clicks the already-active preset, so a misclick doesn't nuke customizations.LocalModelDebugPanelno longer rendersCustomModelSection, and the duplicate file is deleted.Submission Checklist
diff-cover.Impact
Desktop only. No migration or compatibility concerns —
model_routeswas always persisted; this commit just surfaces it on read. No new network calls. The redacted-secret contract is preserved (api_keyis still never returned over RPC; onlyapi_key_set).Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckcargo check --manifest-path Cargo.tomlclean.Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
model_routesnow round-trip into the LLM Provider settings UI on every load; re-clicking the active preset no longer wipes saved values.Parity Contract
update_model_settingsstill REPLACESmodel_routeswholesale;[]still clears them; OpenHuman preset still sends[]on save.get_client_configstill hidesapi_key; newmodel_routesfield is the same{hint, model}shape already accepted byupdate_model_settings.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Removed