perf(ttft): cache skills prompt with shared skill_utils module (salvage #3366) (#3421)

Two-layer caching for build_skills_system_prompt():
  1. In-process LRU (OrderedDict, max 8) — same-process: 546ms → <1ms
  2. Disk snapshot (.skills_prompt_snapshot.json) — cold start: 297ms → 103ms

Key improvements over original PR #3366:
- Extract shared logic into agent/skill_utils.py (parse_frontmatter,
  skill_matches_platform, get_disabled_skill_names, extract_skill_conditions,
  extract_skill_description, iter_skill_index_files)
- tools/skills_tool.py delegates to shared module — zero code duplication
- Proper LRU eviction via OrderedDict.move_to_end + popitem(last=False)
- Cache invalidation on all skill mutation paths:
  - skill_manage tool (in-conversation writes)
  - hermes skills install (CLI hub)
  - hermes skills uninstall (CLI hub)
  - Automatic via mtime/size manifest on cold start

prompt_builder.py no longer imports tools.skills_tool (avoids pulling
in the entire tool registry chain at prompt build time).

6301 tests pass, 0 failures.

Co-authored-by: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com>
This commit is contained in:
Teknium 2026-03-27 10:54:02 -07:00 committed by GitHub
parent cc4514076b
commit 5127567d5d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 527 additions and 196 deletions

View file

@ -589,38 +589,38 @@ class TestSkillMatchesPlatform:
assert skill_matches_platform({"platforms": None}) is True
def test_macos_on_darwin(self):
with patch("tools.skills_tool.sys") as mock_sys:
with patch("agent.skill_utils.sys") as mock_sys:
mock_sys.platform = "darwin"
assert skill_matches_platform({"platforms": ["macos"]}) is True
def test_macos_on_linux(self):
with patch("tools.skills_tool.sys") as mock_sys:
with patch("agent.skill_utils.sys") as mock_sys:
mock_sys.platform = "linux"
assert skill_matches_platform({"platforms": ["macos"]}) is False
def test_linux_on_linux(self):
with patch("tools.skills_tool.sys") as mock_sys:
with patch("agent.skill_utils.sys") as mock_sys:
mock_sys.platform = "linux"
assert skill_matches_platform({"platforms": ["linux"]}) is True
def test_linux_on_darwin(self):
with patch("tools.skills_tool.sys") as mock_sys:
with patch("agent.skill_utils.sys") as mock_sys:
mock_sys.platform = "darwin"
assert skill_matches_platform({"platforms": ["linux"]}) is False
def test_windows_on_win32(self):
with patch("tools.skills_tool.sys") as mock_sys:
with patch("agent.skill_utils.sys") as mock_sys:
mock_sys.platform = "win32"
assert skill_matches_platform({"platforms": ["windows"]}) is True
def test_windows_on_linux(self):
with patch("tools.skills_tool.sys") as mock_sys:
with patch("agent.skill_utils.sys") as mock_sys:
mock_sys.platform = "linux"
assert skill_matches_platform({"platforms": ["windows"]}) is False
def test_multi_platform_match(self):
"""Skills listing multiple platforms should match any of them."""
with patch("tools.skills_tool.sys") as mock_sys:
with patch("agent.skill_utils.sys") as mock_sys:
mock_sys.platform = "darwin"
assert skill_matches_platform({"platforms": ["macos", "linux"]}) is True
mock_sys.platform = "linux"
@ -630,20 +630,20 @@ class TestSkillMatchesPlatform:
def test_string_instead_of_list(self):
"""A single string value should be treated as a one-element list."""
with patch("tools.skills_tool.sys") as mock_sys:
with patch("agent.skill_utils.sys") as mock_sys:
mock_sys.platform = "darwin"
assert skill_matches_platform({"platforms": "macos"}) is True
mock_sys.platform = "linux"
assert skill_matches_platform({"platforms": "macos"}) is False
def test_case_insensitive(self):
with patch("tools.skills_tool.sys") as mock_sys:
with patch("agent.skill_utils.sys") as mock_sys:
mock_sys.platform = "darwin"
assert skill_matches_platform({"platforms": ["MacOS"]}) is True
assert skill_matches_platform({"platforms": ["MACOS"]}) is True
def test_unknown_platform_no_match(self):
with patch("tools.skills_tool.sys") as mock_sys:
with patch("agent.skill_utils.sys") as mock_sys:
mock_sys.platform = "linux"
assert skill_matches_platform({"platforms": ["freebsd"]}) is False
@ -659,7 +659,7 @@ class TestFindAllSkillsPlatformFiltering:
def test_excludes_incompatible_platform(self, tmp_path):
with (
patch("tools.skills_tool.SKILLS_DIR", tmp_path),
patch("tools.skills_tool.sys") as mock_sys,
patch("agent.skill_utils.sys") as mock_sys,
):
mock_sys.platform = "linux"
_make_skill(tmp_path, "universal-skill")
@ -672,7 +672,7 @@ class TestFindAllSkillsPlatformFiltering:
def test_includes_matching_platform(self, tmp_path):
with (
patch("tools.skills_tool.SKILLS_DIR", tmp_path),
patch("tools.skills_tool.sys") as mock_sys,
patch("agent.skill_utils.sys") as mock_sys,
):
mock_sys.platform = "darwin"
_make_skill(tmp_path, "mac-only", frontmatter_extra="platforms: [macos]\n")
@ -684,7 +684,7 @@ class TestFindAllSkillsPlatformFiltering:
"""Skills without platforms field should appear on any platform."""
with (
patch("tools.skills_tool.SKILLS_DIR", tmp_path),
patch("tools.skills_tool.sys") as mock_sys,
patch("agent.skill_utils.sys") as mock_sys,
):
mock_sys.platform = "win32"
_make_skill(tmp_path, "generic-skill")
@ -695,7 +695,7 @@ class TestFindAllSkillsPlatformFiltering:
def test_multi_platform_skill(self, tmp_path):
with (
patch("tools.skills_tool.SKILLS_DIR", tmp_path),
patch("tools.skills_tool.sys") as mock_sys,
patch("agent.skill_utils.sys") as mock_sys,
):
_make_skill(
tmp_path, "cross-plat", frontmatter_extra="platforms: [macos, linux]\n"