fix(provider): persist model enable toggle#7865
fix(provider): persist model enable toggle#7865ACAne0320 wants to merge 2 commits intoAstrBotDevs:masterfrom
Conversation
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Calling
loadConfig()in thefinallyoftoggleProviderEnablefor every toggle may introduce noticeable latency and extra load; consider narrowing the refresh scope (e.g., updating only the changed provider in local state or batching/debouncing full reloads) instead of reloading the entire config each time. - The
savingProviderTogglesguard prevents double-submits per provider, but concurrent toggles of different providers might still interleave with a sharedloadConfig()result; if that’s problematic, you may want to serialize config reloads or ensure newer responses cannot overwrite more recent local changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Calling `loadConfig()` in the `finally` of `toggleProviderEnable` for every toggle may introduce noticeable latency and extra load; consider narrowing the refresh scope (e.g., updating only the changed provider in local state or batching/debouncing full reloads) instead of reloading the entire config each time.
- The `savingProviderToggles` guard prevents double-submits per provider, but concurrent toggles of different providers might still interleave with a shared `loadConfig()` result; if that’s problematic, you may want to serialize config reloads or ensure newer responses cannot overwrite more recent local changes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request introduces a new toggle functionality for providers, allowing users to enable or disable them via the UI with appropriate loading states. I have identified a critical race condition in the toggleProviderEnable function where the loading state is cleared before the configuration reload completes, and a secondary issue where loadConfig fails to properly await the underlying template loading. I have provided a suggestion to reorder these operations to ensure the UI remains disabled until the backend state is fully synchronized.
| savingProviderToggles.value = savingProviderToggles.value.filter((id) => id !== provider.id) | ||
| await loadConfig() |
There was a problem hiding this comment.
There are two issues here that prevent the UI from correctly waiting for the backend to reload the provider, which is the stated goal of this PR:
- Race Condition: The
savingProviderTogglesstate is cleared before the configuration is reloaded. This re-enables the UI components (like the test button) before the backend has finished processing the update and the frontend has received the fresh state. - Broken Await: The
loadConfigfunction (defined at line 659) isasyncbut does notawaitthe call toloadProviderTemplate(). Consequently,await loadConfig()returns immediately without waiting for the network request to complete.
To fix this, you should swap the order here and ensure the definition of loadConfig at line 659 is updated to await loadProviderTemplate().
| savingProviderToggles.value = savingProviderToggles.value.filter((id) => id !== provider.id) | |
| await loadConfig() | |
| await loadConfig() | |
| savingProviderToggles.value = savingProviderToggles.value.filter((id) => id !== provider.id) |
Fixes #7863
This PR fixes a WebUI provider model toggle issue.
When a user fetches models from an OpenAI Compatible provider source, adds a model, enables it, and immediately clicks the test button, the UI previously showed the model as enabled but did not persist the enable state or reload the provider before calling
/config/provider/check_one.As a result, the backend could not find the provider in
provider_manager.inst_mapand returned:Modifications / 改动点
Persist model enable/disable changes through /api/config/provider/update when toggling a configured provider model.
Refresh provider config after the toggle update so the WebUI stays in sync with backend state.
Avoid local-only optimistic switching by replacing the model switch v-model with :model-value.
Disable the model switch and test button while the enable/disable update is being saved, preventing users from testing before the provider reload completes.
Screenshots or Test Results / 运行截图或测试结果
Manual verification path:
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Ensure provider model enable toggles are persisted and synchronized with backend state before testing.
Bug Fixes:
Enhancements: