Add eval coverage for dotnet-test/test-tagging - #830
Merged
Conversation
Enrich the NUnit misleading-names scenario with rubric items and an output_matches assertion covering the [Category] code pattern, reading the test body before classifying, and classifying each test method. Add a Go standard-testing scenario (report-only) with a fixture to cover leaving source files unmodified for report-only frameworks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Skill Coverage Report
Uncovered:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands the dotnet-test/test-tagging evaluation suite to improve SKILL.md teaching-point coverage by adding a new report-only scenario (Go standard testing) and strengthening the existing NUnit “misleading names” scenario so the eval better exercises the skill’s guidance (especially report-only behavior and reading test bodies before classifying).
Changes:
- Enhanced Scenario 7 (NUnit misleading names) with an additional deterministic output assertion and rubric items emphasizing body-based classification and NUnit
[Category]. - Added Scenario 9 for Go standard
testingas a report-only framework, with a small Go fixture and assertions emphasizing reporting without source edits. - Introduced a new Go fixture module under
fixtures/go-report-only/.
Show a summary per file
| File | Description |
|---|---|
| tests/dotnet-test/test-tagging/eval.yaml | Adds a new Go report-only scenario and strengthens NUnit scenario assertions/rubric to better cover SKILL.md teaching points. |
| tests/dotnet-test/test-tagging/fixtures/go-report-only/calculator/go.mod | New Go module fixture used by the report-only evaluation scenario. |
| tests/dotnet-test/test-tagging/fixtures/go-report-only/calculator/calculator.go | Simple Go source fixture providing code under test for trait classification. |
| tests/dotnet-test/test-tagging/fixtures/go-report-only/calculator/calculator_test.go | Go testing-based test suite fixture to drive report-only trait classification. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 0
Member
Author
|
/evaluate |
Contributor
Skill Validation Results
[1] Model: claude-opus-4.6 | Judge: claude-opus-4.6 🔍 Full Results - additional metrics and failure investigation steps
|
Contributor
|
✅ Evaluation passed for |
Evangelink
enabled auto-merge (squash)
June 25, 2026 10:38
YuliiaKovalova
approved these changes
Jun 25, 2026
elvisw
pushed a commit
to elvisw/dotnet-skills
that referenced
this pull request
Jun 29, 2026
Enrich the NUnit misleading-names scenario with rubric items and an output_matches assertion covering the [Category] code pattern, reading the test body before classifying, and classifying each test method. Add a Go standard-testing scenario (report-only) with a fixture to cover leaving source files unmodified for report-only frameworks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Evangelink
added a commit
that referenced
this pull request
Aug 3, 2026
* Capture the repo's hard-won skill-authoring lessons as guidance Mines ~35 merged PRs (#830-#976), issue #899 and eng/eval-quality/README.md into reusable guidance so contributors do not have to rediscover it. - Add .agents/skills/improve-skill-quality: a triage playbook for evals that regress, return no verdict, or fail to activate. Classify before rewriting - broken fixtures, underpowered trial counts, forced tools, stale spec keys and harness errors have all masqueraded as skill regressions. Two reference docs carry the content patterns and the symptom/cause/fix catalogue with citations. - Rewrite .agents/skills/create-skill-test. It documented a pre-Vally schema (scenarios:/assertions:/setup.copy_test_files) that no longer loads: all 97 eval specs use stimuli:/graders:/environment. Anyone following it authored a broken eval. Also folds in the trial floor, dormancy-guard and fixture rules. - Extend .agents/skills/create-skill with the description-as-router rules and a "write for delta over the baseline model" section. - CONTRIBUTING.md: add the quality playbook, document eval sizing, and correct the eval snippet and the stale "raise runs in dotnet-skills.experiment.yaml" advice - an experiment-level runs overwrites every eval's own value. - .gitignore: .agents/skills/ was silently swallowing the repo's own authoring skills. Replaced with .agents/skills/* plus negations for the five owned directories, so installed third-party skills stay ignored. Validated: markdownlint clean, check_eval_quality.py clean, all relative links resolve, frontmatter parses, descriptions under 1024 chars, bodies under 500 lines. Claims fact-checked against the repo by a review pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 771c704d-9ac9-4d5f-8ccb-54c62066630f * Address multi-model review: correct the sign-test arithmetic and eval exceptions Three review rounds (Copilot, Claude Opus 4.8, GPT-5.6 Sol/Terra) against the implementation in eng/vally-adapter/adapt.mjs. Statistics. The docs conflated the gate's two independent bars. adapt.mjs gates on (1) counted trials >= 5, else `underpowered`, and (2) p <= 0.05 on a sign test over the *discordant* (non-tie) trials. So "at 5-7 trials one tie makes a pass unreachable" was wrong: at 6 trials 5W/1T/0L is 5 discordant and passes at p = 0.031, and at 7 trials two ties are survivable. Every trial table is now keyed on discordant trials. eng/eval-quality/README.md carried the same over-generalisation and is corrected too, since these docs restate it. Eval coverage. "Every skill needs an eval" and "no direct eval for a disable-model-invocation skill" contradicted each other. Reference skills are now explicitly exempt and covered through their consumers, and filter-syntax is no longer described as the sole answer-graded exception - platform-detection uses the same pattern. Fixtures. "Every buildable fixture must build" forbade the intentionally broken fixtures the same document demonstrates. Now: healthy fixtures build, deliberately broken ones fail only for the reason their stimulus is about. Triage routing. The first-match classifier sent fixture failures to the harness row before the fixture row could match, and treated any positive-but-failing record as a power problem even though adapt.mjs requires `conclusive` first. Added an inconclusive row, moved the fixture row up, and scoped "quote a losing trial" to content fixes so it no longer blocks the no-results path. Also: expect_tools does not assert activation (the harness reports it separately); CONTRIBUTING claimed evaluations run automatically on plugin changes when a maintainer must trigger /evaluate against a bound commit; the minimal eval example is labelled as a sub-floor skeleton; create-skill scaffolds into plugins/<plugin>/skills/ rather than a non-existent top-level skills/. Validated: markdownlint clean, check_eval_quality.py clean, adapt.test.mjs 21 pass, links and anchors resolve. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 771c704d-9ac9-4d5f-8ccb-54c62066630f * Address review: document that agent evals sit outside the verdict flow Abhitej's review found one actionable gap plus three nits; all four are small enough to land here rather than as follow-ups. Agent evals. create-skill-test told authors to write tests/<plugin>/agent.<name>/ eval.yaml, size it for the trial floor, and validate with run-skill-evals.sh -- none of which applies. The canonical experiment declares `evals: tests/*/!(agent.*)/eval.yaml`, so agent specs are excluded: no verdict is computed, the floor does not apply, and the runner drops them even when named explicitly because --eval-filter is intersected with that glob. Step 1 now says so, Step 10 shows the EXPERIMENT_FILE route for actually exercising one, and the checklist and pitfalls are scoped accordingly. Nits: aligned eng/eval-quality/README.md check 8 on "stimuli" (it still said "scenarios" while every authoring doc says stimuli); added a .gitignore comment so the next repo-owned authoring skill gets a negation instead of being silently untracked; and noted in improve-skill-quality that eng/skill-validator/src/docs/InvestigatingResults.md documents the retired skill-validator evaluate schema, so the vally-adapter one is the guide to read. Validated: markdownlint clean, check_eval_quality.py clean, 22/22 eval-quality self-tests, gitignore negations still exact. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 771c704d-9ac9-4d5f-8ccb-54c62066630f * Generalize two triage rows that were written in test-skill vocabulary The triage catalogue is meant to apply to every plugin, but two rows stated their rule in dotnet-test terms, so an author on another plugin would read past them: - the split-brain fixture row was framed entirely around Cobertura `line-rate`; the rule is that a fixture must never state the same fact twice in two places that disagree, whatever the format. Cobertura is now the named example. - the volume-comparison row said "both arms write tests" / "do not reward test count"; the rule is that when both arms produce the same kind of artifact the judge falls back on comparing quantity. Also generalized the one-line summary of the first rule in SKILL.md Step 4 so the detail and the summary do not disagree. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 771c704d-9ac9-4d5f-8ccb-54c62066630f * Correct the environment.skills guidance and a stale cross-reference Two suppressed findings from the Copilot review summaries, both real. environment.skills. The Step 5 skeleton documented it as "extra skills loaded in the isolated arm only", which is wrong in both halves. The experiment declares `vary: /environment/skills` and supplies the value for each arm itself (`[]` for baseline, `plugins/<plugin>/skills/<skill>` for skilled), so whatever a skill eval declares is replaced in every arm -- it cannot add a skill to one arm, and in a skill eval it does nothing at all. The key is meaningful only in `agent.*` evals, which the experiment does not vary. Removed it from the generic skeleton, documented where it does apply, and pointed at an existing agent eval to copy rather than reproducing a spelling: the three agent specs in this repo disagree about whether entries are bare names or relative paths, so the doc should not assert one. Stale cross-reference. Moving the fixture row to the top of the triage table last round left the sentence below it still saying a fixture failure "also matches the two rows above it", when it is now the first row. Reworded to say why it leads. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 771c704d-9ac9-4d5f-8ccb-54c62066630f --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 771c704d-9ac9-4d5f-8ccb-54c62066630f
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
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.
Summary
Extends
tests/dotnet-test/test-tagging/eval.yamlto raise SKILL.md teaching-point coverage from 85.7% → 96.4% (24/28 → 27/28 points), measured byeng/skill-coverage/Measure-SkillCoverage.ps1.Now-covered target points
report-onlyframeworks, no source files were modified" (SKILL.md ~L243) — added a new Go standard-testingscenario (a report-only framework) with a fixture, plus an outcome-focused rubric item about leaving source files unmodified and producing a report only. Also asserts the test file is not given//go:buildtags (file_not_contains).[Category](~L135) — added a deterministicoutput_matches: "\[Category\b"assertion in the NUnit scenario (whose output genuinely emits the NUnit[Category]attribute), plus a rubric item referencing the[Category]attribute.Changes
tests/dotnet-test/test-tagging/eval.yaml— enriched Scenario 7 (NUnit misleading names); added Scenario 9 (Go report-only).tests/dotnet-test/test-tagging/fixtures/go-report-only/calculator/— new Go module fixture (go.mod,calculator.go,calculator_test.go).Verification
Measure-SkillCoverage.ps1 -PluginName dotnet-test -SkillName test-tagging: 27/28 (96.4%), no regressions.SkillValidator check --plugin ./plugins/dotnet-test: ✅ all checks passed (only pre-existing token-count warnings).Known blocker — Step 3 remains uncovered (measurement-tool bug, not eval.yaml)
The 4th target, [WorkflowStep] "Step 3: Classify each test method" (~L92), cannot be covered by editing
eval.yaml. Root cause is a bug ineng/skill-coverage/Measure-SkillCoverage.ps1:Get-SignificantTermsreturns[string[]]$terms.Keys, where$termsis a hashtable of extracted keywords. SKILL.md line 108 (inside the Step 3 body) contains the word "keys" ("...tests missing keys..."), so$termsgains an entry namedkeys. PowerShell member access$terms.Keysthen returns that entry's value ($true) instead of the key collection — so Step 3's keyword set collapses to the single tokenTrue. The matcher requires ≥2 keyword hits (or 1 distinctive code term), so no eval evidence can ever match this point.Suggested one-line fix (separate PR, broader impact across all skills): use
$terms.get_Keys()(or([System.Collections.IDictionary]$terms).Keys) instead of$terms.Keys. This generally affects any step whose body contains a hashtable member name (keys,values,count,item, ...). The eval.yaml in this PR already includes outcome-focused rubric items for classifying each test method, so Step 3 will be credited automatically once the tool bug is fixed.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com