Fix provider model testing and add MCP coming soon placeholder#2570
Merged
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Contributor
|
Warning Review limit reached
Your plan currently allows 5 reviews/hour. Refill in 27 minutes and 51 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 (4)
📝 WalkthroughWalkthroughAdds end-to-end provider/model testing (backend RPC + frontend Test UI), refactors cloud provider slug generation/validation, integrates LM Studio reasoning_content fallback and local-runtime enable persistence, replaces MCP Servers tab with a "Coming Soon" panel, and adds tests and i18n entries. ChangesProvider Model Testing Feature
Cloud Provider Editor Slug Refactor
LM Studio Integration and Reasoning Content
MCP Coming Soon Placeholder & i18n
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
Bot
added
the
feature
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/components/settings/panels/AIPanel.tsx (2)
2700-2758:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRollback
local_aias well when the local-runtime probe fails.The LM Studio/Ollama branch persists
openhumanUpdateLocalAiSettings(...)before the/modelsverification runs, but the failure path only restorescloud_providers. If the probe rejects, the chip stays off whileconfig.local_aiis still enabled and pointed at the dead endpoint.A simple fix is to move the
openhumanUpdateLocalAiSettings(...)call until afterlistProviderModels(slug)succeeds, or snapshot and restore the previous local settings in the catch path.🤖 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 2700 - 2758, The local-runtime branches call openhumanUpdateLocalAiSettings(...) before probing with listProviderModels(slug), so on probe failure config.local_ai stays enabled; either move the openhumanUpdateLocalAiSettings(...) invocation to after a successful listProviderModels(slug) probe (i.e., perform the upsert/flush/list sequence first, then set local_ai for slug === 'ollama' or 'lmstudio'), or capture the previous local settings into a snapshot variable before updating and, in the catch block (where you already flushCloudProviders(priorWireProviders) and clearCloudProviderKey(slug)), restore that snapshot via openhumanUpdateLocalAiSettings(...) to roll back local_ai; reference functions/values: openhumanUpdateLocalAiSettings, listProviderModels, flushCloudProviders, clearCloudProviderKey, priorWireProviders, upserted, slug, isLocalRuntime.
1942-2051: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winInternationalize the new AI settings copy.
The new test-panel/editor strings here are user-visible (
Temperature override,Test,Model response,Name,OpenAI URL, slug errors, placeholders, etc.), but they’re hard-coded instead of going throughuseT(). That breaks the repo’s app-wide i18n contract.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: 2812-2889
🤖 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 1942 - 2051, Several user-visible literals in the AI settings panel (e.g., "Temperature override", slider/number aria-labels, "Lower = more deterministic. Leave unchecked to use the provider default.", "Test", "Testing…", "Model response", "Test failed", "Testing model…", "Provider:", "Prompt: Hello world", "Waiting for response from the selected model…", etc.) must be internationalized: import and use useT() from app/src/lib/i18n/I18nContext in AIPanel.tsx and replace each hard-coded string used in JSX, aria-labels and button text with t('your.key.here') calls (update references around temperature, handleTest, testBusy, testReply, testError, currentProviderString), then add matching keys and English values to app/src/lib/i18n/en.ts in this PR; ensure placeholders, aria-labels and status messages are covered and preserve existing formatting like interpolation or number formatting where needed.
🤖 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 1748-1752: The useEffect currently performing synchronous state
resets (setTestReply, setTestError, setTestStartedAt) when [source, model,
temperature] change violates the react-hooks/set-state-in-effect rule; remove
these setState calls from the useEffect and instead reset test state inside the
event handlers or setters that update source, model, or temperature (or compute
test state derived from the current selection), e.g., update the functions that
change source, updateModel, and updateTemperature (or the onChange handlers for
those inputs) to call the three reset setters so the resets occur as part of the
initiating action rather than inside useEffect.
In `@app/src/pages/Skills.tsx`:
- Around line 286-310: McpComingSoonPanel currently hard-codes user-facing
strings; replace them with localized keys by adding entries to
app/src/lib/i18n/en.ts (e.g. skills.mcp.title, skills.mcp.description,
skills.mcp.badge) and then update McpComingSoonPanel to import and call useT()
and replace the literal JSX strings with t('skills.mcp.title'),
t('skills.mcp.description'), and t('skills.mcp.badge'); ensure the new keys
exist in en.ts and the component imports useT from the i18n module so runtime
lookup works.
In `@app/src/services/api/aiSettingsApi.ts`:
- Around line 388-393: The current code silently treats a missing RPC payload as
success by returning { reply: '' }; instead, validate the RPC response from
callCoreRpc (the result returned for method
'openhuman.inference_test_provider_model') and if res or res.result is missing
or malformed, throw or return an explicit error (include context like provider,
workload, and method name) rather than returning an empty reply; update the
block around callCoreRpc/ProviderModelTestResult/PROVIDER_MODEL_TEST_TIMEOUT_MS
to check res and res.result and surface a contract failure (throw an Error or
return a rejected Promise) so callers can handle the RPC contract violation.
In `@src/openhuman/inference/local/lm_studio.rs`:
- Around line 195-208: The effective_content() helper currently returns content
or reasoning_content without removing LM Studio "think" blocks, so it can leak
<think>...</think> payloads; update effective_content() to apply the same
think-tag stripping used by the OpenAI-compatible extraction path before
trimming/emptiness checks and before returning a String (i.e., when mapping both
self.content and self.reasoning_content, run a function that removes
surrounding/inner <think>...</think> tags and then trim, filter empty, and
fallback as before); reference the effective_content() method and the fields
content and reasoning_content when making the change so the logic mirrors the
shared think-tag stripping behavior.
---
Outside diff comments:
In `@app/src/components/settings/panels/AIPanel.tsx`:
- Around line 2700-2758: The local-runtime branches call
openhumanUpdateLocalAiSettings(...) before probing with
listProviderModels(slug), so on probe failure config.local_ai stays enabled;
either move the openhumanUpdateLocalAiSettings(...) invocation to after a
successful listProviderModels(slug) probe (i.e., perform the upsert/flush/list
sequence first, then set local_ai for slug === 'ollama' or 'lmstudio'), or
capture the previous local settings into a snapshot variable before updating
and, in the catch block (where you already
flushCloudProviders(priorWireProviders) and clearCloudProviderKey(slug)),
restore that snapshot via openhumanUpdateLocalAiSettings(...) to roll back
local_ai; reference functions/values: openhumanUpdateLocalAiSettings,
listProviderModels, flushCloudProviders, clearCloudProviderKey,
priorWireProviders, upserted, slug, isLocalRuntime.
- Around line 1942-2051: Several user-visible literals in the AI settings panel
(e.g., "Temperature override", slider/number aria-labels, "Lower = more
deterministic. Leave unchecked to use the provider default.", "Test",
"Testing…", "Model response", "Test failed", "Testing model…", "Provider:",
"Prompt: Hello world", "Waiting for response from the selected model…", etc.)
must be internationalized: import and use useT() from
app/src/lib/i18n/I18nContext in AIPanel.tsx and replace each hard-coded string
used in JSX, aria-labels and button text with t('your.key.here') calls (update
references around temperature, handleTest, testBusy, testReply, testError,
currentProviderString), then add matching keys and English values to
app/src/lib/i18n/en.ts in this PR; ensure placeholders, aria-labels and status
messages are covered and preserve existing formatting like interpolation or
number formatting where needed.
🪄 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: 67cc4a25-b806-423d-adf0-2fb4c1d5ea3a
📒 Files selected for processing (12)
app/src/components/settings/panels/AIPanel.tsxapp/src/components/settings/panels/__tests__/AIPanel.test.tsxapp/src/pages/Skills.tsxapp/src/pages/__tests__/Skills.mcp-coming-soon.test.tsxapp/src/services/api/__tests__/aiSettingsApi.test.tsapp/src/services/api/aiSettingsApi.tssrc/openhuman/inference/local/lm_studio.rssrc/openhuman/inference/local/service/lm_studio.rssrc/openhuman/inference/local/service/public_infer_tests.rssrc/openhuman/inference/ops.rssrc/openhuman/inference/ops_tests.rssrc/openhuman/inference/schemas.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/services/api/aiSettingsApi.ts (1)
380-399: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd namespaced debug logs around provider-model test RPC flow.
This changed external-call path should log entry/error/success (without prompt content) using a namespaced
debug()logger for diagnostics parity with app standards.🧭 Proposed patch
+import debug from 'debug'; import { callCoreRpc, getCoreHttpBaseUrl } from '../../services/coreRpcClient'; @@ +const log = debug('openhuman:services:ai-settings-api'); + export async function testProviderModel( workload: WorkloadId, provider: string, prompt = 'Hello world' ): Promise<ProviderModelTestResult> { if (!isTauri()) { + log('[testProviderModel] unavailable-non-tauri', { workload, provider }); throw new Error('Model testing is only available in the desktop app.'); } + log('[testProviderModel] start', { workload, provider }); const res = await callCoreRpc<{ result: ProviderModelTestResult }>({ method: 'openhuman.inference_test_provider_model', params: { workload, provider, prompt }, timeoutMs: PROVIDER_MODEL_TEST_TIMEOUT_MS, }); if (!res?.result) { + log('[testProviderModel] missing-result', { workload, provider }); throw new Error( `Model test RPC returned no result for ${workload} via ${provider} (openhuman.inference_test_provider_model).` ); } + log('[testProviderModel] success', { + workload, + provider, + replyLength: res.result.reply.length, + }); return res.result; }As per coding guidelines "Debug logging in the app should be namespaced using
debug()from a named logger instance and include dev-only detail. Follow the same patterns as Rust logging: entry/exit, branches, external calls, state changes. Never log secrets or full PII."🤖 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 380 - 399, Wrap the testProviderModel flow with namespaced debug logs: create/use a module logger (e.g. debug('aiSettingsApi:testProviderModel')) and add an entry log at the start of testProviderModel that records workload and provider (do not log prompt), a debug log immediately before the callCoreRpc external call, a debug log on successful return recording provider/workload and that result was received, and a debug/error log in the branch where !res?.result or on thrown exceptions that includes the caught error details (but never include prompt or PII). Ensure you call debug() (not console.log) and keep messages concise and namespaced to match app logging conventions.
🤖 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 1748-1752: resetTestState clears visible test fields but does not
invalidate an in-flight testProviderModel(...) request, so stale replies/errors
can be applied to a new selection; add a simple versioning/cancellation
mechanism: introduce a numeric testRequestId (ref or state) used by the async
test call (in the function that calls testProviderModel) and increment it inside
resetTestState and any input-change handlers (provider/model/temperature), then
before setting setTestReply/setTestError/setTestStartedAt verify the requestId
matches the current value so stale completions are ignored; apply the same
pattern to other handlers mentioned (lines around 1850-1857, 1876-1879,
1912-1915, 1923-1926, 1943-1946, 1961-1964, 1984-1987, 1999-2002) or
alternatively disable controls while testBusy is true.
In `@src/openhuman/inference/local/lm_studio.rs`:
- Around line 195-208: The effective_content() flow needs safe diagnostics: add
tracing::debug (or the project's logger) calls that log entry to
effective_content(), which branch was taken ("content" vs "reasoning_content")
and the output length (not the raw content), and a final exit log with the
returned string length; implement this around the existing content ->
strip_think_tags -> trim pipeline and the fallback reasoning_content pipeline
(reference effective_content(), content, reasoning_content, and
strip_think_tags) so you compute and log value.len() after trimming but never
log the value itself.
---
Outside diff comments:
In `@app/src/services/api/aiSettingsApi.ts`:
- Around line 380-399: Wrap the testProviderModel flow with namespaced debug
logs: create/use a module logger (e.g. debug('aiSettingsApi:testProviderModel'))
and add an entry log at the start of testProviderModel that records workload and
provider (do not log prompt), a debug log immediately before the callCoreRpc
external call, a debug log on successful return recording provider/workload and
that result was received, and a debug/error log in the branch where !res?.result
or on thrown exceptions that includes the caught error details (but never
include prompt or PII). Ensure you call debug() (not console.log) and keep
messages concise and namespaced to match app logging conventions.
🪄 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: 9c18a90b-8352-4122-ac77-cf14808c303c
📒 Files selected for processing (17)
app/src/components/settings/panels/AIPanel.tsxapp/src/lib/i18n/chunks/ar-1.tsapp/src/lib/i18n/chunks/bn-1.tsapp/src/lib/i18n/chunks/de-1.tsapp/src/lib/i18n/chunks/es-1.tsapp/src/lib/i18n/chunks/fr-1.tsapp/src/lib/i18n/chunks/hi-1.tsapp/src/lib/i18n/chunks/id-1.tsapp/src/lib/i18n/chunks/it-1.tsapp/src/lib/i18n/chunks/ko-1.tsapp/src/lib/i18n/chunks/pt-1.tsapp/src/lib/i18n/chunks/ru-1.tsapp/src/lib/i18n/chunks/zh-CN-1.tsapp/src/lib/i18n/en.tsapp/src/pages/Skills.tsxapp/src/services/api/aiSettingsApi.tssrc/openhuman/inference/local/lm_studio.rs
✅ Files skipped from review due to trivial changes (7)
- app/src/lib/i18n/chunks/ko-1.ts
- app/src/lib/i18n/chunks/ar-1.ts
- app/src/lib/i18n/chunks/bn-1.ts
- app/src/lib/i18n/chunks/de-1.ts
- app/src/lib/i18n/chunks/id-1.ts
- app/src/lib/i18n/en.ts
- app/src/lib/i18n/chunks/pt-1.ts
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
openhuman.inference_test_provider_modelRPC for one-off provider/model probes.Problem
Solution
Testaction in custom routing, backed by a new inference RPC that exercises an explicit provider/model binding without persisting routing changes.{ reply }payload from the RPC, routing LM Studio through the local-runtime path, falling back toreasoning_contentwhencontentis empty, and removing the overly aggressivemax_tokens: 128cap from provider tests.Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/coverage.yml. Runpnpm test:coverageandpnpm test:rustlocally; PRs below 80% on changed lines will not merge.docs/TEST-COVERAGE-MATRIX.mdreflect this change (N/A: behavior-only/UI/runtime fixes, no matrix rows added or renamed)## Related(N/A: no feature ID additions/removals were required for this change)docs/RELEASE-MANUAL-SMOKE.md) (N/A: no release-cut smoke checklist changes required)Closes #NNNin the## Relatedsection (N/A: no issue number was provided for this branch)Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
codex/production-changes73b3008aeValidation Run
pnpm --filter openhuman-app format:checkpnpm typecheckpnpm debug unit src/services/api/__tests__/aiSettingsApi.test.tspnpm debug unit src/components/settings/panels/__tests__/AIPanel.test.tsxpnpm debug unit src/pages/__tests__/Skills.mcp-coming-soon.test.tsxcargo fmt --manifest-path ../Cargo.toml --all --check(via pre-push)GGML_NATIVE=OFF cargo check --manifest-path Cargo.tomlcargo fmt --manifest-path app/src-tauri/Cargo.toml --all --check(via pre-push)cargo check --manifest-path app/src-tauri/Cargo.toml(via pre-push /pnpm rust:check)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
{ reply }inference RPC response, and added LM Studioreasoning_contentfallback instead of changing upstream payload assumptions.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Improvements
Tests