-
Notifications
You must be signed in to change notification settings - Fork 0
release/v5.3.0 #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
release/v5.3.0 #44
Conversation
code-crusher
commented
Jan 29, 2026
- Implement skill tool functionality and update dependencies
- Update CHANGELOG.md for v5.3.0
- Update assistant message handling and tool integration
- Fix type error in useSkillTool - use ask instead of say
- fix skill use tool
- clean up skill tool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧪 PR Review is completed: The Skills system implementation is robust, with clean integration into the toolchain. The Code Indexing state persistence changes are well-handled. I've identified a prompt optimization to reduce token usage and correct typos.
Skipped files
CHANGELOG.md: Skipped file pattern
| const skillList = skills | ||
| .map((skill) => { | ||
| return ` - ${skill.metadata.name}: ${skill.metadata.description}` | ||
| }) | ||
| .join("\n") | ||
|
|
||
| return `You are provided Skills below, these skills are to be used by you as per your descretion. The purpose of these skills is to provide you additional niche context for you tasks. You might get skills for React, Security or even third-party tools. Use the tool use_skill to get the skill context: | ||
| ${skillList}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 Logic/Performance
Issue: Listing all skills in the system prompt is redundant as they are already listed in the use_skill tool description (which is required for the model to know valid arguments). This consumes unnecessary tokens. Additionally, there are typos in the text ("descretion", "for you tasks").
Fix: Remove the detailed list from the system prompt and direct the model to refer to the use_skill tool definition.
Impact: Reduces token usage and improves prompt clarity.
| const skillList = skills | |
| .map((skill) => { | |
| return ` - ${skill.metadata.name}: ${skill.metadata.description}` | |
| }) | |
| .join("\n") | |
| return `You are provided Skills below, these skills are to be used by you as per your descretion. The purpose of these skills is to provide you additional niche context for you tasks. You might get skills for React, Security or even third-party tools. Use the tool use_skill to get the skill context: | |
| ${skillList}` | |
| // Skills are already listed in the use_skill tool description | |
| return `You have access to a library of skills that provide specialized instructions for tasks. Use the 'use_skill' tool to access them. Refer to the 'use_skill' tool definition for the list of available skills.` |
|
/matter review-max |
|
I couldn't clone the repository. |