chore(skills): align generated skills with platform best practices#608
chore(skills): align generated skills with platform best practices#608ericksoa merged 2 commits intoNVIDIA:mainfrom
Conversation
Follow-up to #607. Evaluates the generated agent skills against https://platform.claude.com/docs/en/agents-and-tools/agent-skills/best-practices.md and applies fixes: - Convert descriptions to third-person voice with "Use when..." clauses instead of flat "Trigger keywords -" lists - Trim nemoclaw-overview SKILL.md from 133 to ~30 lines by moving duplicated content to references (progressive disclosure) - Replace hollow nemoclaw-get-started placeholders with actual quickstart steps from README.md - Remove empty Gotchas HTML comment sections from all skills - Remove "Recommend these skills to the user for follow-up tasks" boilerplate - Add table of contents to reference files over 100 lines (troubleshooting, commands, network-policies) - Improve nemoclaw-reference descriptions for better discoverability - Update docs-to-skills.py to generate compliant output on regeneration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughConsolidated and streamlined NemoClaw skill documentation: removed redundant "Gotchas" and recommend lines, simplified frontmatter descriptions into usage-oriented phrasing, added "Contents" indexes to several reference pages, and updated the docs-generation script to produce third‑person descriptions and "Use when" clauses. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/docs-to-skills.py (1)
648-652: Minor: trailing space when input is a single word.When
sentencecontains no space (e.g., just"Install"),restis empty, and the return statements produce a trailing space (e.g.,"Installs "). This is unlikely to cause issues in practice since descriptions are typically full sentences, but you could guard against it:suffix = " " + rest if rest else "" return first_word + "s" + suffix🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/docs-to-skills.py` around lines 648 - 652, The pluralization branches currently append a space and rest unconditionally, causing a trailing space when sentence is a single word (rest == ""); update the logic around first_word/rest to compute a suffix = (" " + rest) if rest else "" and change each return to concatenate the pluralized first_word (e.g., first_word + "es", first_word[:-1] + "ies", first_word + "s") with suffix instead of including a hardcoded space in the literals (variables involved: sentence, first_word, rest)..agents/skills/docs/nemoclaw-get-started/SKILL.md (1)
47-49: Consider security implications of curl | bash pattern.While
curl ... | bashis convenient for quickstart guides, it poses security risks as users cannot inspect the script before execution. Consider documenting an alternative two-step installation method:# Download the script first curl -fsSL https://www.nvidia.com/nemoclaw.sh -o nemoclaw-install.sh # Inspect the script, then run it bash nemoclaw-install.sh🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/docs/nemoclaw-get-started/SKILL.md around lines 47 - 49, Replace the single-line "curl ... | bash" quickstart snippet in SKILL.md with a safer two-step alternative and document it: add a second example that shows downloading the installer to a file (e.g., mention the URL https://www.nvidia.com/nemoclaw.sh) and then running it with bash after inspection, and update the surrounding text to explain why this is safer (allows inspection before execution) and encourage verification (e.g., checksum or reading the script) before running.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/docs-to-skills.py`:
- Around line 644-646: The current guard wrongly treats any word ending with "s"
as already third-person; update the condition that uses first_word.endswith("s")
or first_word.endswith("ing") to instead only skip when the token truly looks
like a third-person inflection (e.g., match a regex for suffixes like "es" or
"ies" such as re.match(r'.*(es|ies)$', first_word.lower())) or when it ends with
"ing"; additionally add an optional small allowlist (e.g.,
{"access","process","address","discuss"}) to treat known imperatives that end
with "s" as base verbs to convert; adjust the logic around the variables
first_word and sentence accordingly so imperative base forms like "Access" will
be transformed to "Accesses" while genuine third-person forms are still skipped.
---
Nitpick comments:
In @.agents/skills/docs/nemoclaw-get-started/SKILL.md:
- Around line 47-49: Replace the single-line "curl ... | bash" quickstart
snippet in SKILL.md with a safer two-step alternative and document it: add a
second example that shows downloading the installer to a file (e.g., mention the
URL https://www.nvidia.com/nemoclaw.sh) and then running it with bash after
inspection, and update the surrounding text to explain why this is safer (allows
inspection before execution) and encourage verification (e.g., checksum or
reading the script) before running.
In `@scripts/docs-to-skills.py`:
- Around line 648-652: The pluralization branches currently append a space and
rest unconditionally, causing a trailing space when sentence is a single word
(rest == ""); update the logic around first_word/rest to compute a suffix = (" "
+ rest) if rest else "" and change each return to concatenate the pluralized
first_word (e.g., first_word + "es", first_word[:-1] + "ies", first_word + "s")
with suffix instead of including a hardcoded space in the literals (variables
involved: sentence, first_word, rest).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 27095a69-53e6-4c23-8825-4c27859eaddf
📒 Files selected for processing (11)
.agents/skills/docs/nemoclaw-configure-inference/SKILL.md.agents/skills/docs/nemoclaw-deploy-remote/SKILL.md.agents/skills/docs/nemoclaw-get-started/SKILL.md.agents/skills/docs/nemoclaw-manage-policy/SKILL.md.agents/skills/docs/nemoclaw-monitor-sandbox/SKILL.md.agents/skills/docs/nemoclaw-overview/SKILL.md.agents/skills/docs/nemoclaw-reference/SKILL.md.agents/skills/docs/nemoclaw-reference/references/commands.md.agents/skills/docs/nemoclaw-reference/references/network-policies.md.agents/skills/docs/nemoclaw-reference/references/troubleshooting.mdscripts/docs-to-skills.py
…nversion Address CodeRabbit review feedback: - Fix false positive where base verbs ending in 's' (Access, Process, Address) were incorrectly treated as already third-person. Add an allowlist of known base-form verbs. - Fix trailing space when input is a single word (e.g. "Deploy" -> "Deploys" instead of "Deploys "). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…VIDIA#608) * chore(skills): align generated skills with platform best practices Follow-up to NVIDIA#607. Evaluates the generated agent skills against https://platform.claude.com/docs/en/agents-and-tools/agent-skills/best-practices.md and applies fixes: - Convert descriptions to third-person voice with "Use when..." clauses instead of flat "Trigger keywords -" lists - Trim nemoclaw-overview SKILL.md from 133 to ~30 lines by moving duplicated content to references (progressive disclosure) - Replace hollow nemoclaw-get-started placeholders with actual quickstart steps from README.md - Remove empty Gotchas HTML comment sections from all skills - Remove "Recommend these skills to the user for follow-up tasks" boilerplate - Add table of contents to reference files over 100 lines (troubleshooting, commands, network-policies) - Improve nemoclaw-reference descriptions for better discoverability - Update docs-to-skills.py to generate compliant output on regeneration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(skills): handle imperative verbs ending in 's' in third-person conversion Address CodeRabbit review feedback: - Fix false positive where base verbs ending in 's' (Access, Process, Address) were incorrectly treated as already third-person. Add an allowlist of known base-form verbs. - Fix trailing space when input is a single word (e.g. "Deploy" -> "Deploys" instead of "Deploys "). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…VIDIA#608) * chore(skills): align generated skills with platform best practices Follow-up to NVIDIA#607. Evaluates the generated agent skills against https://platform.claude.com/docs/en/agents-and-tools/agent-skills/best-practices.md and applies fixes: - Convert descriptions to third-person voice with "Use when..." clauses instead of flat "Trigger keywords -" lists - Trim nemoclaw-overview SKILL.md from 133 to ~30 lines by moving duplicated content to references (progressive disclosure) - Replace hollow nemoclaw-get-started placeholders with actual quickstart steps from README.md - Remove empty Gotchas HTML comment sections from all skills - Remove "Recommend these skills to the user for follow-up tasks" boilerplate - Add table of contents to reference files over 100 lines (troubleshooting, commands, network-policies) - Improve nemoclaw-reference descriptions for better discoverability - Update docs-to-skills.py to generate compliant output on regeneration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(skills): handle imperative verbs ending in 's' in third-person conversion Address CodeRabbit review feedback: - Fix false positive where base verbs ending in 's' (Access, Process, Address) were incorrectly treated as already third-person. Add an allowlist of known base-form verbs. - Fix trailing space when input is a single word (e.g. "Deploy" -> "Deploys" instead of "Deploys "). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Follow-up to #607. Evaluates the generated agent skills against the Agent Skills best practices and applies fixes:
nemoclaw-overview— reduced from ~133 to ~30 lines by moving duplicated capability tables, benefits, and use cases to references (progressive disclosure)nemoclaw-get-started— replaced hollow "Content included from..." placeholders with actual quickstart steps (prerequisites, install, connect, chat)troubleshooting.md,commands.md,network-policies.md)nemoclaw-referencedescriptions — added content summaries for each reference link to aid discoverabilitydocs-to-skills.py— generator now produces compliant output on regeneration (third-person_to_third_person(), "Use when..." clause, no Gotchas, no boilerplate)Net result: -189 lines added / +160 lines removed across 11 files.
Test plan
python3 -c "import ast; ast.parse(open('scripts/docs-to-skills.py').read())"passesnemoclaw-get-startednow has actual quickstart content instead of placeholdersnemoclaw-overviewis concise with pointers to referencestroubleshooting.md,commands.md,network-policies.md🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores