refactor(skills): consolidate 16 swamp-* skills into 11#1364
Conversation
…rigger routing Five extension-creation skills (model, vault, driver, datastore, quality) merged into one `swamp-extension` skill with type-specific references. swamp-data-query absorbed into swamp-data. swamp-report narrowed to usage-only (creation content moved to swamp-extension). Trigger overlaps cleaned up across swamp-vault, swamp-repo, swamp-troubleshooting, and swamp-extension-publish. Migration added to remove superseded skill directories on `swamp repo upgrade`. Eval baseline: 198/202 (98.0%) → 262/264 (99.2%) with 62 additional test cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…limit Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
Clean consolidation — 16 → 11 skills with improved eval coverage (98.0% → 99.2%). The migration path, asset registry, and cross-references are well-handled.
Blocking Issues
None.
Suggestions
-
Stale cross-reference in
swamp-report/references/report-types.md:180— still links to../swamp-extension-model/references/api.md#datahandle-structure, which no longer exists after the consolidation. Should be../swamp-extension/references/model/api.md#datahandle-structure. This is the only remaining reference to any old skill name across all skill files. -
No unit test for
removeSupersededSkills()(src/domain/repo/repo_service.ts:116) — the function is simple and was verified via manual migration testing per the test plan, but a unit test confirming it removes existing directories and silently skips missing ones would prevent future regressions if the migration list grows.
DDD Assessment
The removeSupersededSkills function is appropriately placed in the repo service — repo lifecycle management (init/upgrade) including skill migration is squarely within this service's responsibility. The SUPERSEDED_SKILLS constant is co-located with its consumer, keeping the blast radius minimal.
Notes
- All bundled skill paths in
skill_assets.tsverified to exist on the branch — no phantom entries. - All skill description lengths are under the 1024-char limit (largest:
swamp-dataat 1001 chars). - New
swamp-extension/SKILL.mdis 401 lines, under the 500-line body limit. SKILL_NAMESingenerate_config.tsnow includesswamp-extension-publishandswamp-getting-startedwhich were previously missing from the eval list despite havingtrigger_evals.json— nice improvement.- No libswamp import boundary violations.
- No security concerns.
There was a problem hiding this comment.
Adversarial Review
Critical / High
None found.
Medium
-
Stale cross-reference — dead link after merge (
swamp-report/references/report-types.md:180):The line reads:
[extension model API reference](../swamp-extension-model/references/api.md#datahandle-structure)
After this PR merges,
swamp-extension-model/is deleted (superseded skill), making this a broken link. The correct post-consolidation path would be../swamp-extension/references/model/api.md#datahandle-structure. This file is a bundled skill asset copied into every user's skills directory onswamp repo init/upgrade, so every new and upgraded repo will receive the dead link.Fix: Update
swamp-report/references/report-types.md:180to../swamp-extension/references/model/api.md#datahandle-structure.
Low
None.
Verdict
PASS — The core logic changes are sound. The skill consolidation is mechanically straightforward: rename directories, update the BUNDLED_SKILLS manifest, fix cross-references, and add an idempotent migration function (removeSupersededSkills) that correctly handles NotFound. The generate_config.ts skill list, repo_service.ts instructions body, and all modified skill files consistently reference the new consolidated names. The one stale cross-reference in an unmodified bundled file is a minor documentation issue, not a code correctness problem.
Summary
swamp-extensionskill with type-specific reference subdirectoriesswamp-data-queryintoswamp-dataswamp-reportto usage-only (report creation content moved toswamp-extension)removeSupersededSkills()migration to clean up old skill directories onswamp repo upgradeskill_assets.ts,repo_service.ts(CLAUDE.md generation), andgenerate_config.ts(eval runner)Eval results
Test plan
deno checkpassesdeno lintpassesdeno fmtpassesdeno run test— 5813 passed, 0 faileddeno run compilesucceedsdeno run review-skills— all skills pass (93-96%)deno run eval-skill-triggers --model sonnet— 262/264 (99.2%)swamp repo init(old binary) →swamp repo upgrade(new binary) correctly removes old skill dirs and creates new structure🤖 Generated with Claude Code