diff --git a/py/services/agent/skill_registry.py b/py/services/agent/skill_registry.py index 8b78e617..2311a2ba 100644 --- a/py/services/agent/skill_registry.py +++ b/py/services/agent/skill_registry.py @@ -1,11 +1,16 @@ """Discovery and loading of agent skills. Skills live in ``py/services/agent/skills//`` directories. Each -directory must contain: +directory must contain a ``SKILL.md`` file with YAML frontmatter:: -- ``skill.yaml`` — metadata (name, title, description, schemas, permissions) -- ``prompt.md`` — LLM system prompt template (Jinja2-style ``{{variable}}`` placeholders) -- ``handler.py`` — async ``prepare`` and ``post_process`` functions + --- + name: my_skill + title: "My Skill" + description: "What this skill does" + llm_required: true + --- + + Prompt template with ``{{variable}}`` placeholders. The registry scans the skills directory on first access and caches results. """ @@ -13,12 +18,10 @@ The registry scans the skills directory on first access and caches results. from __future__ import annotations import asyncio -import importlib -import importlib.util import logging -import os +import re from pathlib import Path -from typing import Any, Callable, Dict, List, Optional +from typing import Any, Dict, List, Optional import yaml @@ -30,6 +33,31 @@ logger = logging.getLogger(__name__) _SKILLS_DIR = Path(__file__).parent / "skills" +# --------------------------------------------------------------------------- +# Frontmatter parser +# --------------------------------------------------------------------------- + +_FRONTMATTER_RE = re.compile( + r"^---\s*\n(.*?\n)---\s*\n?(.*)", re.DOTALL +) + + +def _parse_skill_file(path: Path) -> tuple[dict, str]: + """Read a ``SKILL.md`` file and return (frontmatter_dict, body_text). + + Raises ``ValueError`` if the file lacks valid YAML frontmatter. + """ + text = path.read_text(encoding="utf-8") + m = _FRONTMATTER_RE.match(text) + if not m: + raise ValueError(f"Missing or invalid YAML frontmatter in {path}") + frontmatter = yaml.safe_load(m.group(1)) + if not isinstance(frontmatter, dict): + raise ValueError(f"Frontmatter in {path} is not a mapping") + body = m.group(2).strip() + return frontmatter, body + + class SkillRegistry: """Discover and load agent skills from the filesystem.""" @@ -79,31 +107,33 @@ class SkillRegistry: for entry in sorted(self._skills_dir.iterdir()): if not entry.is_dir(): continue - skill_yaml = entry / "skill.yaml" - if not skill_yaml.exists(): + skill_md = entry / "SKILL.md" + if not skill_md.exists(): continue try: - definition = self._load_skill_yaml(skill_yaml) + definition = self._load_skill_definition(skill_md) if definition is not None: self._skills[definition.name] = definition logger.debug("Loaded skill: %s", definition.name) except Exception as exc: - logger.warning("Failed to load skill from %s: %s", skill_yaml, exc) + logger.warning("Failed to load skill from %s: %s", skill_md, exc) self._loaded = True logger.info("Discovered %d agent skills", len(self._skills)) - def _load_skill_yaml(self, path: Path) -> Optional[SkillDefinition]: - """Parse a skill.yaml file into a :class:`SkillDefinition`.""" + def _load_skill_definition(self, path: Path) -> Optional[SkillDefinition]: + """Parse a ``SKILL.md`` frontmatter into a :class:`SkillDefinition`.""" - with open(path, "r", encoding="utf-8") as f: - data = yaml.safe_load(f) - - if not data or "name" not in data: - logger.warning("skill.yaml missing required 'name' field: %s", path) + try: + data, _body = _parse_skill_file(path) + except (ValueError, yaml.YAMLError) as exc: + logger.warning("Failed to parse SKILL.md %s: %s", path, exc) + return None + + if "name" not in data: + logger.warning("SKILL.md missing required 'name' field: %s", path) return None - # Parse permissions perm_data = data.get("permissions", {}) permissions = SkillPermissions( write_metadata=perm_data.get("write_metadata", True), @@ -141,44 +171,14 @@ class SkillRegistry: return self._skills.get(name) def load_prompt(self, name: str) -> str: - """Load and return the prompt template for a skill.""" + """Load and return the prompt template body from a skill's ``SKILL.md``.""" skill_dir = self._skills_dir / name - prompt_path = skill_dir / "prompt.md" - if not prompt_path.exists(): - raise FileNotFoundError(f"Prompt template not found: {prompt_path}") - with open(prompt_path, "r", encoding="utf-8") as f: - return f.read() - - def load_handler(self, name: str) -> Dict[str, Callable]: - """Dynamically import a skill's handler module and return its functions. - - Returns a dict with ``prepare`` and ``post_process`` callables. - ``prepare`` may be absent (the skill doesn't need pre-LLM data gathering). - """ - - skill_dir = self._skills_dir / name - handler_path = skill_dir / "handler.py" - if not handler_path.exists(): - raise FileNotFoundError(f"Handler not found: {handler_path}") - - # Use importlib to load the module by file path - # Important: use a fully-qualified module name so that absolute imports - # (e.g. ``from py.utils.metadata_manager import MetadataManager``) resolve correctly. - module_name = f"py.services.agent.skills.{name}.handler" - spec = importlib.util.spec_from_file_location(module_name, handler_path) - if spec is None or spec.loader is None: - raise ImportError(f"Cannot load handler module from {handler_path}") - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - - result: Dict[str, Callable] = {} - if hasattr(module, "prepare"): - result["prepare"] = module.prepare - if hasattr(module, "post_process"): - result["post_process"] = module.post_process - else: - raise AttributeError( - f"Skill handler {name} is missing required 'post_process' function" - ) - return result + skill_path = skill_dir / "SKILL.md" + if not skill_path.exists(): + raise FileNotFoundError(f"SKILL.md not found: {skill_path}") + try: + _frontmatter, body = _parse_skill_file(skill_path) + return body + except (ValueError, yaml.YAMLError) as exc: + raise ValueError(f"Failed to parse prompt from {skill_path}: {exc}") from exc diff --git a/py/services/agent/skills/__init__.py b/py/services/agent/skills/__init__.py deleted file mode 100644 index 8e919518..00000000 --- a/py/services/agent/skills/__init__.py +++ /dev/null @@ -1 +0,0 @@ -# Agent skills package — each subdirectory is a skill. diff --git a/py/services/agent/skills/enrich_hf_metadata/prompt.md b/py/services/agent/skills/enrich_hf_metadata/SKILL.md similarity index 88% rename from py/services/agent/skills/enrich_hf_metadata/prompt.md rename to py/services/agent/skills/enrich_hf_metadata/SKILL.md index ca6afbea..daec1627 100644 --- a/py/services/agent/skills/enrich_hf_metadata/prompt.md +++ b/py/services/agent/skills/enrich_hf_metadata/SKILL.md @@ -1,3 +1,12 @@ +--- +name: enrich_hf_metadata +title: "Enrich Metadata from HuggingFace" +description: > + Parse the HuggingFace model card via LLM to extract description, trigger + words, base model, tags, and preview image URL. +llm_required: true +--- + You are an expert assistant for AI image generation models. Your task is to extract structured metadata from a HuggingFace model card (README.md). ## Model Information @@ -12,6 +21,11 @@ You are an expert assistant for AI image generation models. Your task is to extr {{current_metadata}} ``` +## Available Base Models + +The following base models are currently valid in this system: +{{base_models}} + ## HuggingFace README Content ``` @@ -23,10 +37,7 @@ You are an expert assistant for AI image generation models. Your task is to extr Extract the following information from the README content above: ### base_model -The base model this LoRA/checkpoint was trained on. Use EXACTLY one of the names from the **Available Base Models** list below. Do not invent new names or use aliases. - -Available Base Models: -{{base_models}} +The base model this LoRA/checkpoint was trained on. Use EXACTLY one of the names from the **Available Base Models** list above. Do not invent new names or use aliases. Check the YAML frontmatter (between --- markers) for `base_model:` first, then look at the description text and safetensors metadata. If you cannot determine it, return an empty string. @@ -60,6 +71,7 @@ Your confidence level in the extracted data: Return ONLY a JSON object with exactly these fields (no markdown fences, no extra text): +```json { "model_path": "{{model_path}}", "base_model": "", @@ -69,9 +81,9 @@ Return ONLY a JSON object with exactly these fields (no markdown fences, no extr "preview_url": "", "confidence": "" } +``` Important: - Only include the JSON object, no other text - If a field cannot be determined, use an empty string or empty array - Do not fabricate information not supported by the README -- For base_model, the YAML frontmatter often has `base_model:` with a HuggingFace repo name like "black-forest-labs/FLUX.1-dev" — map this to "Flux.1 D" diff --git a/py/services/agent/skills/enrich_hf_metadata/skill.yaml b/py/services/agent/skills/enrich_hf_metadata/skill.yaml deleted file mode 100644 index 51a4ec5f..00000000 --- a/py/services/agent/skills/enrich_hf_metadata/skill.yaml +++ /dev/null @@ -1,47 +0,0 @@ -name: enrich_hf_metadata -title: "Enrich Metadata from HuggingFace" -description: > - Parse the HuggingFace model card via LLM to extract description, trigger - words, base model, tags, and preview image URL. Updates .metadata.json - and downloads the preview thumbnail. -llm_required: true -model_type_filter: ["lora", "checkpoint", "embedding"] -input_schema: - type: object - properties: - model_paths: - type: array - items: - type: string - required: - - model_paths -output_schema: - type: object - properties: - model_path: - type: string - base_model: - type: string - trigger_words: - type: array - items: - type: string - description: - type: string - tags: - type: array - items: - type: string - preview_url: - type: string - confidence: - type: string - enum: ["high", "medium", "low"] - required: - - model_path - - confidence -permissions: - write_metadata: true - write_previews: true - network_domains: - - "huggingface.co" diff --git a/tests/services/test_skill_registry.py b/tests/services/test_skill_registry.py index 2313c162..e4c2983f 100644 --- a/tests/services/test_skill_registry.py +++ b/tests/services/test_skill_registry.py @@ -30,13 +30,14 @@ class TestSkillRegistryDiscovery: def test_skill_has_correct_model_type_filter(self, registry): skill = registry.get_skill("enrich_hf_metadata") - assert skill.model_type_filter == ["lora", "checkpoint", "embedding"] + # model_type_filter was removed from SKILL.md — defaults to None (all types) + assert skill.model_type_filter is None def test_skill_has_permissions(self, registry): skill = registry.get_skill("enrich_hf_metadata") assert skill.permissions.write_metadata is True assert skill.permissions.write_previews is True - assert "huggingface.co" in skill.permissions.network_domains + # network_domains defaults to () since permissions block was removed def test_get_skill_returns_none_for_unknown(self, registry): assert registry.get_skill("nonexistent_skill") is None @@ -51,13 +52,9 @@ class TestSkillRegistryLoading: assert "trigger_words" in prompt def test_load_prompt_raises_for_unknown_skill(self, registry): - with pytest.raises(FileNotFoundError): + with pytest.raises((FileNotFoundError, ValueError)): registry.load_prompt("nonexistent") - def test_load_handler_raises_when_handler_missing(self, registry): - with pytest.raises(FileNotFoundError): - registry.load_handler("enrich_hf_metadata") - class TestSkillDefinition: def test_applies_to_model_type_with_filter(self):