fix(ai): route local inference through provider router + revamp settings UI#2580
Merged
senamakel merged 12 commits intoMay 24, 2026
Merged
Conversation
Drop tests for removed helpers (updateRouting, OpenAI-compat endpoint
key API) and update assertions for the Managed / Use Your Own Models /
Advanced controls. Null routing strings now parse to { kind: 'default' }.
Contributor
|
Warning Review limit reached
Your plan includes 5 reviews of capacity. Refill in 9 minutes and 39 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
📝 WalkthroughWalkthroughThis PR redesigns the AI provider routing model from openhuman-centric to a default-centric approach, replaces local multi-turn chat with single-turn inference and reaction decisions, introduces LM Studio provider support via the factory layer, removes the OpenAI-compatible endpoint API surface, and substantially refactors the AIPanel UI to use managed/own/advanced routing cards instead of per-row Default/Custom segmentation. ChangesProvider routing and local inference refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Adopt upstream's providerToggleAriaLabel and i18n aria-labels while preserving the revamped routing UI and password-manager suppression attributes on the API key input.
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
coderabbitai
Bot
added
feature
The error message format is "{provider_label} API key not set", not
"cloud API key not set". Match on the common suffix instead.
![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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/openhuman/inference/provider/factory_test.rs (1)
580-992: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit live-provider coverage into a separate test module.
This file is now ~992 lines; adding discovery/live streaming coverage here pushes it far beyond the repo’s module-size threshold. Please move the live helpers/tests into a dedicated sibling test module (for example
factory_live_test.rs) and keep this file focused on unit-level factory behavior.As per coding guidelines: "
**/*.{ts,tsx,rs}: File size should not exceed approximately 500 lines. When a module grows beyond this threshold, split it into smaller, more focused modules with clear responsibilities."🤖 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/inference/provider/factory_test.rs` around lines 580 - 992, The live-provider discovery helpers (discover_live_lmstudio_model, discover_live_ollama_model) and the two asynchronous #[tokio::test] functions (live_lmstudio_provider_streams_thinking_and_text, live_ollama_provider_streams_text) should be moved out of this large factory_test.rs into a new dedicated test module (e.g., factory_live_test.rs) to keep file size under the ~500-line guideline; create the new file as a sibling module, copy the helpers and live tests there preserving #[ignore] and imports, update any crate:: paths if necessary (references include inference_test_guard, create_local_chat_provider_from_string, create_local_chat_provider_from_string, Config, ChatMessage, ChatRequest, ProviderDelta, discover_live_lmstudio_model, discover_live_ollama_model) and leave only unit-level factory tests in the original file; run cargo test to ensure module import paths compile and adjust use statements to expose needed symbols.app/src/services/api/aiSettingsApi.ts (1)
211-218:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
ollamain the persisted provider list.
AIPaneluses a sluggedollamaentry indraft.cloudProvidersto keep the local-runtime chip enabled and to probe models after reload. Filtering it out here makes the enablement transient: reload drops the chip state, and any latersaveAISettings()call writes a config without the Ollama entry.Suggested fix
const cloudProviders: CloudProviderView[] = config.cloud_providers - .filter(p => !['', 'cloud', 'openhuman', 'ollama', 'pid'].includes(p.slug.trim())) + .filter(p => !['', 'cloud', 'openhuman', 'pid'].includes(p.slug.trim())) .map(p => { const newKey = authKeyForSlug(p.slug).toLowerCase(); const legacyKey = p.slug.toLowerCase(); const has_api_key = profileProviders.has(newKey) || profileProviders.has(legacyKey); return { ...p, has_api_key }; @@ patch.cloud_providers = next.cloudProviders - .filter(p => !['', 'cloud', 'openhuman', 'ollama', 'pid'].includes(p.slug.trim())) + .filter(p => !['', 'cloud', 'openhuman', 'pid'].includes(p.slug.trim())) .map(({ id, slug, label, endpoint, auth_style }) => ({ id, slug, label, endpoint,Also applies to: 259-267
🤖 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 `@app/src/services/api/aiSettingsApi.ts` around lines 211 - 218, The filter that builds cloudProviders is incorrectly excluding the 'ollama' provider; update the exclusion array used in the config.cloud_providers.filter(...) so it no longer contains 'ollama' (i.e., only exclude ['', 'cloud', 'openhuman', 'pid']), and apply the same change to the second occurrence that covers lines ~259-267 so that p.slug === 'ollama' remains persisted and the AIPanel draft.cloudProviders behavior is preserved; locate these filters around the cloudProviders mapping and the analogous block further down (they reference config.cloud_providers, authKeyForSlug, profileProviders and set has_api_key) and remove 'ollama' from the excluded slugs.app/src/components/settings/panels/AIPanel.tsx (3)
1816-1825:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdvanced routing is missing a way back to Managed/default.
This dialog only lets the user save
cloudorlocalrefs. Once a workload is pinned here, there is no way to restore{ kind: 'openhuman' }or{ kind: 'default' }, even though the surrounding UI copy says rows can stay Managed or use the shared default.Also applies to: 1920-1933, 1998-2028
🤖 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 `@app/src/components/settings/panels/AIPanel.tsx` around lines 1816 - 1825, The initialSource computation currently only returns cloud/local/custom choices and thus prevents restoring managed/default/openhuman kinds; update the logic in the initializer for initialSource (and the analogous blocks around the other ranges) to handle initial.kind values 'openhuman' and 'default' (or 'managed') by returning the corresponding CustomDialogSource objects (e.g., { kind: 'openhuman' } or { kind: 'default' }) instead of falling through to cloud/local defaults, and ensure any UI selection list used by the dialog includes entries for those kinds so a user can re-select Managed/default; modify the branches that reference initial, CustomDialogSource, customCloud, and localAvailable accordingly.
113-149:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLocalize the new provider labels and workload hints.
These strings are rendered in the settings UI, but they are still hard-coded English literals in module-level metadata. Please move them behind translation keys and resolve them through
useT()at render time instead of shipping raw copy here.As per coding guidelines, "Every user-visible string in
app/src/**(headings, labels, button text, placeholders, status chips, toasts, dialog copy,aria-label, etc.) must go throughuseT()fromapp/src/lib/i18n/I18nContext. Hard-coded literals in JSX or props are not allowed. Add the new key toapp/src/lib/i18n/en.tsin the same PR."Also applies to: 198-216
🤖 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 `@app/src/components/settings/panels/AIPanel.tsx` around lines 113 - 149, BUILTIN_PROVIDER_META currently contains hard-coded English labels and tones; change it to only store non-user strings (e.g., keys and tone classes) and relocate all user-visible text (provider labels and workload hint strings referenced near lines 198-216) to i18n keys resolved at render time using useT() from app/src/lib/i18n/I18nContext (call useT() inside the AIPanel component and pass translated strings into the UI instead of literal text). Add the corresponding new keys and English copy to app/src/lib/i18n/en.ts in this PR and update any JSX/props that used the literal labels to use t('...') results; keep the tone CSS classes in BUILTIN_PROVIDER_META as-is.
2874-2886:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDisabling Ollama leaves
kind: 'local'routes behind.The scrub only resets
cloudrefs whoseproviderSlugmatcheslocalKind. Ollama workloads are stored as{ kind: 'local', model }, so turning Ollama off leaves those routes intact and the next inference call still targets a disabled runtime.Suggested fix
const nextRouting = Object.fromEntries( Object.entries(draft.routing).map(([wid, ref]) => [ wid, - ref.kind === 'cloud' && ref.providerSlug === localKind + (localKind === 'ollama' && ref.kind === 'local') || + (ref.kind === 'cloud' && ref.providerSlug === localKind) ? ({ kind: 'default' } as const) : ref, ]) ) as typeof draft.routing;🤖 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 `@app/src/components/settings/panels/AIPanel.tsx` around lines 2874 - 2886, The scrub currently only resets cloud refs matching localKind, leaving Ollama routes of shape { kind: 'local', model } intact; change the routing normalization in nextRouting to also clear refs that are local runtimes for the provider being removed: compute the set of removed provider slugs by comparing draft.cloudProviders (original) against remaining, then map draft.routing and set a route to { kind: 'default' } when (ref.kind === 'cloud' && ref.providerSlug === localKind) OR (ref.kind === 'local' && removedSlugs.has(localKind) / the removed provider was the local runtime), updating nextRouting and then calling persist with the cleaned routing and cloudProviders as you already do (references: nextRouting, draft.routing, draft.cloudProviders, remaining, localKind, persist).
🧹 Nitpick comments (5)
app/src/components/settings/panels/__tests__/AIPanel.test.tsx (2)
717-722: ⚡ Quick winExtract the repeated “open Advanced → open Reasoning model dialog” flow into a test helper.
This sequence is duplicated across multiple tests; pulling it into a helper will reduce drift and make failures easier to diagnose.
Also applies to: 775-780, 822-827, 862-867
🤖 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 `@app/src/components/settings/panels/__tests__/AIPanel.test.tsx` around lines 717 - 722, Extract the repeated "open Advanced → open Reasoning model dialog" sequence into a test helper (e.g., openReasoningModelDialog) so tests call a single function instead of duplicating fireEvent.click(await screen.findByRole('button', { name: /Advanced/i })); locating the Reasoning row via screen.findByText('Reasoning') and clicking the "Change Model" button with within(...).getByRole('button', { name: /Change Model/i }). Implement the helper in the test file (or a shared test-utils module) and replace the duplicated blocks at the locations shown (around the code using fireEvent.click/ screen.findByText/ within) with calls to this helper to ensure DRYness and easier maintenance.
241-247: 🏗️ Heavy liftSplit this spec into smaller feature-focused test files.
This test module is now ~1000+ lines and keeps growing with new scenarios; splitting by feature area (provider chips, custom provider editor, routing dialog, background loop controls) would improve maintainability and reviewability.
As per coding guidelines: "
**/*.{ts,tsx,rs}: File size should not exceed approximately 500 lines. When a module grows beyond this threshold, split it into smaller, more focused modules with clear responsibilities."Also applies to: 249-254, 256-263, 265-284, 398-406, 553-556, 583-586
🤖 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 `@app/src/components/settings/panels/__tests__/AIPanel.test.tsx` around lines 241 - 247, The AIPanel test file is too large and should be split into focused test modules; extract related specs into separate test files organized by feature (e.g., provider-chips.test.tsx for provider chips, custom-provider-editor.test.tsx for the custom provider editor, routing-dialog.test.tsx for routing dialog tests, background-loop-controls.test.tsx for loop controls) while keeping shared setup/helpers (renderWithProviders, any mock providers, and common fixtures) in a test-utils or a shared setup file; update each new test file to import AIPanel only where needed and to use the existing renderWithProviders/utility functions, move the specific tests you see around lines asserting "OpenHuman" and the lists referenced in the review into the appropriate new file (provider chips), and run/adjust any describe/test names and imports to ensure tests still run and maintain the same assertions and waits.scripts/debug/live-local-provider-smoke.sh (1)
18-22: ⚡ Quick winBound model-discovery HTTP calls with timeouts.
discover_json_fieldcurrently has no timeout, so smoke runs can hang on local networking issues.🔧 Proposed patch
discover_json_field() { local url="$1" local js="$2" - curl -fsS "$url" | node -e "$js" + curl -fsS --connect-timeout 2 --max-time 10 "$url" | node -e "$js" }🤖 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 `@scripts/debug/live-local-provider-smoke.sh` around lines 18 - 22, The discover_json_field function makes an unbounded curl call and can hang; update it to pass a curl timeout (e.g., --max-time or -m) and optionally a connect timeout (e.g., --connect-timeout) to the curl invocation so the HTTP request fails fast on network issues; modify the curl line inside discover_json_field (which takes url and js) to include something like --connect-timeout 5 --max-time 10 (or your desired values) so the node -e "$js" invocation only runs on a bounded curl response.src/openhuman/inference/schemas_tests.rs (1)
4-9: ⚡ Quick winPin the exact controller count here.
>= 19weakens this guard enough that a contract shrink can slip through as long asdeclaredandregisteredshrink together. Since this test is already validating the public inference surface, keep the exact expected count instead.Suggested change
- assert!(declared.len() >= 19); + assert_eq!(declared.len(), 19);🤖 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/inference/schemas_tests.rs` around lines 4 - 9, The test inference_catalog_counts_match_and_nonempty currently uses a loose guard assert!(declared.len() >= 19); change this to pin the exact expected controller count by asserting the concrete length (e.g., assert_eq!(declared.len(), 19)); keep the existing assert_eq!(declared.len(), registered.len()) and replace the >= check with the exact-number assertion so the test fails if the public inference surface changes unexpectedly; locate this in the function inference_catalog_counts_match_and_nonempty where declared = all_controller_schemas() and registered = all_registered_controllers().src/openhuman/inference/ops.rs (1)
126-147: ⚡ Quick winAvoid hardcoding the local-provider prefixes in this RPC.
inference_test_provider_model()now owns its ownlmstudio:/ollama:dispatch list even though the factory already owns local-provider parsing. If another local prefix is added inprovider::factory, this handler will silently fall through tocreate_chat_provider_from_string()and regress the test-provider flow. Push the local-vs-remote decision behind the factory so there is a single source of truth.🤖 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/inference/ops.rs` around lines 126 - 147, Remove the hardcoded "lmstudio:"/"ollama:" prefix checks from inference_test_provider_model and delegate the local-vs-remote decision to the provider factory: stop calling create_local_chat_provider_from_string directly from ops.rs and instead call a single factory entry (e.g., create_chat_provider_from_string or add a new create_provider_for_test) that encapsulates local-prefix parsing inside crate::openhuman::inference::provider::factory; if the factory lacks such unified behavior, update the factory so it returns the same (chat_provider, model) tuple for both local and remote providers and use that single factory function in inference_test_provider_model (referencing inference_test_provider_model, create_local_chat_provider_from_string, create_chat_provider_from_string, and provider::factory).
🤖 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 230-240: The EMPTY_ROUTING constant currently sets every workload
to { kind: 'default' }, which causes inferRoutingMode() to treat a fresh config
as 'custom' and open the Advanced editor; change the default initialization
(EMPTY_ROUTING) to use the same sentinel as managed mode (e.g., { kind:
'openhuman' } or whatever inferRoutingMode() expects) OR update
inferRoutingMode() to treat 'default' as equivalent to managed when all refs
match; locate EMPTY_ROUTING and inferRoutingMode() in AIPanel.tsx and make the
defaults and the inference logic consistent so a fresh config lands in the
managed/simple editor rather than Advanced.
- Around line 1693-1713: The JSX in AIPanel (component AIPanel.tsx, e.g., the
resolved display and the button labels like "Choose Model", "Change Model", "No
model selected" and other literals mentioned) contains hard-coded user-visible
strings; replace each literal with useT() lookups (import and call useT() from
app/src/lib/i18n/I18nContext inside AIPanel) and pass the translated keys
instead of raw strings, then add corresponding keys and English values to
app/src/lib/i18n/en.ts in this PR; ensure you update all other mentioned ranges
(around lines ~2430-2533, 2751-2909, 2941-3065) in their components similarly so
every user-visible string uses useT() and has an en.ts entry.
- Around line 2412-2425: The applySelection function sets setSaving(true) then
awaits onApply(...) but never resets setSaving(false) if onApply rejects; update
applySelection to ensure setSaving(false) always runs (e.g., wrap the await
onApply call in try/finally or attach .catch/.finally) so failures from
onApply()/persist() clear the saving state; refer to applySelection, onApply,
and setSaving when making the change.
In `@src/openhuman/inference/provider/factory_test.rs`:
- Around line 931-934: The test currently hard-fails on saw_thinking
(assert!(saw_thinking, ...)) after using the auto-discovery helper that picks
the first non-embedding LM Studio model; change the test so it does not require
a ThinkingDelta unconditionally: detect whether the discovered model can/should
emit reasoning deltas (via whatever capability flag or model metadata the
discovery helper returns) and only assert saw_thinking when that capability is
advertised, otherwise skip or downgrade to a non-failing warning; update the
assertion around saw_thinking and any related test setup that relies on
ThinkingDelta to be conditional based on the model capability advertised by the
discovery helper.
In `@src/openhuman/inference/provider/factory.rs`:
- Around line 263-300: create_local_chat_provider_from_string lacks diagnostic
logs for entry, branch selection, and error cases; add debug/info logs at the
start of the function (logging the raw provider string p), before each branch
decision indicating which prefix matched (use OLLAMA_PROVIDER_PREFIX and
LM_STUDIO_PROVIDER_PREFIX), and on the validation/error paths (empty model and
final unsupported-provider bail) so failures are observable; use the existing
logging facility used elsewhere in this module (or the module's logger) and
include the model and temperature_override where applicable and the original
provider string when logging errors, while keeping logs concise and
non-sensitive.
In `@src/openhuman/subconscious/executor.rs`:
- Around line 205-225: Add diagnostic debug logs around the local-provider path:
log the resolved model id (from effective_chat_model_id), the constructed
provider_string (from provider_from_config and the format branches), and the
resulting provider/model returned by create_local_chat_provider_from_string
before calling chat_with_history; then log a debug/info success message
including provider_string and model on successful completion of
provider.chat_with_history(...) and log an error-level message including the
same provider_string/model and the error when chat_with_history(...) returns
Err. Use the existing logging facility (e.g., debug! / error!) to match project
conventions and include the provider_string, model, and formatted error details
in the messages.
---
Outside diff comments:
In `@app/src/components/settings/panels/AIPanel.tsx`:
- Around line 1816-1825: The initialSource computation currently only returns
cloud/local/custom choices and thus prevents restoring managed/default/openhuman
kinds; update the logic in the initializer for initialSource (and the analogous
blocks around the other ranges) to handle initial.kind values 'openhuman' and
'default' (or 'managed') by returning the corresponding CustomDialogSource
objects (e.g., { kind: 'openhuman' } or { kind: 'default' }) instead of falling
through to cloud/local defaults, and ensure any UI selection list used by the
dialog includes entries for those kinds so a user can re-select Managed/default;
modify the branches that reference initial, CustomDialogSource, customCloud, and
localAvailable accordingly.
- Around line 113-149: BUILTIN_PROVIDER_META currently contains hard-coded
English labels and tones; change it to only store non-user strings (e.g., keys
and tone classes) and relocate all user-visible text (provider labels and
workload hint strings referenced near lines 198-216) to i18n keys resolved at
render time using useT() from app/src/lib/i18n/I18nContext (call useT() inside
the AIPanel component and pass translated strings into the UI instead of literal
text). Add the corresponding new keys and English copy to app/src/lib/i18n/en.ts
in this PR and update any JSX/props that used the literal labels to use t('...')
results; keep the tone CSS classes in BUILTIN_PROVIDER_META as-is.
- Around line 2874-2886: The scrub currently only resets cloud refs matching
localKind, leaving Ollama routes of shape { kind: 'local', model } intact;
change the routing normalization in nextRouting to also clear refs that are
local runtimes for the provider being removed: compute the set of removed
provider slugs by comparing draft.cloudProviders (original) against remaining,
then map draft.routing and set a route to { kind: 'default' } when (ref.kind ===
'cloud' && ref.providerSlug === localKind) OR (ref.kind === 'local' &&
removedSlugs.has(localKind) / the removed provider was the local runtime),
updating nextRouting and then calling persist with the cleaned routing and
cloudProviders as you already do (references: nextRouting, draft.routing,
draft.cloudProviders, remaining, localKind, persist).
In `@app/src/services/api/aiSettingsApi.ts`:
- Around line 211-218: The filter that builds cloudProviders is incorrectly
excluding the 'ollama' provider; update the exclusion array used in the
config.cloud_providers.filter(...) so it no longer contains 'ollama' (i.e., only
exclude ['', 'cloud', 'openhuman', 'pid']), and apply the same change to the
second occurrence that covers lines ~259-267 so that p.slug === 'ollama' remains
persisted and the AIPanel draft.cloudProviders behavior is preserved; locate
these filters around the cloudProviders mapping and the analogous block further
down (they reference config.cloud_providers, authKeyForSlug, profileProviders
and set has_api_key) and remove 'ollama' from the excluded slugs.
In `@src/openhuman/inference/provider/factory_test.rs`:
- Around line 580-992: The live-provider discovery helpers
(discover_live_lmstudio_model, discover_live_ollama_model) and the two
asynchronous #[tokio::test] functions
(live_lmstudio_provider_streams_thinking_and_text,
live_ollama_provider_streams_text) should be moved out of this large
factory_test.rs into a new dedicated test module (e.g., factory_live_test.rs) to
keep file size under the ~500-line guideline; create the new file as a sibling
module, copy the helpers and live tests there preserving #[ignore] and imports,
update any crate:: paths if necessary (references include inference_test_guard,
create_local_chat_provider_from_string, create_local_chat_provider_from_string,
Config, ChatMessage, ChatRequest, ProviderDelta, discover_live_lmstudio_model,
discover_live_ollama_model) and leave only unit-level factory tests in the
original file; run cargo test to ensure module import paths compile and adjust
use statements to expose needed symbols.
---
Nitpick comments:
In `@app/src/components/settings/panels/__tests__/AIPanel.test.tsx`:
- Around line 717-722: Extract the repeated "open Advanced → open Reasoning
model dialog" sequence into a test helper (e.g., openReasoningModelDialog) so
tests call a single function instead of duplicating fireEvent.click(await
screen.findByRole('button', { name: /Advanced/i })); locating the Reasoning row
via screen.findByText('Reasoning') and clicking the "Change Model" button with
within(...).getByRole('button', { name: /Change Model/i }). Implement the helper
in the test file (or a shared test-utils module) and replace the duplicated
blocks at the locations shown (around the code using fireEvent.click/
screen.findByText/ within) with calls to this helper to ensure DRYness and
easier maintenance.
- Around line 241-247: The AIPanel test file is too large and should be split
into focused test modules; extract related specs into separate test files
organized by feature (e.g., provider-chips.test.tsx for provider chips,
custom-provider-editor.test.tsx for the custom provider editor,
routing-dialog.test.tsx for routing dialog tests,
background-loop-controls.test.tsx for loop controls) while keeping shared
setup/helpers (renderWithProviders, any mock providers, and common fixtures) in
a test-utils or a shared setup file; update each new test file to import AIPanel
only where needed and to use the existing renderWithProviders/utility functions,
move the specific tests you see around lines asserting "OpenHuman" and the lists
referenced in the review into the appropriate new file (provider chips), and
run/adjust any describe/test names and imports to ensure tests still run and
maintain the same assertions and waits.
In `@scripts/debug/live-local-provider-smoke.sh`:
- Around line 18-22: The discover_json_field function makes an unbounded curl
call and can hang; update it to pass a curl timeout (e.g., --max-time or -m) and
optionally a connect timeout (e.g., --connect-timeout) to the curl invocation so
the HTTP request fails fast on network issues; modify the curl line inside
discover_json_field (which takes url and js) to include something like
--connect-timeout 5 --max-time 10 (or your desired values) so the node -e "$js"
invocation only runs on a bounded curl response.
In `@src/openhuman/inference/ops.rs`:
- Around line 126-147: Remove the hardcoded "lmstudio:"/"ollama:" prefix checks
from inference_test_provider_model and delegate the local-vs-remote decision to
the provider factory: stop calling create_local_chat_provider_from_string
directly from ops.rs and instead call a single factory entry (e.g.,
create_chat_provider_from_string or add a new create_provider_for_test) that
encapsulates local-prefix parsing inside
crate::openhuman::inference::provider::factory; if the factory lacks such
unified behavior, update the factory so it returns the same (chat_provider,
model) tuple for both local and remote providers and use that single factory
function in inference_test_provider_model (referencing
inference_test_provider_model, create_local_chat_provider_from_string,
create_chat_provider_from_string, and provider::factory).
In `@src/openhuman/inference/schemas_tests.rs`:
- Around line 4-9: The test inference_catalog_counts_match_and_nonempty
currently uses a loose guard assert!(declared.len() >= 19); change this to pin
the exact expected controller count by asserting the concrete length (e.g.,
assert_eq!(declared.len(), 19)); keep the existing assert_eq!(declared.len(),
registered.len()) and replace the >= check with the exact-number assertion so
the test fails if the public inference surface changes unexpectedly; locate this
in the function inference_catalog_counts_match_and_nonempty where declared =
all_controller_schemas() and registered = all_registered_controllers().
🪄 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: 2959572f-c634-4414-8c70-98617c8b3904
📒 Files selected for processing (18)
app/src/components/settings/panels/AIPanel.tsxapp/src/components/settings/panels/__tests__/AIPanel.test.tsxapp/src/services/api/__tests__/aiSettingsApi.test.tsapp/src/services/api/aiSettingsApi.tsapp/src/utils/tauriCommands/localAi.tsscripts/debug/live-local-provider-smoke.shsrc/openhuman/inference/local/mod.rssrc/openhuman/inference/local/ops.rssrc/openhuman/inference/local/ops_tests.rssrc/openhuman/inference/local/service/public_infer.rssrc/openhuman/inference/local/service/public_infer_tests.rssrc/openhuman/inference/ops.rssrc/openhuman/inference/ops_tests.rssrc/openhuman/inference/provider/factory.rssrc/openhuman/inference/provider/factory_test.rssrc/openhuman/inference/schemas.rssrc/openhuman/inference/schemas_tests.rssrc/openhuman/subconscious/executor.rs
💤 Files with no reviewable changes (6)
- src/openhuman/inference/local/ops_tests.rs
- app/src/utils/tauriCommands/localAi.ts
- src/openhuman/inference/local/service/public_infer_tests.rs
- src/openhuman/inference/schemas.rs
- src/openhuman/inference/local/ops.rs
- src/openhuman/inference/local/service/public_infer.rs
…logs - Fix inferRoutingMode to treat 'default' as managed (not custom) - Add try/finally around setSaving in GlobalOwnModelSelector - Extract all hard-coded routing UI strings into i18n keys - Add debug logs to local provider factory and test invocation path
The i18n coverage check requires keys present in en.ts to also exist in the chunked locale files (en-N.ts).
The i18n coverage check requires new keys to exist in every locale's chunk file, not just en-1.ts. English fallback values used until translations are provided.
12 tasks
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
local::service::public_inferpath.external-openai-compatendpoint key surface; null/empty routing strings now parse to{ kind: 'default' }.Problem
The local provider had its own dispatch (
local::service::public_infer) that bypassed the workload-aware routing/factory layer. This led to drift between cloud and local code paths and made the settings UI carry two parallel concepts (workload-level chips and a separate OpenAI-compat endpoint key). The settings panel had also outgrown the per-row Managed/Default/Custom UI as the number of workloads grew.Solution
provider::factory::create_local_chat_provider_from_stringand dispatch throughinference_test_provider_model/ shared chat provider trait.inference/local/service/public_infer.rsandinference/local/ops.rsalong with their tests; collapse the schemas accordingly.AIPanel.tsxaround the Managed / Use Your Own Models / Advanced segmented control. Advanced reveals the full workload routing matrix; Use Your Own Models surfaces the connected providers + model picker.{ kind: 'default' }ProviderRef and treatnull/'cloud'wire values as that.Submission Checklist
Impact
cloud/openhumanrouting sentinels now resolve to{ kind: 'default' }, which preserves prior managed-routing behaviour.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm compile(tsc --noEmit)AIPanel.test,aiSettingsApi.test— all greencargo checkValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
openhumansentinel.Parity Contract
"openhuman"and"openhuman:<model>"as{ kind: 'openhuman' }; the new{ kind: 'default' }covers the historicalnull/"cloud"cases that already routed to managed.create_local_chat_provider_from_stringand back throughsimple_chat, matching the cloud path.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
UI/UX Improvements