diff --git a/.agents/skills/create-skill-test/SKILL.md b/.agents/skills/create-skill-test/SKILL.md index b64215a280..20e27150b6 100644 --- a/.agents/skills/create-skill-test/SKILL.md +++ b/.agents/skills/create-skill-test/SKILL.md @@ -1,414 +1,346 @@ --- name: create-skill-test -description: Scaffolds eval.yaml test files for agent skills in the dotnet/skills repository. Use when creating skill tests, writing evaluation scenarios, defining assertions and rubrics, or setting up test fixture files. Handles eval.yaml generation, fixture organization, and overfitting avoidance. Do not use for running or debugging existing tests nor for skills authoring. +description: Scaffolds eval.yaml evaluation specs for agent skills in the dotnet/skills repository. Use when creating skill tests, writing evaluation stimuli, defining graders and rubrics, sizing an eval for statistical power, or setting up test fixture files. Handles the Vally eval.yaml schema, fixture organization, and overfitting avoidance. Do not use for running or debugging existing evals (use improve-skill-quality) nor for skills authoring (use create-skill). --- # Create Skill Test -This skill helps you scaffold evaluation tests (`eval.yaml`) for agent skills, ensuring they conform to the dotnet/skills repository conventions, pass the skill-validator checks, and avoid common overfitting pitfalls. +Scaffold an evaluation spec (`eval.yaml`) for a skill or agent so it conforms to the Vally schema, +passes `skill-validator check` and `check_eval_quality.py`, is powerful enough to return a verdict, +and does not overfit to the skill's own wording. ## When to Use -- Creating a new `eval.yaml` test file for a skill -- Adding scenarios to an existing eval file -- Setting up test fixture files alongside eval definitions -- Reviewing whether rubric items and assertions risk overfitting +- Creating a new `eval.yaml` for a skill or agent +- Adding stimuli to an existing eval +- Sizing an eval so the pass gate can actually be reached +- Setting up or repairing fixture files alongside an eval +- Reviewing whether rubric items and graders risk overfitting ## When Not to Use -- Running or debugging existing tests (use the skill-validator directly) -- Modifying the skill-validator tool itself -- Creating or editing SKILL.md files (use the `create-skill` skill) +- Diagnosing a failing or regressed eval — use `improve-skill-quality` +- Modifying the skill-validator or the evaluation workflows +- Creating or editing `SKILL.md` files — use `create-skill` ## Inputs | Input | Required | Description | |-------|----------|-------------| -| Skill name | Yes | The skill being tested (must match a skill under `plugins//skills/`) | -| Plugin name | Yes | The plugin the skill belongs to (e.g., `dotnet-msbuild`) | -| Skill content | Recommended | The SKILL.md content to understand what the skill teaches | -| Scenario descriptions | Recommended | What situations the agent should be tested on | +| Skill or agent name | Yes | Must exist under `plugins//skills/` or `plugins//agents/` | +| Plugin name | Yes | e.g. `dotnet-msbuild` | +| Skill content | Yes | Read it — you cannot write non-overfitted rubric items without it | +| Failure modes to discriminate | Recommended | Each becomes one stimulus | ## Workflow -### Step 1: Locate the target and determine the test directory +### Step 1: Locate the target and the test directory -Tests live at: - -``` -# For skills: -tests///eval.yaml - -# For agents (agent. prefix convention): -tests//agent./eval.yaml +```text +tests///eval.yaml # skills +tests//agent./eval.yaml # agents (the agent. prefix disambiguates) ``` -For skills, verify the skill exists at `plugins//skills//SKILL.md`. For agents, verify the agent exists at `plugins//agents/.agent.md`. Read the target content to understand what it does -- this is critical for writing non-overfitted rubric items. - -### Step 2: Create the test directory and eval.yaml +Verify the target exists at `plugins//skills//SKILL.md` or +`plugins//agents/.agent.md`, and read it. -Create the directory and file: - -``` -# For skills: -tests/// -+-- eval.yaml - -# For agents: -tests//agent./ -+-- eval.yaml -``` +**Agent evals sit outside the verdict flow.** The canonical experiment declares +`evals: tests/*/!(agent.*)/eval.yaml`, so `agent.*` specs are excluded: no verdict is ever computed +for them, the trial floor does not apply, and `./eng/run-skill-evals.sh` drops them even when you +name one explicitly (its `--eval-filter` is intersected with that glob). Everything below about +sizing for statistical power therefore applies to **skill** evals. Author agent evals for the +scenario coverage and the deterministic graders, and run them as described in Step 10. -The `agent.` prefix disambiguates agent test directories from skill test directories that might share the same name. +**Be careful with a skill that sets `disable-model-invocation: true`.** The model cannot invoke it, +so any eval graded on the skill self-activating compares two identical arms and returns judge noise. +The honest coverage for such skills is dependency-level — through the evals of the skills that load +them, and through the plugin arm. Two here take the other route and grade the *answer* rather than +activation: `tests/dotnet-test/filter-syntax/eval.yaml` and +`tests/dotnet-test/platform-detection/eval.yaml`. Whether that produces a measurable gap for a skill +the model cannot invoke is still unconfirmed, so read a real verdict before copying the pattern. -### Step 3: Write scenarios +### Step 2: Write the spec skeleton -Each scenario needs a `name`, `prompt`, at least one `assertion`, and a `rubric`. Use this structure: +The spec is Vally format. Every eval in this repo uses `stimuli:` and `graders:`; `scenarios:` and +`assertions:` are a pre-Vally format that no longer loads. ```yaml -scenarios: - - name: "Descriptive scenario name" - prompt: "Natural language task description as a developer would phrase it" - setup: - copy_test_files: true # OR use inline files - assertions: - - type: "output_contains" - value: "expected text" +name: +description: Evaluates the / skill +type: capability +defaults: + timeout: 5m + runs: 1 +stimuli: + - name: + prompt: + environment: + files: + - src: fixtures//Project.csproj + dest: Project.csproj + graders: + - type: output-matches + config: + pattern: (root cause|underlying issue) + - type: exit-success + - type: prompt rubric: - - "The agent correctly identified the root cause" - - "The agent suggested a concrete, actionable fix" - timeout: 120 + - ``` -#### Scenario guidelines +> **`defaults:` replaces `config:` — it does not join it.** `config` is a deprecated alias for the +> same block and vally **throws** on a spec declaring both. Most existing evals here still open with +> `config:`; when you add `runs`, merge the two into one `defaults:` block carrying `timeout` and +> `runs`. The failure is invisible otherwise: the job exits 0 with no verdicts and the PR comment +> blames "transient infrastructure". -- **Name**: Describe *what* is being tested, not *how* (e.g., "Diagnose missing package reference" not "Test binlog replay and error extraction"). -- **Prompt**: Write as a natural developer request. Never mention the skill name or instruct the agent to "use a skill." Neutral prompts prevent prompt overfitting. -- **Timeout**: Default is 120 seconds. Use 300-600 for scenarios requiring builds, benchmarks, or multi-step operations. +### Step 3: Size the eval for power before writing content -### Step 4: Configure setup +`trials = stimuli × runs`, and the gate has two independent bars: -Choose one of three setup strategies: +1. **Counted trials ≥ 5**, else the verdict is `underpowered` — never a pass, never a regression. +2. **p ≤ 0.05 on an exact one-sided sign test over the *discordant* (non-tie) trials.** Ties are not + discarded; they hold the discordant count down. -#### Option A: Copy test files (recommended for complex fixtures) +| discordant trials | records that pass | p | +|---:|---|---:| +| ≤ 4 | none | ≥ 0.0625 | +| 5–7 | zero losses only (5W/0L) | 0.031 | +| 8 | one loss survivable (7W/1L) | 0.035 | -Place fixture files alongside `eval.yaml` and enable auto-copy: +At exactly 5 counted trials a single tie is fatal — it leaves 4 discordant. At 6 counted trials one +tie is survivable (5W/1T/0L); at 7, up to two are (5W/2T/0L). A loss is not. Five is an +**eligibility floor**, not adequate power — one tie at five trials makes a pass +arithmetically unreachable. A run measuring a 32% tie rate certified a genuinely-helping five-trial +eval roughly one time in ten; at fifteen trials, nine times in ten. -```yaml -setup: - copy_test_files: true -``` +Prefer **more stimuli** over more `runs`: repeats measure the same task. Raise `runs` only when a +stimulus is genuinely expensive to add (full build/test pipelines), and write the reasoning in a +comment above `defaults:`. -All files in the directory (except `eval.yaml`) are copied into the agent's working directory. +Do not set `runs` in `dotnet-skills.experiment.yaml`; experiment overrides overwrite every eval's +own value rather than defaulting it. -#### Option B: Inline files (good for small, self-contained scenarios) +### Step 4: Write stimuli -```yaml -setup: - files: - - path: "MyProject/MyProject.csproj" - content: | - - - net10.0 - - - - path: "MyProject/Program.cs" - content: | - Console.WriteLine("Hello"); -``` +- **Name** describes *what* is tested, not *how*. +- **Prompt** is a natural developer request. Never mention the skill, the agent, or its vocabulary — + cued prompts inflate the overfit score and bias the baseline. +- Each stimulus should discriminate a **different** property of the skill. Five stimuli covering one + property give arithmetic, not evidence. +- Include a boundary / no-op stimulus for any skill that migrates or rewrites code, proving it + leaves already-correct input alone. -#### Option C: Reference fixture files from a subdirectory +### Step 5: Configure the environment ```yaml -setup: +environment: files: - - path: "TestProject.csproj" - source: "fixtures/scenario-a/TestProject.csproj" -``` - -Use this when multiple scenarios share a `fixtures/` directory with separate subdirectories. - -#### Setup commands (optional) - -Run shell commands before the agent starts (e.g., to build a project and generate artifacts): - -```yaml -setup: - copy_test_files: true + - src: fixtures/broken-build/App.csproj # path relative to eval.yaml + dest: App.csproj # path in the agent's working directory + - src: fixtures/broken-build # a directory + dest: . commands: - - "dotnet build -bl:build.binlog" -``` - -#### Scenario dependencies (optional) - -Some agents route to specific skills, or some skills depend on sibling agents. In the **isolated** run, only the target is loaded — so the scenario must declare its dependencies using `additional_required_skills` and/or `additional_required_agents`: - -```yaml -setup: - copy_test_files: true - additional_required_skills: - - binlog-failure-analysis # loaded in isolated run alongside the target - additional_required_agents: - - build-perf # registered in isolated run alongside the target + - dotnet build -bl || exit 0 # guard intentional failures ``` -- Names are resolved from the same plugin's `skills/` or `agents/` directory. -- These only affect the **isolated** run. The **plugin** run already loads everything; the **baseline** loads nothing. -- Different scenarios of the same target can declare different dependencies (per-scenario granularity). -- If a declared name cannot be resolved, the validator fails with an error. - -### Step 5: Write assertions - -Assertions are hard pass/fail checks. Use them for objective, binary-verifiable criteria. - -| Type | Required fields | Description | -|------|----------------|-------------| -| `output_contains` | `value` | Agent output contains text (case-insensitive) | -| `output_not_contains` | `value` | Agent output must NOT contain text | -| `output_matches` | `pattern` | Agent output matches regex | -| `output_not_matches` | `pattern` | Agent output does NOT match regex | -| `file_exists` | `path` | File matching glob exists in work dir | -| `file_not_exists` | `path` | No file matching glob exists | -| `file_contains` | `path`, `value` | File at glob path contains text | -| `file_not_contains` | `path`, `value` | File at glob path does NOT contain text | -| `exit_success` | -- | Agent produced non-empty output | - -#### Assertion guidelines - -- Prefer **broad** assertions that multiple valid approaches would satisfy. -- Avoid **narrow** assertions that gate on a specific syntax or flag the LLM already knows. -- Use `output_matches` with regex alternation for flexible matching: `"(root cause|primary error|underlying issue)"`. -- Use `file_contains` / `file_not_contains` to verify the agent modified files correctly. -- Use `output_not_contains` and `file_not_exists` to verify the agent avoided incorrect actions. - -### Step 6: Write rubric items - -Rubric items are evaluated by an LLM judge using pairwise comparison (baseline vs. skill-enhanced). Quality metrics (rubric-based at 40% weight plus overall judgment at 30%) together dominate the composite improvement score. - -#### The three rubric classifications (and how to stay in "outcome") - -The overfitting judge classifies each rubric item: +**Do not set `environment.skills` in a skill eval.** The experiment declares +`vary: /environment/skills` and supplies the value itself — `[]` for the baseline arm and +`plugins//skills/` for the skilled arm — so anything the eval declares is replaced, +in every arm. It cannot add a skill to one arm only. `environment.skills` is meaningful only in an +`agent.*` eval, which the experiment does not vary; there it is the set of skills the agent may +invoke. Copy the shape from an existing agent eval such as +`tests/dotnet-test/agent.test-quality-auditor/eval.yaml` rather than reproducing a remembered form — +the specs in this repo are not consistent about how they spell those entries. + +Fixture rules — each one has already cost a real result: + +- **Every referenced fixture must be tracked by git.** `.gitignore` (e.g. `coverage*.xml`) has + silently swallowed a committed fixture: the eval passed locally and failed at setup in CI. Verify + with `git ls-files`, not by looking at the working tree. +- **Every fixture must behave as its stimulus assumes.** A fixture meant to be healthy must build; a + fixture meant to be broken must fail for the exact reason the stimulus is about, and no other. + Judges penalize agents for unrelated "pre-existing build issues" that the fixture author + introduced. +- **Every fixture must reproduce the bug its stimulus is named for.** If it does not, the baseline + scores well and the skill has nothing to add. +- **Coverage fixtures must be internally consistent.** A Cobertura report whose declared + `line-rate`, summary totals (`lines-covered`/`lines-valid`), and `` elements disagree lets + the two arms read different truths, and the loss is the fixture's fault. Update any rubric item or + prompt that quotes a figure in the same change. +- **Do not wire duplicate fixtures** to raise `n`; rename leftovers add trials without evidence. +- A setup command that is *expected* to fail while still producing its artifact must be guarded + (`|| exit 0`), or vally drops the trial. +- A cleanup command that strips sources must skip directories containing `SKILL.md` — the staged + skill lives there, and deleting it aborts only the skilled arm. + +### Step 6: Write graders + +Graders are hard pass/fail checks evaluated on every arm. + +| Type | Required config | Purpose | +|------|-----------------|---------| +| `output-matches` / `output-not-matches` | `pattern` | Regex over agent output | +| `output-contains` / `output-not-contains` | `substring` | Literal text in output | +| `file-exists` / `file-not-exists` | `path` | Glob against the work directory | +| `file-contains` / `file-not-contains` | `path`, `value` | Content of a produced file | +| `run-command` | `command` (plus optional `expected_exit_code`, `timeout`, `stdout_matches`) | Verify produced code actually builds/runs | +| `exit-success` | — | Agent produced non-empty output | +| `prompt` | — | Runs the LLM judge against the `rubric` | + +Rules: + +- A grader whose `config` is absent or missing its required key parses fine and **enforces nothing**. + The usual cause is an indentation slip during an edit; `check_eval_quality.py` blocks it. +- Prefer broad patterns that several valid approaches satisfy: + `(root cause|primary error|underlying issue)`. +- **If the skill mandates an output shape, assert on it.** A skill required to emit a decisive + `Recommendation:` line can silently stop doing so while the eval still passes. +- Use `file-not-contains` / `file-not-exists` to prove the agent avoided an incorrect action. + +### Step 7: Write rubric items + +Rubric items are judged pairwise (baseline vs. skilled). The overfitting judge classifies each item: | Classification | Description | Goal | |---------------|-------------|------| -| **outcome** | Tests whether the agent reached a correct result. Describes WHAT, not HOW. | Target this | -| **technique** | Tests whether the agent used a skill-specific procedure. | Minimize | -| **vocabulary** | Tests whether the agent used specific terminology from the skill. | Avoid | - -#### Rubric writing rules - -1. **Test outcomes, not methods.** Write "Identified the root cause of the build failure" -- not "Replayed the binlog using `dotnet build /flp`." -2. **Allow alternative approaches.** If multiple valid solutions exist, the rubric item should accept any of them. -3. **Never reference the skill by name** or use phrasing copied directly from the SKILL.md. -4. **Don't test pre-existing LLM knowledge.** If the LLM already knows something (common APIs, standard syntax, basic escaping), testing for it adds no signal. -5. **Test findings, not diagnostic steps.** Write "Correctly determined that the root cause is a missing PackageReference" -- not "Used `dotnet restore` to check package resolution." -6. **Each item should be independently evaluable.** Avoid compound items that test multiple things. +| **outcome** | Whether the agent reached a correct result — WHAT, not HOW | Target this | +| **technique** | Whether the agent used a skill-specific procedure | Minimize | +| **vocabulary** | Whether the agent used the skill's terminology | Avoid | + +1. Test outcomes, not methods: "Identified the root cause of the build failure", not "Replayed the + binlog using `dotnet build /flp`". +2. Accept any valid approach. +3. Never reference the skill by name, and never reuse `SKILL.md` phrasing. +4. Never reward using the skill — the harness reports activation separately, so a rubric item that + does this measures nothing and inflates the overfit score. +5. Do not test knowledge the model already has; it adds no delta. +6. Keep each item independently evaluable. +7. Do not reward raw volume (test count, report length); judges will compare it when both arms act. + +**Good:** -#### Examples - -**Well-designed (outcome-focused):** ```yaml rubric: - - "Correctly identified the missing NuGet package as the root cause of the build failure" - - "Recognized that downstream project failures were cascading from the root cause, not independent errors" - - "Suggested a concrete fix that would resolve the root cause" + - Correctly identified the missing NuGet package as the root cause of the build failure + - Recognized that downstream failures cascaded from that root cause + - Suggested a concrete fix that resolves it ``` -**Overfitted (vocabulary/technique):** -```yaml -rubric: - - "Replayed the binary log using 'dotnet build /flp:v=diag'" # technique: gates on specific command - - "Measured cold, warm, and no-op build scenarios" # vocabulary: uses skill's labels - - "Used the --clreventlevel flag with dotnet trace collect" # vocabulary: gates on specific flag -``` - -### Step 7: Add optional constraints +**Overfitted:** ```yaml -expect_tools: ["bash"] # Agent must use these tools -reject_tools: ["create_file"] # Agent must NOT use these tools -max_turns: 10 # Maximum agent iterations -max_tokens: 5000 # Maximum token budget +rubric: + - Replayed the binary log using 'dotnet build /flp:v=diag' # technique + - Measured cold, warm, and no-op build scenarios # vocabulary + - Used the template-comparison skill # rewards activation ``` -Use constraints sparingly -- only when the scenario specifically requires or forbids certain agent behaviors. - -### Step 8: Add non-activation scenarios with `expect_activation: false` - -Many skills have clear boundaries -- situations where the skill should recognize it does not apply and decline gracefully. Test these boundaries using `expect_activation: false`. - -#### How `expect_activation: false` works - -When a scenario has `expect_activation: false`: - -1. **All three runs still execute** (baseline, skilled-isolated, skilled-plugin) and assertions are evaluated on each. The flag does not change which runs are performed. -2. **Activation verdict is inverted** -- if the skill is not activated for this prompt, the evaluator reports it as `[Info] not activated (expected)` instead of treating it as a failure. -3. **The scenario is excluded from the noise test** -- the multi-skill activation test only runs positive (`expect_activation: true`) scenarios. - -#### When to use non-activation scenarios - -Add `expect_activation: false` scenarios when the skill has explicit "When Not to Use" boundaries. Common patterns: - -| Pattern | Example | -|---------|---------| -| **Wrong input format** | Skill handles Android tombstones; scenario provides an iOS crash log | -| **Out-of-scope request** | Skill collects dumps; scenario asks to *analyze* a dump | -| **Incompatible project type** | Skill converts PackageReference to CPM; scenario has packages.config | -| **Wrong framework version** | Skill migrates .NET 8 to 9; scenario provides a .NET 8 app and asks for .NET 10 migration | -| **Prerequisite not met** | Skill requires a specific file format that isn't present | - -#### Example: Wrong input format +### Step 8: Add constraints sparingly ```yaml -- name: "Reject iOS crash log as wrong format" - prompt: "I have a crash log file at crashlog_ios.txt from a crashed app. Please symbolicate the .NET runtime frames." - expect_activation: false - setup: - copy_test_files: true - assertions: - - type: "output_matches" - pattern: "(iOS|Apple|not.*(Android|tombstone)|wrong.*(format|type))" - rubric: - - "Recognized that this is an iOS crash log, not an Android tombstone" - - "Did NOT attempt to apply the Android tombstone symbolication workflow" - - "Explained that iOS crash logs require a different symbolication process" +constraints: + expect_tools: [bash] + reject_tools: [edit, create] + reject_skills: [some-skill] ``` -#### Example: Out-of-scope request +- `expect_tools: [bash]` on an **advisory** question forces a restore or build and converts an + answer into a timeout with no quality benefit. Only require tools when the task genuinely needs + them. +- `reject_tools` is the right way to keep a read-only stimulus read-only. -```yaml -- name: "Decline dump analysis request" - prompt: | - I already have a .dmp crash dump file from my .NET app. Can you help - me analyze it to find the root cause of the crash? - expect_activation: false - assertions: - - type: "output_matches" - pattern: "(out of scope|not cover|does not|cannot|only.*collect)" - rubric: - - "Clearly states that dump analysis is out of scope for this skill" - - "Does not attempt to open or analyze the dump file" - - "Does not install analysis tools like dotnet-dump analyze, lldb, or windbg" - timeout: 30 -``` +### Step 9: Add dormancy guards -#### Example: Incompatible project type +A dormancy guard proves the skill stays dormant on an off-target request that superficially matches +it. Add one per real "when not to use" boundary: wrong input format, out-of-scope request, +incompatible project type, wrong framework version, prerequisite absent. ```yaml -- name: "Decline CPM conversion for packages.config project" - prompt: "Convert my simple-packages-config/LegacyApp project to Central Package Management." - expect_activation: false - setup: - copy_test_files: true - assertions: - - type: "output_contains" - value: "packages.config" - - type: "file_not_exists" - path: "simple-packages-config/Directory.Packages.props" - rubric: - - "Detected the project uses packages.config instead of PackageReference format" - - "Informed the user that CPM requires PackageReference and cannot be applied to packages.config projects" - - "Suggested migrating from packages.config to PackageReference first" - - "Did not attempt to create Directory.Packages.props or modify any project files" + - name: Decline dump analysis request + prompt: | + I already have a .dmp crash dump from my .NET app. Can you help me + analyze it to find the root cause of the crash? + expect_activation: false + graders: + - type: output-matches + config: + pattern: (out of scope|not cover|does not|cannot|only.*collect) + - type: prompt + rubric: + - Stated that dump analysis is out of scope + - Did not open or analyze the dump file + - Did not install analysis tools such as dotnet-dump analyze, lldb, or windbg + - Suggested the correct alternative ``` -#### Rubric guidelines for non-activation scenarios +> **Never combine `expect_activation: false` with `constraints.reject_skills`.** That forces the +> skilled arm to run skill-free, making it identical to the baseline; the score is then pure judge +> noise. Across four evals the same guard scored −0.4, +0.4, +0.4 and 0, and twice cost a skill its +> pass. `expect_activation: false` **alone** is the repo convention. -Non-activation rubric items typically verify three things: +Guard rubrics verify three things: **recognition** (why it does not apply), **restraint** (no +workflow, no file changes, no installs), **redirection** (the correct next step). -1. **Recognition** -- The agent identified *why* the skill doesn't apply. -2. **Restraint** -- The agent did NOT attempt the skill's workflow (no file modifications, no tool installs). -3. **Redirection** -- The agent suggested the correct alternative approach or next step. - -### Step 9: Validate the eval.yaml - -Run the static validator: +### Step 10: Validate ```bash dotnet run --project eng/skill-validator/src/SkillValidator.csproj -- check --plugin ./plugins/ +python eng/eval-quality/check_eval_quality.py +./eng/run-skill-evals.sh ``` -Then run evaluation (at least 3 runs for reliable results): +For an **agent** eval, the third command is a no-op: `agent.*` is outside the experiment's `evals:` +glob. Exercise one by pointing the runner at an experiment file whose glob includes it: ```bash -# For skills: -dotnet run --project eng/skill-validator/src/SkillValidator.csproj -- evaluate \ - --runs 3 \ - --tests-dir tests/ \ - plugins//skills/ - -# For agents: -dotnet run --project eng/skill-validator/src/SkillValidator.csproj -- evaluate \ - --runs 3 \ - --tests-dir tests/ \ - plugins//agents/.agent.md +# copy dotnet-skills.experiment.yaml, widen its evals: glob to tests/*/agent.*/eval.yaml +EXPERIMENT_FILE=my-agent.experiment.yaml ./eng/run-skill-evals.sh ``` -## eval.yaml Template +Read the trajectories rather than the verdict — there is no sign-test result for an agent eval. -```yaml -scenarios: - - name: "" - prompt: "" - setup: - copy_test_files: true - assertions: - - type: "output_contains" - value: "" - - type: "exit_success" - rubric: - - "" - - "" - - "" - timeout: 120 +`check_eval_quality.py` blocks ten structural defect classes that each already cost a real result: +missing or untracked fixtures, self-contradicting coverage fixtures, empty grader configs, dormancy +guards with `reject_skills`, sub-floor trial counts, duplicate YAML keys, and `config:`/`defaults:` +collisions. Do not add a new eval to `eng/eval-quality/underpowered-allowlist.txt` — the gate rejects +allowlist entries that are new relative to the base branch. - - name: "" - prompt: "" - expect_activation: false - setup: - copy_test_files: true - assertions: - - type: "output_matches" - pattern: "" - - type: "file_not_exists" - path: "" - rubric: - - "" - - "" - - "" - timeout: 120 -``` +For the official run, submit a PR review containing `/evaluate` so it binds to the reviewed commit. ## Validation Checklist -After creating a test, verify: - -- [ ] Test directory matches `tests///` for skills or `tests//agent./` for agents -- [ ] Target exists at `plugins//skills//SKILL.md` (skill) or `plugins//agents/.agent.md` (agent) -- [ ] Every scenario has `name`, `prompt`, at least one assertion, and rubric items -- [ ] Prompts are written as natural developer requests (no skill/agent name references) -- [ ] Assertions are broad enough that multiple valid approaches pass -- [ ] Rubric items test outcomes, not specific techniques or vocabulary -- [ ] Fixture files are present when `copy_test_files: true` is used -- [ ] `source` paths in setup files point to existing fixture files -- [ ] `additional_required_skills`/`additional_required_agents` names exist in the same plugin -- [ ] Timeouts are reasonable for the scenario complexity -- [ ] Non-activation scenarios use `expect_activation: false` and verify recognition, restraint, and redirection -- [ ] `dotnet run --project eng/skill-validator/src/SkillValidator.csproj -- check` passes +- [ ] Directory is `tests///` or `tests//agent./` +- [ ] Spec uses `stimuli:` / `graders:`, and exactly one of `defaults:` or `config:` +- [ ] For a skill eval, `stimuli × runs` clears 5 with room for the expected tie rate (agent evals are exempt — they get no verdict) +- [ ] Each stimulus discriminates a different property +- [ ] Prompts never name the skill, the agent, or its vocabulary +- [ ] Every referenced fixture exists and is tracked by `git ls-files` +- [ ] Every fixture behaves as its stimulus assumes — healthy ones build, deliberately broken ones fail only for the stated reason +- [ ] Every grader has its required `config` key +- [ ] Any output shape the skill mandates has a grader +- [ ] Rubric items are outcome-shaped and never reward using the skill +- [ ] Dormancy guards use `expect_activation: false` alone +- [ ] `skill-validator check` and `check_eval_quality.py` pass ## Common Pitfalls | Pitfall | Solution | |---------|----------| -| Prompt mentions the skill by name | Rewrite as a natural developer request describing the problem | -| Prompt mentions the agent by name | Same as above — agent name in prompts biases the baseline | -| Rubric tests a specific diagnostic command | Rewrite to test the finding or outcome that command produces | -| Assertion gates on syntax the LLM already knows | Use a broader pattern or test the result instead | -| All rubric items test the same aspect | Diversify: test identification, fix quality, and error avoidance | -| Missing fixture files for `copy_test_files` | Add the required project/source files alongside eval.yaml | -| Timeout too short for builds | Use 300-600s for scenarios that compile or run benchmarks | -| Single scenario covers the entire skill | Break into focused scenarios testing different aspects | -| Compound rubric items testing multiple things | Split into separate, independently-evaluable items | -| No non-activation scenarios for skill with clear boundaries | Add `expect_activation: false` scenarios for each "When Not to Use" case | -| Agent test missing `additional_required_skills` | If the agent routes to specific skills, declare them so the isolated run loads them | +| Writing `scenarios:` / `assertions:` | That format no longer loads; use `stimuli:` / `graders:` | +| Adding `defaults: runs:` beside an existing `config:` | Merge into one `defaults:` block | +| Landing an eval at exactly 5 trials | A single tie makes a pass unreachable; size for the tie rate | +| Raising `runs` instead of adding stimuli | Repeats measure one task and add no cross-task evidence | +| Prompt mentions the skill or agent by name | Rewrite as a natural developer request | +| Rubric rewards using the skill | Drop the item — the harness reports activation separately; rubrics measure outcomes | +| Fixture present but ignored by git | Verify with `git ls-files`; CI setup will fail otherwise | +| Fixture that does not build, or breaks for the wrong reason | Fix the fixture before blaming the skill | +| Dormancy guard with `reject_skills` | Use `expect_activation: false` alone | +| `expect_tools: [bash]` on an advisory question | Drop it; it causes timeouts, not quality | +| Timeout too short for code generation | Use ~360s; empty output fails every grader | +| Duplicate YAML key left behind by an edit | It overwrites the next stimulus field by field — delete the stray block | +| Direct activation-graded eval for a `disable-model-invocation: true` skill | Cover it through a consumer skill, or grade the answer content as `filter-syntax` does | +| Agent eval sized for the trial floor | `agent.*` evals get no verdict; size them for scenario coverage instead | +| Agent eval "run" with `./eng/run-skill-evals.sh` | The glob drops it — use a widened `EXPERIMENT_FILE` | +| Agent eval missing `environment.skills` | Declare the skills the agent routes to, or it cannot invoke them | +| `environment.skills` set in a **skill** eval | The experiment varies that key and replaces it in every arm; the declaration does nothing | diff --git a/.agents/skills/create-skill/SKILL.md b/.agents/skills/create-skill/SKILL.md index d47e509222..093c99b1ac 100644 --- a/.agents/skills/create-skill/SKILL.md +++ b/.agents/skills/create-skill/SKILL.md @@ -1,6 +1,6 @@ --- name: create-skill -description: Scaffolds new agent skills for the dotnet/skills repository. Use when creating a new skill, generating SKILL.md files, or setting up skill directory structures. Handles frontmatter generation, section templates, and validation guidance. +description: Scaffolds new agent skills for the dotnet/skills repository. Use when creating a new skill, generating SKILL.md files, writing a skill description that the runtime will actually route to, or setting up skill directory structures. Handles frontmatter generation, section templates, and validation guidance. Do not use for fixing a skill that already fails its evaluation (use improve-skill-quality) or for writing eval.yaml (use create-skill-test). --- # Create Skill @@ -17,6 +17,8 @@ This skill helps you scaffold new agent skills that conform to the Agent Skills ## When Not to Use - Modifying existing skills (edit directly instead) +- Diagnosing or fixing a skill that fails its evaluation (use `improve-skill-quality`) +- Writing the skill's `eval.yaml` (use `create-skill-test`) - Creating custom agents (use the agents/ directory pattern) ## Inputs @@ -38,25 +40,66 @@ Ensure the name: - Does not contain consecutive hyphens - Is between 1-64 characters -### Step 2: Create the skill directory +### Step 2: Write the description — it is the router -``` -skills// -└── SKILL.md -``` - -### Step 3: Generate SKILL.md with frontmatter - -Create the file with required YAML frontmatter: +The `description` is the **only** text the runtime sees when deciding whether to load the skill. +A perfect body behind a weak description never runs. ```yaml --- name: -description: +description: . USE FOR: . DO NOT USE FOR: . --- ``` -### Step 4: Add body content sections +- Lead with an action verb and use the user's own words: symptoms, error codes (`CS1501`, + `MSTEST0014`), artifact names (`.testsettings`, `binlog`), and requests phrased as a developer + would type them. +- Partition against sibling skills on the **real discriminator**, not the topic. "Does the + abstraction already exist?" separates two skills; "testing" does not. Add the matching exclusion + to **both** siblings. +- Claim the ambiguous words that would otherwise route to a sibling. If prompts say "review my + tests" and a sibling owns "review", say so explicitly. +- Check every `DO NOT USE FOR` clause against the scenarios the skill exists to serve — an + exclusion like "already on v3" can lock out the post-upgrade fixes that are the skill's purpose. +- Budget: 1,024 characters per description, and the whole plugin's rendered skill menu is also + budgeted. A helper skill users should never invoke directly can set + `disable-model-invocation: true` to free menu space while staying invocable by name. + +### Step 3: Write for delta over the baseline model + +Every skill is scored head-to-head against the same model with **no skill loaded**. Content the +model already produces unaided is worth zero; content that makes it slower or more hedged is worth +less than zero. See +[improve-skill-quality/references/writing-for-baseline-delta.md](../improve-skill-quality/references/writing-for-baseline-delta.md) +for the full evidence. + +| Do | Instead of | +|----|------------| +| Encode the decision the model would otherwise get wrong | Restating API signatures it already reproduces | +| "When A, do B, never C, verify D" tables | Lists of plausible alternatives | +| A concrete output contract (exact command, verdict line, findings table) | "Consider…", "you may want to…" | +| Scale output structure to input size | A 12-section dashboard for an 8-test suite | +| Stop-conditions that prevent over-applying | Acting before measuring, rewriting working code | +| Instructing the agent to discover repo paths | Marking discoverable paths as required inputs | +| Reporting restore/build/test failures truthfully | Claiming success after a failed command | +| Verifying load-bearing API claims by compiling or probing | Trusting a source read | +| Gating rare or expensive paths behind `references/` | One large SKILL.md carrying every path | + +Do not over-correct: a skilled answer shorter and less actionable than the baseline's still loses. + +### Step 4: Create the skill directory + +``` +plugins//skills// +└── SKILL.md +``` + +### Step 5: Generate SKILL.md with frontmatter + +Create the file with the frontmatter drafted in Step 2. + +### Step 6: Add body content sections Include these recommended sections: @@ -68,17 +111,17 @@ Include these recommended sections: 6. **Validation**: How to confirm the skill worked correctly 7. **Common Pitfalls**: Known traps and how to avoid them -### Step 5: Add optional directories (if needed) +### Step 7: Add optional directories (if needed) ``` -skills// +plugins//skills// ├── SKILL.md ├── scripts/ # Executable code agents can run ├── references/ # Additional documentation loaded on demand └── assets/ # Templates, images, data files ``` -### Step 6: Update CODEOWNERS +### Step 8: Update CODEOWNERS Add entries in `.github/CODEOWNERS` for the new skill and its test directory: @@ -89,12 +132,23 @@ Add entries in `.github/CODEOWNERS` for the new skill and its test directory: Match the owner pattern used by sibling skills in the same plugin. -### Step 7: Validate the skill +### Step 9: Validate the skill - Confirm frontmatter fields are valid - Ensure SKILL.md is under 500 lines - Check that file references use relative paths - Verify instructions are actionable and specific +- Run `dotnet run --project eng/skill-validator/src/SkillValidator.csproj -- check --plugin ./plugins/` + +### Step 10: Add the eval + +A skill without an `eval.yaml` has no evidence that it improves on the baseline. Use +`create-skill-test` to add one in the same pull request, and size it for statistical power — an eval +below five counted trials can never return a passing verdict. + +The exception is a helper skill with `disable-model-invocation: true`: the model cannot +self-activate it, so an activation-graded eval compares two identical arms. Cover it through the +evals of the skills that load it instead. ## SKILL.md Template @@ -161,6 +215,10 @@ After creating a skill, verify: - [ ] Validation section exists with observable success criteria - [ ] No secrets, tokens, or internal URLs included - [ ] `.github/CODEOWNERS` has entries for the new skill and its test directory +- [ ] The description names concrete triggers and excludes the nearest sibling skills +- [ ] Every section changes a decision the unskilled model would otherwise get wrong +- [ ] The skill states when **not** to act, and what a truthful failure report looks like +- [ ] An `eval.yaml` exists and clears the trial floor (or the skill is `disable-model-invocation: true` and covered through its consumers) ## Common Pitfalls @@ -173,9 +231,16 @@ After creating a skill, verify: | SKILL.md too long | Move detailed content to `references/` files | | Hardcoded environment assumptions | Document requirements in `compatibility` field | | Missing CODEOWNERS entry | Add entries for both `/plugins//skills//` and `/tests///` matching sibling skills' owner pattern | +| Skill restates what the model already knows | Cut it; a skill is scored as a delta over the unskilled model | +| Discoverable paths listed as required inputs | Tell the agent to discover them, or it will stop and ask the user | +| Description partitioned by topic against a sibling | Partition on the real discriminator and exclude on both sides | +| Exclusion clause blocks the skill's own use cases | Re-read every "do not use for" clause against real workflow phases | +| Skill added without an eval | Add `eval.yaml` in the same PR; unevaluated skills carry no evidence | ## References - [Agent Skills Specification](https://agentskills.io/specification) -- [Repository README](../../README.md) -- [Contributing Guidelines](../../CONTRIBUTING.md) +- [Repository README](../../../README.md) +- [Contributing Guidelines](../../../CONTRIBUTING.md) +- [create-skill-test](../create-skill-test/SKILL.md) — authoring the skill's `eval.yaml` +- [improve-skill-quality](../improve-skill-quality/SKILL.md) — fixing a skill that loses to its baseline diff --git a/.agents/skills/improve-skill-quality/SKILL.md b/.agents/skills/improve-skill-quality/SKILL.md new file mode 100644 index 0000000000..401e90d42d --- /dev/null +++ b/.agents/skills/improve-skill-quality/SKILL.md @@ -0,0 +1,209 @@ +--- +name: improve-skill-quality +description: Diagnoses and fixes skills in the dotnet/skills repository that lose to their own baseline, fail to activate, time out, or return "no credible improvement". Use when an evaluation verdict is a regression or underpowered, when a skill regressed after a change, when /evaluate reports no results, or when deciding whether a weak skill should be strengthened or retired. Do not use for scaffolding a brand-new skill (use create-skill) or a brand-new eval (use create-skill-test). +--- + +# Improve Skill Quality + +Turn a failing or unconvincing evaluation into a targeted fix. The single most common mistake +in this repo is rewriting skill prose in response to a verdict whose real cause was the eval, +the fixtures, or the harness. Classify first, then fix. + +## When to Use + +- An evaluation verdict is a regression, underpowered, or "no credible improvement". +- A skill wins in the isolated arm but not in the plugin arm, or is reported "not activated". +- `/evaluate` reports "Evaluation ran but produced no results". +- A skill scores well but costs too much (tokens, turns, wall time, plugin menu budget). +- Deciding whether to strengthen or retire a persistently weak skill. + +## When Not to Use + +- Creating a new skill from scratch — use `create-skill`. +- Creating a new `eval.yaml` from scratch — use `create-skill-test`. +- Changing the harness itself (`eng/skill-validator`, `eng/vally-adapter`, `evaluation*.yml`). + +## Inputs + +| Input | Required | Description | +|-------|----------|-------------| +| Verdict evidence | Yes | The `/evaluate` PR comment, or `results.json` from the run artifacts | +| Losing trial transcripts | Yes for content fixes | Baseline vs. skilled output plus the judge's stated reason | +| W/T/L record and trial count | Yes | Distinguishes a real regression from an underpowered eval | +| Activation status per arm | Yes | Isolated and plugin activation are different failures | + +## Workflow + +### Step 1: Get the evidence before forming a hypothesis + +Read [InvestigatingResults.md](../../../eng/vally-adapter/InvestigatingResults.md) for how to +download artifacts and read `results.json`. Extract, per failing stimulus: + +- win / tie / loss record and total trials (`trials = stimuli × runs`) +- activation status in the **isolated** and **plugin** arms, separately +- the judge's verbatim reason on each losing trial +- whether any trial errored, timed out, or produced empty output + +Do not change skill content until you can quote a losing trial and the judge's reason for it. For the +other cause classes the evidence is different: harness failures are diagnosed from the job log and +the spec, and power problems from the trial record — neither has a losing trial to quote, and +demanding one is what sends people rewriting prose instead. + +### Step 2: Classify the failure + +Work down this table and stop at the first row that matches. Rows are ordered by how often the +symptom has been misdiagnosed as a skill-content problem — the fixture row is first because a +fixture failure also presents as a setup or reliability failure and gets misfiled as one. + +| Symptom | Real cause class | Go to | +|---------|------------------|-------| +| A fixture does not build, is untracked by git, breaks for the wrong reason, or contradicts itself | Fixture | Step 4 | +| No `results.json`, "produced no results", or the spec never loaded | Harness / spec-load | Step 3 | +| Trials errored, timed out, or returned empty output | Reliability | Step 3 | +| Trajectories unmatched, a trial errored, or the summary disagrees — verdict reported inconclusive | Reliability (not power) | Step 3 | +| Positive record (e.g. 16W/8T/1L), comparison conclusive, verdict still not a pass | Statistical power | Step 5 | +| Skilled arm equals baseline arm by construction | Eval design | Step 6 | +| Activated and lost on quality, judge names a concrete defect | Skill content | Step 7 | +| Activated in isolation, not in plugin | Activation / routing | Step 8 | +| Not activated in either arm | Frontmatter description | Step 8 | +| Wins but costs far more than baseline | Scope and cost | Step 7 | + +A verdict is only a *measured* result when the comparison was conclusive: `adapt.mjs` requires zero +errored trials, zero unmatched trajectories, and an agreeing summary before it will report a pass or +a regression. Confirm that before reading a record as a power problem. + +### Step 3: Rule out harness and reliability causes + +See [references/eval-triage.md](references/eval-triage.md) for the full catalogue. The recurring ones: + +- A spec declaring both `config:` and `defaults:` is rejected by vally, the job still exits 0, and + the PR comment blames "transient infrastructure". Merge them into one `defaults:` block. +- An errored trial is not automatically a fixture problem — judge-side auth and `session.idle` + failures look identical from the verdict and need harness fixes, not SDK pins. +- `expect_tools: [bash]` on an advisory question forces a restore or build and turns an answer into + a timeout with no quality gain. +- Genuine code-generation stimuli need roughly 360s; a timeout yields empty output, which fails + every grader and hides the real quality signal. +- Unmatched trajectories, an errored trial, or a summary that disagrees make the comparison + **inconclusive**: the remaining matched trials are biased, so the record is not a measured null + and must not be read as a power or content problem. + +### Step 4: Verify the fixtures before touching the skill + +Run `python eng/eval-quality/check_eval_quality.py` — it blocks ten defect classes that each already +cost a real result here. Then confirm by hand: + +- every fixture behaves as its stimulus assumes — a fixture meant to be healthy builds, and one + meant to be broken fails for the exact reason the stimulus is about and no other; +- every referenced fixture is in the git index (`git ls-files`), not merely on disk — `.gitignore` + has silently swallowed committed coverage fixtures; +- a fixture never states the same fact in two places that disagree — a Cobertura report whose + declared `line-rate`, summary totals and `` elements differ is the canonical case — or the + two arms legitimately read different truths. + +### Step 5: Check whether the eval could ever have passed + +The gate has two independent bars, and confusing them is the usual misdiagnosis: + +1. **Counted trials ≥ 5** (`trials = stimuli × runs`). Below that the verdict is reported + `underpowered` — never a pass, never a regression. +2. **The sign test must reach p ≤ 0.05 over the *discordant* (non-tie) trials.** Ties are not + discarded silently; they hold the discordant count down. + +| discordant trials | records that pass | p | +|---:|---|---:| +| ≤ 4 | none, however good the skill | ≥ 0.0625 | +| 5–7 | zero losses only (5W/0L) | 0.031 | +| 8 | one loss survivable (7W/1L) | 0.035 | + +So at exactly 5 counted trials a single tie is fatal — it leaves 4 discordant. At 6 counted trials +one tie is survivable (5W/1T/0L); at 7, up to two are (5W/2T/0L). A loss is not. + +So a positive record with a failing verdict is a power problem, not a content problem. Fix it by +adding **discriminating stimuli** (cross-task evidence) rather than raising `runs` (repetition +only) — except where each stimulus drives an expensive pipeline. Record the reasoning in a comment +above `defaults:`, as `tests/dotnet-test/grade-tests/eval.yaml` does. + +### Step 6: Check whether the two arms differ at all + +An eval that compares the skill against itself measures judge noise: + +- A dormancy guard (`expect_activation: false`) must **not** also set `constraints.reject_skills`. + That makes the skilled arm skill-free, i.e. identical to baseline. Across four evals the same + guard scored −0.4, +0.4, +0.4 and 0, twice costing a skill its pass. +- A skill with `disable-model-invocation: true` cannot self-activate, so an eval graded on + activation compares two identical arms. Cover it through a consumer skill, or grade the answer + content instead, as `tests/dotnet-test/filter-syntax/eval.yaml` and + `tests/dotnet-test/platform-detection/eval.yaml` do. +- A grader whose `config` is missing its required key enforces nothing, so the stimulus has one + fewer assertion than it appears to. + +### Step 7: Fix skill content against the losing trial + +Only now change the skill. Apply the patterns in +[references/writing-for-baseline-delta.md](references/writing-for-baseline-delta.md); the ones that +most often flip a loss: + +- Replace reference prose the model already knows with decisions it would otherwise get wrong. +- Add stop-conditions so a strong skill does not over-apply — but do not over-correct into + answering more narrowly than the baseline did. +- Scale output structure to input size; a dashboard for an 8-test suite loses to a direct answer. +- Require truthful validation reporting; claiming "Build succeeded" after a failed restore is an + automatic loss. +- Verify load-bearing API claims by compiling or probing, not by reading source. +- For cost regressions, gate rare or expensive paths behind `references/` reads and size any + orchestration to the user's scope. + +### Step 8: Fix activation + +Activation failures are frontmatter and routing failures, not body failures. See +[references/eval-triage.md](references/eval-triage.md). Summary: + +| Failure | Fix | +|---------|-----| +| Not activated in any arm | Put the user's own words in `description`: symptoms, error codes, artifact names, quoted requests | +| A sibling skill wins the prompt | Claim the exact ambiguous words in `description`, and add matching exclusions on **both** siblings | +| Model answers with no skill at all | Raise the stakes in the description, de-crowd the plugin menu, verify with the plugin arm | +| Boundary excludes real scenarios | Re-read every "do not use for" clause against every eval prompt and real workflow phase | +| Description at the 1,024-char ceiling | Cut restated body content, not trigger phrases; check the plugin menu budget too | + +### Step 9: Re-validate + +```bash +dotnet run --project eng/skill-validator/src/SkillValidator.csproj -- check --plugin ./plugins/ +python eng/eval-quality/check_eval_quality.py +./eng/run-skill-evals.sh +``` + +Then request the official run by submitting a PR review containing `/evaluate` (Files changed → +Review changes), which binds the run to the reviewed commit. Before declaring a regression on the +result, confirm the skill payload actually changed — reruns on byte-identical content have shifted +7W/2T/2L to 4W/5T/2L. + +## Validation + +- [ ] For a content fix, a losing trial and the judge's stated reason are quoted in the PR description. +- [ ] The failure was classified before any content was edited. +- [ ] `check_eval_quality.py` and `skill-validator check` both pass. +- [ ] Trial count clears the power bar for the observed tie rate, not just the floor of 5. +- [ ] Isolated **and** plugin activation are both reported. +- [ ] The PR body records root cause, fix, and validation so the lesson is reusable. + +## Common Pitfalls + +| Pitfall | Solution | +|---------|----------| +| Rewriting skill prose in response to an underpowered verdict | Underpowered means too few discordant trials; add discriminating stimuli instead | +| Adding `defaults: runs:` to a spec that already has `config:` | Merge into a single `defaults:` block; vally rejects specs with both | +| Padding `runs` to clear the trial floor | Five repeats of one stimulus measure one task; add stimuli | +| Treating an errored trial as fixture nondeterminism | Read the stderr first; judge-side auth failures need harness fixes | +| Fixing a "wrong" answer that the fixture actually made wrong | Check fixture self-consistency before blaming the response | +| Strengthening a skill nobody uses and nothing passes | Weak eval signal plus thin telemetry is a valid retirement case | +| Landing a fix without re-running | Verify the invoked payload contains the fix; judge noise is real | + +## References + +- [references/writing-for-baseline-delta.md](references/writing-for-baseline-delta.md) — content patterns that beat the unskilled model +- [references/eval-triage.md](references/eval-triage.md) — symptom, cause and fix catalogue with PR citations +- [eng/eval-quality/README.md](../../../eng/eval-quality/README.md) — the ten structural gate checks and why each exists +- [eng/vally-adapter/InvestigatingResults.md](../../../eng/vally-adapter/InvestigatingResults.md) — downloading artifacts and reading `results.json`. This is the current guide; the similarly-named `eng/skill-validator/src/docs/InvestigatingResults.md` documents the retired `skill-validator evaluate` schema and does not describe today's results. diff --git a/.agents/skills/improve-skill-quality/references/eval-triage.md b/.agents/skills/improve-skill-quality/references/eval-triage.md new file mode 100644 index 0000000000..87b5ee2139 --- /dev/null +++ b/.agents/skills/improve-skill-quality/references/eval-triage.md @@ -0,0 +1,93 @@ +# Evaluation triage catalogue + +Symptom → cause → fix, with the PR where each was diagnosed. Use with +`SKILL.md` Step 2; this file is the detail behind each cause class. + +## Harness and reliability + +| Symptom | Cause | Fix | Evidence | +|---------|-------|-----|----------| +| "Evaluation ran but produced no results", advice says transient infrastructure | Spec declares both `config:` and `defaults:`; vally throws, the job still exits 0 | Merge into one `defaults:` block carrying `timeout` and `runs` | PR #971 | +| Same message, nothing in `plugins/` changed | Genuine LLM-session auth failure | Re-post `/evaluate`; inspect job logs before touching content | PR #932 | +| Trial errored, avgN unusually low | Judge-side CAPI / `session.idle` timeout, not fixture nondeterminism | Read the trial stderr first; fix the harness, do not pin an SDK | PR #907 | +| Timeout on an advisory question | `expect_tools: [bash]` forced a restore or build | Drop the tool requirement; the answer was always textual | PR #861 | +| Every grader fails and output is empty | Code-generation stimulus timed out | Raise to ~360s | PR #862, PR #863 | +| Only the skilled arm aborts with "contains no SKILL.md" | A setup cleanup command deleted the staged skill directory | Skip directories carrying `SKILL.md` when stripping sources | PR #878 | +| Trials silently dropped | Setup command exited non-zero although its artifact was produced | Guard intentional failures, e.g. `dotnet build -bl \|\| exit 0` | PR #878 | +| One arm has unmatched trajectories | Comparison judge error or arm timeout biases the remaining trials | Report inconclusive, do not read it as a regression | PR #887 | +| Fixture resolves locally, SDK-not-found in CI | Fixture pin without roll-forward | `global.json` with `rollForward: latestMajor` | PR #907 | + +## Fixtures + +| Symptom | Cause | Fix | Evidence | +|---------|-------|-----|----------| +| Judge penalizes the agent for "pre-existing build issues" | A fixture meant to be healthy does not compile | Build every healthy fixture before shipping the eval; a deliberately broken one must fail only for the reason its stimulus is about | PR #949 | +| Scenario fails at setup in CI but passes locally | Fixture is on disk but not in the git index (`.gitignore` swallowed it) | Verify with `git ls-files`; the gate now blocks this | PR #945, PR #953 | +| Judge says the response "made a critical error" about a value the fixture supplies | The fixture states the same fact in two places and they disagree, so each arm can legitimately read a different truth — e.g. a Cobertura report whose declared `line-rate` contradicts its `` payload or summary totals | Make every representation of the value agree, then re-derive any rubric or prompt that quotes it | PR #964, PR #945 | +| Baseline scores suspiciously well | The fixture never reproduces the bug the stimulus is named for | Rebuild the fixture until it produces the real error | PR #974 | +| `n` rose but power did not | Duplicate or rename-leftover fixtures wired in as new stimuli | Delete byte-equivalent leftovers; only wire fixtures exercising new behavior | PR #971, PR #945 | + +## Statistical power + +The gate has two independent bars: **counted trials ≥ 5** (else `underpowered`), and **p ≤ 0.05 on +an exact one-sided sign test over the discordant (non-tie) trials**. `trials = stimuli × runs`. + +| discordant trials | records that pass | p | +|---:|---|---:| +| ≤ 4 | none | ≥ 0.0625 | +| 5–7 | zero losses only (5W/0L) | 0.031 | +| 8 | one loss survivable (7W/1L) | 0.035 | + +At exactly 5 counted trials one tie is fatal — it leaves 4 discordant. At 6 counted trials one tie is +survivable (5W/1T/0L); at 7, up to two are (5W/2T/0L). A loss is not: 4W/3T/1L over eight trials is +five discordant and fails. + +Consequences seen in real runs: + +- Five `dotnet-test` evals raised to exactly 5 trials returned 16W/8T/1L overall — every skill + winning, none regressing — and **all five failed**, four because ties made a pass unreachable + before the run started. (PR #971, `eng/eval-quality/README.md`) +- At the 32% tie rate measured there, a genuinely-helping skill parked at 5 trials is certified + about one run in ten; at 15 trials, about nine in ten. +- Adding stimuli is strictly better than raising `runs`: repeats measure one task. Use `runs` only + where a stimulus is genuinely expensive — `code-testing-agent` uses `defaults.runs: 2` because + each stimulus drives a full npm/pytest/dotnet pipeline inside a 60-minute budget. (PR #974) +- The verdict reads each trial's **winner**, never its magnitude: weighting a confidence interval by + "slightly better" vs "much better" made a stronger win look like variance and reversed verdicts on + identical records. (PR #965, PR #952) + +## Eval design + +| Symptom | Cause | Fix | Evidence | +|---------|-------|-----|----------| +| A dormancy guard scores randomly across runs | `expect_activation: false` combined with `constraints.reject_skills`, making the skilled arm skill-free and identical to baseline | Use `expect_activation: false` alone | PR #945, PR #953 | +| A reference skill shows no improvement | `disable-model-invocation: true` means the model cannot self-activate it, so an activation-graded eval compares identical arms | Cover it through a consumer skill, or grade answer content as `filter-syntax` does | PR #971, PR #976, issue #899 | +| An eval "passes" while the skill stopped emitting its signature output | No grader asserts the mandated shape | Add a grader for the exact contract (e.g. the `Recommendation:` line) | PR #904 | +| Overfit score high, user value unclear | Rubric items reward using the skill, or prompts echo skill vocabulary | Drop them: the harness already reports activation separately, so a rubric never needs to. Keep rubric items outcome-shaped and de-cue the prompt | PR #904 | +| Both arms produce the same kind of artifact and the judge falls back on comparing volume | The rubric rewards raw output instead of the property under test | Add anti-hijack criteria: do not invoke the skill, and do not reward quantity (number of tests, findings, or lines produced) | PR #945 | +| A grader appears to enforce something but does not | `config:` is missing its required key after an indentation slip | `check_eval_quality.py` blocks it; verify the key is present | `eng/eval-quality/README.md` | +| Two stimuli behave identically | Duplicate YAML key — a leftover `prompt:`/`graders:` block overwrites the following stimulus field by field | Delete the stray block after confirming it is not a distinct stimulus that lost its `- name:` | PR #971 | +| Eval measures path recall | The skill is a map to reference files | Do not create the eval; test the consumer's outcome instead | PR #974 | + +## Activation + +| Symptom | Cause | Fix | Evidence | +|---------|-------|-----|----------| +| Not activated in any arm | Triggering words absent from `description` | Add symptoms, error codes, artifact names in user language | PR #974 | +| Not activated for the scenarios the skill exists for | An over-broad exclusion clause | Re-read every "do not use for" clause against every eval prompt | PR #974 | +| Wrong sibling wins the prompt | Descriptions partitioned by topic instead of by discriminator | Partition on the real question, and add handoff exclusions on both sides | PR #864 | +| Sibling wins on one ambiguous word | The target skill never claims that word | Claim it explicitly — "review" had to be claimed by `writing-mstest-tests` | PR #863 | +| Isolated activation perfect, plugin arm fails | The model self-serves: reads the file and answers with no skill at all | Raise stakes in the description, de-crowd the menu, verify in the plugin arm | PR #850 | +| Menu pressure across a plugin | Helper/reference skills consuming budget | `disable-model-invocation: true` keeps them invocable by name only | PR #850 | + +## Process + +| Rule | Evidence | +|------|----------| +| Trigger `/evaluate` by submitting a PR review (Files changed → Review changes) so the run binds to the reviewed commit | PR #956, PR #949 | +| Before declaring a regression, confirm the invoked payload changed — reruns on byte-identical content moved 7W/2T/2L to 4W/5T/2L | PR #974 | +| Use cross-family evaluation for broad rewrites; single-family passes hide model-specific regressions | issue #899, PR #947 | +| Workflow changes cannot be validated by the PR's own evaluation (GitHub runs workflow definitions from `main`) — use manual dispatches | PR #872 | +| Prefer deterministic scripts over agentic workflows for deterministic policy | PR #928 | +| Agent `tools:` allowlists are host-specific and case-sensitive; an allowlist can grant zero tools | PR #856, PR #847 | +| Keep `InvestigatingResults.md` in sync whenever verdict fields, scoring, or PR-comment wording change | PR #965, PR #932 | diff --git a/.agents/skills/improve-skill-quality/references/writing-for-baseline-delta.md b/.agents/skills/improve-skill-quality/references/writing-for-baseline-delta.md new file mode 100644 index 0000000000..b722dce349 --- /dev/null +++ b/.agents/skills/improve-skill-quality/references/writing-for-baseline-delta.md @@ -0,0 +1,151 @@ +# Writing skill content that beats the baseline + +Every skill in this repo is scored head-to-head against the *same model with no skill loaded*. +The score is a **delta**, so content the model already produces unaided is worth zero — and +content that makes the model slower, longer, or more hedged is worth less than zero. + +Each rule below is traced to the PR where it was learned. + +## 1. Encode decisions, not knowledge + +**Rule:** Write what the model should *do* when it sees a symptom, not what an API *is*. + +`system-text-json-net11` scored 0% improvement because it "read as reference prose — API +signatures the model already reproduces". The rewrite into imperative decision guidance is what +moved it. (PR #926) + +Practical test: delete any sentence the unskilled model would have written anyway. If most of the +skill disappears, it is a reference doc, not a skill. + +## 2. Use "when A, do B, never C, verify D" tables + +**Rule:** Route with decision tables so the model picks one answer instead of listing plausible +alternatives. + +PR #926 added a table mapping PascalCase / typed-metadata / probing requests to exact APIs and +their anti-patterns. PR #947 added "Rules That Change the Answer" tables across the MAUI skills. +Tables also give graders something concrete to assert on. + +## 3. Demand a concrete final artifact + +**Rule:** Specify the exact shape of the answer — the command, the verdict line, the findings +table, the recommendation. + +Generic advice did not move the template-engine skills; output contracts did: "exact single +`dotnet new` command", "one-line verdict header", "single findings table", decisive +`Recommendation:` lines. (PR #904) + +If the skill mandates an output shape, the eval must assert on that shape, or the skill can +silently stop emitting it. + +## 4. Scale structure to input size + +**Rule:** Preserve the full report template for large inputs; answer directly for small ones. + +`assertion-quality` regressed on an 8-test fixture because the skill forced a "Summary Dashboard + +12 categories" onto a trivial input and lost to a concise baseline. The fix instructed the skill to +scale report depth to suite size and complexity. (PR #865) + +## 5. Add stop-conditions — then check you have not over-corrected + +**Rule:** Say when *not* to act; then verify the skilled answer is still at least as complete as +the baseline's. + +Strong skills regress by doing too much: rewriting working code, answering beyond the question, +prescribing a remedy before measuring. PR #910 added stop-conditions to `eval-performance` so it +would not act on compile-time slowness or unmeasured builds; PR #947 added scope control to +`maui-collectionview`. + +The opposite failure is just as real: in PR #947 the scope-control wording "over-corrected the +other skills into answering too narrowly", and failing scenarios had skilled answers *shorter* than +baseline. Length is not the goal, but omitting the implementation detail the baseline supplied is a +loss. + +## 6. Do not make discoverable inputs "required" + +**Rule:** Only mark an input required when a human genuinely must supply it. Otherwise instruct the +agent to discover it. + +A `Project or solution path | Yes` row in an Inputs table made the agent answer "I need to see your +project file" while `TestProject.csproj` sat in the working directory. (PR #974) + +## 7. Verify load-bearing claims empirically + +**Rule:** Compile or probe anything the skill asserts about API surface or runtime behavior. + +The most damaging skill defects found in review were factual: MAUI docs taught `ItemSizingStrategy` +on `LinearItemsLayout`, which does not compile (MAUIX2002), and a theming "fix" was reverted after a +runtime probe disproved the source-reading that motivated it. (PR #947) + +## 8. Preserve semantics in migration mappings + +**Rule:** Migration tables need semantic guardrails, not just API substitutions. + +A one-cell mapping steered frontier models into a behavior change: `TimeProvider` guidance using +`.DateTime` silently set `DateTimeKind.Unspecified`; the correct mapping is `.UtcDateTime` / +`.LocalDateTime`. (PR #906) + +## 9. Require truthful validation reporting + +**Rule:** Tell the agent to distinguish restore, build and test failures, and to cite a clean run +before claiming success. + +`migrate-static-to-wrapper` lost trials for claiming "Build succeeded" after a restore failure; +`code-testing-agent` had to be told to cite a clean run. (PR #945) + +## 10. Prove already-correct inputs are left alone + +**Rule:** Any skill that migrates or rewrites code must state the no-op condition, and the eval must +test it. + +PR #929 added an "already on v3" boundary fixture verifying no changes were made; the reviewer +called it "the single best addition" in the PR. + +## 11. Keep the common path in `SKILL.md`, gate the rest + +**Rule:** Rare, expensive or platform-specific paths belong behind a `references/` read. + +`coverage-analysis` went from 30 KB to 15 KB by moving PowerShell and report-generation paths into +references read only when needed — cost down, contract unchanged. (PR #971) + +## 12. Size orchestration to the request + +**Rule:** Do not run a full research → plan → implement pipeline for one function. + +`code-testing-agent` was split into focused and broad paths so a single-function request skips +`.testagent/` artifacts and extra passes. (PR #971) + +## 13. Structure beats verbosity + +**Rule:** When a scenario ties despite a longer skilled answer, the missing differentiator is a +recommended shape, not more words. + +Both arms hardcoded colors in the MAUI theming scenario. The winning change was the rule "define +the palette once — don't scatter literals", not a longer explanation. (PR #947) + +## 14. Retirement is a legitimate outcome + +**Rule:** A skill that is weak across model families, thinly used, and costing menu budget should be +cut, not polished indefinitely. + +`mcp-csharp-debug` was cut after strengthening 0 of 5 families, with owner confirmation; the change +removed the skill, its eval, its CODEOWNERS entry and cross-skill references. (PR #938) +`dotnet-test-frameworks` was removed as a duplicate subset that "nothing actually loaded". (PR #851) + +## Frontmatter: the description is the router + +The `description` is the only text the runtime sees when deciding whether to load the skill. (PR #974) + +| Rule | Evidence | +|------|----------| +| Include symptoms, error codes and artifact names in the user's words (`CS1501`, `.testsettings`, `MSTEST0014`) | PR #974 | +| Lead with an action verb and quote natural requests ("what's wrong with my build file?") | PR #910 | +| Partition siblings by the real discriminator ("abstraction already exists" vs "create wrapper first"), with exclusions on both sides | PR #864 | +| Claim the ambiguous words that route to the wrong sibling — `writing-mstest-tests` had to claim "review" | PR #863 | +| Never exclude a phase the skill exists to serve; "already on MSTest v3+" blocked its own post-bump fixtures | PR #974 | +| Watch both limits: 1,024 characters per description **and** the plugin menu budget | PR #974, PR #910 | +| A helper skill that users should not invoke can set `disable-model-invocation: true` to free menu budget | PR #850 | + +Menu pressure is measurable: disabling model invocation for one helper skill dropped the +`dotnet-test` menu from 14,981 to 14,261 characters (PR #850), and PR #910 tracked the msbuild menu +budget explicitly as part of a description rewrite. diff --git a/.gitignore b/.gitignore index 9481074e8e..dcd7add7d1 100644 --- a/.gitignore +++ b/.gitignore @@ -3,8 +3,17 @@ ## ## Get latest from https://github.com/github/gitignore/blob/main/VisualStudio.gitignore -# Installed skills (other authors' content) -.agents/skills/ +# Installed skills (other authors' content). The repo's own authoring skills are +# re-included below; a directory pattern would make that impossible, so this +# ignores the *entries* under .agents/skills/ rather than the directory itself. +# Adding a new repo-owned authoring skill? Add a negation for it here too, or it +# will be silently untracked. +.agents/skills/* +!.agents/skills/authoring-github-workflows/ +!.agents/skills/create-custom-agent/ +!.agents/skills/create-skill/ +!.agents/skills/create-skill-test/ +!.agents/skills/improve-skill-quality/ # User-specific files *.rsuser diff --git a/AGENTS.md b/AGENTS.md index d472039e21..7bc1805649 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,6 +2,24 @@ This repository contains skill plugins under `plugins/`. Each subdirectory in `plugins/` is an independent plugin (e.g., `plugins/dotnet-msbuild`, `plugins/dotnet`). +## Working on skills and evals + +Use the repository's own authoring skills under `.agents/skills/` instead of improvising: + +- `create-skill` — scaffolding a new skill, and writing a `description` the runtime will route to. +- `create-skill-test` — writing or resizing an `eval.yaml`. Evals use the Vally schema + (`stimuli:` / `graders:` / `defaults:`); `scenarios:` / `assertions:` no longer load. +- `improve-skill-quality` — an eval regressed, produced no verdict, or the skill did not activate. + Classify the failure before editing skill content; broken fixtures, underpowered trial counts and + harness errors routinely masquerade as skill regressions. + +Before pushing eval changes, run `python eng/eval-quality/check_eval_quality.py`. It blocks ten +structural defect classes documented in `eng/eval-quality/README.md`, each of which has already cost +a real evaluation result here. + +The distilled quality rules — what makes a skill beat its own baseline — live in the "Quality bar" +section of `CONTRIBUTING.md`. + ## Skill-Validator The skill-validator is a shipping tool — its NuGet package and `.tar.gz` archives are built from `eng/skill-validator/src/`. Content referenced at runtime or bundled with the tool (docs, README, etc.) must live under `src/` so it is included in the published output. Do not add references from `src/` to files outside of it, except for explicitly linked packaging assets (such as the repo-root `LICENSE` file) referenced by the project file. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ae6608b8c5..7495681057 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -244,14 +244,20 @@ Each skill should have an `eval.yaml` file that defines test scenarios. Tests li tests///eval.yaml ``` -A minimal eval file: +The exception is a helper or reference skill that sets `disable-model-invocation: true`. The model +cannot self-activate it, so an activation-graded eval would compare two identical arms. Cover those +through the evals of the skills that load them and through the plugin arm instead. + +The skeleton below shows the shape only — it declares a single trial and would therefore be rejected +by the quality gate. See [Size the eval so it can return a verdict](#size-the-eval-so-it-can-return-a-verdict) for the real bar. ```yaml name: my-skill description: Evaluates the / skill type: capability -config: +defaults: timeout: 3m + runs: 1 stimuli: - name: "Describe what the agent should do" prompt: | @@ -269,8 +275,44 @@ stimuli: - The agent suggested a concrete fix ``` +> [!IMPORTANT] +> `defaults:` and `config:` are the same block — `config` is a deprecated alias — and vally +> **rejects** a spec declaring both. Many existing evals still open with `config:`; when you add +> `runs`, merge the two into a single `defaults:` block. The failure is silent: the job exits 0 with +> no verdicts and the PR comment blames "transient infrastructure". + Each skill is evaluated in up to three variants — **baseline** (no skills), **skilled** (only the skill under test), and **plugin** (the whole plugin loaded) — and a skill "passes" only when the skilled run is a *credible* improvement over baseline. To assert that a skill should stay dormant for an out-of-scope task, add `expect_activation: false` to that stimulus. See any existing `tests/*/*/eval.yaml` for a fuller example of the grader and stimulus format. +#### Size the eval so it can return a verdict + +The pass gate has two independent bars. `trials = stimuli × runs`. + +1. **Counted trials ≥ 5**, else the verdict is reported `underpowered` — never a pass, never a + regression. +2. **p ≤ 0.05 on an exact one-sided sign test over the *discordant* (non-tie) trials.** Ties are not + discarded; they hold the discordant count down. + +| discordant trials | records that pass | p | +| ---: | --- | ---: | +| ≤ 4 | none, however good the skill | ≥ 0.0625 | +| 5–7 | zero losses only (5W/0L) | 0.031 | +| 8 | one loss survivable (7W/1L) | 0.035 | + +At exactly 5 counted trials a single tie is fatal — it leaves 4 discordant. At 6 counted trials one +tie is survivable (5W/1T/0L); at 7, up to two are (5W/2T/0L). A loss is not. Five is an *eligibility +floor*, not adequate +power. A run that measured a 32% tie rate certified a +genuinely-helping five-trial eval about one time in ten; at fifteen trials, about nine times in ten. +Prefer adding **discriminating stimuli** over raising `runs` — repeats measure the same task. See +[`eng/eval-quality/README.md`](eng/eval-quality/README.md) for the full derivation and for the ten +structural defects the CI quality gate blocks. + +Run the gate locally before pushing: + +```bash +python eng/eval-quality/check_eval_quality.py +``` + @@ -290,14 +332,17 @@ Prerequisites: Node.js 20+ and the [GitHub CLI](https://cli.github.com) signed i ./eng/run-skill-evals.sh ``` -Per-skill verdicts are written to `./eval-results///results.json`, and the raw experiment output goes to `./eval-results/_experiment/`. Model, judge model, and runs-per-stimulus come from the `overrides:` block in `dotnet-skills.experiment.yaml`. +Per-skill verdicts are written to `./eval-results///results.json`, and the raw experiment output goes to `./eval-results/_experiment/`. Model and judge model come from the `overrides:` block in `dotnet-skills.experiment.yaml`. -> [!WARNING] -> LLM evaluations are noisy. For results you intend to share in a Pull Request, raise `runs` in `dotnet-skills.experiment.yaml` to at least 3 (5 is better) for reliable signal. +> [!WARNING] +> LLM evaluations are noisy. Runs-per-stimulus is deliberately **not** set in +> `dotnet-skills.experiment.yaml`: an experiment-level `runs` overwrites every eval's own value +> instead of defaulting it, making per-eval trial counts impossible to express. Raise the eval's own +> `defaults.runs` instead — or, better, add discriminating stimuli. ### CI evaluation -Tests run automatically on pull requests that modify files under `plugins/`. The evaluation workflow discovers changed plugins and evaluates each one. Results are posted as a PR comment and uploaded as build artifacts. +Tests do **not** run automatically on pull requests. When a PR changes skills, the `pr-status` job posts a pending commit status and a maintainer must trigger the evaluation, binding it to a specific reviewed commit — either by submitting a PR review ("Files changed" → "Review changes") whose body contains `/evaluate` (recommended, no SHA to copy), or by commenting `/evaluate `. A bare `/evaluate` comment only posts guidance. Results are posted as a PR comment and uploaded as build artifacts. If a scenario fails or regresses, see [Investigating Results](eng/vally-adapter/InvestigatingResults.md) for how to download artifacts, interpret `results.json`, and diagnose common failure patterns. @@ -374,6 +419,60 @@ Skills and agents in this repo should be: - **Verifiable**: always include a way to validate success. - **Tool-conscious**: don't assume capabilities that might not exist in every runtime. +### What consistently separates a passing skill from a failing one + +Every skill is scored head-to-head against the *same model with no skill loaded*, so the score is a +**delta**. The rules below are the ones this repo has learned the hard way, each from a merged fix. + +**Content** + +- Encode the decisions the model gets wrong; delete anything it already produces unaided. A skill + that reads as reference prose ties its own baseline. +- Prefer "when A, do B, never C, verify D" tables over lists of plausible alternatives, and end with + a concrete output contract (the exact command, the verdict line, the findings table). +- Scale output structure to input size. A twelve-section report for an eight-test suite loses to a + concise direct answer. +- Add stop-conditions so a strong skill doesn't over-apply — then check you haven't over-corrected + into answering more narrowly than the baseline did. +- Tell the agent to discover repo paths rather than listing them as required inputs; a "required" + project path makes the agent ask the user for a file that is already in the working directory. +- Require truthful validation reporting. Claiming "Build succeeded" after a failed restore is an + automatic loss. +- Verify load-bearing API claims by compiling or probing, not by reading source. +- Keep the common path in `SKILL.md` and gate rare or expensive paths behind `references/` reads. + +**Activation** + +- The `description` is the only text the runtime sees when choosing a skill. Put the user's own + words in it: symptoms, error codes, artifact names, quoted requests. +- Partition against sibling skills on the real discriminator, not the shared topic, and add the + matching exclusion to both siblings. +- Re-read every "do not use for" clause against the scenarios the skill exists to serve — an + exclusion can lock out the skill's own purpose. +- Watch both budgets: 1,024 characters per description, and the plugin's rendered skill menu. + +**When something fails** + +- Classify before you rewrite. Broken fixtures, underpowered trial counts, forced tools, stale spec + keys and harness errors have all masqueraded as skill regressions. +- Read the losing trial and the judge's stated reason, and drive the fix from that evidence rather + than from style preference. +- A positive win/tie/loss record with a failing verdict is a power problem, not a content problem. +- A skill that is weak across model families, thinly used, and costing menu budget is a candidate + for retirement, not indefinite polishing. + +### Authoring skills for this repository + +The repository ships agent skills for working on itself, under `.agents/skills/`: + +| Skill | Use it when | +|-------|-------------| +| `create-skill` | Scaffolding a new skill and writing a description the runtime will route to | +| `create-skill-test` | Writing or resizing an `eval.yaml` | +| `improve-skill-quality` | An evaluation regressed, returned no verdict, or the skill didn't activate | +| `create-custom-agent` | Adding an agent definition | +| `authoring-github-workflows` | Editing anything under `.github/workflows/` | + ## Skill-Validator & Evaluation workflow Changes to `eng/skill-validator` or the `.github/workflows/evaluation*.yml` workflows must be made from a branch in the `dotnet/skills` repository (i.e., not from a fork). This is a security measure. diff --git a/eng/eval-quality/README.md b/eng/eval-quality/README.md index c58b9b5114..0ff70f1447 100644 --- a/eng/eval-quality/README.md +++ b/eng/eval-quality/README.md @@ -147,11 +147,11 @@ measures the real property. ### 8. Fewer than 5 trials behind a verdict -Trials, not scenarios, are what the pass gate is computed over. `vally compare` +Trials, not stimuli, are what the pass gate is computed over. `vally compare` produces one head-to-head trial per stimulus per run, so ``` -trials = scenarios × defaults.runs +trials = stimuli × defaults.runs ``` and the gate is an exact one-sided **sign test**: more wins than losses, at @@ -177,10 +177,11 @@ considerably more. Below it, `eng/vally-adapter/adapt.mjs` marks the verdict This check makes that state un-shippable for *new* evals. > **Landing on 5 exactly is a trap, and the gate now warns about it.** The table -> above is the *best possible* record. At 5, 6 or 7 trials the only record that -> passes is every trial a win — no ties, no losses — because the sign test -> conditions on the discordant trials and one tie at n=5 leaves 4, back below the -> floor. Tolerating a single loss needs 8 discordant trials. +> above is the *best possible* record. A pass needs **five discordant (non-tie) +> trials with no losses**, so at exactly 5 trials a single tie is fatal — it +> leaves 4 discordant, back below the floor. At 6 trials one tie is survivable +> (5W/1T/0L is 5 discordant and passes at p = 0.031) and at 7 trials up to two +> are, but a loss still is not: tolerating one needs 8 discordant trials. > > Run `30611635547` is the worked example. Five `dotnet-test` evals had just been > raised to exactly 5 trials. They returned **16W / 8T / 1L** overall — every @@ -351,9 +352,10 @@ available. See check 8 for how, and for why the floor sits at five. ### Evals parked at the floor -Evals at 5–7 trials, where the only passing record is a flawless sweep. These -*are* eligible for a verdict, so they are not underpowered — they are simply -one tie away from being unable to produce one. See the callout under check 8 for +Evals at 5–7 trials, where a pass still requires a loss-free record and enough +non-tie trials to clear the floor. These *are* eligible for a verdict, so they +are not underpowered — but at 5 trials a single tie removes the possibility of +one, and at 6–7 it takes only one or two more. See the callout under check 8 for the run that made this concrete. Raise them unless their scenarios are near-certain discriminators.