refactor(web): update APIs to support web panel configuration #1393
Conversation
This file contains 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
审阅者指南 by Sourcery这个拉取请求重构了框架的几个核心组件，以支持 Web 面板配置并改进整体 API 一致性。主要变更包括：使用虚拟持久化更新内存管理器测试，改进插件加载，对工作流构建器和块注册进行广泛重构（包括解耦构造函数中的容器依赖），为检索规则、LLM 和 IM 适配器的配置架构添加新的 API 端点，以及更新文档。此外，PR 还包括大量测试调整以及日志和错误处理方面的增强。 更新的 PluginLoader 和 WorkflowBuilder 类图（类图）classDiagram
class PluginLoader {
- container: DependencyContainer
- plugin_dir: str
- logger: Logger
- _loaded_entry_points: set
- internal_plugins: List
- config: GlobalConfig
+ register_plugin(plugin_class: Type[Plugin], plugin_name: str)
+ _load_external_plugin(plugin_name: str)
+ instantiate_plugin(plugin_class)
+ enable_plugin(plugin_name: str)
+ disable_plugin(plugin_name: str)
+ update_plugin(plugin_name: str)
+ discover_external_plugins()
}
class WorkflowBuilder {
- name: str
- head: Node
- current: Node
- blocks: List<Block>
- nodes_by_name: Dict<string, Node>
+ __init__(name: str)
+ build(container: DependencyContainer): Workflow
+ save_to_yaml(file_path: str, container: DependencyContainer)
+ load_from_yaml(file_path: str, container: DependencyContainer): WorkflowBuilder
+ chain(block)
+ parallel(blocks: List)
}
class BlockSpec {
- block_class: Type<Block>
- name: string
- kwargs: Dict<string, Any>
- wire_from: Optional<List<string>>
+ __post_init__()
}
PluginLoader ..> GlobalConfig : uses
WorkflowBuilder ..> Node : creates
WorkflowBuilder ..> Block : manages
BlockSpec ..> Block : specifies
文件级变更
提示和命令与 Sourcery 交互
自定义您的体验访问您的仪表板以：
获取帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request refactors several core components of the framework to support web panel configuration and improve overall API consistency. Major changes include updates to memory manager testing with dummy persistence, improvements to plugin loading, extensive refactoring of workflow builders and block registration (including decoupling container dependency from constructors), addition of new API endpoints for retrieving configuration schemas for rules, LLM, and IM adapters, and documentation updates. Additionally, the PR includes numerous test adjustments and enhancements in logging and error handling. Updated class diagram for PluginLoader and WorkflowBuilder (Class Diagram)classDiagram
class PluginLoader {
- container: DependencyContainer
- plugin_dir: str
- logger: Logger
- _loaded_entry_points: set
- internal_plugins: List
- config: GlobalConfig
+ register_plugin(plugin_class: Type[Plugin], plugin_name: str)
+ _load_external_plugin(plugin_name: str)
+ instantiate_plugin(plugin_class)
+ enable_plugin(plugin_name: str)
+ disable_plugin(plugin_name: str)
+ update_plugin(plugin_name: str)
+ discover_external_plugins()
}
class WorkflowBuilder {
- name: str
- head: Node
- current: Node
- blocks: List<Block>
- nodes_by_name: Dict<string, Node>
+ __init__(name: str)
+ build(container: DependencyContainer): Workflow
+ save_to_yaml(file_path: str, container: DependencyContainer)
+ load_from_yaml(file_path: str, container: DependencyContainer): WorkflowBuilder
+ chain(block)
+ parallel(blocks: List)
}
class BlockSpec {
- block_class: Type<Block>
- name: string
- kwargs: Dict<string, Any>
- wire_from: Optional<List<string>>
+ __post_init__()
}
PluginLoader ..> GlobalConfig : uses
WorkflowBuilder ..> Node : creates
WorkflowBuilder ..> Block : manages
BlockSpec ..> Block : specifies
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嘿 @lss233 - 我已经审查了你的更改 - 以下是一些反馈：
整体评论：
- 考虑重构容器分配到块，以便容器注入能够统一处理，而不是在测试中需要手动分配。
- 确保对 WorkflowBuilder 的构建方法和相关工厂方法的更改在所有使用处都一致反映，以避免容器注入方面的混淆。
- 检查 WorkflowRegistry.get_workflow_path 中的新正则表达式检查边缘情况，确保它们匹配所有有效的工作流和组 ID。
以下是我在审查期间查看的内容
- 🟡 一般性问题：发现 4 个问题
- 🟢 安全性：一切看起来都很好
- 🟡 测试：发现 2 个问题
- 🟢 复杂性：一切看起来都很好
- 🟢 文档：一切看起来都很好
帮助我变得更有用！请在每条评论上点击 👍 或 👎，我将使用这些反馈来改进你的评论。
Original comment in English
Hey @lss233 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider refactoring container assignment to blocks so that container injection is handled uniformly rather than needing manual assignment in tests.
- Ensure that the changes to WorkflowBuilder's build method and related factory methods are consistently reflected in all usages to avoid confusion around container injection.
- Review the new regex checks in WorkflowRegistry.get_workflow_path for edge cases and ensure they match all valid workflow and group IDs.
Here's what I looked at during the review
- 🟡 General issues: 4 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return list(self._blocks.values()) | ||
|
|
||
| def extract_block_info(self, block_type: Type[Block]) -> Tuple[Dict[str, BlockInput], Dict[str, BlockOutput], Dict[str, BlockConfig]]: |
There was a problem hiding this comment.
建议： 考虑改进 extract_block_info 中的 Optional 类型检测。
当前的检查使用 'hasattr(param_type, 'args') 和 param_type.name == 'Optional''。更健壮的方法可能涉及使用 'typing.get_origin' 和 'typing.get_args' 来正确解释 Optional[T] 和其他 Union 类型，这可以避免类型注解中的微妙错误。
建议的实现：
from typing import Dict, Tuple, List, Type, get_origin, get_args, Union for name, param_type in block_type.__annotations__.items():
if get_origin(param_type) is Union and type(None) in get_args(param_type):
inner_types = [arg for arg in get_args(param_type) if arg is not type(None)]
inner_type = inner_types[0] if inner_types else param_type注意：
- 此处假设 extract_block_info 中遍历 block_type.annotations 并对 Optional 类型做了检测，
如果代码中 Optional 类型的检测部分不止这一处，请在其他地方也做类似修改。 - 根据项目整体代码风格，请确保导入调整不会与已有的内容冲突。
Original comment in English
suggestion: Consider improving Optional type detection in extract_block_info.
The current check uses 'hasattr(param_type, 'args') and param_type.name == 'Optional''. A more robust approach might involve using 'typing.get_origin' and 'typing.get_args' to correctly interpret Optional[T] and other Union types, which can avoid subtle bugs with type annotations.
Suggested implementation:
from typing import Dict, Tuple, List, Type, get_origin, get_args, Union for name, param_type in block_type.__annotations__.items():
if get_origin(param_type) is Union and type(None) in get_args(param_type):
inner_types = [arg for arg in get_args(param_type) if arg is not type(None)]
inner_type = inner_types[0] if inner_types else param_type注意：
- 此处假设 extract_block_info 中遍历 block_type.annotations 并对 Optional 类型做了检测，
如果代码中 Optional 类型的检测部分不止这一处，请在其他地方也做类似修改。 - 根据项目整体代码风格，请确保导入调整不会与已有的内容冲突。
Comment on lines
+107
to
+108
| # 查询 | ||
| results = memory_manager.query(mock_scope, "user1") |
There was a problem hiding this comment.
问题（测试）： 缺少无匹配条目的查询测试。
添加一个测试用例，验证当没有内存条目匹配提供的发送者时 query 的行为。
Original comment in English
issue (testing): Missing test for query with no matching entries.
Add a test case to verify the behavior of query when no memory entries match the provided sender.
Comment on lines
+133
to
+142
| def test_shutdown(self, memory_manager, test_entry): | ||
| """测试关闭""" | ||
| # 添加一些测试数据 | ||
| self.manager.memories = { | ||
| "scope1": [self.test_entry], | ||
| "scope2": [self.test_entry] | ||
| memory_manager.memories = { | ||
| "scope1": [test_entry], | ||
| "scope2": [test_entry] | ||
| } | ||
|
|
||
| # 测试关闭 | ||
| self.manager.shutdown() | ||
| # 关闭 | ||
| memory_manager.shutdown() |
There was a problem hiding this comment.
问题（测试）： 使用空内存测试 shutdown。
添加一个 memories 为空字典的测试用例，以确保正确处理此边缘情况。
Original comment in English
issue (testing): Test shutdown with empty memories.
Add a test case where memories is an empty dictionary to ensure correct handling of this edge case.
| @@ -129,6 +129,46 @@ DELETE/backend-api/api/llm/backends/{backend_name} | |||
|
|
|||
| 删除指定的后端。如果后端当前已启用，会先自动卸载。 | |||
|
|
|||
| ### 获取适配器配置模式 | |||
There was a problem hiding this comment.
建议： 请阐明 adapter_type 的类型和格式。提供一个示例值将会很有帮助。
Suggested change
| ### 获取适配器配置模式 | |
| ### 获取适配器配置模式 | |
| 注意：路径参数 adapter_type 必须为字符串，表示适配器类型，例如 "openai"。 |
Original comment in English
suggestion: Please clarify the type and format of adapter_type. Providing an example value would be helpful.
Suggested change
| ### 获取适配器配置模式 | |
| ### 获取适配器配置模式 | |
| 注意：路径参数 adapter_type 必须为字符串，表示适配器类型，例如 "openai"。 |
| @@ -152,6 +192,10 @@ DELETE/backend-api/api/llm/backends/{backend_name} | |||
| ### LLMAdapterTypes | |||
| - `types`: 可用的适配器类型列表 | |||
|
|
|||
| ### LLMAdapterConfigSchema | |||
| - `error`: 错误信息(可选) | |||
| - `schema`: JSON Schema 格式的配置字段描述 | |||
There was a problem hiding this comment.
建议： 请为 schema 字段本身添加描述，解释 schema 代表什么。
Suggested change
| - `schema`: JSON Schema 格式的配置字段描述 | |
| - `schema`: JSON Schema 格式的配置字段描述。此 schema 定义了配置字段的结构、数据类型以及验证规则，确保适配器配置的有效性。 |
Original comment in English
suggestion: Please add a description for the schema field itself, explaining what the schema represents.
Suggested change
| - `schema`: JSON Schema 格式的配置字段描述 | |
| - `schema`: JSON Schema 格式的配置字段描述。此 schema 定义了配置字段的结构、数据类型以及验证规则，确保适配器配置的有效性。 |
| if possible_plugin_infos[0].version != old_version: | ||
| return possible_plugin_infos[0] | ||
| else: | ||
| raise Exception(f"Failed to update plugin: {plugin_info.package_name} is already up to date") |
There was a problem hiding this comment.
问题（代码质量）： 引发特定错误而不是通用的 Exception 或 BaseException（raise-specific-error）
解释
如果一段代码引发特定的异常类型而不是通用的 [`BaseException`](https://docs.python.org/3/library/exceptions.html#BaseException) 或 [`Exception`](https://docs.python.org/3/library/exceptions.html#Exception)，调用代码可以：- 获取有关错误类型的更多信息
- 为其定义特定的异常处理
这样，代码的调用者可以适当地处理错误。
如何解决？
所以，不是像这样引发 Exception 或 BaseException
if incorrect_input(value):
raise Exception("输入不正确")你可以像这样引发特定错误
if incorrect_input(value):
raise ValueError("输入不正确")或
class IncorrectInputError(Exception):
pass
if incorrect_input(value):
raise IncorrectInputError("输入不正确")Original comment in English
issue (code-quality): Raise a specific error instead of the general Exception or BaseException (raise-specific-error)
Explanation
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 of the standard library.
- Define your own error class that subclasses
Exception.
So instead of having code raising Exception or BaseException like
if incorrect_input(value):
raise Exception("The input is incorrect")you can have code raising a specific error like
if incorrect_input(value):
raise ValueError("The input is incorrect")or
class IncorrectInputError(Exception):
pass
if incorrect_input(value):
raise IncorrectInputError("The input is incorrect")| metadata=builder.metadata if hasattr(builder, 'metadata') else None | ||
| )) | ||
|
|
||
|
|
||
| return WorkflowList(workflows=workflows).model_dump() | ||
|
|
||
| @workflow_bp.route('/<group_id>/<workflow_id>', methods=['GET']) | ||
| @require_auth | ||
| async def get_workflow(group_id: str, workflow_id: str): | ||
| """获取特定工作流的详细信息""" |
There was a problem hiding this comment.
问题（代码质量）： 将 for 循环转换为列表推导式 [×2]（list-comprehension）
Original comment in English
issue (code-quality): Convert for loop into list comprehension [×2] (list-comprehension)
| @@ -177,15 +176,18 @@ def _parse_block_spec(self, block_spec: Union[Type[Block], tuple]) -> BlockSpec: | |||
|
|
|||
| def _create_node(self, spec: BlockSpec, is_parallel: bool = False) -> Node: | |||
| """创建并初始化一个新的节点""" | |||
| block = spec.block_class(self.container, **spec.kwargs) | |||
| try: | |||
There was a problem hiding this comment.
问题（代码质量）： 我们发现了以下问题：
- 用 if 表达式替换 if 语句（
assign-if-exp） - 使用命名表达式简化赋值和条件（
use-named-expression） - 从先前的错误显式引发（
raise-from-previous-error）
Original comment in English
issue (code-quality): We've found these issues:
- Replace if statement with if expression (
assign-if-exp) - Use named expression to simplify assignment and conditional (
use-named-expression) - Explicitly raise from a previous error (
raise-from-previous-error)
Comment on lines
+65
to
+67
| if builder and container: | ||
| return builder.build(container) | ||
| return builder |
There was a problem hiding this comment.
建议（代码质量）： 我们发现了以下问题：
- 在控制流跳转后提升代码到 else（
reintroduce-else） - 用 if 表达式替换 if 语句（
assign-if-exp）
Suggested change
| if builder and container: | |
| return builder.build(container) | |
| return builder | |
| return builder.build(container) if builder and container else builder |
Original comment in English
suggestion (code-quality): We've found these issues:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
Suggested change
| if builder and container: | |
| return builder.build(container) | |
| return builder | |
| return builder.build(container) if builder and container else builder |
Comment on lines
+53
to
+54
| manager = MemoryManager(container, persistence=dummy_persistence) | ||
| return manager |
There was a problem hiding this comment.
建议（代码质量）： 内联立即返回的变量（inline-immediately-returned-variable）
Suggested change
| manager = MemoryManager(container, persistence=dummy_persistence) | |
| return manager | |
| return MemoryManager(container, persistence=dummy_persistence) |
Original comment in English
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable)
Suggested change
| manager = MemoryManager(container, persistence=dummy_persistence) | |
| return manager | |
| return MemoryManager(container, persistence=dummy_persistence) |
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.
Summary by Sourcery
更新 API 以支持调度规则的 Web 面板配置。在规则创建和更新端点中添加规则类型和配置。添加端点以检索可用的规则类型及其配置架构。
新功能：
测试：
Original summary in English
Summary by Sourcery
Update APIs to support web panel configuration for dispatch rules. Add rule type and configuration to rule creation and update endpoints. Add endpoint to retrieve available rule types and their configuration schemas.
New Features:
Tests: