mirror of
https://github.com/alirezarezvani/claude-skills.git
synced 2026-08-28 04:24:58 +00:00
Merge pull request #937 from benrfairless/fix/validator-and-model-freshness
fix(skill-tester): recalibrate validator to the real schema + add gate G7
This commit is contained in:
commit
d9ae390afa
5 changed files with 339 additions and 47 deletions
8
.github/workflows/ci-quality-gate.yml
vendored
8
.github/workflows/ci-quality-gate.yml
vendored
|
|
@ -113,6 +113,14 @@ jobs:
|
|||
run: |
|
||||
python3 scripts/check_dual_publish.py
|
||||
|
||||
# Proposed by audit/newgen-2026-06 and never built, which is why retired
|
||||
# model IDs survived two later audits. Advisory while the existing
|
||||
# references are cleaned up; flip to blocking once the tree is clean.
|
||||
- name: Retired model identifier lint (gate G7 — advisory)
|
||||
continue-on-error: true
|
||||
run: |
|
||||
python3 scripts/check_model_freshness.py --all
|
||||
|
||||
- name: Script --help smoke gate (gate G8 — blocking)
|
||||
run: |
|
||||
python3 scripts/smoke_scripts.py
|
||||
|
|
|
|||
|
|
@ -1,16 +1,16 @@
|
|||
---
|
||||
name: sample-text-processor
|
||||
description: "Reference BASIC-tier skill used as a fixture by skill-tester. Counts words and characters and applies basic text transformations. Use when validating skill-tester itself or when you need a minimal, known-good skill layout to copy. Not a production skill."
|
||||
---
|
||||
|
||||
# Sample Text Processor
|
||||
|
||||
---
|
||||
This file is the fixture skill_validator.py and script_tester.py run against.
|
||||
It is deliberately minimal. Keep its frontmatter valid YAML and limited to the
|
||||
fields Claude Code reads: anything else here gets copied into new skills by
|
||||
authors treating it as a template.
|
||||
|
||||
**Name**: sample-text-processor
|
||||
**Tier**: BASIC
|
||||
**Category**: Text Processing
|
||||
**Dependencies**: None (Python Standard Library Only)
|
||||
**Author**: Claude Skills Engineering Team
|
||||
**Version**: 1.0.0
|
||||
**Last Updated**: 2026-02-16
|
||||
|
||||
---
|
||||
Tier: BASIC. Dependencies: none, Python standard library only.
|
||||
|
||||
## Description
|
||||
|
||||
|
|
|
|||
|
|
@ -132,14 +132,37 @@ class SkillValidator:
|
|||
}
|
||||
}
|
||||
|
||||
REQUIRED_SKILL_MD_SECTIONS = [
|
||||
"Name", "Description", "Features", "Usage", "Examples"
|
||||
# Sections the repo actually uses. Measured across all 361 real SKILL.md
|
||||
# files: no single heading appears in even 30% of them, so requiring a
|
||||
# fixed list is not defensible. These are scored as a recommendation
|
||||
# (RECOMMENDED_SECTIONS_THRESHOLD of them present earns full marks) and
|
||||
# never raise an error.
|
||||
RECOMMENDED_SKILL_MD_SECTIONS = [
|
||||
"References", "Quick Start", "Related Skills", "Workflow", "Workflows",
|
||||
"When to Use", "Proactive Triggers", "Output Artifacts",
|
||||
"Anti-Patterns", "Output Format", "Overview", "Core Capabilities",
|
||||
]
|
||||
|
||||
FRONTMATTER_REQUIRED_FIELDS = [
|
||||
"Name", "Tier", "Category", "Dependencies", "Author", "Version"
|
||||
RECOMMENDED_SECTIONS_THRESHOLD = 2
|
||||
|
||||
# What Claude Code actually reads from SKILL.md frontmatter. `description`
|
||||
# is what the skill listing matches against, so a skill without one is
|
||||
# invisible to model invocation.
|
||||
#
|
||||
# This list previously read ["Name", "Tier", "Category", "Dependencies",
|
||||
# "Author", "Version"] — the bold key/value convention used by the
|
||||
# assets/sample-skill fixture, not YAML frontmatter and not a schema any
|
||||
# real skill has ever followed. Every one of the 362 skills failed both
|
||||
# checks identically, so the output was noise nobody acted on.
|
||||
FRONTMATTER_REQUIRED_FIELDS = ["name", "description"]
|
||||
|
||||
# Present on many skills and harmless, but no runtime reads them.
|
||||
FRONTMATTER_OPTIONAL_FIELDS = [
|
||||
"when_to_use", "argument-hint", "arguments", "allowed-tools",
|
||||
"disallowed-tools", "disable-model-invocation", "user-invocable",
|
||||
"model", "effort", "context", "agent", "background", "hooks",
|
||||
"paths", "shell",
|
||||
]
|
||||
|
||||
|
||||
def __init__(self, skill_path: str, target_tier: Optional[str] = None, verbose: bool = False):
|
||||
self.skill_path = Path(skill_path).resolve()
|
||||
self.target_tier = target_tier
|
||||
|
|
@ -283,23 +306,33 @@ class SkillValidator:
|
|||
self.report.add_error("SKILL.md must start with YAML frontmatter")
|
||||
|
||||
def _validate_required_sections(self, content: str):
|
||||
"""Validate required sections in SKILL.md"""
|
||||
self.log_verbose("Checking required sections...")
|
||||
|
||||
missing_sections = []
|
||||
for section in self.REQUIRED_SKILL_MD_SECTIONS:
|
||||
"""Score SKILL.md against the repo's common section conventions.
|
||||
|
||||
Advisory only: the repo has no universal section schema, so a miss is
|
||||
a hint to the author, not a validation error.
|
||||
"""
|
||||
self.log_verbose("Checking recommended sections...")
|
||||
|
||||
present = []
|
||||
for section in self.RECOMMENDED_SKILL_MD_SECTIONS:
|
||||
pattern = rf'^#+\s*{re.escape(section)}\s*$'
|
||||
if not re.search(pattern, content, re.MULTILINE | re.IGNORECASE):
|
||||
missing_sections.append(section)
|
||||
|
||||
if not missing_sections:
|
||||
self.report.add_check("required_sections", True,
|
||||
"All required sections present", 1.0)
|
||||
if re.search(pattern, content, re.MULTILINE | re.IGNORECASE):
|
||||
present.append(section)
|
||||
|
||||
threshold = self.RECOMMENDED_SECTIONS_THRESHOLD
|
||||
if len(present) >= threshold:
|
||||
self.report.add_check("recommended_sections", True,
|
||||
f"{len(present)} recommended section(s) present: "
|
||||
f"{', '.join(present)}", 1.0)
|
||||
else:
|
||||
self.report.add_check("required_sections", False,
|
||||
f"Missing sections: {', '.join(missing_sections)}", 0.0)
|
||||
self.report.add_error(f"Missing required sections: {', '.join(missing_sections)}")
|
||||
|
||||
self.report.add_check("recommended_sections", False,
|
||||
f"Only {len(present)} recommended section(s) present "
|
||||
f"(suggest at least {threshold} of: "
|
||||
f"{', '.join(self.RECOMMENDED_SKILL_MD_SECTIONS)})", 0.0)
|
||||
self.report.add_warning(
|
||||
f"SKILL.md has {len(present)} of the repo's common sections; "
|
||||
f"consider adding at least {threshold}")
|
||||
|
||||
def _validate_readme(self):
|
||||
"""Validate README.md content"""
|
||||
self.log_verbose("Validating README.md...")
|
||||
|
|
@ -469,22 +502,19 @@ class SkillValidator:
|
|||
return False
|
||||
|
||||
def _check_external_imports(self, tree: ast.AST) -> List[str]:
|
||||
"""Check for external (non-stdlib) imports"""
|
||||
# Simplified check - a more comprehensive solution would use a stdlib module list
|
||||
stdlib_modules = {
|
||||
'argparse', 'ast', 'json', 'os', 'sys', 'pathlib', 'datetime', 'typing',
|
||||
'collections', 're', 'math', 'random', 'itertools', 'functools', 'operator',
|
||||
'csv', 'sqlite3', 'urllib', 'http', 'html', 'xml', 'email', 'base64',
|
||||
'hashlib', 'hmac', 'secrets', 'tempfile', 'shutil', 'glob', 'fnmatch',
|
||||
'subprocess', 'threading', 'multiprocessing', 'queue', 'time', 'calendar',
|
||||
'zoneinfo', 'locale', 'gettext', 'logging', 'warnings', 'unittest',
|
||||
'doctest', 'pickle', 'copy', 'pprint', 'reprlib', 'enum', 'dataclasses',
|
||||
'contextlib', 'abc', 'atexit', 'traceback', 'gc', 'weakref', 'types',
|
||||
'copy', 'pprint', 'reprlib', 'enum', 'decimal', 'fractions', 'statistics',
|
||||
'cmath', 'platform', 'errno', 'io', 'codecs', 'unicodedata', 'stringprep',
|
||||
'textwrap', 'string', 'struct', 'difflib', 'heapq', 'bisect', 'array',
|
||||
'weakref', 'types', 'copyreg', 'uuid', 'mmap', 'ctypes'
|
||||
}
|
||||
"""Check for external (non-stdlib) imports.
|
||||
|
||||
Uses the interpreter's own module list rather than a hand-maintained
|
||||
set, which is what this function's original comment asked for. The old
|
||||
set omitted `__future__`, so every script using
|
||||
`from __future__ import annotations` was reported as carrying an
|
||||
external dependency.
|
||||
"""
|
||||
stdlib_modules = set(getattr(sys, "stdlib_module_names", ()))
|
||||
if not stdlib_modules: # Python < 3.10 has no sys.stdlib_module_names
|
||||
stdlib_modules = set(sys.builtin_module_names)
|
||||
# A compiler directive, not a dependency.
|
||||
stdlib_modules.add('__future__')
|
||||
|
||||
external_imports = []
|
||||
for node in ast.walk(tree):
|
||||
|
|
|
|||
228
scripts/check_model_freshness.py
Normal file
228
scripts/check_model_freshness.py
Normal file
|
|
@ -0,0 +1,228 @@
|
|||
#!/usr/bin/env python3
|
||||
"""check_model_freshness.py — retired-model-identifier linter (gate G7).
|
||||
|
||||
Proposed by audit/newgen-2026-06/00-MASTER.md and never built, which is why
|
||||
retired model IDs and 2024 price tables kept drifting past two later audits.
|
||||
|
||||
Flags references to model identifiers that no longer exist, are no longer
|
||||
current, or are presented as current when they are not. The point is not to
|
||||
chase whatever is newest: it is to catch the cases that mislead a reader or
|
||||
break on execution, namely
|
||||
|
||||
- script defaults and config values naming a retired model
|
||||
- cost/pricing tables keyed on a retired model
|
||||
- copy-pasteable CLI examples pinning a retired versioned ID
|
||||
|
||||
Dated citations are legitimate and must NOT be flagged. A reference is treated
|
||||
as a citation when the same line carries a year, an arXiv ID, the words
|
||||
"paper"/"technical report"/"model card"/"as of"/"historical", or a
|
||||
"snapshot"/"deprecated"/"retired" marker. Anything else needs an entry in
|
||||
scripts/check_model_freshness_allowlist.txt with a reason.
|
||||
|
||||
Exit codes: 0 = clean, 1 = at least one unexplained retired identifier.
|
||||
|
||||
Usage:
|
||||
python3 scripts/check_model_freshness.py --all
|
||||
python3 scripts/check_model_freshness.py FILE [FILE ...]
|
||||
python3 scripts/check_model_freshness.py --all --json
|
||||
python3 scripts/check_model_freshness.py --list-patterns
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import fnmatch
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
|
||||
EXCLUDED_DIRS = {
|
||||
".git", ".codex", ".gemini", ".hermes", ".vibe", "node_modules",
|
||||
"docs", # generated mirror; fix the source instead
|
||||
"audit", # audit records quote stale IDs on purpose, that is their job
|
||||
"CHANGELOG.md",
|
||||
}
|
||||
|
||||
SCAN_EXTENSIONS = (".md", ".py", ".json", ".yaml", ".yml", ".sh", ".txt")
|
||||
|
||||
# Retired or superseded identifiers, as (regex, label) pairs. Word-ish bounded
|
||||
# so `claude-3` does not match `claude-3x-something` unintentionally.
|
||||
RETIRED_PATTERNS = [
|
||||
(r"\bclaude-instant\b", "claude-instant (retired)"),
|
||||
(r"\bclaude-2(?:\.\d+)?\b", "Claude 2 family (retired)"),
|
||||
(r"\bclaude-3(?:[.-]\d+)?(?:-(?:opus|sonnet|haiku))?(?:-\d{8})?\b",
|
||||
"Claude 3 family (retired)"),
|
||||
(r"\bClaude\s+3(?:\.\d+)?\s+(?:Opus|Sonnet|Haiku)\b", "Claude 3 family (retired)"),
|
||||
# Haiku 4.5 (claude-haiku-4-5-20251001) is still current, so it is
|
||||
# excluded from the Claude 4 sweep rather than allowlisted per-file.
|
||||
(r"\bclaude-(?:opus|sonnet)-4(?:[.-]\d+)?(?:-\d{8})?\b",
|
||||
"Claude 4 family (superseded by Claude 5)"),
|
||||
(r"\bclaude-haiku-4(?!\W*5)(?:[.-]\d+)?(?:-\d{8})?\b",
|
||||
"Claude 4 family (superseded by Claude 5)"),
|
||||
(r"\bClaude\s+(?:Opus|Sonnet)\s+4(?:\.\d+)?\b",
|
||||
"Claude 4 family (superseded by Claude 5)"),
|
||||
(r"\bClaude\s+Haiku\s+4(?!\.5)(?:\.\d+)?\b",
|
||||
"Claude 4 family (superseded by Claude 5)"),
|
||||
(r"\banthropic/claude-[a-z]+-4[.-]\d+\b", "Claude 4 family (superseded by Claude 5)"),
|
||||
(r"\bgpt-3\.5(?:-turbo)?\b", "gpt-3.5 (retired)"),
|
||||
(r"\bgpt-4(?:-32k|o|o-mini)?\b(?!\S)", "gpt-4 family (superseded)"),
|
||||
(r"\bGPT-4(?:o|-32k)?\b", "gpt-4 family (superseded)"),
|
||||
(r"\btext-embedding-ada-002\b", "ada-002 embeddings (superseded)"),
|
||||
(r"\bgemini-1\.5(?:-[a-z]+)?\b", "Gemini 1.5 (superseded)"),
|
||||
]
|
||||
|
||||
# Signals that a line is quoting a source or explicitly labelling age.
|
||||
CITATION_HINTS = (
|
||||
"paper", "technical report", "model card", "as of", "historical",
|
||||
"snapshot", "deprecated", "retired", "superseded", "arxiv", "et al",
|
||||
"changelog", "was ", "formerly", "legacy", "pre-", "no longer",
|
||||
)
|
||||
YEAR_RE = re.compile(r"\b(19|20)\d{2}\b")
|
||||
|
||||
# Lines that are much more likely to be a live default than prose.
|
||||
EXECUTABLE_HINTS = (
|
||||
"model=", "model =", '"model"', "'model'", "model:", "--model",
|
||||
"-m ", "default=", "MODEL", "input\":", "output\":",
|
||||
)
|
||||
|
||||
|
||||
def load_allowlist(repo_root):
|
||||
"""<file-glob> :: <substring> — one per line, '#' comments."""
|
||||
path = os.path.join(repo_root, "scripts", "check_model_freshness_allowlist.txt")
|
||||
entries = []
|
||||
if not os.path.exists(path):
|
||||
return entries
|
||||
with open(path, encoding="utf-8") as handle:
|
||||
for line in handle:
|
||||
line = line.strip()
|
||||
if not line or line.startswith("#") or "::" not in line:
|
||||
continue
|
||||
file_glob, needle = (part.strip() for part in line.split("::", 1))
|
||||
entries.append((file_glob, needle))
|
||||
return entries
|
||||
|
||||
|
||||
def allowlisted(rel_file, line_text, allowlist):
|
||||
posix = rel_file.replace(os.sep, "/")
|
||||
return any(fnmatch.fnmatch(posix, glob) and needle in line_text
|
||||
for glob, needle in allowlist)
|
||||
|
||||
|
||||
def is_citation(line_text):
|
||||
lowered = line_text.lower()
|
||||
if YEAR_RE.search(line_text):
|
||||
return True
|
||||
return any(hint in lowered for hint in CITATION_HINTS)
|
||||
|
||||
|
||||
def looks_executable(line_text):
|
||||
return any(hint in line_text for hint in EXECUTABLE_HINTS)
|
||||
|
||||
|
||||
def scan_file(path, repo_root, allowlist):
|
||||
rel = os.path.relpath(path, repo_root)
|
||||
findings = []
|
||||
try:
|
||||
text = open(path, encoding="utf-8", errors="replace").read()
|
||||
except OSError:
|
||||
return findings
|
||||
|
||||
for lineno, line in enumerate(text.splitlines(), 1):
|
||||
if allowlisted(rel, line, allowlist):
|
||||
continue
|
||||
for pattern, label in RETIRED_PATTERNS:
|
||||
match = re.search(pattern, line)
|
||||
if not match:
|
||||
continue
|
||||
# An executable default outweighs a citation hint on the same line:
|
||||
# `model: str = "claude-3-opus" # 2024 default` still breaks.
|
||||
if is_citation(line) and not looks_executable(line):
|
||||
continue
|
||||
findings.append({
|
||||
"line": lineno,
|
||||
"match": match.group(0),
|
||||
"label": label,
|
||||
"executable": looks_executable(line),
|
||||
"text": line.strip()[:160],
|
||||
})
|
||||
break
|
||||
return findings
|
||||
|
||||
|
||||
def collect(repo_root):
|
||||
targets = []
|
||||
for dirpath, dirnames, filenames in os.walk(repo_root):
|
||||
dirnames[:] = [d for d in dirnames if d not in EXCLUDED_DIRS]
|
||||
for fn in filenames:
|
||||
if fn in EXCLUDED_DIRS:
|
||||
continue
|
||||
if fn.endswith(SCAN_EXTENSIONS):
|
||||
targets.append(os.path.join(dirpath, fn))
|
||||
return sorted(targets)
|
||||
|
||||
|
||||
def main():
|
||||
ap = argparse.ArgumentParser(
|
||||
description="Flag retired model identifiers presented as current."
|
||||
)
|
||||
ap.add_argument("files", nargs="*", help="Specific files to scan")
|
||||
ap.add_argument("--all", action="store_true", help="Scan the canonical tree")
|
||||
ap.add_argument("--json", action="store_true", help="Emit JSON")
|
||||
ap.add_argument("--list-patterns", action="store_true",
|
||||
help="Print the retired-identifier deny-list and exit")
|
||||
ap.add_argument("--executable-only", action="store_true",
|
||||
help="Report only script defaults, config values and CLI examples")
|
||||
ap.add_argument("--root", default=os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
|
||||
help="Repo root (default: parent of this script)")
|
||||
args = ap.parse_args()
|
||||
|
||||
if args.list_patterns:
|
||||
for pattern, label in RETIRED_PATTERNS:
|
||||
print(f"{label}\n {pattern}")
|
||||
return 0
|
||||
|
||||
repo_root = os.path.abspath(args.root)
|
||||
if args.all:
|
||||
targets = collect(repo_root)
|
||||
elif args.files:
|
||||
targets = [os.path.abspath(f) for f in args.files]
|
||||
else:
|
||||
ap.print_help()
|
||||
return 0
|
||||
|
||||
allowlist = load_allowlist(repo_root)
|
||||
findings = {}
|
||||
for path in targets:
|
||||
hits = scan_file(path, repo_root, allowlist)
|
||||
if args.executable_only:
|
||||
hits = [h for h in hits if h["executable"]]
|
||||
if hits:
|
||||
findings[os.path.relpath(path, repo_root)] = hits
|
||||
|
||||
total = sum(len(v) for v in findings.values())
|
||||
n_exec = sum(1 for v in findings.values() for h in v if h["executable"])
|
||||
|
||||
if args.json:
|
||||
print(json.dumps({
|
||||
"files_scanned": len(targets),
|
||||
"total_findings": total,
|
||||
"executable_findings": n_exec,
|
||||
"findings": findings,
|
||||
}, indent=2))
|
||||
else:
|
||||
for rel in sorted(findings):
|
||||
print(f"{rel}:")
|
||||
for hit in findings[rel]:
|
||||
tag = "EXEC" if hit["executable"] else "PROSE"
|
||||
print(f" {tag} L{hit['line']}: {hit['match']} — {hit['label']}")
|
||||
print(f" {hit['text']}")
|
||||
print(f"\nScanned {len(targets)} files; {total} retired-identifier references "
|
||||
f"({n_exec} in executable positions).")
|
||||
if total:
|
||||
print("Fix the reference, or add an allowlist entry with a reason to "
|
||||
"scripts/check_model_freshness_allowlist.txt")
|
||||
|
||||
return 1 if total else 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
26
scripts/check_model_freshness_allowlist.txt
Normal file
26
scripts/check_model_freshness_allowlist.txt
Normal file
|
|
@ -0,0 +1,26 @@
|
|||
# check_model_freshness.py allowlist — deliberate references only.
|
||||
# Format: <file-glob> :: <substring that must appear on the line>
|
||||
#
|
||||
# A retired model identifier belongs here only when naming it IS the point:
|
||||
# a regulatory list frozen at a date, a benchmark labelled as historical, or
|
||||
# the deny-list in the linter itself. If the reference is a script default,
|
||||
# a price table, or a copy-pasteable command, fix the reference instead.
|
||||
#
|
||||
# Every entry needs a one-line reason above it.
|
||||
|
||||
# The linter's own deny-list and this allowlist's examples.
|
||||
scripts/check_model_freshness.py :: claude
|
||||
scripts/check_model_freshness.py :: gpt-
|
||||
scripts/check_model_freshness.py :: gemini-
|
||||
scripts/check_model_freshness.py :: GPT-4
|
||||
scripts/check_model_freshness_allowlist.txt :: gpt-
|
||||
scripts/check_model_freshness_allowlist.txt :: GPT-4
|
||||
|
||||
# The EU AI Act's own systemic-risk examples are frozen in the cited text;
|
||||
# renaming them would misquote the regulation.
|
||||
*/references/ai_risk_governance.md :: systemic risk
|
||||
|
||||
# Benchmarks explicitly published as a dated snapshot, with a staleness
|
||||
# warning already at the top of the file.
|
||||
*/references/embedding_model_benchmark.md :: ada-002
|
||||
*/references/embedding_model_benchmark.md :: text-embedding
|
||||
Loading…
Add table
Reference in a new issue