docs: require skill evals before publishing - #318
Conversation
|
|
Important Review skippedToo many files! This PR contains 53 files, which is 3 over the limit of 50. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (53)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughSkill evaluation guidance was added and incorporated into contribution instructions. New skills now require documented happy-path, negative near-miss, and edge-case prompts, with testing focused on expected triggering and non-triggering behavior. ChangesSkill evaluation workflow
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a new documentation file docs/skill-evals.md detailing the process and template for writing skill evaluations, and updates CONTRIBUTING.md to integrate this new evaluation step into the skill creation workflow. Feedback on the pull request points out an inconsistency in the required number of evaluation prompts between CONTRIBUTING.md (which mentions 3-5 prompts) and docs/skill-evals.md (which defines a minimum of 5-6 prompts), suggesting an update to maintain consistency.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| 2. **Create SKILL.md file**: Add `SKILL.md` with skill frontmatter and content | ||
| 3. **Update skill-rules.json**: Add triggers and keywords for your new skill | ||
| 4. **Document**: Add skill to `skills/README.md` if it exists | ||
| 3. **Write evals**: Add 3-5 prompts that prove the skill works, including at least one negative case. See [`docs/skill-evals.md`](docs/skill-evals.md). |
There was a problem hiding this comment.
The prompt count mentioned here ("3-5 prompts") is inconsistent with the minimal eval set defined in docs/skill-evals.md, which requires at least 5 prompts (3 happy-path, 1-2 near-miss, and 1 edge-case). Update this to "5-6 prompts" to maintain consistency across the documentation.
| 3. **Write evals**: Add 3-5 prompts that prove the skill works, including at least one negative case. See [`docs/skill-evals.md`](docs/skill-evals.md). | |
| 3. **Write evals**: Add 5-6 prompts that prove the skill works, including at least one negative case. See [`docs/skill-evals.md`](docs/skill-evals.md). |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@CONTRIBUTING.md`:
- Around line 53-54: Update CONTRIBUTING.md lines 53-54 to require 3 happy-path,
1-2 near-miss, and 1 edge-case prompts, consistent with docs/skill-evals.md.
Also update CONTRIBUTING.md line 131 to require contributors to run and record
results for edge-case prompts alongside happy-path and near-miss prompts.
In `@docs/skill-evals.md`:
- Around line 42-62: Update every evaluation entry under “Happy path prompts,”
“Near-miss prompts,” and “Edge cases” to include an explicit “Result/Notes”
field alongside Prompt and Expected, preserving the existing template structure
and guidance for recording manual pass/fail observations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 271222b7-5a32-49af-b3a2-21c475cba158
📒 Files selected for processing (2)
CONTRIBUTING.mddocs/skill-evals.md
| 3. **Write evals**: Add 3-5 prompts that prove the skill works, including at least one negative case. See [`docs/skill-evals.md`](docs/skill-evals.md). | ||
| 4. **Update skill-rules.json**: Add triggers and keywords for your new skill |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Keep the contribution workflow consistent with the documented eval standard.
The workflow currently allows incomplete eval coverage and does not require running edge-case prompts.
CONTRIBUTING.md#L53-L54: require 3 happy-path, 1-2 near-miss, and 1 edge-case prompts, matchingdocs/skill-evals.md.CONTRIBUTING.md#L131-L131: require contributors to run and record results for edge-case prompts in addition to happy-path and near-miss prompts.
📍 Affects 1 file
CONTRIBUTING.md#L53-L54(this comment)CONTRIBUTING.md#L131-L131
🤖 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 `@CONTRIBUTING.md` around lines 53 - 54, Update CONTRIBUTING.md lines 53-54 to
require 3 happy-path, 1-2 near-miss, and 1 edge-case prompts, consistent with
docs/skill-evals.md. Also update CONTRIBUTING.md line 131 to require
contributors to run and record results for edge-case prompts alongside
happy-path and near-miss prompts.
| ## Happy path prompts | ||
|
|
||
| 1. Prompt: ... | ||
| Expected: ... | ||
|
|
||
| 2. Prompt: ... | ||
| Expected: ... | ||
|
|
||
| 3. Prompt: ... | ||
| Expected: ... | ||
|
|
||
| ## Near-miss prompts | ||
|
|
||
| 1. Prompt: ... | ||
| Expected: skill should not trigger / should ask for clarification / should route elsewhere | ||
|
|
||
| ## Edge cases | ||
|
|
||
| 1. Prompt: ... | ||
| Expected: ... | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a result/notes field to every template eval.
The guide requires recording manual pass/fail notes, but the template only captures prompts and expected behavior. Add an explicit Result/Notes field to the happy-path, near-miss, and edge-case entries so eval records are auditable.
Suggested template addition
1. Prompt: ...
Expected: ...
+ Result/Notes: Pass/Fail — ...🤖 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 `@docs/skill-evals.md` around lines 42 - 62, Update every evaluation entry
under “Happy path prompts,” “Near-miss prompts,” and “Edge cases” to include an
explicit “Result/Notes” field alongside Prompt and Expected, preserving the
existing template structure and guidance for recording manual pass/fail
observations.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@docs/skill-evals.md`:
- Around line 106-114: Add a checklist item in the “Review checklist” section
requiring at least one prompt that exercises the skill’s documented edge case,
boundary condition, or failure mode, alongside the existing negative-case
requirement.
- Around line 51-56: Update the recommended test-case schema in the
skill-evaluation guide to include a required should_trigger field indicating
whether the skill is expected to trigger. Keep the existing prompt,
expected_output, files, and assertions fields, and clarify that should_trigger
supports unambiguous near-miss cases in evals.json.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 739f7fbe-a3f7-4f4c-adbd-3a095a607dac
📒 Files selected for processing (1)
docs/skill-evals.md
| A test case should usually include: | ||
|
|
||
| - `prompt`: realistic user input | ||
| - `expected_output`: human-readable success criteria | ||
| - `files` (optional): input files the skill needs | ||
| - `assertions` (optional at first): concrete checks added after the first run |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Include the trigger expectation in the eval schema.
The guide requires recording whether each skill should trigger, but the recommended test-case structure has no should_trigger field. Without it, near-miss evaluations cannot be represented unambiguously in evals.json.
Suggested documentation update
- `prompt`: realistic user input
+ - `should_trigger`: whether the skill is expected to trigger
- `expected_output`: human-readable success criteria📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| A test case should usually include: | |
| - `prompt`: realistic user input | |
| - `expected_output`: human-readable success criteria | |
| - `files` (optional): input files the skill needs | |
| - `assertions` (optional at first): concrete checks added after the first run | |
| A test case should usually include: | |
| - `prompt`: realistic user input | |
| - `should_trigger`: whether the skill is expected to trigger | |
| - `expected_output`: human-readable success criteria | |
| - `files` (optional): input files the skill needs | |
| - `assertions` (optional at first): concrete checks added after the first run |
🤖 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 `@docs/skill-evals.md` around lines 51 - 56, Update the recommended test-case
schema in the skill-evaluation guide to include a required should_trigger field
indicating whether the skill is expected to trigger. Keep the existing prompt,
expected_output, files, and assertions fields, and clarify that should_trigger
supports unambiguous near-miss cases in evals.json.
| ## Review checklist | ||
|
|
||
| - [ ] The skill has eval prompts, not just a description | ||
| - [ ] At least one negative case is included | ||
| - [ ] The prompts are realistic and varied | ||
| - [ ] Expected outputs are specific enough to review manually | ||
| - [ ] The evals were run before the skill was published | ||
| - [ ] Any failures led to an update in the skill or its trigger rules | ||
| - [ ] The skill was compared against a baseline or previous version |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add an explicit edge-case requirement to the checklist.
The minimum eval set requires an edge-case prompt at Line 22, but this checklist only verifies negative coverage. A skill could pass the checklist without testing its documented boundary or failure mode.
Suggested checklist addition
- [ ] At least one negative case is included
+ - [ ] At least one edge-case prompt is included📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Review checklist | |
| - [ ] The skill has eval prompts, not just a description | |
| - [ ] At least one negative case is included | |
| - [ ] The prompts are realistic and varied | |
| - [ ] Expected outputs are specific enough to review manually | |
| - [ ] The evals were run before the skill was published | |
| - [ ] Any failures led to an update in the skill or its trigger rules | |
| - [ ] The skill was compared against a baseline or previous version | |
| ## Review checklist | |
| - [ ] The skill has eval prompts, not just a description | |
| - [ ] At least one negative case is included | |
| - [ ] At least one edge-case prompt is included | |
| - [ ] The prompts are realistic and varied | |
| - [ ] Expected outputs are specific enough to review manually | |
| - [ ] The evals were run before the skill was published | |
| - [ ] Any failures led to an update in the skill or its trigger rules | |
| - [ ] The skill was compared against a baseline or previous version |
🤖 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 `@docs/skill-evals.md` around lines 106 - 114, Add a checklist item in the
“Review checklist” section requiring at least one prompt that exercises the
skill’s documented edge case, boundary condition, or failure mode, alongside the
existing negative-case requirement.
What
docs/skill-evals.mdguide for defining skill evals before publishing.CONTRIBUTING.mdto require eval prompts and a negative case for each new skill.Why
How
Summary by CodeRabbit