Skip to content

Commit 6866070

Browse files
authored
feat(validator): add SOFT eval-coverage check (check #8) (#481)
Every skill under skills/ must ship a matching behavioural eval suite under tools/skill-evals/evals/<slug>/. The new validate_eval_coverage function surfaces missing suites as SOFT advisory violations so that in-flight eval PRs do not fail the gate while their branches are pending review. Against the live repo the check correctly flags the two skills that currently have in-flight eval branches (pr-management-quick-merge and setup-status) and is silent on all others. 8 new test cases cover the happy path, the missing-eval path, missing-both-dirs paths, the soft-category membership, and the non-directory skip. Addresses the Known Gap in specs/meta-and-quality-tooling.md: "Eval coverage is incomplete — skills added before the per-skill-eval convention have no suite." The check prevents future regressions. Generated-by: Claude (Opus 4.7)
1 parent bf416a1 commit 6866070

2 files changed

Lines changed: 130 additions & 1 deletion

File tree

tools/skill-and-tool-validator/src/skill_and_tool_validator/__init__.py

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
"""Validate framework skill definitions.
1919
20-
This module validates eight aspects of every skill under
20+
This module validates nine aspects of every skill under
2121
skills/:
2222
2323
1. YAML frontmatter — every SKILL.md must have a valid frontmatter
@@ -49,6 +49,11 @@
4949
Apache Software Foundation license preamble. Skill ``.md`` files
5050
declare their license via the required ``license:`` frontmatter key
5151
(checked by aspect 1), so they need no separate header.
52+
9. Eval-coverage (SOFT) — every skill directory under ``skills/``
53+
must have a matching behavioural eval suite under
54+
``tools/skill-evals/evals/<slug>/``. Missing suites are
55+
advisories so in-flight eval PRs do not block the gate while
56+
their branches are pending review.
5257
5358
SOFT categories surface as advisory warnings (stderr) without
5459
failing the run unless ``--strict`` is passed.
@@ -74,6 +79,7 @@
7479
SKILLS_DIR = Path("skills")
7580
TOOLS_DIR = Path("tools")
7681
DOCS_DIR = Path("docs")
82+
SKILL_EVALS_DIR = Path("tools/skill-evals/evals")
7783
PROJECTS_TEMPLATE_DIR = Path("projects/_template")
7884

7985
# Categories for the tool-validator block. Both HARD by default — every
@@ -89,6 +95,8 @@
8995
# with live skill frontmatter + tool README declarations.
9096
DOCS_LABELS_AND_CAPABILITIES = Path("docs/labels-and-capabilities.md")
9197
CAPABILITY_SYNC_CATEGORY = "capability-sync"
98+
# Eval-coverage check: every skill must have a matching eval suite.
99+
EVAL_COVERAGE_CATEGORY = "eval-coverage"
92100
_SKILL_TABLE_HEADER = "## Capability to skill map"
93101
_TOOL_TABLE_HEADER = "## Capability to tool map"
94102
# Tokens like `capability:setup`. Optional backticks around the token.
@@ -262,6 +270,7 @@ def _read_mode_table() -> dict[str, str]:
262270
GH_LIST_CATEGORY,
263271
PRIVACY_CATEGORY,
264272
LOWERCASE_F_FIELD_CATEGORY,
273+
EVAL_COVERAGE_CATEGORY,
265274
}
266275
)
267276
HARD_CATEGORIES: frozenset[str] = frozenset(
@@ -1728,6 +1737,40 @@ def collect_doc_files(root: Path | None = None) -> set[Path]:
17281737
return files
17291738

17301739

1740+
# ---------------------------------------------------------------------------
1741+
# Eval-coverage check (check #9, SOFT)
1742+
# ---------------------------------------------------------------------------
1743+
1744+
1745+
def validate_eval_coverage(root: Path | None = None) -> Iterable[Violation]:
1746+
"""Warn when a skill directory has no matching eval suite.
1747+
1748+
Every skill under skills/ must have a behavioural eval suite under
1749+
tools/skill-evals/evals/<slug>/. Missing suites surface as SOFT
1750+
advisories so in-flight eval PRs do not fail the gate while their
1751+
branches are pending review.
1752+
"""
1753+
repo_root = root or find_repo_root()
1754+
skills_base = repo_root / SKILLS_DIR
1755+
evals_base = repo_root / SKILL_EVALS_DIR
1756+
if not skills_base.exists():
1757+
return
1758+
eval_slugs: set[str] = set()
1759+
if evals_base.exists():
1760+
eval_slugs = {p.name for p in evals_base.iterdir() if p.is_dir()}
1761+
for skill_dir in sorted(skills_base.iterdir()):
1762+
if not skill_dir.is_dir():
1763+
continue
1764+
slug = skill_dir.name
1765+
if slug not in eval_slugs:
1766+
yield Violation(
1767+
skill_dir / "SKILL.md",
1768+
None,
1769+
f"eval-coverage: no eval suite at tools/skill-evals/evals/{slug}/ — add one before shipping",
1770+
category=EVAL_COVERAGE_CATEGORY,
1771+
)
1772+
1773+
17311774
def run_validation(root: Path | None = None) -> list[Violation]:
17321775
"""Run the full validation suite and return all violations."""
17331776
repo_root = root or find_repo_root()
@@ -1774,6 +1817,9 @@ def run_validation(root: Path | None = None) -> list[Violation]:
17741817
# Capability-sync check: the doc tables and the source must agree.
17751818
violations.extend(validate_capability_sync(repo_root))
17761819

1820+
# Eval-coverage check: every skill must have a matching eval suite.
1821+
violations.extend(validate_eval_coverage(repo_root))
1822+
17771823
return violations
17781824

17791825

tools/skill-and-tool-validator/tests/test_validator.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
_PRIVACY_EXTERNAL_CONTENT_MODES,
3131
ALL_CATEGORIES,
3232
ALLOWED_MODES,
33+
EVAL_COVERAGE_CATEGORY,
3334
FORBIDDEN_PATTERNS,
3435
GH_LIST_CATEGORY,
3536
HARD_CATEGORIES,
@@ -61,6 +62,7 @@
6162
run_validation,
6263
slugify,
6364
validate_capability_sync,
65+
validate_eval_coverage,
6466
validate_frontmatter,
6567
validate_gh_list_limit,
6668
validate_injection_guard,
@@ -2399,3 +2401,84 @@ def test_italic_parens_annotation_is_stripped(self, tmp_path: Path) -> None:
23992401
# The parenthetical capability:reconciliation must NOT be flagged as a doc-side declared capability;
24002402
# the row's authoritative capability is just intake, which matches the live skill.
24012403
assert violations == [], [v.message for v in violations]
2404+
2405+
2406+
# ---------------------------------------------------------------------------
2407+
# Eval-coverage check
2408+
# ---------------------------------------------------------------------------
2409+
2410+
2411+
class TestValidateEvalCoverage:
2412+
"""Tests for validate_eval_coverage (check #9 — SOFT)."""
2413+
2414+
def _make_skill(self, root: Path, slug: str) -> None:
2415+
skill_dir = root / "skills" / slug
2416+
skill_dir.mkdir(parents=True, exist_ok=True)
2417+
(skill_dir / "SKILL.md").write_text(
2418+
f"---\nname: magpie-{slug}\ndescription: test\ncapability: capability:triage\nlicense: Apache-2.0\n---\n"
2419+
)
2420+
2421+
def _make_eval(self, root: Path, slug: str) -> None:
2422+
eval_dir = root / "tools" / "skill-evals" / "evals" / slug
2423+
eval_dir.mkdir(parents=True, exist_ok=True)
2424+
(eval_dir / "README.md").write_text(f"# {slug} evals\n")
2425+
2426+
def test_skill_with_matching_eval_passes(self, tmp_path: Path) -> None:
2427+
self._make_skill(tmp_path, "issue-triage")
2428+
self._make_eval(tmp_path, "issue-triage")
2429+
violations = list(validate_eval_coverage(tmp_path))
2430+
assert violations == []
2431+
2432+
def test_skill_without_eval_yields_soft_violation(self, tmp_path: Path) -> None:
2433+
self._make_skill(tmp_path, "new-skill")
2434+
# No matching eval directory.
2435+
violations = list(validate_eval_coverage(tmp_path))
2436+
assert len(violations) == 1
2437+
v = violations[0]
2438+
assert v.category == EVAL_COVERAGE_CATEGORY
2439+
assert "new-skill" in v.message
2440+
assert "tools/skill-evals/evals/new-skill/" in v.message
2441+
2442+
def test_multiple_skills_some_missing_evals(self, tmp_path: Path) -> None:
2443+
self._make_skill(tmp_path, "alpha")
2444+
self._make_skill(tmp_path, "beta")
2445+
self._make_skill(tmp_path, "gamma")
2446+
self._make_eval(tmp_path, "alpha")
2447+
# beta and gamma have no evals.
2448+
violations = list(validate_eval_coverage(tmp_path))
2449+
assert len(violations) == 2
2450+
slugs = {v.path.parent.name for v in violations}
2451+
assert slugs == {"beta", "gamma"}
2452+
assert all(v.category == EVAL_COVERAGE_CATEGORY for v in violations)
2453+
2454+
def test_no_skills_dir_returns_no_violations(self, tmp_path: Path) -> None:
2455+
# skills/ does not exist at all.
2456+
violations = list(validate_eval_coverage(tmp_path))
2457+
assert violations == []
2458+
2459+
def test_no_evals_dir_all_skills_flagged(self, tmp_path: Path) -> None:
2460+
self._make_skill(tmp_path, "alpha")
2461+
self._make_skill(tmp_path, "beta")
2462+
# tools/skill-evals/evals/ does not exist.
2463+
violations = list(validate_eval_coverage(tmp_path))
2464+
assert len(violations) == 2
2465+
assert all(v.category == EVAL_COVERAGE_CATEGORY for v in violations)
2466+
2467+
def test_eval_coverage_is_soft_category(self) -> None:
2468+
assert EVAL_COVERAGE_CATEGORY in SOFT_CATEGORIES
2469+
assert EVAL_COVERAGE_CATEGORY not in ALL_CATEGORIES - SOFT_CATEGORIES
2470+
2471+
def test_violation_path_points_to_skill_md(self, tmp_path: Path) -> None:
2472+
self._make_skill(tmp_path, "orphan")
2473+
violations = list(validate_eval_coverage(tmp_path))
2474+
assert len(violations) == 1
2475+
assert violations[0].path.name == "SKILL.md"
2476+
assert violations[0].path.parent.name == "orphan"
2477+
2478+
def test_non_directory_entries_in_skills_are_skipped(self, tmp_path: Path) -> None:
2479+
skills_dir = tmp_path / "skills"
2480+
skills_dir.mkdir(parents=True)
2481+
# A plain file (not a directory) must not be treated as a skill.
2482+
(skills_dir / "README.md").write_text("# skills\n")
2483+
violations = list(validate_eval_coverage(tmp_path))
2484+
assert violations == []

0 commit comments

Comments
 (0)