Repository navigation
Add shared TypeSpec naming skill for authoring - #17121
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 1 pipeline(s). 68 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The skill currently excludes direct client.tsp naming edits, and version-sensitive exact-name behavior lacks evaluation coverage.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds authoring-time SDK naming guidance for C# ARM TypeSpec projects and integrates hermetic naming evaluations.
Changes:
- Adds C# ARM member/model naming profiles and workflow integration.
- Adds 17 routing and capability evaluation scenarios.
- Registers the suite in local and shared CI discovery.
| File | Description |
|---|---|
eng/common/pipelines/skill-eval.yml |
Discovers the naming eval suite. |
.github/skills/.vally.yaml |
Registers its eval directory. |
.github/skills/azure-typespec-author/SKILL.md |
Integrates naming into authoring. |
references/intake.md |
Selects naming profiles during intake. |
references/authoring-plan.md |
Adds naming decisions to plans. |
references/validation.md |
Adds naming validation. |
references/naming-conventions.md |
Defines dispatch and safeguards. |
references/csharp-naming.md |
Defines C# ARM member rules. |
references/csharp-model-naming.md |
Defines C# ARM model rules. |
evals/README.md |
Documents the eval suite. |
evals/eval.yaml |
Adds routing and naming scenarios. |
|
The following pipelines have been queued for testing: |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The following pipelines have been queued for testing: |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The following pipelines have been queued for testing: |
There was a problem hiding this comment.
Hi JoshLove-msft , I like the naming skill, it looks good to me.
Also thanks for contributing to the authoring skill and assessment skill, really appreciate it! I think the key point for us to align on is this comment. It would be helpful to agree on that first so we have a shared direction for how to integrate.
Also two general comments:
-
if there is any changes to azure-typespec-author skill, please run the benchmark and make sure benchmark passes without regression. https://dev.azure.com/azure-sdk/internal/_build?definitionId=8178&_a=summary
-
As Ray Chen (@raych1) mentioned, the current suggestion from engsys is to commit the code directly to spec repo and make spec repo as source of truth for azure-typespec-author and azure-typespec-assessment skill code. I've looped you into the email thraed as well. Please feel free to let us know if you have any concern or comments. So if you decided to continue developing authoring skill and assessment skill in this PR, you may need to create a separate PR to sync them to spec repo. An alternative is to only include the naming skill in this PR and have another PR to integrate it with authoring skill and assessment skill.
…ions Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The following pipelines have been queued for testing: |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
Replace naming-only customization tool calls with scoped client.tsp edits and align authoring handoffs and regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The following pipelines have been queued for testing: |
Restore source intake confirmations and strict assertions, use document-led versioning guidance, and align naming benchmarks with numbered cases and comparable no-skill baselines. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The following pipelines have been queued for testing: |
Resolve the TypeSpec author benchmark conflicts while preserving the naming workflow and current main state. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6952059b-ae31-4269-80d8-07264176b55e
|
Hi Josh, quick update: I merged the TypeSpec authoring benchmark PR in Looks your latest pipeline run fails on env set up. I retriggered the TypeSpec author benchmark pipeline on the updated branch using the live MCP configuration. The run is currently in progress. |
|
The following pipelines have been queued for testing: |
There was a problem hiding this comment.
The changes on benchmark overall looks good to me, I've improved the benchmark stability, here is the latest benchmark result triggered from main, https://dev.azure.com/azure-sdk/internal/_build/results?buildId=6972379&view=results, pass rate is 100%.
I've merged my improvements to your pr and help to solve conflicts now. And I've triggered two pipeline runs.
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=6972773
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=6972554
Let's make sure the benchmark looks good (success rate > 90%) before merge.
update:
I've checked the failed non-naming cases in this completed run, they are caused by network/test writing issues, not changes introduced by this pr. But there are a few naming cases failure that may need your investigation.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The following pipelines have been queued for testing: |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The following pipelines have been queued for testing: |
Haoling Dong (haolingdong-msft)
left a comment
There was a problem hiding this comment.
Thanks JoshLove-msft for the changes! The skill and latest benchmark both look great to me now!
I've approved the PR.
Just a friendly reminder: after this PR is merged, please also create a sync PR to the spec repo so the change can take effect. Thanks!
|
/check-enforcer override |
Include the authoring workflow, client configuration reference, validation safeguards, naming evaluations, and compiled-service grader from Azure/azure-sdk-tools#17121. Preserve REST specs' local evaluation setup and adapt the grader for type-aware linting. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Extract shared TypeSpec naming skill and tool-based customization Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Cover exact numeric enum naming across TCGC capabilities Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Test generic customization routing Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Refine TypeSpec naming orchestration Make client configuration a separate default authoring step with an explicit opt-out, clarify customization ownership and verification, and document assessment evidence. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Make client configuration opt-out non-blocking Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Harden TypeSpec author benchmark behavior Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Fix remaining author benchmark failures Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Address naming review safeguards Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Remove naming MCP dependency Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Restore naming MCP workflow Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Clarify TypeSpec naming skill routing Exclude naming-only client.tsp overrides from author routing and make shipped-name compatibility a hard gate before naming style. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Fix naming and preview-version benchmark failures Require naming profile reads during default client configuration planning and clarify preview replacement and excluded-default reversal. Add an offline planning regression without relaxing the existing live assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Address C# naming review feedback Remove the underscore enum convention and automatic casing guidance, correct date/time examples, and incorporate applicable semantic naming checks from the .NET review rules. Replace obsolete enum expectations and add focused naming regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Defer TypeSpec assessment naming integration Restore the assessment skill to the PR base and remove its naming handoff cases and integration claims. Retain authoring, standalone read-only naming review, and routing boundaries. Track the deferred integration in #17297. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Add fallback for unavailable naming customization tool Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Apply TypeSpec naming overrides directly Replace naming-only customization tool calls with scoped client.tsp edits and align authoring handoffs and regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Fix TypeSpec naming benchmark feedback Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Format shared TypeSpec naming skill fixtures Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Sync naming fixture lint fixes from Azure/azure-sdk-tools#17326 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Sync TypeSpec authoring skill naming integration Include the authoring workflow, client configuration reference, validation safeguards, naming evaluations, and compiled-service grader from Azure/azure-sdk-tools#17121. Preserve REST specs' local evaluation setup and adapt the grader for type-aware linting. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Sync latest naming fixture scope validation from tools PR 17326 Import upstream 0996ff06f86f scope-specific customization path validation and expanded regression coverage. Preserve the authoring integration and explicitly type the test response helper for downstream TypeScript narrowing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Summary
Partially implements #17120 with reusable TypeSpec SDK naming guidance, initially for C# ARM management SDKs. Additional language/service profiles remain follow-up work. This is proactive guidance independent of the linter work in Azure/typespec-azure#4442.
azsdk-common-typespec-naming, distributed through the existing shared-skill sync, with a dispatcher and authoritative C# member/model profiles.client.tspand necessary imports, notazsdk_customized_code_update. General SDK customization retains its existing workflow.Evaluation coverage
007001–007008compare trigger, forced, and no-skill task outcomes;007009–007014cover routing boundaries. Naming-only continuations may invoke naming directly; forced handoff cases still require authoring. Outcome assertions remain identical across modes..vally.yamland mount definitions/references inside trial workspaces. The existing runner isolates no-skill runs, excludes evaluator content and fixture dependencies, and supports olderSkillBranchsources without importing candidate naming rules. The separate bundle-preparation script is removed.001014; existing001005coverage remains.Deferred assessment integration
Assessment integration is excluded and tracked separately in #17297. Assessment is retained only as a competing skill in routing-boundary tests; this PR does not add an assessment naming-report dimension or naming handoff.
Validation
002005.Enabledstayed unchanged, new minute-based TTL planned asTimeToLiveInMinutes, no edits, andgeneratedNamesVerified: false. This is not a full Vally benchmark result.Investigated naming failures in benchmark 6972773, including failed reference reads and an author-invocation requirement on naming-only continuations. Live-MCP benchmark 6974159 completed successfully on
e753da21f: 98/98 published tests passed (100%) across the forced and trigger tracks, exceeding the requested >90% success rate. The two completed test runs report 52/52 and 46/46 passed, with no incomplete tests.Formatting-only follow-up
3484b4725fixes the four Prettier violations reported bysync-skills; the exact CI formatting command passes locally.