mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
Amaury Levé
38bf797c27
* 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