Skip to content

feat(skills): two-tier index + signal-based nudge (gated) - #21841

Closed
zhuosama wants to merge 19 commits into
NousResearch:mainfrom
zhuosama:skills-index-v2-and-nudge-signals
Closed

feat(skills): two-tier index + signal-based nudge (gated)#21841
zhuosama wants to merge 19 commits into
NousResearch:mainfrom
zhuosama:skills-index-v2-and-nudge-signals

Conversation

@zhuosama

@zhuosama zhuosama commented May 8, 2026

Copy link
Copy Markdown

Summary

Two-tier skill index + signal-based skill-creation nudge, both gated behind feature flags. Targets the spec in plans/skills-prompt-budget-and-nudge-redesign.md (~80% reduction in per-turn skill-prompt tokens at 100 skills, signal-driven nudges that fire on evidence rather than a wall-clock counter).

Phase A — skill index v2 (gated skills.index_v2, default false)

  • Extracted agent/skill_inventory.py as a typed source of truth for parsed skill metadata, consumed by both build_skills_system_prompt() and the new skill_describe tool. Snapshot version bumped to 2.
  • New SKILL.md frontmatter field: priority: critical | normal (default normal). Validated by tools/skill_manager_tool.py:_validate_frontmatter.
  • New rendering when skills.index_v2: true:
    • Tier 1 (system prompt) shows category labels + skill names, with full descriptions reserved for priority: critical entries.
    • Tier 2 on demand via skill_describe(category=…) / skill_describe(names=[…]).
  • Token budget skills.index_token_budget (default 2000) with category-fold-back ranked by recent usage. Categories beyond budget are listed by name behind a … and N more categories hint.
  • New ~/.hermes/.skill_usage.json tracker (agent/skill_usage_tracker.py) updated on skill_view / skill_describe; best-effort, falls back to empty on corruption.
  • Warning logged at index-build time when more than 15 skills are marked priority: critical (spec §9 Q1).

Phase B — signal-based nudge (gated skills.nudge_signals.enabled, default false)

  • New agent/skill_nudge_signals.py SignalEvaluator covering:
    • S1 repeated tool pattern (per-tool argument signature, threshold repeated_pattern_threshold default 3)
    • S2 novel external CLI on terminal/process (window novel_cli_window_days default 30, plus a default suppression list of git, python, pytest, etc.)
    • S3 explicit user-message phrases (next time, remember, from now on, 记一下, 下次, 以后)
    • S4 resolved repeated error (error_repeat_threshold default 2)
  • ~/.hermes/.skill_known_clis.json persists CLI history with daily pruning (best-effort).
  • End-of-turn skill-review gate flipped from time-only to signal-first: signals fire it immediately, the legacy creation_nudge_interval counter remains as a fallback (kept at 15 in this PR; bumps to 50 in Phase 3).
  • _spawn_background_review now receives triggered_signals: set[str] so the reviewer prompt can reference which signal fired.
  • Per-session disable:
    • Slash command /skills nudge off|on (intercepted in cli.py:HermesCLI._handle_skills_command before delegating to the skills_hub router; help line added).
    • Env var HERMES_SKILL_NUDGE_DISABLE=1 honoured at session init.

Phase 3 (separate PR after dogfooding)

  • Flip skills.index_v2 and skills.nudge_signals.enabled defaults to true
  • Bump skills.creation_nudge_interval default from 15 → 50
  • Add TUI parity (ui-tui/src/app/slash/commands/ops.ts) and gateway parity for /skills nudge off|on
  • Phase 4 cleanup: delete the v1 rendering path once v2 has been default for one minor version

Tracked in plans/skills-prompt-budget-and-nudge-phase-3-checklist.md.

Files

  • New: agent/skill_inventory.py, agent/skill_usage_tracker.py, agent/skill_nudge_signals.py
  • Modified: agent/prompt_builder.py, tools/skills_tool.py, tools/skill_manager_tool.py, run_agent.py, cli.py, hermes_cli/skills_hub.py, cli-config.yaml.example, tests/conftest.py
  • Plans: plans/skills-prompt-budget-and-nudge-redesign.md (spec), plans/skills-prompt-budget-and-nudge-implementation.md (plan), plans/skills-prompt-budget-and-nudge-phase-3-checklist.md (follow-up)

Test status

  • Focused suite (this PR's tests + test_concurrent_interrupt): 216 passed, 1 skipped, 0 failed.
  • Wider xdist suite (tests/agent/ tests/tools/ tests/run_agent/ tests/cli/ tests/integration/): 27–47 failures across runs vs 26 on main with the same xdist conditions. The delta is within ambient cross-module xdist race variance; no deterministic regressions were found. Same-directory parallel runs (e.g. tests/run_agent/ alone) pass cleanly: 992/0 on branch, 982/0 on main.
  • Pre-existing failures unrelated to this PR (visible on both sides): tests/agent/test_anthropic_adapter.py (reads real ~/.claude credentials instead of fixtures), tests/tools/test_file_staleness.py / test_file_state_registry.py (/var/folders path-validation gate), tests/tools/test_local_interrupt_cleanup.py.

Dogfood checklist (complete before merging out of draft)

Phase A — index v2

Set in ~/.hermes/config.yaml:

skills:
  index_v2: true
  index_token_budget: 2000

Then exercise:

  • Start a fresh session; confirm system prompt logs show the v2 format (category-grouped names, only priority: critical skills with descriptions). Suggested: hermes --debug 2>&1 | grep -A 3 "## Skills".
  • Estimate Tier 1 size; on a 100-skill library it should sit near 1.5–2k tokens (the spec's measured target was ~1300; budget is 2000).
  • Mark one frequently-used skill priority: critical; verify its description reappears in Tier 1.
  • Run skill_describe(category="<some-cat>") from inside a session; confirm it returns the expected one-line descriptions.
  • Run skill_describe(names=["<known-name>"]); confirm it returns the entry. Try an unknown name; confirm success: false with a useful error.
  • Force budget overflow (drop index_token_budget to e.g. 300); confirm the fold-back line … and N more categories: … appears with comma-separated names.
  • Confirm ~/.hermes/.skill_usage.json is created/updated when skill_view or skill_describe runs.

Phase B — signal-based nudge

Set in ~/.hermes/config.yaml:

skills:
  nudge_signals:
    enabled: true

Then exercise:

  • S1: in one turn, run a repeated tool pattern 3+ times (e.g. gh pr view 1, gh pr view 2, gh pr view 3). Confirm a background skill review fires after the response.
  • S2: in one turn, invoke a CLI that hasn't been used in the last 30 days (e.g. an exotic binary with terminal); confirm the signal fires once the call succeeds.
  • S3: send a user message containing "next time" or "记一下"; confirm the signal fires and the background review runs.
  • S4: trigger the same error twice (e.g. wrong path), then succeed; confirm the signal fires.
  • Run /skills nudge off; confirm subsequent signals are suppressed in that session. Run /skills nudge on; confirm they fire again.
  • Set HERMES_SKILL_NUDGE_DISABLE=1 in the env; confirm a fresh session is muted.
  • Run a long task with no reusable patterns and creation_nudge_interval: 50; confirm the time fallback eventually triggers when no signals fired.

Cleanup

  • Revert local ~/.hermes/config.yaml overrides used during dogfooding (or leave them on if you want the new behavior — Phase 3 is what flips the defaults globally).

Spec / plan

  • Spec: plans/skills-prompt-budget-and-nudge-redesign.md
  • Plan: plans/skills-prompt-budget-and-nudge-implementation.md
  • Phase 3 checklist: plans/skills-prompt-budget-and-nudge-phase-3-checklist.md

🤖 Generated with Claude Code

codex and others added 19 commits May 8, 2026 01:55
The _Stub class fakes a minimal AIAgent for the concurrent-tool-execution
tests. After Codex's signal-based-nudge work added a _observe_tool_activity
call inside _execute_tool_calls_concurrent (run_agent.py:8399, 8765), the
stub started failing with AttributeError because it didn't fake the new
method.  Add a no-op stub method to match the existing pattern (_vprint,
_safe_print, _touch_activity, etc.).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex's refactor introduced a module-level OrderedDict LRU
(_INVENTORY_CACHE in agent/skill_inventory.py) that survives across
xdist worker tests, since pytest-xdist reuses workers across tests in
the same module/group. Each test gets a fresh HERMES_HOME tmp dir, so
the cache key (skills_dir, ...) changes — but stale entries from
previous tests' tmp dirs remain in the OrderedDict until evicted by
_INVENTORY_CACHE_MAX (8 entries). Clearing both caches in
_reset_module_state up front matches how production behaves on a
fresh process.

Note: this addresses correctness hygiene, not the wider xdist flake
variance — those are cross-module races that exist on main too
(measured: main wider suite has 26 failures, branch has 27-47, similar
variance distribution).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/skills Skills system (list, view, manage) labels May 8, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks @zhuosama — closing without merging.

This is the third PR proposing to compact the skills system prompt (after #14319 and #20644, both closed). The core token-saving idea is real, but each version has come back heavier than the last and we've consistently found the same architectural objections. The size makes the review cost worse than the prompt cost it's trying to save.

On the prompt compaction (Phase A): same pattern as the two earlier PRs. The current prompt's strong "MUST load skills" framing is doing real work for models that don't tool-call aggressively, and we're not willing to trade that for prompt size. priority: critical would become a third orthogonal flag alongside pinned and locked. skill_describe duplicates what skills_list already does (it has a category= filter and we just landed a query= parameter discussion).

On the signal-based nudge (Phase B): this is the more interesting half but solves a problem we already patched at the right layer in #23004 (just merged). That PR added explicit "Do NOT capture" guidance to both review prompts — the reviewer now refuses transient env failures, negative tool claims, one-off task narratives, and session-specific errors directly in its own prompt. The reviewer makes context-aware decisions; the proposed S1-S4 signal gate replaces context with hardcoded heuristics (3 repeats = class of work, 30 days = novel CLI, etc.) which will be wrong often. If the reviewer's own judgment is correctly calibrated — which is what #23004 set out to do — the signal gate is unnecessary. If it's still miscalibrated after #23004, the fix belongs inside the reviewer's prompt, not as a heuristic gatekeeper outside it.

Other concerns that hurt this PR independent of the design:

  • 2,522 LOC of plans/ and docs/superpowers/plans/ markdown — those are agent working notes, not Hermes documentation, and shouldn't ship.
  • "Wider xdist suite: 27-47 failures vs 26 on main, ambient race variance" isn't a passing test suite. Real engineers don't merge with that delta and call it ambient.
  • The dogfood checklist in the PR body is entirely unchecked.
  • All commits are authored by codex / claude-code with no human-authored validation pass on top.
  • Phase 3 plans to flip the feature-flagged defaults later — that escalates this from "opt-in experiment" to "mandatory change" without a separate review.

If you want to push specifically on the signal-based nudge as a useful idea independent of prompt compaction, the path that would actually get merged: a focused PR (~400-600 LOC) with just the S1-S4 evaluator, no priority frontmatter, no skill_describe tool, no plans/ dump, and benchmarks showing it reduces useless reviewer fires more than #23004 already does. Without measured-improvement-over-current-main, we can't justify the new heuristic layer.

Closing this so the queue stays legible.

@teknium1 teknium1 closed this May 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have tool/skills Skills system (list, view, manage) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants