Skip to content

test(dotnet-test): pin fixture SDK for P0 reliability skills (writing-mstest-tests, detect-static-dependencies) - #907

Merged
Evangelink merged 3 commits into
mainfrom
dev/amauryleve/p0-reliability-eval-fixes
Jul 16, 2026
Merged

test(dotnet-test): pin fixture SDK for P0 reliability skills (writing-mstest-tests, detect-static-dependencies)#907
Evangelink merged 3 commits into
mainfrom
dev/amauryleve/p0-reliability-eval-fixes

Conversation

@Evangelink

@Evangelink Evangelink commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary

Addresses the two P0 FIX-RELIABILITY items in #899 from the dotnet-test cross-family skill evaluation:

  • writing-mstest-tests — 2 errored trials
  • detect-static-dependencies — 1 errored trial

Per the InvestigatingResults guide, I downloaded the eval artifacts from run 29228914412 and captured the actual stderr of every errored trial before changing anything.

Root cause (what the stderr actually showed)

All three "errored" trials are judge-side infrastructure failures — not fixture nondeterminism:

Skill Family Scenario Error
detect-static-dependencies haiku Verify structured report… Comparison judge failed: CAPIError: 400 This organization has been disabled
writing-mstest-tests haiku Write async tests with cancellation Comparison judge failed: Timeout after 120000ms waiting for session.idle
writing-mstest-tests sonnet46 Use comparison assertions for boundary testing Comparison judge failed: Timeout after 120000ms waiting for session.idle

Evidence the executor setup is already sound:

  • 42/42 opus trials for both skills completed successfully.
  • detect-static-dependencies never builds (0/30 trials ran dotnet build/test) — it is a pure source scan.
  • writing-mstest-tests ran dotnet test in 19/75 trials with no SDK-not-found or NuGet errors.
  • The Use comparison assertions scenario that timed out has reject_tools: [bash] and no fixtures at all — it cannot be a fixture/SDK problem.

The eval.vally.yaml schema exposes only config.timeout (executor); there is no per-eval judge timeout/retry knob, and the judge token pool is infra managed by validate-pat-pool.yml. So the report's generic "pin SDK/tool versions" prescription does not match these specific errors.

What this PR changes (defense-in-depth determinism insurance)

Even though pinning wouldn't have prevented these judge-side flakes, the fixtures previously floated on the runner's ambient SDK (no global.json), unlike reliable siblings such as mtp-hot-reload and run-tests. This PR closes that gap:

  1. Adds a pinned global.json (rollForward: latestMajor) co-located with every buildable fixture:
    • writing-mstest: legacy-antipatterns (net8), modern-mstest (net9), service-registry (net9)
    • detect-static: static-heavy-project, lambda-statics-project, obj-exclusion-project (net10)
  2. Wires the pin into the flat-copy build-capable scenarios in both eval.vally.yaml and eval.yaml (whole-./fixtures and whole-folder copies auto-inherit it).

rollForward: latestMajor guarantees the pin never causes an "SDK not found" failure. Verified locally: all fixtures resolve their SDK cleanly with the pins in place.

No skill content was changed — this is strictly reliability/determinism.

The real fix (tracked separately)

The observed errors are judge-side and belong in the harness, not the fixtures — tracked in #909:

  • Retry the comparison judge on transient failures (CAPIError 4xx, session.idle timeout).
  • Add a token-pool pre-flight to drop disabled/throttled COPILOT_PAT entries before a run.

Refs #899. Follow-up: #909.

…-tests and detect-static-dependencies

P0 reliability hardening for the cross-family eval (#899). Adds a pinned
global.json (rollForward: latestMajor) to every buildable fixture in both
skills and wires it into the flat-copy scenarios so the executor arm no
longer floats on the runner's ambient SDK.

Note: the 3 errored trials in run 29228914412 were judge-side infra
failures (CAPIError 400 'organization disabled' + 120s judge session.idle
timeout), not fixture nondeterminism. These pins are defense-in-depth; the
real fix (judge-error retry / token-pool pre-flight) is a harness follow-up.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5474f870-6be7-4b24-a5e1-a6e1e6de4ace
Copilot AI review requested due to automatic review settings July 16, 2026 13:30
@github-actions

Copy link
Copy Markdown
Contributor

Skill Coverage Report

Plugin Skill Covered Coverage
dotnet-test detect-static-dependencies 15/15 100%
dotnet-test writing-mstest-tests 45/45 100%

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves reliability for dotnet-test skill evaluations by pinning a global.json (with rollForward: latestMajor) into buildable fixtures and ensuring evaluation scenarios copy it into the working directory.

Changes:

  • Added global.json pins to multiple buildable fixture folders for writing-mstest-tests and detect-static-dependencies.
  • Updated writing-mstest-tests eval.yaml and eval.vally.yaml scenarios to include global.json in the scratch environment for build-capable scenarios.
Show a summary per file
File Description
tests/dotnet-test/writing-mstest-tests/fixtures/service-registry/global.json Adds an SDK pin for the service-registry fixture.
tests/dotnet-test/writing-mstest-tests/fixtures/modern-mstest/global.json Adds an SDK pin for the modern-mstest fixture.
tests/dotnet-test/writing-mstest-tests/fixtures/legacy-antipatterns/global.json Adds an SDK pin for the legacy-antipatterns fixture.
tests/dotnet-test/writing-mstest-tests/eval.yaml Copies global.json into relevant scenarios (one scenario currently embeds it inline).
tests/dotnet-test/writing-mstest-tests/eval.vally.yaml Copies global.json into relevant Vally scenarios.
tests/dotnet-test/detect-static-dependencies/fixtures/static-heavy-project/global.json Adds an SDK pin for the fixture.
tests/dotnet-test/detect-static-dependencies/fixtures/obj-exclusion-project/global.json Adds an SDK pin for the fixture.
tests/dotnet-test/detect-static-dependencies/fixtures/lambda-statics-project/global.json Adds an SDK pin for the fixture.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 8/8 changed files
  • Comments generated: 1

Comment thread tests/dotnet-test/writing-mstest-tests/eval.yaml Outdated
@Evangelink
Evangelink enabled auto-merge (squash) July 16, 2026 13:41
…tead of inline pin

Addresses PR review: the base eval.yaml scenario embedded global.json inline
while the same pin exists at fixtures/service-registry/global.json, creating
two sources of truth that could drift between the vally and skill-validator
evals. Point the setup file at the fixture so the SDK pin has one source.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5474f870-6be7-4b24-a5e1-a6e1e6de4ace
Copilot AI review requested due to automatic review settings July 16, 2026 13:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new

@Evangelink

Copy link
Copy Markdown
Member Author

/evaluate

@github-actions github-actions Bot added the pr-state/ready-for-eval PR is mergeable and awaiting evaluation label Jul 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Skill Validation Results

Skill Scenario Quality Skills Loaded Overfit Verdict
detect-static-dependencies Identify static dependencies in a multi-class project 4.3/5 → 4.3/5 ✅ detect-static-dependencies; tools: skill, glob ✅ 0.08 [1]
detect-static-dependencies Detect time-related statics and recommend TimeProvider 5.0/5 → 4.7/5 🔴 ✅ detect-static-dependencies; tools: skill, glob / ✅ detect-static-dependencies; tools: skill ✅ 0.08 [2]
detect-static-dependencies Decline scan for non-C# project 4.3/5 → 5.0/5 🟢 ✅ detect-static-dependencies; tools: skill / ℹ️ not activated (expected) ✅ 0.08 [3]
detect-static-dependencies Verify structured report includes file count, categories, and top patterns 5.0/5 → 5.0/5 ✅ detect-static-dependencies; tools: skill, glob ✅ 0.08 [4]
detect-static-dependencies Exclude obj and bin directories from the scan 5.0/5 → 5.0/5 ✅ detect-static-dependencies; tools: skill, glob ✅ 0.08 [5]
detect-static-dependencies Detect statics inside lambda expressions and LINQ queries 4.0/5 → 3.7/5 🔴 ✅ detect-static-dependencies; tools: skill, glob ✅ 0.08 [6]
writing-mstest-tests Write unit tests for a service class 3.7/5 → 4.0/5 🟢 ✅ writing-mstest-tests; tools: skill, glob / ✅ writing-mstest-tests; tools: skill 🟡 0.40 [7]
writing-mstest-tests Write data-driven tests for a calculator 2.7/5 → 3.7/5 🟢 ✅ writing-mstest-tests; tools: skill 🟡 0.40 [8]
writing-mstest-tests Write async tests with cancellation 3.0/5 → 5.0/5 🟢 ✅ writing-mstest-tests; tools: skill / ✅ writing-mstest-tests; tools: report_intent, skill 🟡 0.40
writing-mstest-tests Fix swapped Assert.AreEqual arguments 3.0/5 → 4.3/5 🟢 ✅ writing-mstest-tests; tools: report_intent, skill / ⚠️ NOT ACTIVATED 🟡 0.40 [9]
writing-mstest-tests Modernize legacy test patterns 4.3/5 → 4.3/5 ✅ writing-mstest-tests; tools: skill 🟡 0.40 [10]
writing-mstest-tests Replace ExpectedException with Assert.Throws 2.7/5 → 3.0/5 🟢 ⚠️ NOT ACTIVATED 🟡 0.40 [11]
writing-mstest-tests Use proper collection assertions 3.0/5 → 5.0/5 🟢 ✅ writing-mstest-tests; tools: skill 🟡 0.40 [12]
writing-mstest-tests Use proper type assertions instead of casts 3.3/5 → 2.7/5 🔴 ⚠️ NOT ACTIVATED 🟡 0.40 [13]
writing-mstest-tests Set up test lifecycle correctly 2.3/5 → 4.0/5 🟢 ✅ writing-mstest-tests; tools: skill, glob, create, bash, edit, report_intent / ✅ writing-mstest-tests; tools: skill, report_intent 🟡 0.40 [14]
writing-mstest-tests Use DynamicData with ValueTuples over object arrays 3.0/5 → 5.0/5 🟢 ✅ writing-mstest-tests; tools: skill / ⚠️ NOT ACTIVATED 🟡 0.40 [15]
writing-mstest-tests Use string assertions for format validation 4.0/5 → 4.3/5 🟢 ✅ writing-mstest-tests; tools: skill 🟡 0.40 [16]
writing-mstest-tests Use comparison assertions for boundary testing 2.7/5 → 4.7/5 🟢 ✅ writing-mstest-tests; tools: skill / ✅ writing-mstest-tests; tools: skill, report_intent 🟡 0.40
writing-mstest-tests Write tests with collection, null, and reference assertions 3.3/5 → 3.3/5 ✅ writing-mstest-tests; tools: skill 🟡 0.40
writing-mstest-tests Configure conditional execution, retry, and cleanup 2.3/5 → 5.0/5 🟢 ✅ writing-mstest-tests; tools: skill / ✅ writing-mstest-tests; tools: report_intent, skill 🟡 0.40
writing-mstest-tests Configure test parallelization and MSTest.Sdk project 3.0/5 → 5.0/5 🟢 ✅ writing-mstest-tests; tools: skill 🟡 0.40

[1] ⚠️ High run-to-run variance (CV=130%) — consider re-running with --runs 5
[2] ⚠️ High run-to-run variance (CV=73%) — consider re-running with --runs 5
[3] ⚠️ High run-to-run variance (CV=749%) — consider re-running with --runs 5. (Plugin) Quality unchanged but weighted score is -4.0% due to: quality, tokens (38734 → 52554), time (18.9s → 24.5s)
[4] (Isolated) Quality unchanged but weighted score is -15.4% due to: completion (✓ → ✗), tokens (56288 → 79837), time (55.2s → 75.6s), tool calls (8 → 10)
[5] ⚠️ High run-to-run variance (CV=207%) — consider re-running with --runs 5
[6] ⚠️ High run-to-run variance (CV=791%) — consider re-running with --runs 5. (Plugin) Quality unchanged but weighted score is -6.8% due to: tokens (52291 → 98347), time (21.5s → 35.1s)
[7] ⚠️ High run-to-run variance (CV=1304%) — consider re-running with --runs 5. (Isolated) Quality improved but weighted score is -7.3% due to: judgment
[8] ⚠️ High run-to-run variance (CV=98%) — consider re-running with --runs 5
[9] ⚠️ High run-to-run variance (CV=72%) — consider re-running with --runs 5. (Plugin) Quality unchanged but weighted score is -1.6% due to: tokens (12916 → 17681)
[10] ⚠️ High run-to-run variance (CV=144%) — consider re-running with --runs 5. (Isolated) Quality unchanged but weighted score is -23.3% due to: judgment, quality, tokens (169281 → 214080)
[11] ⚠️ High run-to-run variance (CV=100%) — consider re-running with --runs 5
[12] ⚠️ High run-to-run variance (CV=61%) — consider re-running with --runs 5
[13] ⚠️ High run-to-run variance (CV=138%) — consider re-running with --runs 5
[14] ⚠️ High run-to-run variance (CV=68%) — consider re-running with --runs 5
[15] ⚠️ High run-to-run variance (CV=85%) — consider re-running with --runs 5. (Plugin) Quality unchanged but weighted score is -12.8% due to: judgment, tokens (12929 → 17480)
[16] ⚠️ High run-to-run variance (CV=55%) — consider re-running with --runs 5

Model: claude-opus-4.6 | Judge: claude-opus-4.6

🔍 Full Results - additional metrics and failure investigation steps

To investigate failures, paste this to your AI coding agent:

For PR 907 in dotnet/skills, download eval artifacts with gh run download 29504559036 --repo dotnet/skills --pattern "skill-validator-results-*" --dir ./eval-results, then fetch https://raw.githubusercontent.com/dotnet/skills/6db3862b6c5c65ce081bc81edbba3393b3549aa1/eng/skill-validator/src/docs/InvestigatingResults.md and follow it to analyze the results.json files. Diagnose each failure, suggest fixes to the eval.yaml and skill content, and tell me what to fix first.

▶ Sessions Visualisation -- interactive replay of all evaluation sessions
📊 Session Analytics (preview) -- aggregated metrics across evaluation sessions

…k-imperatives

Fixes skill activation for 4 scenarios where eager models answered the passive
advice-question prompts inline without invoking the skill (misses concentrated
on weaker models). Reframes 'Is there a better...?' / 'How should I...?' into
directive tasks ('Review and fix...', 'Modernize...', 'Rewrite...') so the
request clearly warrants the skill, matching its review/fix/modernize triggers.

Scenarios: Fix swapped Assert.AreEqual arguments, Replace ExpectedException with
Assert.Throws, Use proper type assertions instead of casts, Use DynamicData with
ValueTuples over object arrays. Code blocks, graders, and rubrics unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5474f870-6be7-4b24-a5e1-a6e1e6de4ace
Copilot AI review requested due to automatic review settings July 16, 2026 14:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 8/8 changed files
  • Comments generated: 0 new

@github-actions

Copy link
Copy Markdown
Contributor

Skill Validation Results

Skill Scenario Quality Skills Loaded Overfit Verdict
detect-static-dependencies Identify static dependencies in a multi-class project 5.0/5 → 5.0/5 ✅ detect-static-dependencies; tools: skill / ✅ detect-static-dependencies; tools: skill, glob ✅ 0.10 [1]
detect-static-dependencies Detect time-related statics and recommend TimeProvider 4.0/5 → 4.0/5 ✅ detect-static-dependencies; tools: skill, glob ✅ 0.10 [2]
detect-static-dependencies Decline scan for non-C# project 4.7/5 → 5.0/5 🟢 ✅ detect-static-dependencies; tools: skill, glob / ℹ️ not activated (expected) ✅ 0.10 [3]
detect-static-dependencies Verify structured report includes file count, categories, and top patterns 4.7/5 → 5.0/5 🟢 ✅ detect-static-dependencies; tools: skill, glob / ✅ detect-static-dependencies; tools: skill ✅ 0.10 [4]
detect-static-dependencies Exclude obj and bin directories from the scan 5.0/5 → 5.0/5 ✅ detect-static-dependencies; tools: skill, glob ✅ 0.10 [5]
detect-static-dependencies Detect statics inside lambda expressions and LINQ queries 4.0/5 → 4.0/5 ✅ detect-static-dependencies; tools: skill / ✅ detect-static-dependencies; tools: skill, glob ✅ 0.10 [6]
writing-mstest-tests Write unit tests for a service class 3.0/5 → 3.3/5 🟢 ✅ writing-mstest-tests; tools: skill, glob, report_intent, view, create 🟡 0.31 [7]
writing-mstest-tests Write data-driven tests for a calculator 1.7/5 → 3.7/5 🟢 ✅ writing-mstest-tests; tools: report_intent, skill 🟡 0.31 [8]
writing-mstest-tests Write async tests with cancellation 3.0/5 → 5.0/5 🟢 ✅ writing-mstest-tests; tools: skill / ✅ writing-mstest-tests; tools: report_intent, skill 🟡 0.31
writing-mstest-tests Fix swapped Assert.AreEqual arguments 3.3/5 → 3.3/5 ✅ writing-mstest-tests; tools: skill, report_intent / ⚠️ NOT ACTIVATED 🟡 0.31 [9]
writing-mstest-tests Modernize legacy test patterns 4.0/5 → 5.0/5 🟢 ✅ writing-mstest-tests; tools: skill / ⚠️ NOT ACTIVATED 🟡 0.31 [10]
writing-mstest-tests Replace ExpectedException with Assert.Throws 3.3/5 → 3.3/5 ✅ writing-mstest-tests; tools: skill / ⚠️ NOT ACTIVATED 🟡 0.31 [11]
writing-mstest-tests Use proper collection assertions 3.0/5 → 4.0/5 🟢 ✅ writing-mstest-tests; tools: skill / ✅ writing-mstest-tests; tools: report_intent, skill 🟡 0.31
writing-mstest-tests Use proper type assertions instead of casts 4.0/5 → 3.3/5 🔴 ✅ writing-mstest-tests; tools: skill / ✅ writing-mstest-tests; tools: report_intent, skill 🟡 0.31 [12]
writing-mstest-tests Set up test lifecycle correctly 2.0/5 → 4.3/5 🟢 ✅ writing-mstest-tests; tools: report_intent, skill 🟡 0.31
writing-mstest-tests Use DynamicData with ValueTuples over object arrays 3.0/5 → 3.0/5 ✅ writing-mstest-tests; tools: report_intent, skill 🟡 0.31 [13]
writing-mstest-tests Use string assertions for format validation 3.7/5 → 4.0/5 🟢 ✅ writing-mstest-tests; tools: skill, bash, edit, view 🟡 0.31 [14]
writing-mstest-tests Use comparison assertions for boundary testing 2.7/5 → 4.0/5 🟢 ✅ writing-mstest-tests; tools: skill / ✅ writing-mstest-tests; tools: report_intent, skill 🟡 0.31
writing-mstest-tests Write tests with collection, null, and reference assertions 3.7/5 → 3.0/5 🔴 ✅ writing-mstest-tests; tools: glob, skill 🟡 0.31
writing-mstest-tests Configure conditional execution, retry, and cleanup 3.0/5 → 5.0/5 🟢 ✅ writing-mstest-tests; tools: report_intent, skill 🟡 0.31
writing-mstest-tests Configure test parallelization and MSTest.Sdk project 3.0/5 → 5.0/5 🟢 ✅ writing-mstest-tests; tools: skill 🟡 0.31

[1] ⚠️ High run-to-run variance (CV=96%) — consider re-running with --runs 5. (Plugin) Quality unchanged but weighted score is -2.2% due to: tokens (54170 → 92892)
[2] (Plugin) Quality unchanged but weighted score is -10.4% due to: tokens (36341 → 100479), tool calls (5 → 10), time (26.3s → 46.3s), quality
[3] (Plugin) Quality unchanged but weighted score is -8.7% due to: quality, tokens (38571 → 65360), time (23.2s → 31.4s), tool calls (4 → 5)
[4] ⚠️ High run-to-run variance (CV=142%) — consider re-running with --runs 5
[5] ⚠️ High run-to-run variance (CV=54%) — consider re-running with --runs 5. (Plugin) Quality unchanged but weighted score is -3.9% due to: tokens (52621 → 94326)
[6] ⚠️ High run-to-run variance (CV=70%) — consider re-running with --runs 5. (Isolated) Quality unchanged but weighted score is -27.9% due to: quality, judgment, tokens (52269 → 75342), tool calls (5 → 7), time (22.4s → 28.9s)
[7] ⚠️ High run-to-run variance (CV=3075%) — consider re-running with --runs 5. (Isolated) Quality improved but weighted score is -6.8% due to: judgment, quality
[8] ⚠️ High run-to-run variance (CV=128%) — consider re-running with --runs 5
[9] ⚠️ High run-to-run variance (CV=165%) — consider re-running with --runs 5
[10] ⚠️ High run-to-run variance (CV=189%) — consider re-running with --runs 5
[11] ⚠️ High run-to-run variance (CV=700%) — consider re-running with --runs 5. (Isolated) Quality unchanged but weighted score is -2.2% due to: tokens (25140 → 30841), time (10.0s → 14.4s)
[12] ⚠️ High run-to-run variance (CV=64%) — consider re-running with --runs 5
[13] ⚠️ High run-to-run variance (CV=1123%) — consider re-running with --runs 5. (Plugin) Quality unchanged but weighted score is -4.2% due to: tokens (12899 → 40025), tool calls (0 → 2), time (10.2s → 13.2s)
[14] ⚠️ High run-to-run variance (CV=83%) — consider re-running with --runs 5. (Isolated) Quality improved but weighted score is -6.0% due to: judgment

Model: claude-opus-4.6 | Judge: claude-opus-4.6

🔍 Full Results - additional metrics and failure investigation steps

To investigate failures, paste this to your AI coding agent:

For PR 907 in dotnet/skills, download eval artifacts with gh run download 29509131231 --repo dotnet/skills --pattern "skill-validator-results-*" --dir ./eval-results, then fetch https://raw.githubusercontent.com/dotnet/skills/17d0e206c4d2fe3d88561eaa3ae71de5feea0339/eng/skill-validator/src/docs/InvestigatingResults.md and follow it to analyze the results.json files. Diagnose each failure, suggest fixes to the eval.yaml and skill content, and tell me what to fix first.

▶ Sessions Visualisation -- interactive replay of all evaluation sessions
📊 Session Analytics (preview) -- aggregated metrics across evaluation sessions

@github-actions github-actions Bot added waiting-on-review PR state label and removed pr-state/ready-for-eval PR is mergeable and awaiting evaluation labels Jul 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

✅ Evaluation passed for 17d0e20. cc @dotnet/dotnet-testing — please review.

@Evangelink
Evangelink merged commit a9fc55a into main Jul 16, 2026
37 checks passed
@Evangelink
Evangelink deleted the dev/amauryleve/p0-reliability-eval-fixes branch July 16, 2026 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-on-review PR state label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants