mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
Update experiment name in evaluation workflow and enhance csharp-refactoring skill documentation and tests
This commit is contained in:
Wendy Breiding (She/Her)
committed by
Abhitej John
Abhitej John
parent
31ca050056
commit
72fc029f9f
@@ -2253,7 +2253,7 @@ jobs:
|
||||
$rate = if ($all.Count -gt 0) { [math]::Round($agree / $all.Count, 4) } else { 0 }
|
||||
|
||||
[pscustomobject]@{
|
||||
experiment = "opus-4.8-vs-sonnet-4.6"
|
||||
experiment = "opus-4.8-vs-sonnet-5"
|
||||
lastUpdate = $date
|
||||
summary = [pscustomobject]@{ pairs = $all.Count; agree = $agree; agreementRate = $rate }
|
||||
entries = $all
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
---
|
||||
name: csharp-refactoring
|
||||
description: "Performs safe, behavior-preserving refactoring of C#/.NET code, verified with build, tests, and analyzers. USE FOR: requests to rename, move, extract, split, consolidate, de-duplicate, modernize, or otherwise restructure C# code without changing behavior, including 'rename X to Y', generated/partial declarations, public or serialized contracts, multi-targeted code, and mixed requests where a feature, bug fix, package/framework upgrade, nullability change, or other behavior/contract change is presented as a refactor and must be separated or declined. DO NOT USE FOR: ordinary feature or bug-fix requests that are not framed as refactoring; package/framework upgrades after they have been reclassified (use dotnet-upgrade); new tests; or formatting-only passes (use dotnet format)."
|
||||
description: "Performs safe, behavior-preserving refactoring of C#/.NET code, verified with build, tests, and analyzers. USE FOR: rename or move a symbol/type/file; extract a method/type/interface; inline a wrapper/method/local; merge or consolidate near-identical classes or duplicate helpers; split or modernize C# code; generated/partial declarations; public, serialized, friend-assembly, or multi-targeted contracts; and mixed requests where a feature, bug fix, package/framework upgrade, public nullability change, or other behavior/contract change is presented as a refactor and must be separated or declined. DO NOT USE FOR: ordinary feature or bug-fix requests not framed as refactoring; upgrades after reclassification (use dotnet-upgrade); new tests; or formatting-only passes (use dotnet format)."
|
||||
license: MIT
|
||||
---
|
||||
|
||||
@@ -10,30 +10,34 @@ A refactor changes **structure**, never observable **behavior**. Do the edit wit
|
||||
then confirm behavior held with a build + the relevant tests. Keep the effort proportional to the change:
|
||||
a one-line local rename does not need the ceremony a public multi-targeted change does.
|
||||
|
||||
## First, is this actually a refactor?
|
||||
## Mandatory gate: classify before searching or editing
|
||||
|
||||
The most valuable thing this skill does is *not* restructure code you were told to restructure — it is
|
||||
catching a request that is **not** behavior-preserving before you run it through a refactor's contract.
|
||||
When the request changes results, decline the refactor framing and handle it honestly:
|
||||
Do this before reading project files, restoring, building, or making an edit. If the requested operation
|
||||
changes behavior or a public/source contract, the correct result of this skill is a decisive handoff,
|
||||
not an implementation attempt:
|
||||
|
||||
- **Framework / NuGet version bump** → not a refactor. Stop this workflow without editing project or
|
||||
package files, explain the reclassification, and redirect to the `dotnet-upgrade` skills. A successful
|
||||
build does not make an upgrade behavior-preserving.
|
||||
- **New feature** (e.g. add a capability, a flag, or an endpoint) → not a refactor. Stop this
|
||||
workflow and route it to the appropriate feature workflow; do not implement it here. If a separately
|
||||
authorized feature also has a structural cleanup, keep the two changes distinct in the implementation
|
||||
and final report.
|
||||
- **Bug fix or "simplification" that changes output** (e.g. change a threshold or calculation) →
|
||||
a behavior **change**. It is a legitimate task — do it as an explicit, tested change and update the
|
||||
tests that lock in the new behavior — but only after it is authorized as a behavior change. Under an
|
||||
explicitly behavior-preserving request, leave that edit undone, complete only any separable structural
|
||||
operation, and report the deferred change. Never label the behavior change behavior-preserving.
|
||||
- **A rename/move with a behavior tweak smuggled in** ("rename X, and while you're there change the result")
|
||||
→ do the rename/move as the behavior-preserving operation and defer the tweak. Perform the tweak only
|
||||
after the user separately accepts it as a tested behavior change; do not silently turn one
|
||||
"behavior-preserving" task into two edits.
|
||||
1. State: `Not a behavior-preserving refactor: <specific reason>.`
|
||||
2. State that no files were changed.
|
||||
3. Name the correct next workflow. Do not offer to perform the reclassified work inside this skill and
|
||||
do not ask whether to proceed anyway.
|
||||
|
||||
Only when the request is genuinely structure-only do you proceed as a refactor.
|
||||
| Requested as a "refactor" | Classification and action |
|
||||
|---|---|
|
||||
| Framework or NuGet version change | **Upgrade.** Do not edit or validate the upgrade here; hand off to `dotnet-upgrade`. |
|
||||
| New capability, flag, endpoint, tier, or behavior | **Feature.** Do not implement it in this workflow. |
|
||||
| Threshold, rate, output, or bug-result change | **Behavior change.** Defer it unless separately authorized outside the refactor. |
|
||||
| Tighten or loosen a shipped/public nullable annotation | **Source-contract change.** Leave the declaration and API record unchanged. |
|
||||
|
||||
For a mixed request, perform only a clearly separable structural operation and explicitly defer the
|
||||
behavior/contract change. Never modify tests to make an unauthorized behavior change appear preserved.
|
||||
|
||||
## Work only in the current repository
|
||||
|
||||
Resolve the repository root first (`git rev-parse --show-toplevel`) and resolve any prompt-provided
|
||||
relative solution/project path inside that root. Search and edit only that workspace. Never use
|
||||
filesystem-wide search or select a similarly named clone, temporary directory, build output, or
|
||||
another worktree because a file also exists there. If the named path is absent from the current
|
||||
repository, stop and report that mismatch instead of guessing another workspace.
|
||||
|
||||
## Rename / move by bindings, not text
|
||||
|
||||
@@ -65,7 +69,7 @@ injection, configuration binding, source generators, P/Invoke, or `dynamic`.
|
||||
| Boundary | Required decision |
|
||||
|---|---|
|
||||
| Serialized/configuration name | Preserve the external name with the repository's existing mechanism (for example, `JsonPropertyName`) while migrating C# callers; run a focused round-trip or payload test. |
|
||||
| Public nullable annotation | Treat tightening or loosening nullability as a source-contract change, not a behavior-preserving refactor. Leave it unchanged unless the contract change is explicitly authorized and validated. |
|
||||
| Public nullable annotation | The mandatory classification gate applies: leave it unchanged and hand off as a source-contract change. |
|
||||
| Uncovered reflection or runtime lookup | Do not guess that a compile-clean rename is safe. Preserve the observed name or stop and report the unverified runtime boundary. |
|
||||
|
||||
## Verify proportionally
|
||||
|
||||
@@ -180,6 +180,7 @@ stimuli:
|
||||
- Finds the true binding references to the wrapper before removing it, not just textual matches
|
||||
- Updates every caller to the underlying call and removes the wrapper in one focused operation
|
||||
- Lets the compiler catch any missed reference, then rebuilds and re-runs the tests to confirm green
|
||||
- Works only in the provided repository workspace and does not inspect or modify another clone, worktree, or temporary directory
|
||||
|
||||
- name: Merge two near-identical types into one parameterized type
|
||||
prompt: "`GoldPricing` and `SilverPricing` are near-identical. Consolidate them into a single parameterized pricing type and update any usages, without changing behavior. The solution is at Fixture.sln."
|
||||
@@ -236,6 +237,7 @@ stimuli:
|
||||
- Recognizes that a framework or package major-version upgrade is not a behavior-preserving refactor
|
||||
- Declines or redirects to the appropriate upgrade workflow instead of treating it as a refactor
|
||||
- Does not silently perform a behavior-changing version bump under the guise of refactoring
|
||||
- Gives a decisive, actionable handoff without offering to execute the upgrade inside the refactoring workflow or asking whether to proceed anyway
|
||||
|
||||
- name: Decline a new-feature request dressed up as a refactor
|
||||
prompt: 'In Fixture.sln, add a new "platinum" loyalty tier at 15% off and wire it into the pricing tiers so orders can use it, treating this as a behavior-preserving refactor.'
|
||||
@@ -254,6 +256,7 @@ stimuli:
|
||||
- Recognizes that adding a new pricing tier is a new feature, not a behavior-preserving refactor
|
||||
- Declines or redirects to an appropriate feature workflow rather than implementing the tier here
|
||||
- Leaves the pricing behavior unchanged and does not claim the requested feature is behavior-preserving
|
||||
- Stops before editing instead of implementing first and merely acknowledging afterward that behavior changed
|
||||
|
||||
- name: Keep behavior-changing bug fixes out of a behavior-preserving refactor
|
||||
prompt: "In Fixture.sln, rename `OrderProcessor.DoStuff` to `ComputeInvoice` and, while you are there, change the free-shipping threshold from $100 to $50. Treat the whole change as one behavior-preserving refactor."
|
||||
@@ -331,6 +334,7 @@ stimuli:
|
||||
- Recognizes that tightening a nullable annotation on a shipped public API changes its source contract and is not a behavior-preserving cleanup
|
||||
- Leaves both the declaration and shipped API record unchanged instead of forcing callers to satisfy a newly non-null contract
|
||||
- Clearly reclassifies the requested edit as a separately authorized contract change
|
||||
- Classifies the requested contract change before attempting edits; compiler or analyzer feedback is not a substitute for that decision
|
||||
|
||||
- name: Rename one binding without touching same-text symbols
|
||||
prompt: "Rename only `OrderProcessor.DoStuff` to `CalculateInvoice` and migrate its callers. Do not rename unrelated members or text that happen to contain `DoStuff`. Verify Fixture.sln."
|
||||
|
||||
Reference in New Issue
Block a user