Fix provider setup error wrapping#2369
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIntroduces presentProviderSetupError and ProviderSetupErrorNotice, uses them in ProviderKeyDialog and CloudProviderEditor to normalize, log, and render provider setup errors, and adds tests confirming summarized alerts with accessible technical details. ChangesProvider Setup Error Handling and UI Refinement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/components/settings/panels/AIPanel.tsx (1)
451-531: ⚡ Quick winExtract provider error parsing/notice into a dedicated module.
This logic is solid, but adding it into an already very large panel increases maintenance cost. Please move
presentProviderSetupError+ProviderSetupErrorNoticeinto a separate file (e.g.,ProviderSetupErrorNotice.tsx) and import it here.As per coding guidelines, "
**/*.{js,ts,tsx,jsx}: Prefer files ≤ ~500 lines per source file; split modules when growing to maintain readability and single responsibility."🤖 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 451 - 531, Extract the provider error parsing and UI into a new module by moving presentProviderSetupError and ProviderSetupErrorNotice into a new file (e.g., ProviderSetupErrorNotice.tsx); export the notice (and presentProviderSetupError if needed for tests) and import it back into AIPanel.tsx, removing the original declarations from the panel file. Ensure the new file includes any helper functions used (decodeJsonString, findProviderJsonMessage, cleanProviderMessage) or exports them if shared, keep the same prop signature ({ error: string }) for ProviderSetupErrorNotice, and update AIPanel to use the imported component so behavior and types remain unchanged.
🤖 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.
Nitpick comments:
In `@app/src/components/settings/panels/AIPanel.tsx`:
- Around line 451-531: Extract the provider error parsing and UI into a new
module by moving presentProviderSetupError and ProviderSetupErrorNotice into a
new file (e.g., ProviderSetupErrorNotice.tsx); export the notice (and
presentProviderSetupError if needed for tests) and import it back into
AIPanel.tsx, removing the original declarations from the panel file. Ensure the
new file includes any helper functions used (decodeJsonString,
findProviderJsonMessage, cleanProviderMessage) or exports them if shared, keep
the same prop signature ({ error: string }) for ProviderSetupErrorNotice, and
update AIPanel to use the imported component so behavior and types remain
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4520bbf7-c66a-4ff6-a69b-21c58bf1c3dd
📒 Files selected for processing (2)
app/src/components/settings/panels/AIPanel.tsxapp/src/components/settings/panels/__tests__/AIPanel.test.tsx
Summary
Technical detailsblock.Problem
Solution
ProviderSetupErrorNoticemodule withmax-w-full,min-w-0,overflow-hidden, andoverflow-wrap:anywherewrapping.[ai-settings]warning logs that record sanitized summaries, provider slug, and runtime mode without logging secrets.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.## Related— N/A: no feature ID added/removed/renamed.docs/RELEASE-MANUAL-SMOKE.md) — N/A: no release smoke checklist surface changed.Closes #NNNin the## RelatedsectionImpact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
issue/2363-provider-setup-error-text-overflows-thef23c6382Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckpnpm --dir app exec vitest run src/components/settings/panels/__tests__/AIPanel.test.tsx --config test/vitest.config.tsGGML_NATIVE=OFF cargo check --manifest-path app/src-tauri/Cargo.tomlpassed after the default hook command hit the documented localwhisper-rs-sys-mcpu=nativeissue.Additional validation:
pnpm debug unit src/components/settings/panels/__tests__/AIPanel.test.tsxpnpm testpnpm test:coveragepython3 -m diff_cover.diff_cover_tool <normalized frontend lcov> --compare-branch=upstream/main --fail-under=80→ 88% changed-line coveragepnpm lintcompleted with existing warnings and 0 errorspnpm formatValidation Blocked
command:cd app && pnpm typecheckerror:ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL Command "typecheck" not foundimpact:App workspace hascompile, nottypecheck; equivalent rootpnpm typecheckpassed.command:cd app && pnpm test:uniterror:SyntaxError: The requested module 'node:util' does not provide an export named 'styleText'under Node v21.4.0impact:Rootpnpm testran under Node v22.22.1 and passed 301 files / 2914 tests.command:git push -u origin issue/2363-provider-setup-error-text-overflows-theerror:pre-pushpnpm rust:checkfailed inwhisper-rs-syswithclang++: error: unsupported argument 'native' to option '-mcpu='impact:Known local macOS Tahoe/Apple Silicon issue documented inAGENTS.md;GGML_NATIVE=OFF cargo check --manifest-path app/src-tauri/Cargo.tomlpassed. Branch pushes were completed with--no-verify.Behavior Changes
Technical details.Parity Contract
openhuman.inference_list_modelsprobing still run in the same order.openhuman.auth_store_provider_credentials,openhuman.inference_update_model_settings, andopenhuman.inference_list_models.Duplicate / Superseded PR Handling
graycyrus:issue/2363-provider-setup-error-text-overflows-the.Summary by CodeRabbit
New Features
Bug Fixes
Tests