Feature Description
SkillLoader.load_skill() currently returns a raw dict {"module": module, "manifest": ..., "instructions": ..., "card": ...}. To instantiate the skill, callers must know the class name — there is no enforced convention and no helper. The comment at loader.py:153 reads:
# Find the class that inherits from BaseSkill?
# For now assume the user looks for the exported class or we inspect.
This is unresolved technical debt. The result is that:
- Integration tests and examples must hardcode class names (e.g.
bundle["module"].IssueResolverSkill()).
- No validation is done that the loaded module actually contains a
BaseSkill subclass.
SkillLoader cannot programmatically instantiate a skill without caller-side knowledge of the class name.
Proposed change:
Add a get_skill_class(bundle) helper (or auto-populate bundle["class"]) that:
- Inspects the loaded module for subclasses of
BaseSkill.
- Returns the first (and ideally only) match, or raises a clear error if zero or more than one subclass is found.
- Optionally: enforce a naming convention (
<TitleCase>Skill) as an alternative auto-discovery path.
Update load_skill() to always populate bundle["class"] with the discovered class (uninstantiated), making it a first-class field alongside module, manifest, instructions, and card.
Rationale
The current design requires every caller of load_skill() to have out-of-band knowledge of the skill's class name. This makes integration tests fragile, blocks programmatic skill loading (e.g. skillware test, skillware doctor), and contradicts the registry model where skills should be fully self-describing.
The fix is small (10-20 lines of inspection logic) but unblocks: the two-tier test improvements in #86, the CLI commands in #83 and #81, and any future auto-execution of skills from the registry without source code inspection.
Implementation Idea
In loader.py, after spec.loader.exec_module(module):
import inspect
from skillware.core.base_skill import BaseSkill
skill_classes = [
obj for _, obj in inspect.getmembers(module, inspect.isclass)
if issubclass(obj, BaseSkill) and obj is not BaseSkill
]
if len(skill_classes) != 1:
raise ImportError(
f"Expected exactly one BaseSkill subclass in {skill_file}, "
f"found: {[c.__name__ for c in skill_classes]}"
)
Return bundle["class"] = skill_classes[0].
Update tests/test_loader.py to assert bundle["class"] is present and instantiable.
Update CONTRIBUTING.md to note that each skill.py must export exactly one BaseSkill subclass.
Feature Description
SkillLoader.load_skill()currently returns a raw dict{"module": module, "manifest": ..., "instructions": ..., "card": ...}. To instantiate the skill, callers must know the class name — there is no enforced convention and no helper. The comment atloader.py:153reads:This is unresolved technical debt. The result is that:
bundle["module"].IssueResolverSkill()).BaseSkillsubclass.SkillLoadercannot programmatically instantiate a skill without caller-side knowledge of the class name.Proposed change:
Add a
get_skill_class(bundle)helper (or auto-populatebundle["class"]) that:BaseSkill.<TitleCase>Skill) as an alternative auto-discovery path.Update
load_skill()to always populatebundle["class"]with the discovered class (uninstantiated), making it a first-class field alongsidemodule,manifest,instructions, andcard.Rationale
The current design requires every caller of
load_skill()to have out-of-band knowledge of the skill's class name. This makes integration tests fragile, blocks programmatic skill loading (e.g.skillware test,skillware doctor), and contradicts the registry model where skills should be fully self-describing.The fix is small (10-20 lines of inspection logic) but unblocks: the two-tier test improvements in #86, the CLI commands in #83 and #81, and any future auto-execution of skills from the registry without source code inspection.
Implementation Idea
In
loader.py, afterspec.loader.exec_module(module):Return
bundle["class"] = skill_classes[0].Update
tests/test_loader.pyto assertbundle["class"]is present and instantiable.Update
CONTRIBUTING.mdto note that eachskill.pymust export exactly oneBaseSkillsubclass.