Conversation
There was a problem hiding this comment.
Thanks for the thorough redesign work — the architecture improvements (Astro content collections, lib/ separation, richer JSON-LD, RSS feed, per-skill JSON API) are genuinely good. A few things need to be addressed before this can merge, ranging from a silent data-loss bug to several UX regressions.
Critical: multi-file skills are silently incomplete
The new /skills/<slug>.md endpoint and llms-full.txt both call getSkillRawMarkdown(), which reads only SKILL.md. Two skills currently ship additional files that agents need:
canhelp—scripts/fetch-candid.sh,scripts/resolve-canister-id.shicp-cli—references/binding-generation.md,references/dev-server.md,references/dfx-migration.md
An agent fetching /skills/canhelp.md gets the skill stub without the scripts it references — broken skill, no error, no indication anything is missing.
More importantly, llms-full.txt explicitly claims in its own header to be "the entire corpus" — but it silently omits reference files and scripts for multi-file skills. A training scraper ingesting it would get an incomplete dataset with no warning.
The existing /.well-known/skills/<name>/SKILL.zip endpoint handles multi-file skills correctly, and getSkillFiles() / getSkillFileEntries() in the new skills.ts already enumerate the extra files. Three things need fixing:
llms-full.txt: for multi-file skills, append reference files and scripts after the SKILL.md block so the corpus is actually complete/skills/<slug>.md: either serve the ZIP for multi-file skills, or document clearly that it is SKILL.md only and point to/.well-known/skills/<name>/SKILL.zipfor the full skill- Add
fileCountand azipURL to/api/skills/<slug>.jsonwhen a skill has more than one file so programmatic consumers can detect this
Critical: CATEGORY_ORDER in skills.ts uses stale category names
const CATEGORY_ORDER = [
'Architecture', // renamed → Core in PR #168
...
'Tokens', // removed in PR #168
'Wallet', // removed in PR #168
];If this merges after #168, Core skills sort last (after Security, alphabetically) and the old names match nothing. Needs updating to: Auth, Core, DeFi, Frontend, Governance, Infrastructure, Integration, Security.
Significant regressions
Copy-prompt button removed. The "Give your agent ICP skills" button that copies Fetch /llms.txt and follow its instructions was the primary call-to-action for agent users — the whole point of the site. The new hero has "Get started" and "How it works" links, which point to documentation. The prompt-copy affordance needs to come back.
"Browse" nav item has no effect on the homepage. The nav has <a href="/">Browse</a>. Clicking it from / reloads the same page with no feedback — it looks broken. The current site links "browse" to /skills/, a distinct URL. With the new design merging the browse page into the homepage, either (a) give the Browse link an active/current state so users know they're already there, or (b) restore /skills/ as a real route.
Two markdown endpoints for the same content. /skills/<slug>.md (new) and /.well-known/skills/<name>/SKILL.md (RFC standard, unchanged) both serve SKILL.md. The How It Works page promotes the non-standard one and doesn't mention the /.well-known/ RFC endpoints at all — these are the standardised paths that agent frameworks like icp-cli auto-detect and should be documented.
Minor regressions
- Theme toggle removed (
ThemeToggle.tsxdeleted) — dark/light preference lost - Category filter pills gone — no interactive filtering; the homepage lists all skills in a flat grouped layout with no way to narrow by category without scrolling
What's better in this PR
To be clear, the following are genuine improvements:
- Skills linked directly from the homepage with real
<a href>— better PageRank distribution than the/skills/hub approach in #166 - Much richer
TechArticleJSON-LD (dateModified from git, encoding variants, publisher, license) BreadcrumbListJSON-LD preserved on skill pages- RSS feed is a useful addition
llms-full.txtis a valid convention (llms.txt spec recommends it for bulk corpus access and training data) — just needs to include reference files to live up to its "entire corpus" claim- Astro content collections +
lib/site.ts/lib/skills.tsare a cleaner architecture - Dynamic
robots.txtis the right approach - Per-skill JSON API endpoint
getSkillFiles()/getSkillFileEntries()inskills.tscorrectly enumerate multi-file skills — the/.well-known/layer is intact
The SEO fundamentals from #166 are preserved or improved. The issues above are about agent correctness (multi-file skills), UX regressions (copy prompt, Browse nav), and one architectural point (canonical markdown URL).
Skill Validation ReportNo skill files were changed in this PR — validation skipped. |
No description provided.