fix: validate all provider selections after server sync#266
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace inline LLM-only validation from PR #232 with unified validation for all provider types (LLM, TTS, ASR, PDF, Image, Video). - Extract isProviderUsable, validateProvider, validateModel into lib/store/settings-validation.ts - Usability checks server config and client API key, not requiresApiKey - Auto-select fallback to usable providers instead of clearing to empty - TTS/ASR/PDF fall back to local defaults (browser-native, unpdf) - Image/Video clear to empty when no server provider available Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Server-configured providers auto-select on first load, making the manual configuration prompt unnecessary.
45fe0ba to
2b73dfb
Compare
Auto-close imageGenerationEnabled/videoGenerationEnabled when server removes all providers. Prevent re-enabling until a provider becomes available.
When server adds image/video providers after empty state, auto-select the first available provider+model and enable generation. When server removes all providers, disable generation and prevent re-enabling.
Fix cases where provider matches default but generation stays disabled, and where model is empty after provider recovery. Split provider and model spreads so model updates independently of provider changes.
loadEnvSection now accepts allowBaseUrlOnly option. PDF providers like mineru that only need a baseUrl (no API key) are now correctly included in the server provider response.
ccda4ee to
6985225
Compare
Only auto-disable image/video generation when no provider is usable. Never auto-enable — user controls when to turn it on. First-run auto-enable via autoConfigApplied guard is preserved. Also remove dead setup-needed CSS animations from globals.css.
cosarah
left a comment
There was a problem hiding this comment.
Issues
Important
settings.ts:925-939 — Fallback arrays for TTS/ASR/PDF/Image/Video only include server-configured providers, while LLM's fallback also includes providers with client-side API keys. This means if a user manually sets a client API key for a non-LLM provider (e.g. OpenAI TTS) and the server config for that provider is later removed, the fallback will skip it and force-switch to browser-native — even though the client key is still valid. Consider unifying the fallback strategy across all provider types.
Minor
settings.ts:1150-1171— Auto-select spreads after validation on first load, silently overriding validated results. A comment documenting the intended precedence would help maintainability.settings-validation.ts:30/settings.ts:977—validateProvider('')short-circuits to'', forcing auto-recover to duplicate the "pick first fallback" logic.settings.ts:1000-1003—|| imageModels[0]?.idis redundant withvalidateModel's own fallback.
Summary
Solid PR — clean extraction of validation helpers, unified approach for all 6 provider types, thorough test coverage. Main concern is the inconsistent fallback strategy for non-LLM providers that could drop valid client-key configurations.
…types All provider types now fall back to client-key-only providers (not just server-configured ones), matching the LLM fallback strategy. Extracted buildFallback helper to reduce duplication. Added spread precedence comment per review feedback.
cosarah
left a comment
There was a problem hiding this comment.
All previous review items addressed:
- Fallback strategy unified via
buildFallback<T>()— all 6 provider types now include both server-configured and client-key providers. Core issue resolved. - Auto-select override precedence documented with clear comments.
- Redundant
imageModels[0]?.idfallback commented.
Clean fix. LGTM.
Summary
isProviderUsable,validateProvider,validateModelintolib/store/settings-validation.tsisServerConfigured || apiKey— does NOT userequiresApiKey(e.g. mineru needs server baseUrl, not API key)requiresBaseUrloption inloadEnvSection)Closes #263
Test plan
🤖 Generated with Claude Code