skills/magnus919_agent-skills
1
0
Fork 0
mirror of https://github.com/magnus919/agent-skills.git synced 2026-09-11 19:47:12 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix: close skill quality lint gaps (#98)

Browse Source 
Co-authored-by: magnus919 <magnus919>
This commit is contained in:
Magnus Hedemark
2026-07-21 20:47:16 -04:00
committed by GitHub
co-authored by magnus919 <magnus919>
parent c092a14c8e
commit dd457f1170
2 changed files with 68 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/test-validate-skill-quality.rb
+56 -2
View File
@@ -66,7 +66,9 @@ class ValidateSkillQualityTest < Minitest::Test
"## When not to use\n\n> <!-- boundary goes here -->\n",
"## When not to use\n\n```text\nUse another skill instead.\n```\n",
"## When not to use\n\n> ```text\n> Use another skill instead.\n> ```\n",
"## When not to use\n\n`TODO: Document this later.`\n"
"## When not to use\n\n`TODO: Document this later.`\n",
"## When not to use\n\nDetails will be added later.\n",
"## When not to use\n\nThis section explains deployment behavior.\n"
]
bodies.each do |body|
@@ -126,6 +128,56 @@ class ValidateSkillQualityTest < Minitest::Test
end
end
def test_inline_markdown_does_not_hide_no_op_phrases
body = <<~MARKDOWN
# Ruby review
Follow **best practices**.
Write *maintainable* code.
Follow __best practices__.
Write _maintainable_ code.
`Follow best practices.`
Call `follow_best_practices()`.
Call `follow__best__practices()`.
Follow ***best practices***.
Follow **_best practices_**.
Follow _**best practices**_.
Follow **best** *practices*.
Follow *best* **practices**.
Write **maintainable** **code**.
## When not to use
Use a deployment skill for releases.
MARKDOWN
with_skill("Review Ruby code for correctness.", body) do |path|
warnings = SkillQualityValidator.new.validate(path).select { |finding| finding.severity == :warning }
assert_equal 11, warnings.length
assert_equal [6, 7, 8, 9, 10, 13, 14, 15, 16, 17, 18], warnings.map(&:line).sort
end
end
def test_whole_phrase_emphasis_is_detected_for_every_no_op_family
phrases = [
"Write clear code",
"Follow best practices",
"Handle errors gracefully",
"Ensure high quality",
"Make it easy to read",
"Write maintainable code"
]
phrases.each do |phrase|
["**_#{phrase}_**.", "__#{phrase}__."].each do |formatted|
body = "#{formatted}\n\n## When not to use\nUse a deployment skill for releases.\n"
with_skill("Review Ruby code for correctness.", body) do |path|
warnings = SkillQualityValidator.new.validate(path).select { |finding| finding.severity == :warning }
assert_equal 1, warnings.length, formatted
assert_equal 5, warnings.first.line, formatted
end
end
end
end
def test_malformed_frontmatter_is_reported_without_crashing
Dir.mktmpdir do |directory|
path = File.join(directory, "SKILL.md")
@@ -153,9 +205,11 @@ class ValidateSkillQualityTest < Minitest::Test
git(root, "mv", "old-name/RENAMED.md", "renamed/SKILL.md")
write_skill(root, "untracked/SKILL.md", valid_description("untracked"))
write_skill(root, "SKILL.md", valid_description("root-level"))
write_skill(root, "agent-council/profiles/skills/vendored/SKILL.md", valid_description("vendored"))
write_skill(root, "not-agent-council/profiles/skills/changed/SKILL.md", valid_description("similarly named"))
assert_equal(
%w[SKILL.md added/SKILL.md modified/SKILL.md renamed/SKILL.md untracked/SKILL.md],
%w[SKILL.md added/SKILL.md modified/SKILL.md not-agent-council/profiles/skills/changed/SKILL.md renamed/SKILL.md untracked/SKILL.md],
ChangedSkillSelector.new(root: root, base: base).paths
)
end
scripts/validate-skill-quality.rb
+12 -8
View File
@@ -28,13 +28,17 @@ class SkillQualityValidator
/(?:\A|[.!?;:]\s+)distinct from\s+[`*_"']*[[:alnum:]]/i
].freeze
BOUNDARY_HEADING = /^\#{1,6}\s+When not to use\s*\#*\s*$/i
BOUNDARY_CONTENT_PATTERN = /\b(?:avoid|belongs?|choose|defer|do not|don't|does not|doesn't|instead|never|no|not|only|outside|prefer|prerequisite|required?|requires?|rather than|route|see|skip|use|when)\b/i
MARKDOWN_LEAD = "(?<![[:alnum:]])[*_`~]*"
MARKDOWN_GAP = "[*_`~]*\\s+[*_`~]*"
MARKDOWN_TRAIL = "[*_`~]*"
NO_OP_PATTERNS = {
"write clear or clean code" => /\bwrite\s+(?:clear|clean)\s+code\b/i,
"follow best practices" => /\bfollow\s+best\s+practices\b/i,
"handle errors appropriately or gracefully" => /\bhandle\s+errors\s+(?:appropriately|gracefully)\b/i,
"ensure high quality" => /\bensure\s+high\s+quality\b/i,
"make it easy to read" => /\bmake\s+it\s+easy\s+to\s+read\b/i,
"write maintainable code" => /\bwrite\s+maintainable\s+code\b/i
"write clear or clean code" => /#{MARKDOWN_LEAD}write#{MARKDOWN_GAP}(?:clear|clean)#{MARKDOWN_GAP}code#{MARKDOWN_TRAIL}\b/i,
"follow best practices" => /#{MARKDOWN_LEAD}follow#{MARKDOWN_GAP}best#{MARKDOWN_GAP}practices#{MARKDOWN_TRAIL}\b/i,
"handle errors appropriately or gracefully" => /#{MARKDOWN_LEAD}handle#{MARKDOWN_GAP}errors#{MARKDOWN_GAP}(?:appropriately|gracefully)#{MARKDOWN_TRAIL}\b/i,
"ensure high quality" => /#{MARKDOWN_LEAD}ensure#{MARKDOWN_GAP}high#{MARKDOWN_GAP}quality#{MARKDOWN_TRAIL}\b/i,
"make it easy to read" => /#{MARKDOWN_LEAD}make#{MARKDOWN_GAP}it#{MARKDOWN_GAP}easy#{MARKDOWN_GAP}to#{MARKDOWN_GAP}read#{MARKDOWN_TRAIL}\b/i,
"write maintainable code" => /#{MARKDOWN_LEAD}write#{MARKDOWN_GAP}maintainable#{MARKDOWN_GAP}code#{MARKDOWN_TRAIL}\b/i
}.freeze
Finding = Struct.new(:severity, :path, :line, :message)
@@ -144,7 +148,7 @@ class SkillQualityValidator
next false if visible.empty? || visible.match?(/\A[-*_]{3,}\z/)
next false if visible.match?(/\A(?:TODO|TBD|TBA)\b/i)
visible.scan(/[[:alnum:]]+/).length >= 3
visible.scan(/[[:alnum:]]+/).length >= 3 && visible.match?(BOUNDARY_CONTENT_PATTERN)
end
end
@@ -182,7 +186,7 @@ class ChangedSkillSelector
selected.merge(git_paths("ls-files", "--others", "--exclude-standard", "-z", "--", SKILL_PATHSPEC))
selected
.reject { |path| path.include?("agent-council/profiles/skills/") }
.reject { |path| path.start_with?("agent-council/profiles/skills/") }
.select { |path| (@root / path).file? }
.sort
end