refactor(core): use frontmatter parser for skills

[omarluq]

No description provided.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ef85447a-5e30-4e0e-a6e0-191743469e0a

📥 Commits

Reviewing files that changed from the base of the PR and between e542a28 and 313449a.

📒 Files selected for processing (1)

• internal/core/frontmatter_internal_test.go

---

📝 Walkthrough

Summary by CodeRabbit

Release Notes

• Refactor

  • Updated skill YAML frontmatter parsing to use a standardized parser, improving handling of --- delimiters and extracting YAML metadata more reliably.

• Tests

  • Added unit tests covering valid frontmatter, missing delimiters, unterminated blocks, and invalid YAML (including error messaging).
  • Removed older frontmatter-related tests that no longer apply to the updated parsing behavior.

Walkthrough

Replaces the hand-rolled frontmatter extractor with the github.com/adrg/frontmatter library. The exported Frontmatter helper is removed; parseSkillFrontmatter now delegates to frontmatter.Parse configured with a ---/--- YAML format. Three new go.mod entries cover the library and its transitive deps. The old exported-function test file is deleted and replaced with internal package tests.

Changes

Frontmatter parser replacement

Layer / File(s)	Summary
Dependency additions and parser reimplementation go.mod, internal/core/frontmatter.go	go.mod adds github.com/adrg/frontmatter v0.2.0 (direct) plus BurntSushi/toml and gopkg.in/yaml.v2 (indirect). frontmatter.go removes the exported Frontmatter helper, adds skillFrontmatterFormat to configure --- delimiters with yaml.Unmarshal, and rewrites parseSkillFrontmatter to call frontmatter.Parse.
Internal tests replacing exported-function tests internal/core/frontmatter_internal_test.go, internal/core/frontmatter_test.go	frontmatter_test.go (tests for the removed exported helper) is deleted. frontmatter_internal_test.go is added with TestParseSkillFrontmatter (valid YAML, no delimiters, unterminated block, invalid YAML) and TestParseSkillFrontmatterParsesNameDescriptionAndBody (metadata field and body extraction).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A bunny once parsed with great toil by hand,
Each --- delimiter painstakingly scanned.
Now a library leaps in with elegant grace,
frontmatter.Parse hops right into place!
The exported helper has hopped away free,
And tests dance inside the same package, you see. 🐇✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to assess whether the description relates to the changeset.	Add a description explaining the rationale for switching to an external frontmatter parser, any benefits gained, and testing performed.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: replacing the local frontmatter implementation with an external parser library for skill parsing.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/frontmatter

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

internal/core/frontmatter_internal_test.go (1)

79-95: ⚡ Quick win

Rename this test to match what it exercises.

TestLoadSkillsParsesFrontmatterWithoutExportedExtractor never calls LoadSkills; it only calls parseSkillFrontmatter. A more precise name will avoid false integration-scope signaling.

✏️ Suggested rename

-func TestLoadSkillsParsesFrontmatterWithoutExportedExtractor(t *testing.T) {
+func TestParseSkillFrontmatterParsesNameDescriptionAndBody(t *testing.T) {

🤖 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 `@internal/core/frontmatter_internal_test.go` around lines 79 - 95, The test
function TestLoadSkillsParsesFrontmatterWithoutExportedExtractor has a
misleading name because it does not actually call LoadSkills, only
parseSkillFrontmatter. Rename this test function to accurately reflect what it
tests, such as TestParseSkillFrontmatterWithoutExportedExtractor, to provide
clear signaling about the scope of the test and avoid confusion about what
function is being exercised.

🤖 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 `@internal/core/frontmatter_internal_test.go`:
- Around line 79-95: The test function
TestLoadSkillsParsesFrontmatterWithoutExportedExtractor has a misleading name
because it does not actually call LoadSkills, only parseSkillFrontmatter. Rename
this test function to accurately reflect what it tests, such as
TestParseSkillFrontmatterWithoutExportedExtractor, to provide clear signaling
about the scope of the test and avoid confusion about what function is being
exercised.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5949e8c3-001b-47b0-b1e0-5aa7d1e4999c

📥 Commits

Reviewing files that changed from the base of the PR and between d760571 and e542a28.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (4)

• go.mod
• internal/core/frontmatter.go
• internal/core/frontmatter_internal_test.go
• internal/core/frontmatter_test.go

💤 Files with no reviewable changes (1)

• internal/core/frontmatter_test.go

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.40%. Comparing base (d760571) to head (313449a).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #143      +/-   ##
==========================================
+ Coverage   80.36%   80.40%   +0.04%     
==========================================
  Files         277      277              
  Lines       21913    21898      -15     
==========================================
- Hits        17610    17607       -3     
+ Misses       3086     3079       -7     
+ Partials     1217     1212       -5     

Flag	Coverage Δ
unittests	80.40% <100.00%> (+0.04%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud