Skip to content

Improve .NET performance skill review workflow#912

Draft
artl93 wants to merge 3 commits into
artl93-improve-performance-skill-qualityfrom
artl93-stack-performance-skill-guidance
Draft

Improve .NET performance skill review workflow#912
artl93 wants to merge 3 commits into
artl93-improve-performance-skill-qualityfrom
artl93-stack-performance-skill-guidance

Conversation

@artl93

@artl93 artl93 commented Jul 17, 2026

Copy link
Copy Markdown
Member

Follow-up to #886.

This stacked PR applies only the evidence-backed skill guidance changes after the eval criteria introduced by #905:

  • read source before selecting confirmation searches
  • load only references needed for the detected categories
  • batch confirmation searches by category
  • report a concise category checklist instead of per-recipe zero-hit tool calls

Baseline coverage support

The automated skill coverage report is unchanged at 11/21 units (52.4%) on the frozen evaluator and this stacked follow-up. The ten uncovered units are five final-output validation checks (including the checklist requirement) and five separate pitfall guardrails; this PR therefore does not claim increased declarative coverage or broaden the evaluator. It preserves the existing baseline while changing how the agent gathers evidence during covered focused reviews.

Scenarios 8 and 9 provide the targeted behavioral probe for this workflow: they review hot-path string/collection code with outcome-focused criteria and no prescribed search vocabulary. In the independent five-family combined rerun used as supporting—not causal—evidence, both scenarios activated in all 10 skilled cells, all 10 graders passed, 53/55 rubric criteria were satisfied, average skilled tool calls fell 41.3% versus the historical run, and skilled-versus-baseline overhead fell from +5.93 to +1.36 calls (-77%). Those results support source-first review and batched confirmation while the unchanged 11/21 coverage boundary makes clear what remains untested.

#905 must merge first so the evaluation criteria are frozen before this skill-only treatment is considered. This PR intentionally does not modify eval YAML, fixtures, workflows, the vally adapter, or reliability behavior.

Validation:

  • skill-validator check --skills plugins/dotnet-diag/skills/analyzing-dotnet-performance
  • markdownlint-cli2 plugins/dotnet-diag/skills/analyzing-dotnet-performance/SKILL.md
  • base-relative diff contains exactly plugins/dotnet-diag/skills/analyzing-dotnet-performance/SKILL.md

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c7960b02-a0d3-4c71-8b65-3b238a9b7b42
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Skill Coverage Report

Plugin Skill Covered Coverage
⚠️ dotnet-diag analyzing-dotnet-performance 12/24 50%
Uncovered: dotnet-diag/analyzing-dotnet-performance
  • [Validation] Source was read without modification unless implementation was requested (line 192)
  • [Validation] Available references were loaded only for selected topics and applicable critical/structural checks (line 193)
  • [Validation] Coverage ledger reconciles every applicable recipe and manual-review pattern (line 194)
  • [Validation] Each finding has verified counts, locations, evidence, and a concrete fix (line 195)
  • [Validation] Relevant positive/inverse patterns and exclusions were preserved (line 196)
  • [Validation] Category checklist covers all applicable performance categories (line 197)
  • [Validation] Summary table included at end (line 198)
  • [Pitfall] Suggesting Span in async methods (line 206)
  • [Pitfall] Suggesting ConfigureAwait(false) in app code (line 208)
  • [Pitfall] Recommending ValueTask everywhere (line 209)
  • [Pitfall] Flagging new HttpClient() in DI services (line 210)
  • [Pitfall] Suggesting CollectionsMarshal.AsSpan broadly (line 212)

@artl93

artl93 commented Jul 18, 2026

Copy link
Copy Markdown
Member Author

Superseded: This no-skill-vs-skill run did not measure PR #912’s incremental change. See the corrected main-skill-vs-PR-skill A/B: #912 (comment)

Five-family no-skill vs skill A/B evidence

Run: https://github.com/dotnet/skills/actions/runs/29625558673
Disposable integration branch/SHA: artl93-run-performance-ab / fdfece783f28c633d03e26de7661caad4462382d

