Add .NET refactoring skill - #873
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Skill Coverage Report
Uncovered:
|
|
/evaluate |
Skill Validation Results
[1]
Model: claude-opus-4.6 | Judge: claude-opus-4.6 🔍 Full Results - additional metrics and failure investigation steps
▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
There was a problem hiding this comment.
Pull request overview
This PR adds two new .NET-focused skills to the plugins/dotnet plugin—one for behavior-preserving C# refactoring workflows and one for .NET-specific breaking-change/compatibility guardrails—along with eval coverage and fixtures that exercise the key hazards (public API baselines, multi-targeting #if, generated/partial code, and friend assemblies).
Changes:
- Added
csharp-refactoringskill guidance plus an operation catalog reference. - Added
dotnet-breaking-changesskill guidance plus focused reference docs for the four “hidden surfaces”. - Added eval suites + identical lightweight Billing fixture solutions under
tests/dotnet/for both skills; updated CODEOWNERS and dotnet plugin README.
Show a summary per file
| File | Description |
|---|---|
| tests/dotnet/dotnet-breaking-changes/tests/Billing.Tests/BillingTests.cs | Adds fixture tests used by the breaking-change eval scenarios. |
| tests/dotnet/dotnet-breaking-changes/tests/Billing.Tests/Billing.Tests.csproj | Adds an xUnit test project targeting net10.0 for the breaking-change fixture solution. |
| tests/dotnet/dotnet-breaking-changes/src/Billing/PublicAPI.Shipped.txt | Adds a public API baseline file used by eval prompts/assertions. |
| tests/dotnet/dotnet-breaking-changes/src/Billing/Pricing.cs | Adds pricing/tax types used for refactor/breaking-change exercises. |
| tests/dotnet/dotnet-breaking-changes/src/Billing/PlatformInfo.cs | Adds multi-targeting #if example surface for breaking-change checks. |
| tests/dotnet/dotnet-breaking-changes/src/Billing/OrderProcessor.cs | Adds intentionally-refactorable implementation used by scenarios. |
| tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.g.cs.template | Adds generator-input template to simulate generated/partial hazards. |
| tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.cs | Adds hand-authored partial type paired with the generated template. |
| tests/dotnet/dotnet-breaking-changes/src/Billing/Billing.csproj | Adds multi-targeted library project and build-time generation hook. |
| tests/dotnet/dotnet-breaking-changes/src/Billing/AppSettingsHelper.cs | Adds duplicated parsing helpers for public-API consolidation scenarios. |
| tests/dotnet/dotnet-breaking-changes/Fixture.sln | Adds the breaking-change fixture solution container. |
| tests/dotnet/dotnet-breaking-changes/eval.yaml | Adds breaking-change eval scenarios and build/test gates. |
| tests/dotnet/csharp-refactoring/tests/Billing.Tests/BillingTests.cs | Adds fixture tests used by the refactoring eval scenarios. |
| tests/dotnet/csharp-refactoring/tests/Billing.Tests/Billing.Tests.csproj | Adds an xUnit test project targeting net10.0 for the refactoring fixture solution. |
| tests/dotnet/csharp-refactoring/src/Billing/PublicAPI.Shipped.txt | Adds a public API baseline file used by refactoring prompts/assertions. |
| tests/dotnet/csharp-refactoring/src/Billing/Pricing.cs | Adds pricing/tax types used for refactoring exercises. |
| tests/dotnet/csharp-refactoring/src/Billing/PlatformInfo.cs | Adds multi-targeting #if example surface for refactoring checks. |
| tests/dotnet/csharp-refactoring/src/Billing/OrderProcessor.cs | Adds intentionally-refactorable implementation used by scenarios. |
| tests/dotnet/csharp-refactoring/src/Billing/Coupons.g.cs.template | Adds generator-input template to simulate generated/partial hazards. |
| tests/dotnet/csharp-refactoring/src/Billing/Coupons.cs | Adds hand-authored partial type paired with the generated template. |
| tests/dotnet/csharp-refactoring/src/Billing/Billing.csproj | Adds multi-targeted library project and build-time generation hook. |
| tests/dotnet/csharp-refactoring/src/Billing/AppSettingsHelper.cs | Adds duplicated parsing helpers used in refactoring scenarios. |
| tests/dotnet/csharp-refactoring/Fixture.sln | Adds the refactoring fixture solution container. |
| tests/dotnet/csharp-refactoring/eval.yaml | Adds refactoring eval scenarios and build/test gates. |
| plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md | Introduces the breaking-change skill and when/when-not-to-use guidance. |
| plugins/dotnet/skills/dotnet-breaking-changes/references/source-generation.md | Adds detailed guidance for generated/partial code hazards. |
| plugins/dotnet/skills/dotnet-breaking-changes/references/public-api.md | Adds detailed guidance for public API gating and compatibility decisions. |
| plugins/dotnet/skills/dotnet-breaking-changes/references/multi-targeting.md | Adds detailed guidance for multi-targeting and #if branch correctness. |
| plugins/dotnet/skills/dotnet-breaking-changes/references/internals-visible-to.md | Adds detailed guidance for friend-assembly/internal-surface hazards. |
| plugins/dotnet/skills/csharp-refactoring/SKILL.md | Introduces the refactoring skill and a stepwise safety contract. |
| plugins/dotnet/skills/csharp-refactoring/references/operation-catalog.md | Adds a more detailed refactoring operation taxonomy reference. |
| plugins/dotnet/README.md | Updates the dotnet plugin skill list to include the new skills. |
| eng/known-domains.txt | Adds github.com/dotnet/skills to known domains. |
| .github/CODEOWNERS | Adds ownership entries for the new skills and their tests. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 34/34 changed files
- Comments generated: 0
|
/evaluate |
|
👋 @AbhitejJohn — this PR has 1 unresolved review thread(s). When you're ready, please address the feedback and push an update; the triage bot will pick up the next state automatically. (Add the |
Skill Validation Results
[1]
Model: claude-opus-4.6 | Judge: claude-opus-4.6 🔍 Full Results - additional metrics and failure investigation steps
▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
JanKrivanek
left a comment
There was a problem hiding this comment.
Since the skills have nontrivial size - the main concern now is overlap between the two skills + cross-loading cost. csharp-refactoring instructs the agent to also load dotnet-breaking-changes, and the two share material ("stop and escalate," public API, forwarders, partial/generated). The split is defensible (breaking-changes is meant to apply to features/fixes too, not just refactors), but loading two long skills for one refactor is a real context cost that the evals don't yet justify.
Trim study for PR #873: csharp-refactoring-trim is a ~30% shorter rewrite of csharp-refactoring (drops When-to-use/Inputs/Outputs boilerplate + verbose LSP launch prose, compresses the dbc-overlapping hazards section) while preserving every rubric-rewarded behavior. Adds a 'trimmed' experiment arm and bumps runs to 3 so CI generates baseline/skilled/trimmed trajectories for local cross-family re-judging. Eval-only scratch branch; not part of the PR. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3c9f9823-7f2f-4f7d-9d1b-f2b9e7a20c60
|
👋 @AbhitejJohn — this PR has 1 unresolved review thread(s). When you're ready, please address the feedback and push an update; the triage bot will pick up the next state automatically. (Add the |
Robustness probe for the csharp-refactoring trim: all prior CI trajectories were executed by claude-opus-4.6. Flip executor to gpt-5.5 (GPT family) and CI judge to claude-opus-4.8 so the (executor, judge) pair stays cross-family. Tests whether trimmed-vs-current non-inferiority holds under a different model family. Eval-branch only; not on PR #873. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3c9f9823-7f2f-4f7d-9d1b-f2b9e7a20c60
Closes the 'refactoring-only endpoint too narrow' risk flagged by the
cross-family rubber-duck. Adds two boundary stimuli never used in trim tuning:
9. Separate a smuggled behavior change from a requested rename (rename +
smuggled 10%->12% discount change) -> should separate/flag, not bundle.
10. Recognize a behavior-changing simplification is not a refactor (remove
free-shipping-over-\ rule under a 'cleanup' framing) -> should not
silently change observable behavior.
Executor reverted to claude-opus-4.6 / judge gpt-5.5 (re-judge) to match the
stringent Confirm A family where boundary behavior looked weakest.
Eval-branch only; not on PR #873.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3c9f9823-7f2f-4f7d-9d1b-f2b9e7a20c60
Replaces the shipped skill body with the validated v3 variant: judgment-first, rigor proportional to blast radius, redundant catalog/list scaffolding removed. ~53% smaller (12,811->6,039 chars) with equal or better cross-family eval quality and no measured regression. Description (929 chars) and all safety rules retained. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3c9f9823-7f2f-4f7d-9d1b-f2b9e7a20c60
|
👋 @AbhitejJohn — this PR has 1 unresolved review thread(s). When you're ready, please address the feedback and push an update; the triage bot will pick up the next state automatically. (Add the |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3c9f9823-7f2f-4f7d-9d1b-f2b9e7a20c60
Co-authored-by: webreidi <55603905+webreidi@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Unresolved moderate guidance and evaluation issues, plus one build-target nit, remain.
Review tier: Lite
Findings: 2
Pre-existing issues (2)
| Severity | Finding |
|---|---|
tests/dotnet/csharp-refactoring/golden-patches/rename-generated-member.patch — Align the generated-member golden patch with its scenario View comment |
|
tests/dotnet/csharp-refactoring/eval.yaml — Copy CollisionExamples into the shared fixture View comment |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
tests/dotnet/csharp-refactoring/eval.yaml — Use the xUnit pass marker for this test gate View resolved comment |
Previously missed findings (2)
In code that hasn't changed since last review
tests/dotnet/csharp-refactoring/eval.yaml:201
- This stimulus is exclusively a framework/package upgrade, while the new skill explicitly says not to use it for upgrades. Add
expect_activation: false; otherwise a correct routing decision to leave this skill dormant is counted as a missed activation by the eval harness.
tests/dotnet/csharp-refactoring/eval.yaml:214 - This stimulus is exclusively a new-feature request, while the new skill explicitly says not to use it for adding features. Add
expect_activation: false; otherwise a correct routing decision to leave this skill dormant is counted as a missed activation by the eval harness.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved moderate findings remain in workflow routing, promised skill scope, and evaluation test-count assertions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 2
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
.github/workflows/evaluation.yml — Remove duplicate sonnet-5 judge route |
Pre-existing issues (2)
| Severity | Finding |
|---|---|
tests/dotnet/csharp-refactoring/golden-patches/rename-generated-member.patch — Align the generated-member golden patch with its scenario View comment |
|
tests/dotnet/csharp-refactoring/eval.yaml — Copy CollisionExamples into the shared fixture View comment |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The promised breaking-changes skill is absent, several evaluation graders/fixtures are incorrect or unused, and the workflow includes an unrelated model change.
Review tier: Lite
Findings: 1
Pre-existing issues (1)
| Severity | Finding |
|---|---|
tests/dotnet/csharp-refactoring/golden-patches/rename-generated-member.patch — Align the generated-member golden patch with its scenario View comment |
Issues resolved since last review (2)
| Severity | Finding |
|---|---|
.github/workflows/evaluation.yml — Remove duplicate sonnet-5 judge route View resolved comment |
|
tests/dotnet/csharp-refactoring/eval.yaml — Copy CollisionExamples into the shared fixture View resolved comment |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The generated fixture is syntactically invalid, and unresolved plugin-scope and evaluation issues remain.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
tests/dotnet-msbuild/item-management/Constants.g.cs — Do not ship a malformed generated C# file |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
tests/dotnet/csharp-refactoring/golden-patches/rename-generated-member.patch — Align the generated-member golden patch with its scenario View resolved comment |
Previously missed findings (1)
In code that hasn't changed since last review
.github/workflows/evaluation.yml:1098
- This hunk changes the default model for every PR and scheduled evaluation, but the PR description only covers adding refactoring skills. That changes evaluation cost and results outside this feature and makes failures harder to attribute; move the matrix rollout to a separate PR or document and validate it as part of this change.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
❌ Evaluation did not complete successfully (the evaluate job reported 2 partial result file(s) were preserved for diagnosis but were not consolidated because the full matrix did not complete. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8c45529b-2515-483d-9e51-e6c0b7cb6852
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8c45529b-2515-483d-9e51-e6c0b7cb6852
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8c45529b-2515-483d-9e51-e6c0b7cb6852
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8c45529b-2515-483d-9e51-e6c0b7cb6852
…et-refactoring-skills
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Unresolved scope, routing, and evaluation issues remain.
Review tier: Lite
Findings: None
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
tests/dotnet-msbuild/item-management/Constants.g.cs — Do not ship a malformed generated C# file View resolved comment |
Previously missed findings (1)
In code that hasn't changed since last review
.github/workflows/evaluation.yml:1095
- This changes the default executor for every evaluation from
claude-sonnet-4.6toclaude-sonnet-5, affecting unrelated plugin results and making this PR's eval comparison non-comparable. The PR description does not mention this CI-wide behavior change; please split it into a separate change or document and validate the rollout here.
…tnet/skills into add-dotnet-refactoring-skills
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Required fixes remain for the evaluation graders, workflow model metadata and scope, missing companion skill, and validation heading.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
tests/dotnet/csharp-refactoring/eval.yaml — Make the consolidation grader accept block-bodied methods |
|
tests/dotnet/csharp-refactoring/eval.yaml — Verify the old API stays in the shipped surface |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Unresolved moderate findings cover the missing companion skill, stale workflow labeling, and insufficient evaluation checks.
Review tier: Lite
Findings: None
Issues resolved since last review (2)
| Severity | Finding |
|---|---|
tests/dotnet/csharp-refactoring/eval.yaml — Verify the old API stays in the shipped surface View resolved comment |
|
tests/dotnet/csharp-refactoring/eval.yaml — Make the consolidation grader accept block-bodied methods View resolved comment |
Previously missed findings (1)
In code that hasn't changed since last review
.github/workflows/evaluation.yml:1095
- This changes the default executor from
claude-sonnet-4.6toclaude-sonnet-5, but the workflow still writes the comparison report underopus-4.8-vs-sonnet-4.6later in the same file. New data will therefore be published with a stale model label; update the report identifier with this change or keep this unrelated matrix change separate.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8c45529b-2515-483d-9e51-e6c0b7cb6852
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Unresolved moderate issues affect the promised skill scope and evaluation coverage.
Review tier: Lite
Findings: None
Previously missed findings (1)
In code that hasn't changed since last review
tests/dotnet/csharp-refactoring/eval.yaml:10
- The eval adds golden patches for
extract-subtotal,rename-local, andrename-friend-member, but no stimulus includes those files or asks for those operations; the shared environment copies only the fixture source and test files. The extraction and friend-assembly cases therefore provide no evaluation coverage despite being listed in the PR summary. Add stimuli that exercise these cases or remove the unconsumed patches and narrow the coverage claim.
This issue also appears on line 51 of the same file.


Summary
Refactoring is a common operation for .NET developers, and a dedicated skill can help agents make these changes in the way .NET teams expect: behavior-preserving, incremental, and validated with build/test gates.
This PR adds two related skills:
csharp-refactoring: guides safe C#/.NET refactoring work such as rename, move, extract, inline, split, consolidate/de-duplicate, and modernization while preserving behavior.dotnet-breaking-changes: provides the compatibility guardrails needed when a refactor touches public API, multi-targeting, source-generated/partial code, orInternalsVisibleTosurfaces.Why these skills
csharp-refactoringfocuses on the refactoring workflow itself: establish a green baseline, choose one named operation, find true references, prefer semantics-aware edits, and re-gate after each step. Its reference file expands the operation catalog and maps common refactoring operations to the kinds of changes .NET teams regularly make.dotnet-breaking-changescovers the .NET-specific surfaces where a change can compile and pass tests while still breaking downstream consumers. Its reference files explain how to inspect and handle:PublicAPI.*, ApiCompat, package validation)#ifbranchesInternalsVisibleToTogether, the skills let an agent keep refactoring focused on behavior preservation while still checking the .NET compatibility surfaces that matter in real repos.
Validation