feat(skills): compact system prompt index via sidecar-driven pinned skills - #23663
feat(skills): compact system prompt index via sidecar-driven pinned skills#23663sontianye wants to merge 1 commit into
Conversation
|
I hit the exact same problem — the skills block is eating ~4,500 tokens every turn on my install (90 skills across 20 categories), and there's no config workaround. This PR's approach (pinned from |
|
Friendly ping — happy to rebase or adjust if you have feedback. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for preserving the MUST-load and hermes-agent guidance and for using the existing sidecar rather than state.db. The token-cost premise is still present on current main, but the implementation needs re-scoping against later skill-index decisions.
Problems
agent/prompt_builder.py:1246removes every unpinned skill name from the default prompt. Current main deliberately preserves all names after a real capability-loss regression: commitee1a744acandagent/prompt_builder.py:1622-1667state that models do not reliably rediscover hidden skills throughskills_list. Commit4d6a133a9also makes even names-only demotion opt-in.agent/prompt_builder.py:1270aggregates nested categories, but the footer callsskills_list(category="<name>").tools/skills_tool.py:827-830filters categories by exact equality, so a top-levelmlopslisting omitsmlops/evaluation,mlops/inference, andmlops/modelsskills included in its count.
Suggested changes
- Rework this on the current demote-never-hide contract, retaining names and treating any broader default compaction as a deliberate design decision.
- Align category summaries with exact filtering or add prefix-aware filtering, with nested-category coverage.
Automated hermes-sweeper review.
| @@ -1162,26 +1246,54 @@ def build_skills_system_prompt( | |||
| if not skills_by_category: | |||
There was a problem hiding this comment.
Blocking: this global replacement removes unpinned skill names from the default prompt. Current main intentionally preserves every name after a real capability-loss regression (ee1a744ac; see current agent/prompt_builder.py:1622-1667), and even names-only demotion is opt-in (4d6a133a9). Please re-scope onto that demote-never-hide contract.
| if top not in top_descs and category_descriptions.get(cat): | ||
| top_descs[top] = category_descriptions[cat] | ||
|
|
||
| total_skills = sum(top_counts.values()) |
There was a problem hiding this comment.
Blocking: these counts combine nested categories under top, but the footer later directs skills_list(category="<name>"). Current skills_list uses exact equality (tools/skills_tool.py:827-830), so mlops will not return skills in mlops/evaluation, mlops/inference, or mlops/models even though they are counted here.
b55362c to
8c4eaff
Compare
…erwise The current index shows every skill with its full description, costing ~3,500 tokens on a fully-loaded install. This PR reduces that to a fixed-budget format that still honours the demote-never-hide contract: every skill name stays visible, but descriptions appear only for skills explicitly pinned by the user or agent via the skill_usage sidecar. How it works - _get_pinned_candidates() reads ~/.hermes/skills/.usage.json via tools/skill_usage.agent_created_report(), filters pinned=True && state != archived, orders by activity_count descending, and trims to _PINNED_SKILLS_CHAR_BUDGET (1200 chars) so the description budget stays bounded regardless of how many skills are pinned. - The index loop shows "name: description" for pinned skills and "name" for everything else — no entries ever removed, no skills_list() needed for discovery. - Posture-driven compact_categories demotion (names-only lines for non-coding categories in coding posture) is unchanged. - _skill_usage_epoch() adds sidecar mtime_ns to the cache key so pin state changes take effect on the next prompt build automatically. No new settings. build_skills_system_prompt() signature unchanged. Closes NousResearch#14319
8c4eaff to
f518658
Compare
|
Reworked and force-pushed. The branch is now a single clean commit on top of current main — only What changed vs the previous push The earlier push had unintended noise from a soft-reset accident. This is the correct minimal patch. Demote-never-hide — Every skill name stays in No category mismatch — The previous approach's Cache — Tests — Four existing assertions updated ( Diff stat: +196 / -7 lines across two files. |
Re-spin of #14319 along the path @teknium1 outlined when closing it.
Why
The skills section of the system prompt currently dumps every skill name + description on every turn — ~3,500 tokens on a fully-loaded install. The PR shrinks that to a fixed-budget compact format while keeping the existing framing the dispatcher relies on.
Mapping to the closing review on #14319
tools/skill_usage.py(agent_created_report()+ sidecar lookup) instead ofstate.db"_get_pinned_candidates()reads only viaagent_created_report(), filterspinned=True && state != archived, orders byactivity_countdesc. Nostate.dbaccess, no duplicate tracking.test_preserves_must_load_and_hermes_agent_framing.hermes-agentinstruction"hermes config set …paragraph kept verbatim. Same test.queryparam onskills_listis independently nice but small enough we can do it ourselves"What it looks like
When nothing is pinned, the
<pinned_skills>block is omitted entirely — pinned means pinned, never an alphabetical placeholder.Implementation notes
_PINNED_SKILLS_CHAR_BUDGET = 1200keeps the pinned block bounded regardless of how many skills get pinned.<skill_categories>reuses the existingcategory_descriptionsmapping; counts are deduped per top-level category._skill_usage_epoch()(sidecarmtime_ns) so toggling pin state takes effect on the next build with no manual invalidation.skills_list(category=...)) is how they stay reachable.build_skills_system_prompt()unchanged.Tests
TestBuildSkillsSystemPromptandTestBuildSkillsSystemPromptConditionaltests updated to assert the new contract (category presence + count) instead of raw name dumps.TestCompactSkillsPrompt:test_preserves_must_load_and_hermes_agent_framingtest_pinned_candidates_from_sidecartest_no_pinned_block_when_no_pinstest_no_pinned_block_when_skill_set_is_emptytest_skill_categories_summary_shows_countstest_pin_change_invalidates_prompt_cachetests/agent/test_prompt_builder.py: 127 passed, 1 skipped.Test plan
uv run pytest tests/agent/test_prompt_builder.py— 127 passedgateway_service/anthropic_adapterOAuth /kanban_cli/e2e/test_platform_commandstests, none touching skills code)Closes #14319