fix #4097 & show tools added by Context.add_llm_tools in web panel#4098
Open
lgc2333 wants to merge 2 commits intoAstrBotDevs:masterfrom
Open
fix #4097 & show tools added by Context.add_llm_tools in web panel#4098lgc2333 wants to merge 2 commits intoAstrBotDevs:masterfrom
Context.add_llm_tools in web panel#4098lgc2333 wants to merge 2 commits intoAstrBotDevs:masterfrom
Conversation
Contributor
There was a problem hiding this comment.
你好——我已经审阅了你的改动,这里是一些反馈:
- 在
star_manager.load中,建议把elif ft.__module__ == metadata.module_path改成单独的if,这样对于既有handler属性、又依赖 metadata 派生的模块路径的工具,也能正确设置handler_module_path。 - 相等判断
ft.__module__ == metadata.module_path对于定义在子模块中的工具来说可能过于严格;可以考虑使用前缀或层级式检查(例如startswith),以便把位于metadata.module_path.*下的工具也正确关联起来。
给 AI Agent 的提示
请根据这次代码审查中的评论进行修改:
## 总体评论
- 在 `star_manager.load` 中,建议把 `elif ft.__module__ == metadata.module_path` 改成单独的 `if`,这样对于既有 `handler` 属性、又依赖 metadata 派生的模块路径的工具,也能正确设置 `handler_module_path`。
- 相等判断 `ft.__module__ == metadata.module_path` 对于定义在子模块中的工具来说可能过于严格;可以考虑使用前缀或层级式检查(例如 `startswith`),以便把位于 `metadata.module_path.*` 下的工具也正确关联起来。帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的审查。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- In
star_manager.load, consider changing theelif ft.__module__ == metadata.module_pathto a separateifso thathandler_module_pathcan still be set for tools that have ahandlerattribute but also rely on the metadata-derived module path. - The equality check
ft.__module__ == metadata.module_pathmay be too strict for tools defined in submodules; consider using a prefix or hierarchical check (e.g.,startswith) so that tools inmetadata.module_path.*are also correctly associated.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `star_manager.load`, consider changing the `elif ft.__module__ == metadata.module_path` to a separate `if` so that `handler_module_path` can still be set for tools that have a `handler` attribute but also rely on the metadata-derived module path.
- The equality check `ft.__module__ == metadata.module_path` may be too strict for tools defined in submodules; consider using a prefix or hierarchical check (e.g., `startswith`) so that tools in `metadata.module_path.*` are also correctly associated.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
81d361f to
7a1d943
Compare
7a1d943 to
5a25b9d
Compare
Context.add_llm_tools in web panel
Member
|
抱歉,由于目前 astrbot plugin 系统中一些设计问题,这个 PR 可能还需要一段时间才能被处理和合并!设计问题主要指的是目前 astrbot 对 handler 和 star 的绑定关系采用 handler_module_path,这导致 handler(比如 tools、command、regex 等)不能在插件的其他模块被注册。我会尽快修复这个问题! |
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.
Modifications / 改动点
handler_module_path统一移至astrbot/core/star/star_manager.py处理,以解决模块路径差异问题Context.add_llm_tools注册的 Tool 能够展示在网页上Screenshots or Test Results / 运行截图或测试结果
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.Summary by Sourcery
通过在 star manager 中集中管理 LLM 工具 handler 模块路径的赋值,并在加载时与插件元数据对齐,修复其路径处理问题。
Bug Fixes:
Enhancements:
Original summary in English
Summary by Sourcery
Fix LLM tool handler module path handling by centralizing its assignment in the star manager and aligning it with plugin metadata during load.
Bug Fixes:
Enhancements: