Add promptfoo eval infrastructure for skills - #28
Open
sachiniSam wants to merge 2 commits into
Open
Conversation
Introduce the "how to evaluate skills" tooling and docs so contributors can add a promptfoo eval suite for any skill: - EVALS.md: contributor guide (layout, prerequisites, scaffold/sync/run) - tools/scaffold-eval.js: generate a starter suite for a skill - tools/sync-fixtures.js: refresh fixture copies from the live skill - AGENTS.md / CLAUDE.md / DEVELOPMENT.md: wire evals into the skill-authoring and directory-layout docs The api-design reference suite is intentionally left out of this commit; it will land separately once its tests are finalized. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughSummary
WalkthroughAdds repository guidance for Promptfoo-based skill evaluations, including suite structure, test expectations, execution, tracing, and fixture maintenance. Adds a scaffolding CLI that generates provider-specific eval suites with starter tests and configuration. Adds a synchronization CLI that refreshes fixture copies for individual skills or all eligible skills. Updates contributor documentation to reference the new evaluation workflow. Sequence Diagram(s)sequenceDiagram
participant Contributor
participant ScaffoldEval
participant SkillWorkspace
participant Promptfoo
Contributor->>ScaffoldEval: Provide plugin, skill, providers, and grader
ScaffoldEval->>SkillWorkspace: Validate skill and create fixture copies
ScaffoldEval-->>Contributor: Generate eval configuration and starter tests
Contributor->>Promptfoo: Run the skill evaluation suite
Promptfoo->>SkillWorkspace: Discover provider-specific fixtures
Promptfoo-->>Contributor: Return triggering and task-quality results
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
EVALS.md (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpecify a language for the fenced code block.
Specifying a language resolves the linter warning.
♻️ Proposed fix
-``` +```text🤖 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 `@EVALS.md` at line 13, Specify the appropriate language identifier on the fenced code block in EVALS.md, using text if the block contains plain text, to resolve the linter warning.Source: Linters/SAST tools
DEVELOPMENT.md (1)
110-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpecify a language for the fenced code block.
Specifying a language resolves the linter warning and improves syntax highlighting.
♻️ Proposed fix
-``` +```bash🤖 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 `@DEVELOPMENT.md` at line 110, Specify the Bash language on the fenced code block in DEVELOPMENT.md by changing the opening fence to use the bash language identifier, while preserving the block’s existing contents and closing fence.Source: Linters/SAST tools
tools/sync-fixtures.js (1)
29-31: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPrevent early termination during batch processing.
When
syncSkillis called via the--allflag, a missingSKILL.mdtriggersfail(), which terminates the script and prevents any remaining valid skills from syncing. Logging an error and returning0allows the batch process to continue gracefully.♻️ Proposed fix
if (!fs.existsSync(path.join(skillDir, 'SKILL.md'))) { - fail(`no SKILL.md at plugins/${plugin}/skills/${skill}`); + console.error(` (error) no SKILL.md at plugins/${plugin}/skills/${skill}`); + return 0; }🤖 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 `@tools/sync-fixtures.js` around lines 29 - 31, Update syncSkill’s missing-SKILL.md branch to log an error instead of calling fail(), then return 0 so --all batch processing continues with remaining skills while preserving the existing successful sync behavior.
🤖 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 `@tools/scaffold-eval.js`:
- Around line 102-104: Update the positional-argument validation near plugin and
skill so both values are restricted to safe directory-name components before any
downstream path construction. Reject values containing path separators,
traversal segments, or other invalid path characters, and call fail(USAGE)
consistently with the existing missing-argument handling.
In `@tools/sync-fixtures.js`:
- Around line 84-87: Update the CLI argument validation around `plugin` and
`skill` to reject either input when it contains path separators, while
preserving the existing missing-argument usage failure and `--all` handling.
Keep subsequent file operations limited to validated directory-name inputs.
---
Nitpick comments:
In `@DEVELOPMENT.md`:
- Line 110: Specify the Bash language on the fenced code block in DEVELOPMENT.md
by changing the opening fence to use the bash language identifier, while
preserving the block’s existing contents and closing fence.
In `@EVALS.md`:
- Line 13: Specify the appropriate language identifier on the fenced code block
in EVALS.md, using text if the block contains plain text, to resolve the linter
warning.
In `@tools/sync-fixtures.js`:
- Around line 29-31: Update syncSkill’s missing-SKILL.md branch to log an error
instead of calling fail(), then return 0 so --all batch processing continues
with remaining skills while preserving the existing successful sync behavior.
🪄 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: 6a653972-12dc-43c1-aa16-124bca4a309b
📒 Files selected for processing (6)
AGENTS.mdCLAUDE.mdDEVELOPMENT.mdEVALS.mdtools/scaffold-eval.jstools/sync-fixtures.js
- scaffold-eval.js / sync-fixtures.js: validate <plugin>/<skill> as plain directory names before using them as path segments - sync-fixtures.js: skip (warn) instead of fail() on a missing SKILL.md so a single bad entry no longer aborts a --all run - EVALS.md / DEVELOPMENT.md: add languages to fenced code blocks Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
DEVELOPMENT.md (1)
20-20: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winQualify the “each skill” eval-suite claim.
The PR objective states that the
api-designreference suite is intentionally excluded, so saying every skill carries anevals/suite is currently inaccurate. Qualify this as applying to eligible/new skills or document the exception explicitly.Proposed wording
- evals/ — promptfoo eval suite for the skill (see EVALS.md) + evals/ — promptfoo eval suite for eligible skills (see EVALS.md) -Each skill carries a promptfoo eval suite under `evals/` that runs the skill the +Eligible skills carry a promptfoo eval suite under `evals/` that runs the skill theAs per path instructions, this is a concise documentation correctness fix.
Also applies to: 106-108
🤖 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 `@DEVELOPMENT.md` at line 20, Update the DEVELOPMENT.md description of the evals/ directory to qualify the claim that each skill has an eval suite, limiting it to eligible or new skills and explicitly acknowledging the excluded api-design reference suite. Apply the same wording correction to the corresponding repeated references.Source: Path instructions
🤖 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.
Outside diff comments:
In `@DEVELOPMENT.md`:
- Line 20: Update the DEVELOPMENT.md description of the evals/ directory to
qualify the claim that each skill has an eval suite, limiting it to eligible or
new skills and explicitly acknowledging the excluded api-design reference suite.
Apply the same wording correction to the corresponding repeated references.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e67cbf78-8736-4506-a763-593ce0333cac
📒 Files selected for processing (4)
DEVELOPMENT.mdEVALS.mdtools/scaffold-eval.jstools/sync-fixtures.js
🚧 Files skipped from review as they are similar to previous changes (3)
- tools/sync-fixtures.js
- tools/scaffold-eval.js
- EVALS.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Introduces the "how to evaluate skills" tooling and docs so contributors can add a promptfoo eval suite for any skill.
EVALS.md— contributor guide: suite layout, prerequisites, and the scaffold → sync → run workflow.tools/scaffold-eval.js— generates a starter eval suite (config, triggering + task-quality tests, fixtures) for a given<plugin> <skill>.tools/sync-fixtures.js— refreshes the fixture workspace copies from the live skill.AGENTS.md/CLAUDE.md/DEVELOPMENT.md— wire evals into the skill-authoring flow and directory-layout docs.Scope
This PR is the generic infrastructure only. The
api-designreference suite is intentionally not included — it lands separately once its tests are finalized.How it works
An eval runs a skill the way it runs in production: the agent SDK discovers the skill from a fixture workspace and invokes it, then assertions check both triggering (does it activate on the right prompts / stay out of the way on the wrong ones) and task quality (is the output correct). Promptfoo is a dev/test tool here — configs and fixtures are shipped, contributors run promptfoo locally.
🤖 Generated with Claude Code