Fail closed on missing or duplicate comparison slots, reject duplicate stimulus names before execution, and clarify how the practical floor changes power at its boundary.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1c6716a2-172b-461a-adb5-127b1ba7e96b
* 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
* Remove agentic workflows
The agentic workflows were useful when CCA didn't support loading skills. That's now solved and supported. The agentic workflows come with a couple of issues (limited chars) which make it hard to use. Remove them in favor of the plugin system.
* Remove Agentic Workflows section from README
Removed section about Agentic Workflows from README.
* Remove stale /agentic-workflows references from CONTRIBUTING.md, CODEOWNERS, and docs
Agent-Logs-Url: https://github.com/dotnet/skills/sessions/8f129e2f-156e-4ee9-9897-896cd1981513
Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
* Move skill-validator docs under src/ and include in packages
Move README.md to src/ and InvestigatingResults.md + OverfittingDetection.md
to src/docs/ so they ship in the NuGet package and .tar.gz archives.
- Include docs/*.md in NuGet pack and publish output via CopyToPublishDirectory
- Add LICENSE to publish output (tar.gz only, not NuGet — uses PackageLicenseExpression)
- Replace enumerated docOnlyFiles lists with a regex pattern in CI workflows
- Deduplicate eval.yaml docs between CONTRIBUTING.md and the shipping README
- Add test fixture files walkthrough to README (moved from CONTRIBUTING.md)
- Add AGENTS.md guidance: src/ is the packaging boundary
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Update link format in failure diagnosis message
* Update AGENTS.md
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Viktor Hofer <vihofer@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Refactor skill-validator: add check command, rename validate to evaluate
- Add new 'check' command for CI skill validation (frontmatter, structure)
- Rename ValidateCommand to EvaluateCommand to clarify its purpose
- Add skill-check.yml GitHub Actions workflow
- Refactor SkillDiscovery for improved skill detection
- Simplify OverfittingJudge and SkillProfiler
- Update tests to match refactored APIs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix check subcommand: use EvaluateCommand as root command
System.CommandLine 2.0.4 requires the primary command to be the
RootCommand. Restores the upstream pattern where the evaluate/validate
command is the root and check/consolidate/rejudge are subcommands.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Pass all plugins in single check invocation
Build plugin args dynamically and pass them all at once instead of
looping per plugin.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address PR review comments
- Remove 'evaluate' subcommand token from README, evaluation-run.yml,
and RunArguments in csproj (evaluate is now the root command)
- Wrap ParsePluginJson in try/catch for JsonException with clean error
- Add CheckCommandTests with aggregate description limit coverage
(under/at/over limit and multi-plugin cases)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Abhitej John