docs: require eval coverage for skill changes - #127
Conversation
Signed-off-by: Polly Labs <pollychen.lab@gmail.com>
|
Thank you for your interest in contributing, @pollychen-lab. This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer. To get vouched:
See CONTRIBUTING.md for details. |
Functional tests did not runFunctional tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the |
PR Summary by QodoDocs: require eval coverage before skill behavior changes
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
55 rules 1. eval/<agent-or-skill-name> path mismatch
|
| verify that the affected skill has functional eval coverage. For local skill | ||
| definitions under `skills/<skill-name>/SKILL.md`, check for the corresponding | ||
| eval config under `eval/<agent-or-skill-name>/eval.yaml`, using the repo's | ||
| established mapping if the skill name and eval directory differ. |
There was a problem hiding this comment.
1. eval/ path mismatch 📎 Requirement gap ⚙ Maintainability
AGENTS.md documents eval coverage using eval/<agent-or-skill-name>/eval.yaml and references an “established mapping” when names differ, but it does not specify the required canonical path eval/<skill-name>/eval.yaml, the current coverage state, or how to determine/apply the mapping. This ambiguity can lead to inconsistent triage and readiness-to-code gating for skill behavior changes, contrary to the documented prerequisite requirement.
Agent Prompt
## Issue description
Update `AGENTS.md` so the eval-coverage prerequisite for skill behavior changes is unambiguous and compliant: explicitly require eval coverage at the canonical path `eval/<skill-name>/eval.yaml` for behavior changes under `skills/<skill-name>/SKILL.md`, clearly state the current eval coverage state, and remove/replace the vague “established mapping” guidance by documenting exactly where the mapping is defined and how to apply it (including clarifying the relationship to agent-name-based tooling like `fullsend run <agent>`).
## Issue Context
PR Compliance ID 1540707 requires AGENTS.md to (a) specify the expected eval path `eval/{skill-name}/eval.yaml`, (b) state that missing evals should block readiness-to-code, and (c) note the current coverage state so triage is consistent. The current text instead uses `eval/<agent-or-skill-name>/eval.yaml` and refers to an “established mapping” without explaining how to determine it, even though eval execution is driven by the agent name argument to `fullsend run` (which resolves to `eval/<agent>/eval.yaml`) and agents can include skills whose directory names don’t match the agent name (e.g., `review` including `skills/pr-review`), making the mapping non-obvious.
## Fix Focus Areas
- AGENTS.md[91-105]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| ## 8. Eval coverage for skill changes | ||
|
|
||
| Before marking an issue `ready-to-code` for changes to skill behavior, | ||
| verify that the affected skill has functional eval coverage. For local skill | ||
| definitions under `skills/<skill-name>/SKILL.md`, check for the corresponding | ||
| eval config under `eval/<agent-or-skill-name>/eval.yaml`, using the repo's | ||
| established mapping if the skill name and eval directory differ. | ||
|
|
||
| If no matching eval coverage exists on the base branch, treat eval creation | ||
| as a prerequisite and do not promote the issue to implementation. Ask for an | ||
| eval task first, or split the work so coverage lands before behavior changes. | ||
|
|
||
| If coverage exists, require the implementation plan to update or add eval | ||
| cases for the changed behavior, or explicitly explain why the existing cases | ||
| already cover the change. |
There was a problem hiding this comment.
Summary
Closes #116.
Validation
Note: make script-test currently fails in scripts/post-triage-test.sh at prerequisites-creates-allowed-issue; that failure is unrelated to this AGENTS.md-only docs change.