fix: filter stale fallback chat models#8480
Open
Clhikari wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/unit/test_config_route_provider_list.py" line_range="13-21" />
<code_context>
+ assert not _provider_config_enabled({"id": "disabled-provider", "enable": False})
+
+
+def test_provider_config_selectable_requires_loaded_non_agent_provider():
+ inst_map = {"loaded-provider": object()}
+
+ assert _provider_config_selectable(
+ {"id": "loaded-provider", "enable": True},
+ ["chat_completion"],
+ inst_map,
+ )
+ assert not _provider_config_selectable(
+ {"id": "unloaded-provider", "enable": True},
+ ["chat_completion"],
</code_context>
<issue_to_address>
**suggestion (testing):** Include tests for `_provider_config_selectable` when `id` is missing or not a string.
Since the helper does `provider_id = provider_config.get("id")` and `isinstance(provider_id, str)`, please add tests that cover configs with no `id` key and with a non-string `id`, asserting they are treated as non-selectable. This will document and preserve behavior for malformed or legacy configs.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces functionality to prune stale or disabled provider IDs from fallback chat models and adds helper functions to filter selectable provider configurations. The review feedback highlights a potential bug in _provider_config_selectable where checking for agent_runner in the type list could incorrectly bypass loaded checks for other provider types, and suggests checking the provider's type directly instead. Additionally, the feedback recommends using boolean or for default values to handle None or empty strings correctly, and suggests simplifying the fallback logic for provider_type in get_enabled_chat_provider_ids.
- prune disabled, missing, duplicate, and non-chat providers from fallback_chat_models when core config or provider configs change - hide disabled or unloaded providers from provider selection lists while keeping agent runners selectable - add regression coverage for fallback pruning and provider list filtering
0fe77e0 to
b7e8b95
Compare
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
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.
Fixes #8479
这个 PR 修复“回退对话模型列表”里会保留已禁用或已删除模型的问题
之前如果某个对话模型已经被删除、禁用,或者因为配置变更不再可用,它仍然可能留在“回退对话模型列表”中。主对话模型请求失败后,AstrBot 会继续尝试读取这个无效模型,然后在日志里出现类似下面的警告:
本次改动在后端做了统一过滤:
设计说明
这里没有新增 hook,也没有在聊天请求运行时直接写配置
原因是这个问题本质上是 provider 配置的一致性问题,放在配置保存和 provider 生命周期管理处处理更直观;如果在普通聊天请求里发现无效 ID 后再持久化配置,副作用会更大,也更难排查
测试
已运行:
测试结果:
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 fallback chat model configuration only references currently enabled and loaded chat providers and stays consistent with provider lifecycle changes.
Bug Fixes:
Enhancements:
Tests: