[None][chore] Enforce Claude Code skill and agent naming convention via pre-commit#14285
Conversation
📝 WalkthroughWalkthroughThis PR introduces a naming convention validation system for Claude skills and agents. A new Python script discovers all skill and agent files, validates their names against allowed prefixes, and ensures YAML frontmatter ChangesClaude Skill/Agent Naming Convention Validation
🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/check_skill_naming_convention.py (2)
22-22: 💤 Low valueOptional: Remove unnecessary future import.
The
from __future__ import annotationsimport is not required in Python 3.10+, which this codebase targets. Modern type hints likestr | Noneandlist[tuple[Path, str]]work natively.Based on learnings: "In TensorRT-LLM (Python requires >=3.10 and <4 as per setup.py), you can use Python 3.10+ features (e.g., PEP 585 generics like dict[str, int], list[str], etc.) throughout the codebase, and you do not need to add from future import annotations."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check_skill_naming_convention.py` at line 22, Remove the unnecessary future import by deleting the line "from __future__ import annotations" at the top of the module; the codebase targets Python 3.10+ (so annotations are native) and no other changes to functions or type hints (e.g., any usage of PEP 585 generics or union operators) are needed in this file.
46-52: ⚡ Quick winConsider adding error handling for file I/O and YAML parsing.
The function can raise exceptions from
path.read_text()(e.g.,FileNotFoundError,PermissionError,UnicodeDecodeError) andyaml.safe_load()(e.g.,yaml.YAMLError). While unhandled exceptions are visible in pre-commit output, catching these and reporting them as violations would provide clearer feedback.🛡️ Suggested improvement
def load_frontmatter_name(path: Path) -> str | None: - m = FRONTMATTER_RE.match(path.read_text()) + try: + content = path.read_text() + except (FileNotFoundError, PermissionError, UnicodeDecodeError) as e: + return None # Caller can detect missing name and report violation + m = FRONTMATTER_RE.match(content) if not m: return None - data = yaml.safe_load(m.group(1)) or {} + try: + data = yaml.safe_load(m.group(1)) or {} + except yaml.YAMLError: + return None # Malformed YAML treated as missing name name = data.get("name") return name if isinstance(name, str) else None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check_skill_naming_convention.py` around lines 46 - 52, Wrap the body of load_frontmatter_name in a try/except that surrounds path.read_text() and yaml.safe_load() to prevent uncaught I/O and parse errors; specifically catch FileNotFoundError, PermissionError, UnicodeDecodeError and yaml.YAMLError, and on those exceptions log a concise error (e.g., via logging.error or writing to stderr) that includes the path and exception details, then return None so the caller treats it as a missing/invalid frontmatter; preserve the existing FRONTMATTER_RE matching and the final type-check for name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/check_skill_naming_convention.py`:
- Line 22: Remove the unnecessary future import by deleting the line "from
__future__ import annotations" at the top of the module; the codebase targets
Python 3.10+ (so annotations are native) and no other changes to functions or
type hints (e.g., any usage of PEP 585 generics or union operators) are needed
in this file.
- Around line 46-52: Wrap the body of load_frontmatter_name in a try/except that
surrounds path.read_text() and yaml.safe_load() to prevent uncaught I/O and
parse errors; specifically catch FileNotFoundError, PermissionError,
UnicodeDecodeError and yaml.YAMLError, and on those exceptions log a concise
error (e.g., via logging.error or writing to stderr) that includes the path and
exception details, then return None so the caller treats it as a missing/invalid
frontmatter; preserve the existing FRONTMATTER_RE matching and the final
type-check for name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1cf2f749-a0cb-4663-882d-d923bd3439a5
📒 Files selected for processing (2)
.pre-commit-config.yamlscripts/check_skill_naming_convention.py
kaiyux
changed the title
Enforces the Claude Code skill (and agent) naming convention documented in .claude/README.md: every entry under .claude/skills/*/SKILL.md and .claude/agents/*.md must start with an approved domain prefix (ad-, exec-, kernel-, perf-, trtllm-) and its on-disk name must match the frontmatter `name:` field. Signed-off-by: Kaiyu Xie <26294424+kaiyux@users.noreply.github.com>
kaiyux
force-pushed
the
user/kaiyu/skills_name_convention
branch
from
1be6b91 to
3db81d1
Compare
|
/bot skip --comment "skill related changes" |
|
PR_Github #49142 [ skip ] triggered by Bot. Commit: |
|
PR_Github #49142 [ skip ] completed with state |
4 participants
Summary by CodeRabbit
Description
Enforces the Claude Code skill (and agent) naming convention documented in
.claude/README.mdvia a pre-commit hook:.claude/skills/*/SKILL.md) and agent (.claude/agents/*.md) on-disk name must start with one of the approved domain prefixes —ad-,exec-,kernel-,perf-,trtllm-— followed by a descriptive suffix.name:field must match the on-disk name (directory name for skills, file stem for agents).The hook runs
scripts/check_skill_naming_convention.pywhenever a skill, agent, or the checker itself is touched, and fails the commit with a list of violations. This keeps new contributions aligned with the convention without manual review.Test Coverage
python scripts/check_skill_naming_convention.pyexits 0 on the current tree; all existing skills and agents already comply with the documented prefixes.files:filter scopes runs to.claude/skills/<name>/SKILL.md,.claude/agents/*.md, and the checker script itself, so it only fires on relevant changes.PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.