fix: wire bump_use() into skill loading paths - #17865
Closed
Giggitycountless wants to merge 1 commit into
Closed
Conversation
bump_use() in tools/skill_usage.py existed and was tested, but had zero production call sites — use_count and last_used_at stayed at their defaults forever, making the Curator's stale timer useless for distinguishing actively-used skills from never-used ones. Add bump_use() calls to: - build_skill_invocation_message() — when a /skill slash command is resolved and its content is injected into the prompt - build_preloaded_skills_prompt() — when skills are loaded via the CLI --skills flag at session start Both calls are best-effort (wrapped in try/except) to never break skill loading if the usage sidecar is unavailable. Fixes NousResearch#17782
Collaborator
|
Likely duplicate of #17818 — same fix for bump_use() zero call sites, same target file (agent/skill_commands.py). |
Collaborator
|
Likely duplicate of #17818 |
Author
|
Duplicate of #17818 — closing mine. Good to see this fix already in the pipeline! 👍 |
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.
Summary
bump_use()intools/skill_usage.pyexisted and was tested, but had zero production call sites —use_countstayed at 0 andlast_used_atstayedNoneforever, making the Curator's stale timer unable to distinguish actively-used skills from never-used ones.Changes
Adds
bump_use()calls to two skill loading paths inagent/skill_commands.py:build_skill_invocation_message()— when a/skillslash command is resolved and its content is injected into the promptbuild_preloaded_skills_prompt()— when skills are loaded via the CLI--skillsflag at session startBoth calls follow the same pattern as the existing
bump_view()call inskills_tool.py: wrapped intry/except Exceptionfor best-effort operation that never breaks skill loading.Why these paths
Per the issue's analysis, option 1 (slash command resolution) is the most precise: a skill is "used" when its content is actually injected to solve a task, not just when it's listed or browsed. The preload path is included because it represents intentional, active use of a skill for a session.
Before
After
Fixes #17782