From 74645123bbfcede0b87d33aedf3d5ca364169c0b Mon Sep 17 00:00:00 2001 From: Teknium Date: Wed, 1 Apr 2026 02:57:34 -0700 Subject: [PATCH] feat(skills): add fuzzy matching to skill patch _patch_skill now uses the same 8-strategy fuzzy matching engine (tools/fuzzy_match.py) as the file patch tool. Handles whitespace normalization, indentation differences, escape sequences, and block-anchor matching. Eliminates exact-match failures when agents patch skills with minor formatting mismatches. --- tests/tools/test_skill_improvements.py | 174 +++++++++++++++++++++++++ tests/tools/test_skill_manager_tool.py | 4 +- tools/skill_manager_tool.py | 29 ++--- 3 files changed, 188 insertions(+), 19 deletions(-) create mode 100644 tests/tools/test_skill_improvements.py diff --git a/tests/tools/test_skill_improvements.py b/tests/tools/test_skill_improvements.py new file mode 100644 index 0000000000..6e781309f2 --- /dev/null +++ b/tests/tools/test_skill_improvements.py @@ -0,0 +1,174 @@ +"""Tests for skill fuzzy patching via tools.fuzzy_match.""" + +import json +import os +from pathlib import Path +from unittest.mock import patch + +import pytest + +from tools.skill_manager_tool import ( + _create_skill, + _patch_skill, + _write_file, + skill_manage, +) + + +SKILL_CONTENT = """\ +--- +name: test-skill +description: A test skill for unit testing. +--- + +# Test Skill + +Step 1: Do the thing. +Step 2: Do another thing. +Step 3: Final step. +""" + + +# --------------------------------------------------------------------------- +# Fuzzy patching +# --------------------------------------------------------------------------- + + +class TestFuzzyPatchSkill: + @pytest.fixture(autouse=True) + def setup_skills(self, tmp_path, monkeypatch): + skills_dir = tmp_path / "skills" + skills_dir.mkdir() + monkeypatch.setattr("tools.skill_manager_tool.SKILLS_DIR", skills_dir) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + self.skills_dir = skills_dir + + def test_exact_match_still_works(self): + _create_skill("test-skill", SKILL_CONTENT) + result = _patch_skill("test-skill", "Step 1: Do the thing.", "Step 1: Done!") + assert result["success"] is True + content = (self.skills_dir / "test-skill" / "SKILL.md").read_text() + assert "Step 1: Done!" in content + + def test_whitespace_trimmed_match(self): + """Patch with extra leading whitespace should still find the target.""" + skill = """\ +--- +name: ws-skill +description: Whitespace test +--- + +# Commands + + def hello(): + print("hi") +""" + _create_skill("ws-skill", skill) + # Agent sends patch with no leading whitespace (common LLM behaviour) + result = _patch_skill("ws-skill", "def hello():\n print(\"hi\")", "def hello():\n print(\"hello world\")") + assert result["success"] is True + content = (self.skills_dir / "ws-skill" / "SKILL.md").read_text() + assert 'print("hello world")' in content + + def test_indentation_flexible_match(self): + """Patch where only indentation differs should succeed.""" + skill = """\ +--- +name: indent-skill +description: Indentation test +--- + +# Steps + + 1. First step + 2. Second step + 3. Third step +""" + _create_skill("indent-skill", skill) + # Agent sends with different indentation + result = _patch_skill( + "indent-skill", + "1. First step\n2. Second step", + "1. Updated first\n2. Updated second" + ) + assert result["success"] is True + content = (self.skills_dir / "indent-skill" / "SKILL.md").read_text() + assert "Updated first" in content + + def test_multiple_matches_blocked_without_replace_all(self): + """Multiple fuzzy matches should return an error without replace_all.""" + skill = """\ +--- +name: dup-skill +description: Duplicate test +--- + +# Steps + +word word word +""" + _create_skill("dup-skill", skill) + result = _patch_skill("dup-skill", "word", "replaced") + assert result["success"] is False + assert "match" in result["error"].lower() + + def test_replace_all_with_fuzzy(self): + skill = """\ +--- +name: dup-skill +description: Duplicate test +--- + +# Steps + +word word word +""" + _create_skill("dup-skill", skill) + result = _patch_skill("dup-skill", "word", "replaced", replace_all=True) + assert result["success"] is True + content = (self.skills_dir / "dup-skill" / "SKILL.md").read_text() + assert "word" not in content + assert "replaced" in content + + def test_no_match_returns_preview(self): + _create_skill("test-skill", SKILL_CONTENT) + result = _patch_skill("test-skill", "this does not exist anywhere", "replacement") + assert result["success"] is False + assert "file_preview" in result + + def test_fuzzy_patch_on_supporting_file(self): + """Fuzzy matching should also work on supporting files.""" + _create_skill("test-skill", SKILL_CONTENT) + ref_content = " function hello() {\n console.log('hi');\n }" + _write_file("test-skill", "references/code.js", ref_content) + # Patch with stripped indentation + result = _patch_skill( + "test-skill", + "function hello() {\nconsole.log('hi');\n}", + "function hello() {\nconsole.log('hello world');\n}", + file_path="references/code.js" + ) + assert result["success"] is True + content = (self.skills_dir / "test-skill" / "references" / "code.js").read_text() + assert "hello world" in content + + def test_patch_preserves_frontmatter_validation(self): + """Fuzzy matching should still run frontmatter validation on SKILL.md.""" + _create_skill("test-skill", SKILL_CONTENT) + # Try to destroy the frontmatter via patch + result = _patch_skill("test-skill", "---\nname: test-skill", "BROKEN") + assert result["success"] is False + assert "structure" in result["error"].lower() or "frontmatter" in result["error"].lower() + + def test_skill_manage_patch_uses_fuzzy(self): + """The dispatcher should route to the fuzzy-matching patch.""" + _create_skill("test-skill", SKILL_CONTENT) + raw = skill_manage( + action="patch", + name="test-skill", + old_string=" Step 1: Do the thing.", # extra leading space + new_string="Step 1: Updated.", + ) + result = json.loads(raw) + # Should succeed via line-trimmed or indentation-flexible matching + assert result["success"] is True diff --git a/tests/tools/test_skill_manager_tool.py b/tests/tools/test_skill_manager_tool.py index 06a2f88aed..a20d23fcbd 100644 --- a/tests/tools/test_skill_manager_tool.py +++ b/tests/tools/test_skill_manager_tool.py @@ -271,7 +271,7 @@ class TestPatchSkill: _create_skill("my-skill", VALID_SKILL_CONTENT) result = _patch_skill("my-skill", "this text does not exist", "replacement") assert result["success"] is False - assert "not found" in result["error"] + assert "not found" in result["error"].lower() or "could not find" in result["error"].lower() def test_patch_ambiguous_match_rejected(self, tmp_path): content = """\ @@ -288,7 +288,7 @@ word word _create_skill("my-skill", content) result = _patch_skill("my-skill", "word", "replaced") assert result["success"] is False - assert "matched" in result["error"] + assert "match" in result["error"].lower() def test_patch_replace_all(self, tmp_path): content = """\ diff --git a/tools/skill_manager_tool.py b/tools/skill_manager_tool.py index 6899ba51ce..d6d2f6f78d 100644 --- a/tools/skill_manager_tool.py +++ b/tools/skill_manager_tool.py @@ -404,28 +404,24 @@ def _patch_skill( content = target.read_text(encoding="utf-8") - count = content.count(old_string) - if count == 0: + # Use the same fuzzy matching engine as the file patch tool. + # This handles whitespace normalization, indentation differences, + # escape sequences, and block-anchor matching — saving the agent + # from exact-match failures on minor formatting mismatches. + from tools.fuzzy_match import fuzzy_find_and_replace + + new_content, match_count, match_error = fuzzy_find_and_replace( + content, old_string, new_string, replace_all + ) + if match_error: # Show a short preview of the file so the model can self-correct preview = content[:500] + ("..." if len(content) > 500 else "") return { "success": False, - "error": "old_string not found in the file.", + "error": match_error, "file_preview": preview, } - if count > 1 and not replace_all: - return { - "success": False, - "error": ( - f"old_string matched {count} times. Provide more surrounding context " - f"to make the match unique, or set replace_all=true to replace all occurrences." - ), - "match_count": count, - } - - new_content = content.replace(old_string, new_string) if replace_all else content.replace(old_string, new_string, 1) - # Check size limit on the result target_label = "SKILL.md" if not file_path else file_path err = _validate_content_size(new_content, label=target_label) @@ -450,10 +446,9 @@ def _patch_skill( _atomic_write_text(target, original_content) return {"success": False, "error": scan_error} - replacements = count if replace_all else 1 return { "success": True, - "message": f"Patched {'SKILL.md' if not file_path else file_path} in skill '{name}' ({replacements} replacement{'s' if replacements > 1 else ''}).", + "message": f"Patched {'SKILL.md' if not file_path else file_path} in skill '{name}' ({match_count} replacement{'s' if match_count > 1 else ''}).", }