Add refactoring-to-async skill#269
Conversation
Migration NoteThis PR replaces #79 which was opened from
All prior review feedback from #79 still applies — please see that PR for the full discussion history. |
There was a problem hiding this comment.
Pull request overview
Adds a new refactoring-to-async skill to the dotnet plugin, along with an evaluation scenario and fixture project intended to test bottom-up async conversion guidance.
Changes:
- Added
refactoring-to-asyncskill documentation underplugins/dotnet/skills/. - Added a new eval scenario plus C# fixture project under
tests/dotnet/refactoring-to-async/. - Updated
.github/CODEOWNERSto assign ownership for the new skill and test directory.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/dotnet/skills/refactoring-to-async/SKILL.md | New skill content describing async refactoring workflow and pitfalls. |
| tests/dotnet/refactoring-to-async/eval.yaml | New eval scenarios/assertions for async refactoring and CPU-bound non-async guidance. |
| tests/dotnet/refactoring-to-async/UserService.cs | Fixture code containing sync I/O and blocking patterns to be refactored by the model. |
| tests/dotnet/refactoring-to-async/SyncService.csproj | Fixture project enabling dotnet build-style validation. |
| .github/CODEOWNERS | Adds code owners for the new skill and tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Feedback carried over from #79 (refactoring-to-async)Code Review Comments (10 threads)@copilot —
@copilot —
@copilot —
@danmoseley —
@danmoseley —
@danmoseley —
@danmoseley —
@danmoseley —
@danmoseley —
@danmoseley —
Discussion Comments (5)
|
|
/evaluate |
Skill Validation Results
Model: claude-opus-4.6 | Judge: claude-opus-4.6 |
|
/evaluate |
|
✅ Evaluation completed. View results | View workflow run |
|
@danmoseley - could you please take another look? I believe I have addressed your feedback. |
|
@mrsharm check the one scenario in which the skill doesn't get activated. |
@ViktorHofer: Intentional - is a negative test and is the correct outcome. The eval suggests optimizing matrix multiplication (CPU-bound), not about converting I/O to async. The skill's description targets I/O-bound async refactoring, so the agent should recognize this isn't an async problem and not load the skill. That's exactly what happened in all 3 runs. |
|
@mrsharm such tests must be annotated with Otherwise we have no way to distinguish between intentionally not activated and unintentionally not activated. |
|
/evaluate |
|
✅ Evaluation completed. View results | View workflow run |
|
@ViktorHofer - thanks! Been implemented for this and the other PRs. |
|
@mrsharm - the repo has undergone some restructuring to make everything more organized. Hence, we are asking all open PRs to update the branch. Sorry about this. @jasonmalinowski @jaredpar - please review. |
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 |
Thanks - done! Evals look good - can merge if things look good. |
2fa9826 to
00ae665
Compare
|
@mrsharm This PR was approved back in March and evals pass — is this still something you'd like to land? It likely needs a rebase on main at this point. Let us know if you need help getting it across the finish line. |
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 |
|
✅ Approved by @ViktorHofer. cc @dotnet/skills-merge-approvers — ready to merge. |
|
✅ Approved by @ViktorHofer. cc @dotnet/skills-merge-approvers — ready to merge. |
Adds the refactoring-to-async skill for bottom-up async conversion patterns in .NET.
Note: Replaces #79 (migrated from skills-old repo to new plugins/ structure).
What the Skill Teaches