fix: 修复 astrbot.core.star 等包下的 type checking error #2787
Merged
Conversation
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
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
你好 - 我已审阅了你的更改 - 以下是一些反馈：
- 在
register_command和register_command_group中，缺少必需参数目前只会记录警告，但仍然返回一个不起作用的装饰器；考虑抛出错误或明确返回一个无操作装饰器以实现快速失败并避免静默的错误配置。 - 你正在对插件和 MCP 客户端错误使用通用的
Exception/ValueError——引入更具体的自定义异常类型将使错误处理更清晰、更容易捕获。 - 宽泛的
Callable[..., Awaitable[Any]]注释涵盖了所有异步签名；考虑定义一个Protocol或TypeVar来缩小预期的处理程序签名并提高类型安全性。
Prompt for AI Agents
请处理此代码审查中的评论：
## 总体评论
- 在 `register_command` 和 `register_command_group` 中，缺少必需参数目前只会记录警告，但仍然返回一个不起作用的装饰器；考虑抛出错误或明确返回一个无操作装饰器以实现快速失败并避免静默的错误配置。
- 你正在对插件和 MCP 客户端错误使用通用的 `Exception`/`ValueError`——引入更具体的自定义异常类型将使错误处理更清晰、更容易捕获。
- 宽泛的 `Callable[..., Awaitable[Any]]` 注释涵盖了所有异步签名；考虑定义一个 `Protocol` 或 `TypeVar` 来缩小预期的处理程序签名并提高类型安全性。
## 独立评论
### 评论 1
<location> `astrbot/core/star/session_plugin_manager.py:143-141` </location>
<code_context>
filtered_handlers.append(handler)
continue
+ if plugin.name is None:
+ continue
+
# 检查插件是否在当前会话中启用
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 插件名称为 None 时跳过可能会隐藏注册问题。
在跳过没有名称的插件时记录警告或错误，以帮助诊断潜在的注册问题。
建议实现：
```python
filtered_handlers.append(handler)
continue
if plugin.name is None:
import logging
logging.warning(f"Skipping plugin with no name: {plugin!r}")
continue
# 检查插件是否在当前会话中启用
```
如果文件中其他地方已经导入了 logging，你可以从这个块中删除 `import logging` 行。
你可能还需要在应用程序的其他地方配置日志级别和格式（如果尚未完成）。
</issue_to_address>
### 评论 2
<location> `astrbot/core/provider/manager.py:91` </location>
<code_context>
- self.curr_provider_inst = self.inst_map[provider_id]
- if provider_type == ProviderType.TEXT_TO_SPEECH:
+
+ prov = self.inst_map[provider_id]
+ if provider_type == ProviderType.TEXT_TO_SPEECH and isinstance(
+ prov, TTSProvider
</code_context>
<issue_to_address>
**issue (complexity):** 考虑将重复的按类型分支重构为简洁的查找表和循环，以进行提供者管理。
你可以在 `get_using_provider` 和 `terminate_provider` 中将所有这些按类型分支折叠成一个小的查找表 + 单个循环。例如：
```python
# at class‐level
_PROVIDER_CONFIG = {
ProviderType.TEXT_TO_SPEECH: ("curr_tts_provider_inst", TTSProvider, "curr_provider_tts"),
ProviderType.SPEECH_TO_TEXT: ("curr_stt_provider_inst", STTProvider, "curr_provider_stt"),
ProviderType.CHAT_COMPLETION: ("curr_provider_inst", Provider, "curr_provider"),
}
```
然后在 `get_using_provider` 中：
```python
prov = self.inst_map[provider_id]
attr, cls, sp_key = self._PROVIDER_CONFIG[provider_type]
if isinstance(prov, cls):
setattr(self, attr, prov)
sp.put(sp_key, provider_id, scope="global", scope_id="global")
```
在 `terminate_provider` 中，你可以通过迭代列表和 `curr_*` 属性来删除所有 `isinstance` 检查/remove 调用：
```python
inst = self.inst_map[provider_id]
# remove from any list it lives in
for lst in (
self.provider_insts,
self.stt_provider_insts,
self.tts_provider_insts,
self.embedding_provider_insts,
):
if inst in lst:
lst.remove(inst)
# clear any curr_* pointer
for attr, _, _ in self._PROVIDER_CONFIG.values():
if getattr(self, attr) is inst:
setattr(self, attr, None)
if getattr(inst, "terminate", None):
await inst.terminate()
```
这保留了完全相同的行为，但将所有重复的 `if isinstance(...)` / `remove(...)` / `setattr(...)` / `sp.put(...)` 代码折叠成了两个非常简洁的循环。
</issue_to_address>
### 评论 3
<location> `astrbot/core/agent/mcp_client.py:202` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 抛出特定错误而非通用的 `Exception` 或 `BaseException` ([`raise-specific-error`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/raise-specific-error))
<details><summary>Explanation</summary>如果一段代码抛出特定异常类型
而不是通用的
[`BaseException`](https://docs.python.org/3/library/exceptions.html#BaseException)
或 [`Exception`](https://docs.python.org/3/library/exceptions.html#Exception)，
则调用代码可以：
- 获取有关错误类型的更多信息
- 为其定义特定的异常处理
这样，代码的调用者可以适当地处理错误。
如何解决这个问题？
- 使用标准库中的 [内置异常](https://docs.python.org/3/library/exceptions.html)。
- [定义自己的错误类](https://docs.python.org/3/tutorial/errors.html#tut-userexceptions)，该类继承自 `Exception`。
因此，与其让代码抛出 `Exception` 或 `BaseException`，例如
```python
if incorrect_input(value):
raise Exception("The input is incorrect")
```
你可以让代码抛出特定错误，例如
```python
if incorrect_input(value):
raise ValueError("The input is incorrect")
```
或者
```python
class IncorrectInputError(Exception):
pass
if incorrect_input(value):
raise IncorrectInputError("The input is incorrect")
```
</details>
</issue_to_address>
### 评论 4
<location> `astrbot/core/star/star_manager.py:691` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 抛出特定错误而非通用的 `Exception` 或 `BaseException` ([`raise-specific-error`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/raise-specific-error))
<details><summary>Explanation</summary>如果一段代码抛出特定异常类型
而不是通用的
[`BaseException`](https://docs.python.org/3/library/exceptions.html#BaseException)
或 [`Exception`](https://docs.python.org/3/library/exceptions.html#Exception)，
则调用代码可以：
- 获取有关错误类型的更多信息
- 为其定义特定的异常处理
这样，代码的调用者可以适当地处理错误。
如何解决这个问题？
- 使用标准库中的 [内置异常](https://docs.python.org/3/library/exceptions.html)。
- [定义自己的错误类](https://docs.python.org/3/tutorial/errors.html#tut-userexceptions)，该类继承自 `Exception`。
因此，与其让代码抛出 `Exception` 或 `BaseException`，例如
```python
if incorrect_input(value):
raise Exception("The input is incorrect")
```
你可以让代码抛出特定错误，例如
```python
if incorrect_input(value):
raise ValueError("The input is incorrect")
```
或者
```python
class IncorrectInputError(Exception):
pass
if incorrect_input(value):
raise IncorrectInputError("The input is incorrect")
```
</details>
</issue_to_address>
### 评论 5
<location> `astrbot/core/star/star_manager.py:808` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 抛出特定错误而非通用的 `Exception` 或 `BaseException` ([`raise-specific-error`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/raise-specific-error))
<details><summary>Explanation</summary>如果一段代码抛出特定异常类型
而不是通用的
[`BaseException`](https://docs.python.org/3/library/exceptions.html#BaseException)
或 [`Exception`](https://docs.python.org/3/library/exceptions.html#Exception)，
则调用代码可以：
- 获取有关错误类型的更多信息
- 为其定义特定的异常处理
这样，代码的调用者可以适当地处理错误。
如何解决这个问题？
- 使用标准库中的 [内置异常](https://docs.python.org/3/library/exceptions.html)。
- [定义自己的错误类](https://docs.python.org/3/tutorial/errors.html#tut-userexceptions)，该类继承自 `Exception`。
因此，与其让代码抛出 `Exception` 或 `BaseException`，例如
```python
if incorrect_input(value):
raise Exception("The input is incorrect")
```
你可以让代码抛出特定错误，例如
```python
if incorrect_input(value):
raise ValueError("The input is incorrect")
```
或者
```python
class IncorrectInputError(Exception):
pass
if incorrect_input(value):
raise IncorrectInputError("The input is incorrect")
```
</details>
</issue_to_address>
### 评论 6
<location> `astrbot/core/star/updator.py:36` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** 抛出特定错误而非通用的 `Exception` 或 `BaseException` ([`raise-specific-error`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/raise-specific-error))
<details><summary>Explanation</summary>如果一段代码抛出特定异常类型
而不是通用的
[`BaseException`](https://docs.python.org/3/library/exceptions.html#BaseException)
或 [`Exception`](https://docs.python.org/3/library/exceptions.html#Exception)，
则调用代码可以：
- 获取有关错误类型的更多信息
- 为其定义特定的异常处理
这样，代码的调用者可以适当地处理错误。
如何解决这个问题？
- 使用标准库中的 [内置异常](https://docs.python.org/3/library/exceptions.html)。
- [定义自己的错误类](https://docs.python.org/3/tutorial/errors.html#tut-userexceptions)，该类继承自 `Exception`。
因此，与其让代码抛出 `Exception` 或 `BaseException`，例如
```python
if incorrect_input(value):
raise Exception("The input is incorrect")
```
你可以让代码抛出特定错误，例如
```python
if incorrect_input(value):
raise ValueError("The input is incorrect")
```
或者
```python
class IncorrectInputError(Exception):
pass
if incorrect_input(value):
raise IncorrectInputError("The input is incorrect")
```
</details>
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- In register_command and register_command_group, missing required parameters currently only log warnings but still return a decorator that does nothing; consider raising errors or explicitly returning a no-op decorator to fail fast and avoid silent misconfigurations.
- You’re using generic Exception/ValueError for plugin and MCP client errors—introducing more specific custom exception types would make error handling clearer and easier to catch.
- The broad Callable[..., Awaitable[Any]] annotations cover all async signatures; consider defining a Protocol or TypeVar to narrow the expected handler signature and improve type safety.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In register_command and register_command_group, missing required parameters currently only log warnings but still return a decorator that does nothing; consider raising errors or explicitly returning a no-op decorator to fail fast and avoid silent misconfigurations.
- You’re using generic Exception/ValueError for plugin and MCP client errors—introducing more specific custom exception types would make error handling clearer and easier to catch.
- The broad Callable[..., Awaitable[Any]] annotations cover all async signatures; consider defining a Protocol or TypeVar to narrow the expected handler signature and improve type safety.
## Individual Comments
### Comment 1
<location> `astrbot/core/star/session_plugin_manager.py:143-141` </location>
<code_context>
filtered_handlers.append(handler)
continue
+ if plugin.name is None:
+ continue
+
# 检查插件是否在当前会话中启用
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Skipping plugins with None name may hide registry issues.
Log a warning or error when skipping plugins with no name to aid in diagnosing potential registry problems.
Suggested implementation:
```python
filtered_handlers.append(handler)
continue
if plugin.name is None:
import logging
logging.warning(f"Skipping plugin with no name: {plugin!r}")
continue
# 检查插件是否在当前会话中启用
```
If logging is already imported elsewhere in the file, you can remove the `import logging` line from this block.
You may also want to configure the logging level and format elsewhere in your application if not already done.
</issue_to_address>
### Comment 2
<location> `astrbot/core/provider/manager.py:91` </location>
<code_context>
- self.curr_provider_inst = self.inst_map[provider_id]
- if provider_type == ProviderType.TEXT_TO_SPEECH:
+
+ prov = self.inst_map[provider_id]
+ if provider_type == ProviderType.TEXT_TO_SPEECH and isinstance(
+ prov, TTSProvider
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring repetitive per-type branches into concise lookup tables and loops for provider management.
You can collapse all of those per-type branches into a small lookup table + single loop in both `get_using_provider` and `terminate_provider`. For example:
```python
# at class‐level
_PROVIDER_CONFIG = {
ProviderType.TEXT_TO_SPEECH: ("curr_tts_provider_inst", TTSProvider, "curr_provider_tts"),
ProviderType.SPEECH_TO_TEXT: ("curr_stt_provider_inst", STTProvider, "curr_provider_stt"),
ProviderType.CHAT_COMPLETION: ("curr_provider_inst", Provider, "curr_provider"),
}
```
Then in `get_using_provider`:
```python
prov = self.inst_map[provider_id]
attr, cls, sp_key = self._PROVIDER_CONFIG[provider_type]
if isinstance(prov, cls):
setattr(self, attr, prov)
sp.put(sp_key, provider_id, scope="global", scope_id="global")
```
And in `terminate_provider` you can drop all of the `isinstance` checks/remove calls by iterating your lists and `curr_*` attrs:
```python
inst = self.inst_map[provider_id]
# remove from any list it lives in
for lst in (
self.provider_insts,
self.stt_provider_insts,
self.tts_provider_insts,
self.embedding_provider_insts,
):
if inst in lst:
lst.remove(inst)
# clear any curr_* pointer
for attr, _, _ in self._PROVIDER_CONFIG.values():
if getattr(self, attr) is inst:
setattr(self, attr, None)
if getattr(inst, "terminate", None):
await inst.terminate()
```
This preserves exactly the same behavior but collapses all of your repetitive `if isinstance(...)` / `remove(...)` / `setattr(...)` / `sp.put(...)` code into two very concise loops.
</issue_to_address>
### Comment 3
<location> `astrbot/core/agent/mcp_client.py:202` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Raise a specific error instead of the general `Exception` or `BaseException` ([`raise-specific-error`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/raise-specific-error))
<details><summary>Explanation</summary>If a piece of code raises a specific exception type
rather than the generic
[`BaseException`](https://docs.python.org/3/library/exceptions.html#BaseException)
or [`Exception`](https://docs.python.org/3/library/exceptions.html#Exception),
the calling code can:
- get more information about what type of error it is
- define specific exception handling for it
This way, callers of the code can handle the error appropriately.
How can you solve this?
- Use one of the [built-in exceptions](https://docs.python.org/3/library/exceptions.html) of the standard library.
- [Define your own error class](https://docs.python.org/3/tutorial/errors.html#tut-userexceptions) that subclasses `Exception`.
So instead of having code raising `Exception` or `BaseException` like
```python
if incorrect_input(value):
raise Exception("The input is incorrect")
```
you can have code raising a specific error like
```python
if incorrect_input(value):
raise ValueError("The input is incorrect")
```
or
```python
class IncorrectInputError(Exception):
pass
if incorrect_input(value):
raise IncorrectInputError("The input is incorrect")
```
</details>
</issue_to_address>
### Comment 4
<location> `astrbot/core/star/star_manager.py:691` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Raise a specific error instead of the general `Exception` or `BaseException` ([`raise-specific-error`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/raise-specific-error))
<details><summary>Explanation</summary>If a piece of code raises a specific exception type
rather than the generic
[`BaseException`](https://docs.python.org/3/library/exceptions.html#BaseException)
or [`Exception`](https://docs.python.org/3/library/exceptions.html#Exception),
the calling code can:
- get more information about what type of error it is
- define specific exception handling for it
This way, callers of the code can handle the error appropriately.
How can you solve this?
- Use one of the [built-in exceptions](https://docs.python.org/3/library/exceptions.html) of the standard library.
- [Define your own error class](https://docs.python.org/3/tutorial/errors.html#tut-userexceptions) that subclasses `Exception`.
So instead of having code raising `Exception` or `BaseException` like
```python
if incorrect_input(value):
raise Exception("The input is incorrect")
```
you can have code raising a specific error like
```python
if incorrect_input(value):
raise ValueError("The input is incorrect")
```
or
```python
class IncorrectInputError(Exception):
pass
if incorrect_input(value):
raise IncorrectInputError("The input is incorrect")
```
</details>
</issue_to_address>
### Comment 5
<location> `astrbot/core/star/star_manager.py:808` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Raise a specific error instead of the general `Exception` or `BaseException` ([`raise-specific-error`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/raise-specific-error))
<details><summary>Explanation</summary>If a piece of code raises a specific exception type
rather than the generic
[`BaseException`](https://docs.python.org/3/library/exceptions.html#BaseException)
or [`Exception`](https://docs.python.org/3/library/exceptions.html#Exception),
the calling code can:
- get more information about what type of error it is
- define specific exception handling for it
This way, callers of the code can handle the error appropriately.
How can you solve this?
- Use one of the [built-in exceptions](https://docs.python.org/3/library/exceptions.html) of the standard library.
- [Define your own error class](https://docs.python.org/3/tutorial/errors.html#tut-userexceptions) that subclasses `Exception`.
So instead of having code raising `Exception` or `BaseException` like
```python
if incorrect_input(value):
raise Exception("The input is incorrect")
```
you can have code raising a specific error like
```python
if incorrect_input(value):
raise ValueError("The input is incorrect")
```
or
```python
class IncorrectInputError(Exception):
pass
if incorrect_input(value):
raise IncorrectInputError("The input is incorrect")
```
</details>
</issue_to_address>
### Comment 6
<location> `astrbot/core/star/updator.py:36` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Raise a specific error instead of the general `Exception` or `BaseException` ([`raise-specific-error`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/raise-specific-error))
<details><summary>Explanation</summary>If a piece of code raises a specific exception type
rather than the generic
[`BaseException`](https://docs.python.org/3/library/exceptions.html#BaseException)
or [`Exception`](https://docs.python.org/3/library/exceptions.html#Exception),
the calling code can:
- get more information about what type of error it is
- define specific exception handling for it
This way, callers of the code can handle the error appropriately.
How can you solve this?
- Use one of the [built-in exceptions](https://docs.python.org/3/library/exceptions.html) of the standard library.
- [Define your own error class](https://docs.python.org/3/tutorial/errors.html#tut-userexceptions) that subclasses `Exception`.
So instead of having code raising `Exception` or `BaseException` like
```python
if incorrect_input(value):
raise Exception("The input is incorrect")
```
you can have code raising a specific error like
```python
if incorrect_input(value):
raise ValueError("The input is incorrect")
```
or
```python
class IncorrectInputError(Exception):
pass
if incorrect_input(value):
raise IncorrectInputError("The input is incorrect")
```
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Motivation / 动机
减少 unexpected behavior
Modifications / 改动点
主要都集中于 pyright type checking 的报错
Verification Steps / 验证步骤
Screenshots or Test Results / 运行截图或测试结果
Compatibility & Breaking Changes / 兼容性与破坏性变更
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Sourcery 总结
优化类型注解并添加运行时检查，以消除核心 star、provider、agent 和 tool 模块中的 Pyright 类型检查错误。
改进：
Callable[..., Awaitable[Any]]，并对可选参数使用联合管道语法。register_command和register_command_group装饰器中，针对缺失的参数添加日志警告。ValueError。Original summary in English
Summary by Sourcery
Refine type annotations and add runtime checks to eliminate Pyright type checking errors across core star, provider, agent, and tool modules.
Enhancements: