fix(prompt_builder): include skills manifest digest in cache key - #18443
Open
leon7609 wants to merge 1 commit into
Open
fix(prompt_builder): include skills manifest digest in cache key#18443leon7609 wants to merge 1 commit into
leon7609 wants to merge 1 commit into
Conversation
The in-process LRU cache for `build_skills_system_prompt` was keyed only on directory paths (`skills_dir.resolve()` plus a tuple of external dir paths). Edits to skill files within those directories — add, edit, remove, rename — did not invalidate the cache, so a running agent kept serving a stale system prompt until the process restarted. The disk snapshot (Layer 2) already validates against a manifest of mtime/size pairs; this commit folds the same manifest digest into the in-process cache key (Layer 1) so both layers share invalidation semantics. Add `_skills_manifest_digest()` and `_skills_cache_state()` helpers and use the latter in place of the bare external-dir tuple.
This was referenced May 3, 2026
13 tasks
teknium1
reviewed
Jul 12, 2026
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for isolating a real Layer-1 cache staleness path. Current main still keys the skills LRU by directory paths at agent/prompt_builder.py:1481-1489 and returns it before Layer-2 manifest validation at :1490-1497.
Problems
- The new
_skills_cache_state()calls_build_skills_manifest()before every LRU lookup. On currentmain, that function performsos.walk()andos.stat()across indexed files (agent/prompt_builder.py:1281-1297), so unchanged cache hits regain the filesystem traversal that cache commit5127567d5specifically reduced from 546ms to <1ms. - Current reload behavior deliberately preserves this prompt cache:
agent/skill_commands.py:405-416rescans commands and queues a user-turn note instead of invalidating the system-prompt cache. The intended automatic-refresh boundary needs a maintainer decision before this changes it implicitly. - The test adds only a local-file-addition case; it does not cover edits, removals, renames, external directories, or unchanged-hit performance.
Suggested changes
- Establish the intended refresh boundary, then preserve a cheap unchanged LRU hit.
- Add coverage for each claimed mutation class and external directories.
Automated hermes-sweeper review.
|
|
||
|
|
||
| def _skills_manifest_digest(skills_dir: Path) -> str: | ||
| """Return a stable digest of one skills directory's manifest.""" |
Contributor
There was a problem hiding this comment.
This manifest is evaluated before the Layer-1 lookup, so every otherwise-unchanged cache hit now walks and stats the complete skills tree. Current _build_skills_manifest() uses os.walk() plus per-file os.stat(); please retain a cheap unchanged-hit path before adding this to the key.
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.
What does this PR do?
Fixes a stale-cache bug in
build_skills_system_prompt's in-process LRU.The cache key was based only on directory paths (
skills_dir.resolve()+ a tuple of external dir paths). Edits to skill files within those directories — add, edit, remove, rename — did not invalidate the cache, so a long-running agent kept serving a stale system prompt until the process restarted.The disk snapshot (Layer 2) already validates against a manifest of
mtime/sizepairs; this PR folds the same manifest digest into the in-process cache key (Layer 1) so both layers share invalidation semantics. Two new helpers —_skills_manifest_digest()and_skills_cache_state()— compute a per-directory SHA-1 over a sorted manifest and feed the result into the existing cache key tuple.Related Issue
No existing issue. Discovered while iterating on a skill — agent kept serving the old prompt until I restarted the gateway.
Type of Change
Changes Made
agent/prompt_builder.py_skills_manifest_digest(skills_dir)— SHA-1 over a sorted JSON dump of_build_skills_manifest()'s output._skills_cache_state(skills_dir, external_dirs)— tuple of(resolved_path, digest)pairs across local + external dirs.tuple(str(d) for d in external_dirs)in the cache key with_skills_cache_state(...).tests/agent/test_prompt_builder.py— extend with cases that mutate skill files and assert the cache key changes.How to Test
hermes chat -q "list skills".SKILL.mdcontent in~/.hermes/skills/<some-skill>/(without restarting hermes).Or run the unit tests directly:
Checklist
Code
fix(prompt_builder): ...)pytest tests/ -qand the changes don't introduce new failures (vs. main baseline)Documentation & Housekeeping