Skill catalog: label a skill whose frontmatter does not parse by its directory, and say why - #6
Merged
Merged
Conversation
…directory, and say why A SKILL.md whose YAML frontmatter fails to parse rendered in the session catalog as a bare "SKILL" -- no name, no description, nothing logged. _read_skill_metadata swallowed the YAMLError and returned None, and _readable_skill_name fell back to path.stem, which for every Agent-Skills-style skill is the constant "SKILL". Two shared skills (workshop-problem, research-writer) shipped that way for ten weeks because an unquoted ': ' in the description is a mapping indicator to YAML (ligon/sucoder-skills#1, fixed there by ligon/sucoder-skills#2). - _read_skill_metadata returns a SkillMetadata(name, description, error) NamedTuple. A frontmatter block that is present but unusable -- YAML error, or a non-mapping document, which used to raise AttributeError out of the prelude build -- yields name "" and an actionable one-line error with FILE coordinates ("mapping values are not allowed here at line 3, column 163"). The reader does not invent a name. - _readable_skill_name falls back to the directory name for a SKILL.md (the stem otherwise) and puts "(frontmatter did not parse: ...)" in the description slot, so the catalog line carries the diagnosis. - The three call sites (_skill_blocks, _render_skill_catalog, _format_skill_reference) test metadata.name rather than truthiness and log a warning per broken skill per launch. A catalog file with broken frontmatter keeps its generic "SKILL CATALOG" header. Tests: unit tests for the broken / valid / non-mapping cases and the display-name fallback, plus a prelude test asserting the entry reads "- workshop-problem — (frontmatter did not parse: ..." and no line starts with "- SKILL". Full suite 655 passed (650 before). mypy on sucoder/mirror.py reports the same two pre-existing errors before and after. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EVtHa4ZXum7AbKRRSsSNxm
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Companion to ligon/sucoder-skills#1 / ligon/sucoder-skills#2 (which fix the two affected skill files). This fixes the generator side so the next broken skill is visible instead of silent.
What was wrong. A
SKILL.mdwhose YAML frontmatter fails to parse rendered in the session catalog as_read_skill_metadata(sucoder/mirror.py) caught theYAMLError, setdata = {}, and returnedNone;_readable_skill_namethen fell back topath.stem, which for every Agent-Skills-style skill is the constantSKILL. Nothing was logged. Two shared skills shipped that way for ten weeks. A frontmatter that parsed to a non-mapping (a bare list) was worse:data.getraisedAttributeErrorout of the prelude build.Change.
_read_skill_metadatareturnsSkillMetadata(name, description, error)(aNamedTuple). A present-but-unusable frontmatter yieldsname=""and a one-lineerrorwith file coordinates —mapping values are not allowed here at line 3, column 164— from PyYAML'sproblem/problem_mark. The reader does not invent a name._readable_skill_namefalls back to the directory name for aSKILL.md(stem otherwise) and puts(frontmatter did not parse: …)in the description slot, so the catalog line carries the diagnosis.metadata.namerather than truthiness andlogger.warningonce per broken skill per launch. A catalog file with broken frontmatter keeps its genericSKILL CATALOGheader rather than being renamed to its directory.Rendered against the live
~/.sucoder/skillson this node (pre-#2 checkout):Tests. Five added in
tests/test_mirror.py: broken / valid (regression pin) / non-mapping frontmatter at the unit level, the display-name fallback, and a prelude test that a catalog referencing a broken skill renders- workshop-problem — (frontmatter did not parse: mapping values are not allowed hereand that no line starts with- SKILL. Full suite: 655 passed (650 onf72f815).mypy sucoder/mirror.pyunder the repo's[tool.mypy]: the same two pre-existingvar-annotatederrors before and after, none in the changed code.Not changed.
scripts/generate_unified_skills_catalog.pydoes not read local frontmatter (it fetches curated remote skills), so there is nothing to fail closed there — closing out a remark I made earlier that it might.🤖 Generated with Claude Code
https://claude.ai/code/session_01EVtHa4ZXum7AbKRRSsSNxm