fix(setting-model): fix instance display and api_key loss after auto-save - #16853
Conversation
dosubot
Bot
added
🐞 bug
📝 WalkthroughWalkthroughProvider settings now validate added-provider availability, refresh provider caches after instance creation, defer credential-dependent model loading, preserve form baselines, refine verification output, and render secret credentials as password fields. ChangesProvider model settings
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SettingModelV2
participant ModelsSection
participant useModelsCatalog
participant LLMFactory
SettingModelV2->>ModelsSection: pass instanceDetailsLoaded
ModelsSection->>useModelsCatalog: provide readiness state
useModelsCatalog->>LLMFactory: identify provider credential requirements
useModelsCatalog->>useModelsCatalog: resolve credentials and conditionally fetch models
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/src/pages/user-setting/setting-model/index.tsx (1)
65-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
handleDraftSaveinvalidation becomes a no-op for first-instance saves.The new
providerQueryNamereturns''when a provider has nohas_instance, so thequeryClient.invalidateQueriesat line 139–141 (which usesproviderQueryName) is a no-op for the first instance of a provider. This is safe becauseuseAddProviderInstanceinternally invalidatesproviderInstances(params.llm_factory)with the correct provider name (lines 297–299 ofuse-llm-request.tsx), and theaddedProvidersrefetch will flipproviderQueryNameto enable the query. Consider replacingproviderQueryNamewithselection as stringat line 140 to keep the invalidation effective for all cases, or remove it entirely sinceuseAddProviderInstancealready handles it.🤖 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 `@web/src/pages/user-setting/setting-model/index.tsx` around lines 65 - 71, The handleDraftSave invalidation currently uses providerQueryName, which is empty when saving a provider’s first instance. Update the queryClient.invalidateQueries call in handleDraftSave to use selection as the provider name, or remove the redundant invalidation because useAddProviderInstance already invalidates providerInstances. Preserve effective invalidation for both existing and first-instance saves.
🤖 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
`@web/src/pages/user-setting/setting-model/provider-schema/field-config/provider-config-map.ts`:
- Around line 246-252: Update the XunFei Spark provider configuration entries
for spark_api_secret and spark_api_key to use FormFieldType.Password, matching
spark_api_password, while preserving their existing labels, required settings,
placeholders, and rendering conditions.
---
Nitpick comments:
In `@web/src/pages/user-setting/setting-model/index.tsx`:
- Around line 65-71: The handleDraftSave invalidation currently uses
providerQueryName, which is empty when saving a provider’s first instance.
Update the queryClient.invalidateQueries call in handleDraftSave to use
selection as the provider name, or remove the redundant invalidation because
useAddProviderInstance already invalidates providerInstances. Preserve effective
invalidation for both existing and first-instance saves.
🪄 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: d3744f96-d12c-447e-b2b8-8ae9b54fbfa8
📒 Files selected for processing (11)
web/src/hooks/use-llm-request.tsxweb/src/pages/user-setting/setting-model/index.tsxweb/src/pages/user-setting/setting-model/instance-card/components/saved-mode-card.tsxweb/src/pages/user-setting/setting-model/instance-card/hooks.tsxweb/src/pages/user-setting/setting-model/instance-card/models-section/hooks.tsweb/src/pages/user-setting/setting-model/instance-card/models-section/index.tsxweb/src/pages/user-setting/setting-model/instance-card/models-section/interface.tsweb/src/pages/user-setting/setting-model/instance-card/verify-button.tsxweb/src/pages/user-setting/setting-model/provider-schema/field-config/generic-api-key-config.tsweb/src/pages/user-setting/setting-model/provider-schema/field-config/local-llm-configs.tsweb/src/pages/user-setting/setting-model/provider-schema/field-config/provider-config-map.ts
| { | ||
| name: 'spark_api_password', | ||
| label: 'addSparkAPIPassword', | ||
| type: FormFieldType.Text, | ||
| type: FormFieldType.Password, | ||
| required: true, | ||
| placeholder: 'SparkAPIPasswordMessage', | ||
| shouldRender: 'hideWhenInstanceExists', |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Mask all XunFei Spark credentials.
spark_api_password is now rendered as a password, but the same provider still renders spark_api_secret (Lines 265-271) and spark_api_key (Lines 274-280) as plain text. These values are submitted as credentials and should receive the same protection.
🤖 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
`@web/src/pages/user-setting/setting-model/provider-schema/field-config/provider-config-map.ts`
around lines 246 - 252, Update the XunFei Spark provider configuration entries
for spark_api_secret and spark_api_key to use FormFieldType.Password, matching
spark_api_password, while preserving their existing labels, required settings,
placeholders, and rendering conditions.
2 participants
Summary
fix(setting-model): fix instance display and api_key loss after auto-save