This run started from current main (ab72985132b79adcc4818d1fc5c41d9543f12498, merged #887 reliability behavior), layered exact PR #905 content commit 0b7ae60bec7c7fa37b98ae6c3fb4f286d70f426a, then exact PR #912 content commit 16268bb04a4a3008ec82eff42fc6800da682bcf1, plus a temporary manual cross-family dispatch harness. Before the harness edit, the product diff from main was exactly the two eval files and SKILL.md.

Pre-dispatch validation passed: pinned actionlint 1.7.7 across all 16 hand-authored workflows; all 4 focused Vally adapter reliability tests; static skill-validator check for dotnet-diag; exact canonical/Vally parity for all 11 scenario names, prompts, fixtures, assertions, and rubrics; timeout coverage; git diff --check; intended-diff and #887 ancestry checks. The 10 run summaries also confirm runs: 1, the five pinned executor models, environment.skills: [] for every baseline, and exactly plugins/dotnet-diag/skills/analyzing-dotnet-performance for every treatment.

Quality result

All five executor jobs completed successfully and produced the expected 110 raw trajectories (5 families × 11 stimuli × 2 variants). Vally produced 47 matched comparisons and 8 explicit unmatched baseline-error cases; there were 0 comparison-judge errors. Across matched comparisons the descriptive W/T/L count was 37/7/3, but I do not pool preference scores across families because the executor/judge pair differs.

Family Executor / comparison judge Treatment preference (95% CI) W/T/L McNemar B-only/T-only (p) Executor success B/T Grader pass B/T Unmatched Treatment activation Interpretation
GPT gpt-5.5 / claude-opus-4.8 +58.0% (+37.3%, +78.7%) 10/0/0 0/3 (0.25) 10/11 / 11/11 7/11 / 11/11 1 11/11 Strong positive matched evidence, formally inconclusive because one baseline trajectory is unmatched
Haiku claude-haiku-4.5 / claude-opus-4.8 +45.5% (+33.3%, +57.6%) 11/0/0 0/2 (0.50) 11/11 / 11/11 9/11 / 11/11 0 11/11 Complete and credibly better
MAI mai-code-1-flash-picker / claude-opus-4.8 +26.7% (+4.9%, +48.4%) 7/1/1 0/2 (0.50) 9/11 / 11/11 4/11 / 8/11 2 10/11 Positive matched evidence, formally inconclusive because two baselines are unmatched
Opus claude-opus-4.8 / gpt-5.5 +12.7% (-18.1%, +43.5%) 7/2/2 0/0 (n/a) 11/11 / 11/11 11/11 / 11/11 0 11/11 Complete but not credible; CI includes zero
Sonnet 4.6 claude-sonnet-4.6 / claude-opus-4.8 +13.3% (-8.3%, +35.0%) 2/4/0 0/0 (n/a) 6/11 / 11/11 6/11 / 11/11 5 11/11 Inconclusive because five baseline executor errors are unmatched

The causal evidence is the within-run pairing above. It supports a clear quality gain for Haiku and strong positive but incomplete evidence for GPT and MAI. It does not establish a credible gain for Opus, and Sonnet is too incomplete to interpret beyond the six matched trials.

Per-scenario evidence

Each cell is descriptive only: there is one trial per family/scenario, so scenario-level results are not statistically significant. U is unmatched; grader outcomes are treatment-only / baseline-only / both-pass / both-fail among matched pairs.

Scenario W/T/L/U Grader outcomes T-only/B-only/BP/BF
Catches LINQ on hot-path string processing 4/1/0/0 0/0/5/0
Compound allocations in recursive converter 4/1/0/0 0/0/5/0
Aggregate + Replace / missing IEquatable 4/0/0/1 2/0/1/1
CurrentCulture comparer / compiled regex budget 4/0/0/1 2/0/2/0
TimeSpan and collection LINQ pipeline 3/1/1/0 0/0/5/0
Compiled regex startup / regex-chain allocations 4/1/0/0 0/0/4/1
Ordinal / FrozenDictionary opportunities 3/0/0/2 1/0/2/0
Branched Replace format manipulation 3/1/0/1 1/0/2/1
Per-call Dictionary allocation 4/1/0/0 1/0/4/0
Span inconsistencies / compound chains 1/1/1/2 0/0/3/0
Unsealed leaf classes / locale hierarchy 3/0/1/1 0/0/4/0

Key judge evidence:

  • The strongest repeated gains were completeness and precision: treatment identified rubric-specific details the controls omitted, such as existing [GeneratedRegex] positives, the exact compiled-regex budget, Ordinal/FrozenDictionary, culture-sensitive casing, and span-helper positives.
  • GPT treatment won all 10 matched scenarios; three were treatment-only grader passes.
  • Haiku treatment won all 11, including two treatment-only grader passes.
  • The MAI loss on the locale hierarchy was substantive: the treatment dismantled the inheritance chain and missed the requirement to keep DefaultOrdinalizer as an unsealed base. MAI also failed to activate the skill on the per-call Dictionary scenario, though that trajectory still passed.
  • Opus lost the TimeSpan scenario because the control found blocking correctness bugs that treatment missed, and lost the truncation scenario because the control was judged slightly more focused/actionable. Two Opus comparisons were position-swap ties.
  • The fragile truncation scenario had only three matched families (1W/1T/1L) and two unmatched baselines; it should not be used as standalone evidence.

Explicit missing/error evidence: GPT baseline failed to execute the truncation scenario; MAI baselines failed the Ordinal/FrozenDictionary and truncation scenarios; Sonnet baselines failed Aggregate/Replace, CurrentCulture/regex budget, Ordinal/FrozenDictionary, branched Replace, and locale hierarchy. These eight pairs are unmatched and were not silently converted into wins.

Efficiency (separate from quality)

These are Vally's standard treatment-minus-baseline deltas over matched trajectories. HV is Vally's high-variance flag. Fewer resources are not treated as a quality win.

Family Total tokens Δ (95% CI) Turns Δ (95% CI) Tool calls Δ (95% CI) Wall time Δ ms (95% CI) Errors Δ
GPT +31,304 [-15,912, +78,520] HV +1.20 [-0.90, +3.30] HV +5.70 [+1.03, +10.37] HV +7,364 [-6,335, +21,063] HV 0
Haiku +31,525 [+4,148, +58,903] HV +1.45 [-0.12, +3.03] HV +2.18 [+1.19, +3.17] +12,827 [+8,215, +17,439] 0
MAI -74,700 [-206,265, +56,866] HV -3.78 [-8.73, +1.17] HV -3.33 [-8.57, +1.91] HV -39,064 [-80,074, +1,945] HV 0
Opus -60,589 [-160,353, +39,175] HV -2.55 [-6.37, +1.28] HV -2.27 [-6.19, +1.65] HV -17,280 [-43,995, +9,435] HV 0
Sonnet 4.6 +10,857 [-99,571, +121,284] HV +0.17 [-4.31, +4.64] HV +2.00 [-2.60, +6.60] HV +29,943 [-42,983, +102,869] HV 0

Raw means below use every trajectory with available metrics; baseline sample counts are GPT 10, Haiku 11, MAI 9, Opus 11, Sonnet 6, while every treatment has 11. Token columns are input/output/total/cache-read/cache-write.

Family Variant Token means I/O/T/CR/CW Turns Tools Wall ms Errors
GPT baseline 61,079 / 2,085 / 63,164 / 42,035 / 0 4.20 5.00 29,991 0
GPT treatment 91,355 / 2,779 / 94,134 / 61,254 / 0 5.36 10.45 36,970 0
Haiku baseline 55,954 / 1,382 / 57,336 / 42,467 / 13,458 4.18 3.55 16,570 0
Haiku treatment 86,315 / 2,546 / 88,861 / 67,826 / 18,422 5.64 5.73 29,397 0
MAI baseline 214,516 / 9,920 / 224,436 / 150,443 / 0 12.22 11.56 83,904 0
MAI treatment 139,619 / 6,020 / 145,639 / 103,936 / 0 8.36 8.27 44,518 0
Opus baseline 140,635 / 4,055 / 144,690 / 123,352 / 17,267 6.64 5.73 64,131 0
Opus treatment 80,871 / 3,230 / 84,101 / 64,396 / 16,449 4.09 3.45 46,851 0
Sonnet 4.6 baseline 85,357 / 4,884 / 90,241 / 68,844 / 16,506 5.17 4.00 71,938 0
Sonnet 4.6 treatment 89,801 / 6,349 / 96,150 / 71,457 / 18,060 5.09 5.73 106,025 0
Aggregate tool-call breakdown
Family Baseline Treatment
GPT rg=15, view=14, glob=12, bash=6, apply_patch=3 rg=59, view=30, glob=15, skill=11
Haiku view=23, bash=12, edit=3, create=1 bash=23, view=22, skill=11, grep=7
MAI bash=45, edit=28, view=17, grep=10, glob=4 view=19, bash=18, grep=18, edit=14, glob=11, skill=10, list_bash=1
Opus bash=31, edit=14, view=13, glob=3, grep=2 view=13, skill=11, bash=9, grep=3, glob=2
Sonnet 4.6 view=14, edit=8, read_agent=1, task=1 view=43, skill=11, bash=5, glob=4

Reproducibility against run 29537550152

This is secondary evidence only; the within-run A/B above is causal.

Family Current mean (95% CI), W/T/L, unmatched Prior mean (95% CI), W/T/L, unmatched
GPT +58.0% (+37.3%, +78.7%), 10/0/0, U=1 +52.7% (+30.8%, +74.6%), 10/1/0, U=0
Haiku +45.5% (+33.3%, +57.6%), 11/0/0, U=0 +58.2% (+34.6%, +81.8%), 10/1/0, U=0
MAI +26.7% (+4.9%, +48.4%), 7/1/1, U=2 +34.0% (+12.6%, +55.4%), 7/3/0, U=1
Opus +12.7% (-18.1%, +43.5%), 7/2/2, U=0 +14.5% (-10.3%, +39.4%), 7/1/3, U=0
Sonnet 4.6 +13.3% (-8.3%, +35.0%), 2/4/0, U=5 +15.6% (-20.3%, +51.4%), 6/2/1, U=2

The broad pattern reproduced: strong credible positive preference for Haiku; strong positive GPT preference (current formal result limited only by one unmatched baseline); positive but incomplete MAI evidence; Opus positive point estimate with CI crossing zero; Sonnet positive point estimate but unreliable because of missing baselines. With one trial per scenario, all scenario-level findings remain descriptive rather than statistically significant.

@artl93

artl93 commented Jul 18, 2026

Copy link
Copy Markdown
Member Author

Corrected old-vs-new skill A/B

This supersedes my earlier no-skill-vs-skill comment. That run established that the skill is useful, but did not measure PR #912's incremental change. This run compares the existing main skill directly with PR #912's treatment skill.

Run: https://github.com/dotnet/skills/actions/runs/29628141153
Integration branch/SHA: artl93-run-performance-ab / 6ee4e3377b0064a308ff5b5625ec123d8b449e4c

Exact inputs

All five artifacts recorded the same hashes. All 10 run summaries confirmed one trial per stimulus and the expected old/new skill paths. Pre-dispatch validation passed: pinned actionlint 1.7.7, all four focused Vally reliability tests, static dotnet-diag skill validation, exact PR #905 canonical/Vally parity across 11 scenarios, exact baseline/treatment hashes, git diff --check, and intended-diff checks.

Quality result

The run produced all expected 110 raw trajectories, 54 matched comparisons, one explicit unmatched MAI baseline executor error, and zero comparison errors. Treatment had 14 wins, 16 ties, and 24 losses descriptively across matched family/scenario pairs; scores are not pooled because judge families differ.

Family Treatment preference (95% CI) W/T/L McNemar old-only/new-only Executor success old/new Grader pass old/new Unmatched Activation old/new Interpretation
GPT +10.9% (-10.2%, +32.0%) 5/4/2 0/0 11/11 / 11/11 10/11 / 10/11 0 11/11 / 11/11 No credible difference
Haiku -14.5% (-39.4%, +10.3%) 3/1/7 0/0 11/11 / 11/11 11/11 / 11/11 0 11/11 / 11/11 No credible difference; point estimate favors old
MAI 0.0% (-27.0%, +27.0%) 4/2/4 2/1 (p=1.0) 10/11 / 11/11 7/11 / 7/11 1 10/11 / 10/11 Inconclusive due unmatched baseline
Opus -21.8% (-35.9%, -7.8%) 0/5/6 0/0 11/11 / 11/11 11/11 / 11/11 0 11/11 / 11/11 Credible quality regression
Sonnet 4.6 -16.4% (-43.8%, +11.1%) 2/4/5 0/0 11/11 / 11/11 10/11 / 10/11 0 11/11 / 11/11 No credible difference; point estimate favors old

No family showed a credible quality improvement. Opus showed a statistically credible regression. Haiku and Sonnet also leaned toward the old skill, although their intervals cross zero. GPT leaned toward treatment but was inconclusive; MAI was exactly neutral on matched preference and had one unmatched baseline.

Scenario evidence

One trial per family/scenario makes these descriptive, not independently significant.

Scenario Treatment W/T/L/U Direction
Branched Replace format manipulation 0/0/5/0 Old skill won every family
Unsealed leaf classes / locale hierarchy 1/0/4/0 Strongly favors old
Compiled regex startup / regex-chain allocations 0/2/3/0 Favors old
LINQ hot-path string processing 0/2/3/0 Favors old
Aggregate + Replace / missing IEquatable 2/0/3/0 Mixed, favors old
CurrentCulture comparer / compiled regex budget 2/1/2/0 Mixed
TimeSpan / collection LINQ pipeline 1/3/1/0 Neutral
Recursive converter ToLower allocations 1/3/1/0 Neutral
Ordinal / FrozenDictionary opportunities 2/2/0/1 Favors treatment among matched
Per-call Dictionary allocation 3/1/1/0 Favors treatment
Span inconsistencies / compound chains 2/2/1/0 Slightly favors treatment

The systematic losses align with the workflow change: the old skill's more exhaustive scans produced broader coverage, exact counts/locations, and positive findings that treatment sometimes omitted. The clearest regression was branched Replace:

  • All five families preferred the old skill.
  • One treatment failed to inspect the provided source at all.
  • Another treatment elevated a likely false positive about local-function closure allocation.
  • Other treatment responses missed string.Create, precise Replace locations, or IEquatable positives.

For Opus specifically, treatment had no wins: 0W/5T/6L. Its losses were generally small (-0.4) but consistent: less coverage, fewer exact counts/locations, less precise allocation analysis, weaker caveats/positive findings, or less review-aligned framing.

Treatment did have real strengths: it improved focused coverage for per-call Dictionary, Ordinal/FrozenDictionary, and some span scenarios. These gains did not offset the broader losses.

Efficiency

These are treatment-minus-baseline deltas over matched trajectories. Negative means PR #912 used fewer resources. HV means scenario-to-scenario variance was high.

Family Total tokens delta (95% CI) Turns delta Tool calls delta Wall-time delta Interpretation
GPT -21,538 [-46,589, +3,514] HV 0.00 [-0.95, +0.95] HV -15.64 [-20.14, -11.14] -12.9s [-20.6s, -5.2s] Clearly fewer tools and faster; token direction uncertain
Haiku -45,728 [-80,269, -11,186] HV -1.82 [-3.37, -0.26] HV -5.27 [-8.08, -2.47] -9.5s [-15.0s, -3.9s] Clearly more efficient
MAI +49,119 [-68,762, +167,000] HV +1.10 [-2.88, +5.08] HV +0.50 [-3.45, +4.45] HV +8.7s [-18.6s, +36.1s] HV No reliable efficiency difference
Opus -25,843 [-36,283, -15,403] -1.00 [-1.42, -0.58] -0.27 [-3.23, +2.69] HV -5.1s [-14.5s, +4.2s] HV Clearly fewer tokens and turns
Sonnet 4.6 -52,293 [-69,393, -35,193] -0.82 [-1.48, -0.16] HV -5.09 [-6.42, -3.77] -27.3s [-56.1s, +1.6s] HV Clearly fewer tokens, turns, and tools

Raw means confirm the intended mechanism: for example, GPT tool calls fell from 29.45 to 13.82; Haiku from 11.73 to 6.45; Sonnet from 10.27 to 5.18. All recorded metric errorCount deltas were zero.

This establishes an efficiency/quality trade-off, not an overall improvement: PR #912 makes GPT, Haiku, Opus, and Sonnet materially less resource-intensive, but provides no credible quality gain and causes a credible Opus regression.

Recommendation

Do not accept PR #912 as written. The old-vs-new evidence does not support the change as an overall improvement.

A narrower revision may be worthwhile:

  • Keep reading the supplied source before searching.
  • Keep batching related searches and avoiding repeated zero-value rescans.
  • Restore the old skill's explicit coverage guarantees, relevant reference loading, exact-count discipline, and positive-finding checks.
  • Re-run this same old-vs-revised A/B, with particular attention to branched Replace, locale hierarchy, compiled regex, and hot-path string processing.

Art Leonard added 2 commits July 18, 2026 21:24
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c7960b02-a0d3-4c71-8b65-3b238a9b7b42
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c7960b02-a0d3-4c71-8b65-3b238a9b7b42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant