refactor(toolset): move _load_tools from agent.py to KimiToolset.load_tools#493
refactor(toolset): move _load_tools from agent.py to KimiToolset.load_tools#493
_load_tools from agent.py to KimiToolset.load_tools#493Conversation
….load_tools` Signed-off-by: Richard Chien <stdrc@outlook.com>
|
This change is part of the following stack: Change managed by git-spice. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the tool loading functionality by moving the _load_tools and _load_tool functions from src/kimi_cli/soul/agent.py into the KimiToolset class as instance methods (load_tools and _load_tool). This improves encapsulation by placing tool loading logic closer to the toolset class that manages the tools.
Key changes:
- Moved
_load_toolsand_load_toolfunctions fromagent.pytoKimiToolsetclass as methods - Updated imports in both
agent.pyand test files to reflect the refactoring - Changed from standalone functions to instance methods for better object-oriented design
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/kimi_cli/soul/agent.py |
Removed _load_tools and _load_tool functions, updated call to use toolset.load_tools() method, removed unused imports |
src/kimi_cli/soul/toolset.py |
Added load_tools and _load_tool instance methods to KimiToolset class with necessary imports |
tests/test_load_agent.py |
Updated test imports and calls to use the new toolset.load_tools() method instead of the standalone _load_tools function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| good_tools.append(tool_path) | ||
| else: | ||
| bad_tools.append(tool_path) | ||
| logger.info("Loaded tools: {good_tools}", good_tools=good_tools) |
There was a problem hiding this comment.
The logging behavior has changed. The original implementation logged the tool names ([tool.name for tool in toolset.tools]), but the new implementation logs the tool paths (good_tools). This changes what information is displayed in the logs - tool paths like "kimi_cli.tools.shell:Shell" instead of tool names like "shell". Consider whether this change in logging output is intentional or if the original behavior should be preserved.
| logger.info("Loaded tools: {good_tools}", good_tools=good_tools) | |
| logger.info( | |
| "Loaded tools: {tool_names}", | |
| tool_names=[tool.name for tool in self.tools], | |
| ) |
Signed-off-by: Richard Chien <stdrc@outlook.com>
Signed-off-by: Richard Chien stdrc@outlook.com
Related Issue
Resolve #(issue_number)
Description