Skip to content

Commit cab168a

Browse files
Kasper JungeRalphify
authored andcommitted
refactor: rename _is_excluded_skill_path to _is_excluded_resource_path
This function and EXCLUDED_DIRS are used by generic resource discovery functions that handle both skills and ralphs, but still had "skill" in their names from before the generification. Rename to accurately reflect their generic purpose. Co-authored-by: Ralphify <noreply@ralphify.co>
1 parent f799acb commit cab168a

File tree

2 files changed

+30
-30
lines changed

2 files changed

+30
-30
lines changed

agr/skill.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
# no leading/trailing/consecutive hyphens.
2323
_VALID_SKILL_NAME_RE = re.compile(r"^[a-z0-9]+(-[a-z0-9]+)*$")
2424

25-
# Directories to exclude from skill discovery
25+
# Directories to exclude from resource discovery (skills, ralphs, etc.)
2626
EXCLUDED_DIRS = {
2727
".git",
2828
"node_modules",
@@ -50,31 +50,31 @@ def _shallowest(paths: list[_P]) -> _P:
5050
return min(paths, key=lambda p: len(p.parts))
5151

5252

53-
def _is_excluded_skill_path(parts: tuple[str, ...]) -> bool:
54-
"""Check if a relative SKILL.md path should be excluded from discovery.
53+
def _is_excluded_resource_path(parts: tuple[str, ...]) -> bool:
54+
"""Check if a relative marker-file path should be excluded from discovery.
5555
5656
Centralizes the two exclusion rules shared by both filesystem and
57-
git-listing discovery:
57+
git-listing discovery for all resource types (skills, ralphs, etc.):
5858
59-
1. Root-level SKILL.md (single component, e.g. just ``SKILL.md``)
60-
is a repo marker, not a skill directory.
59+
1. Root-level marker (single component, e.g. just ``SKILL.md``)
60+
is a repo marker, not a resource directory.
6161
2. Any **ancestor** directory matching ``EXCLUDED_DIRS`` (.git,
6262
node_modules, __pycache__, etc.) disqualifies the entry.
63-
The skill directory itself (``parts[-2]``) is NOT checked —
64-
a skill legitimately named ``build`` or ``dist`` should be
63+
The resource directory itself (``parts[-2]``) is NOT checked —
64+
a resource legitimately named ``build`` or ``dist`` should be
6565
discoverable.
6666
6767
Args:
6868
parts: Components of the path *relative to the repo root*
6969
(e.g. ``("skills", "my-skill", "SKILL.md")``).
7070
7171
Returns:
72-
True if the path should be excluded from skill discovery.
72+
True if the path should be excluded from resource discovery.
7373
"""
7474
if len(parts) == 1:
7575
return True
76-
# parts[-1] is the marker file, parts[-2] is the skill directory.
77-
# Only check ancestor directories above the skill directory.
76+
# parts[-1] is the marker file, parts[-2] is the resource directory.
77+
# Only check ancestor directories above the resource directory.
7878
ancestors = parts[:-2]
7979
return any(part in EXCLUDED_DIRS for part in ancestors)
8080

@@ -99,7 +99,7 @@ def _find_resource_dirs(repo_dir: Path, marker: str) -> list[Path]:
9999
dirs: list[Path] = []
100100
for marker_path in repo_dir.rglob(marker):
101101
rel = marker_path.relative_to(repo_dir)
102-
if _is_excluded_skill_path(rel.parts):
102+
if _is_excluded_resource_path(rel.parts):
103103
continue
104104
dirs.append(marker_path.parent)
105105
return dirs
@@ -122,7 +122,7 @@ def _find_resource_dirs_in_listing(
122122
rel_path = PurePosixPath(rel)
123123
if rel_path.name != marker:
124124
continue
125-
if _is_excluded_skill_path(rel_path.parts):
125+
if _is_excluded_resource_path(rel_path.parts):
126126
continue
127127
results.append(rel_path.parent)
128128
return results

tests/test_skill.py

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
from agr.skill import (
66
SKILL_MARKER,
7-
_is_excluded_skill_path,
7+
_is_excluded_resource_path,
88
create_skill_scaffold,
99
discover_skills_in_repo_listing,
1010
find_skill_in_repo,
@@ -428,41 +428,41 @@ def test_excludes_root_and_excluded_dirs(self):
428428
assert result["commit"].as_posix() == "skills/commit"
429429

430430

431-
class TestIsExcludedSkillPath:
432-
"""Tests for _is_excluded_skill_path — the shared exclusion predicate."""
431+
class TestIsExcludedResourcePath:
432+
"""Tests for _is_excluded_resource_path — the shared exclusion predicate."""
433433

434434
def test_root_level_skill_md_excluded(self):
435435
"""A single-component path (root SKILL.md) is excluded."""
436-
assert _is_excluded_skill_path(("SKILL.md",)) is True
436+
assert _is_excluded_resource_path(("SKILL.md",)) is True
437437

438438
def test_nested_skill_md_not_excluded(self):
439439
"""A normal skill path like skills/my-skill/SKILL.md is included."""
440-
assert _is_excluded_skill_path(("skills", "my-skill", "SKILL.md")) is False
440+
assert _is_excluded_resource_path(("skills", "my-skill", "SKILL.md")) is False
441441

442442
def test_git_dir_excluded(self):
443443
"""Paths under .git are excluded."""
444-
assert _is_excluded_skill_path((".git", "hooks", "SKILL.md")) is True
444+
assert _is_excluded_resource_path((".git", "hooks", "SKILL.md")) is True
445445

446446
def test_node_modules_excluded(self):
447447
"""Paths under node_modules are excluded."""
448-
assert _is_excluded_skill_path(("node_modules", "pkg", "SKILL.md")) is True
448+
assert _is_excluded_resource_path(("node_modules", "pkg", "SKILL.md")) is True
449449

450450
def test_pycache_excluded(self):
451451
"""Paths under __pycache__ are excluded."""
452-
assert _is_excluded_skill_path(("__pycache__", "my-skill", "SKILL.md")) is True
452+
assert _is_excluded_resource_path(("__pycache__", "my-skill", "SKILL.md")) is True
453453

454454
def test_venv_excluded(self):
455455
"""Paths under .venv are excluded."""
456-
assert _is_excluded_skill_path((".venv", "lib", "SKILL.md")) is True
456+
assert _is_excluded_resource_path((".venv", "lib", "SKILL.md")) is True
457457

458458
def test_build_dir_excluded(self):
459459
"""Paths under build/ are excluded."""
460-
assert _is_excluded_skill_path(("build", "output", "SKILL.md")) is True
460+
assert _is_excluded_resource_path(("build", "output", "SKILL.md")) is True
461461

462462
def test_excluded_dir_deep_in_path(self):
463463
"""An excluded dir anywhere in the path triggers exclusion."""
464464
assert (
465-
_is_excluded_skill_path(("a", "b", "node_modules", "c", "SKILL.md")) is True
465+
_is_excluded_resource_path(("a", "b", "node_modules", "c", "SKILL.md")) is True
466466
)
467467

468468
def test_skill_named_as_excluded_dir_not_excluded(self):
@@ -473,16 +473,16 @@ def test_skill_named_as_excluded_dir_not_excluded(self):
473473
legitimate skill named "build", not a build artifact.
474474
"""
475475
# "build" is the skill directory (parts[-2]), "skills" is an ancestor
476-
assert _is_excluded_skill_path(("skills", "build", "SKILL.md")) is False
476+
assert _is_excluded_resource_path(("skills", "build", "SKILL.md")) is False
477477
# Same for other excluded names used as skill directories
478-
assert _is_excluded_skill_path(("skills", "dist", "SKILL.md")) is False
479-
assert _is_excluded_skill_path(("skills", "vendor", "SKILL.md")) is False
478+
assert _is_excluded_resource_path(("skills", "dist", "SKILL.md")) is False
479+
assert _is_excluded_resource_path(("skills", "vendor", "SKILL.md")) is False
480480

481481
def test_skill_named_as_excluded_dir_at_top_level(self):
482482
"""A top-level skill named after an excluded dir should NOT be excluded."""
483-
assert _is_excluded_skill_path(("build", "SKILL.md")) is False
484-
assert _is_excluded_skill_path(("dist", "SKILL.md")) is False
483+
assert _is_excluded_resource_path(("build", "SKILL.md")) is False
484+
assert _is_excluded_resource_path(("dist", "SKILL.md")) is False
485485

486486
def test_empty_tuple_not_excluded(self):
487487
"""Edge case: empty parts tuple is not excluded (no excluded dir check)."""
488-
assert _is_excluded_skill_path(()) is False
488+
assert _is_excluded_resource_path(()) is False

0 commit comments

Comments
 (0)