Extend azsdk_typespec_generate_authoring_plan with SDK breaking change detection#15077
Extend azsdk_typespec_generate_authoring_plan with SDK breaking change detection#15077samvaity wants to merge 26 commits into
Conversation
Catalogs 10 known TypeSpec change patterns that cause SDK breaking changes: - Per-language impact matrix (Java/.NET/Python/Go) - Language-scoped client.tsp mitigations with concrete code examples - Detection sources (TypeSpec diff vs SDK changelog) - Complex enum split case with multi-step decorator guidance - Language-specific breaking change policies Used by azsdk_typespec_generate_authoring_plan via RAG to warn developers about SDK impact during TypeSpec authoring, before merge. Ref: #15069, #13972, #14675 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
azsdk-cli
|
The following pipelines have been queued for testing: |
…e detection - Load sdk-breaking-patterns.md from eng/common/knowledge/ and inject as additional context so the AI agent includes SDK IMPACT warnings when planned TypeSpec changes match known breaking patterns - Add optional sdkChangelog parameter for deeper detection from SDK changelog - Add IGitHelper dependency for repo root discovery (same pattern as TypeSpecCustomizationService) - Update tool description to mention SDK breaking change detection - Build succeeds with 0 errors Ref: #15069, #13972, #14675 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
samvaity
force-pushed
the
savaity/sdk-breaking-patterns
branch
from
6e4401c to
cb61d4e
Compare
The most common failure in enum split mitigation is forgetting to rename the original enum out of the way before creating a replacement with the same @@clientName. This was the exact gap found in Storage testing (Issue #14675) — tool got steps 2-4 right but missed step 1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
…reaking change detection
|
The following pipelines have been queued for testing: |
… Java, Python, Go, JS Added 10 new patterns (total 20) sourced from official per-language breaking change guides: - Enum value name changes (numeric names, Pattern 3) - Client name changes (Pattern 13) - Resource base type changes, .NET-specific (Pattern 14) - WirePathAttribute missing, .NET MPG-specific (Pattern 15) - Common types upgrade - accept (Pattern 16) - Unreferenced models removed - accept (Pattern 17) - Multi-level unflattened properties, Python-specific (Pattern 18) - Property name conflicts with base methods, Python-specific (Pattern 19) - LRO/Paging operation type changes, Go-specific (Pattern 20) - Enhanced operation signature pattern with Python @@OverRide example (Pattern 10) Enhanced Language-Specific Breaking Change Policies section with per-language references, customization mechanisms, and known accept-patterns. Key additions: - Go: many breaking changes CANNOT be resolved via client.tsp - .NET: @@markAsPageable, @@hierarchyBuilding, WirePathAttribute via tspconfig - Python: @@OverRide for parameter reorder, accept patterns for common types - JS: three-way merge workflow, TypeSpec-first recommendation - Per-language changelog pattern examples for each pattern Sources: - .NET: azure-sdk-for-net/.github/skills/mitigate-breaking-changes/SKILL.md - Java: azure-sdk-for-java/docs/contributor/typespec-quickstart.md - Python: azure-sdk-for-python/doc/dev/mgmt/sdk-breaking-changes-guide.md - Go: azure-sdk-for-go/documentation/development/breaking-changes/sdk-breaking-changes-guide.md - JS: aka.ms/azsdk/js/customization Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
…plan Breaking change detection via SDK changelog will be handled by the separate azsdk_package_detect_breaking_change tool (Crystal's PR #15248) in the CI/release pipeline stage. The authoring plan tool focuses on pattern-based detection using sdk-breaking-patterns.md during TypeSpec authoring. Removed: - sdkChangelog parameter from GenerateTypeSpecAuthoringPlan MCP tool - --sdk-changelog CLI option - Changelog injection into AdditionalInfo - Changelog reference in tool description Kept: - TryLoadBreakingPatternsAsync() for loading sdk-breaking-patterns.md - IGitHelper dependency for repo root discovery - SDK breaking change detection via pattern matching (no changelog needed) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
The following pipelines have been queued for testing: |
chunyu3
force-pushed
the
savaity/sdk-breaking-patterns
branch
from
bb1a9a2 to
ffd22e0
Compare
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
|
Hey @samvaity and @chunyu3 , thanks for the pr! while I like detecting breaking change earlier, after discussing with Shanghai side in the scrum meeting, we may have some concerns about putting the breaking detection and mitigation into each individual authoring task. /cc @ArthurMa1978 @josefree @lirenhe
|
|
|
||
| ## Pattern Catalog | ||
|
|
||
| ### 1. Enum Changed to Extensible Union |
There was a problem hiding this comment.
We’ve discussed this before: on the TypeSpec side, we should not allow this kind of change. Instead of modifying an existing API, we should use version decorators to deprecate the old version and introduce a new one.
So this pattern should be invalid and not allowed.
There was a problem hiding this comment.
In this case, yes, we can use version decorate to insure that extensible union only start from a special version, as following:
before
enum DocumentFormulaKind {
@doc("A formula embedded within the content of a paragraph.")
inline: "inline",
@doc("A formula in display mode that takes up an entire line.")
display: "display",
}
after:
union DocumentFormulaKind {
@doc("A formula embedded within the content of a paragraph.")
inline: "inline",
@doc("A formula in display mode that takes up an entire line.")
display: "display",
@added(Versions.v2026_01_30)
string,
}
The pattern still be enum -> union
There was a problem hiding this comment.
Addressed in 6245127 — added a note that this change should use version decorators (@added) to introduce the union in a new API version. Also updated .NET impact to ❌ Breaking with a note that @@alternateType may not fully resolve it for .NET and customization code (partial classes) may be needed.
|
|
||
| **Per-Language Impact:** | ||
| - **Java:** ❌ Breaking — enum ordinal values change, deserialization breaks | ||
| - **.NET:** ⚠️ May break — strong typing affected, but extensible enums are handled |
There was a problem hiding this comment.
Breaking for .Net and can't be fixed by client.tsp
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
|
I re-run Benchmark with/without extending azsdk_typespec_generate_authoring_plan with the latest We ran the existing 21 authoring tasks using two versions of the authoring plan—one with the extension and one without. The time cost of invoking the MCP tool shows no significant difference between the two. Extending azsdk_typespec_generate_authoring_plan with SDK breaking change detection does not introduce any noticeable performance overhead.
|
|
Thanks @Sameeksha and @chunyu3 for the PR!
|
There was a problem hiding this comment.
Pull request overview
This PR extends the TypeSpec authoring-plan workflow to proactively surface SDK breaking-change risks by (1) adding a breaking-change pattern knowledge document to the knowledge base and (2) updating the QA bot / authoring prompts and tooling so the plan can incorporate SDK-impact guidance (including using a local TypeSpec diff as additional context).
Changes:
- Added a new shared knowledge document (
eng/common/knowledge/sdk-breaking-patterns.md) intended for KB/RAG retrieval to drive SDK-impact warnings and mitigations. - Updated knowledge-sync config and TypeSpec authoring prompts to include the breaking-change pattern content and require an explicit “SDK breaking changes checked” step.
- Updated
azsdk_typespec_generate_authoring_planimplementation to accept--target-branchand attach a localgit diffof the TypeSpec project as additional context.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/sdk-ai-bots/azure-sdk-qa-bot-knowledge-sync/config/knowledge-config.json | Adds eng/common/knowledge as an indexed knowledge source for the KB. |
| tools/sdk-ai-bots/azure-sdk-qa-bot-backend/service/prompt/prompt.go | Extends prompt {{include ...}} handling to support blob: includes (KB content). |
| tools/sdk-ai-bots/azure-sdk-qa-bot-backend/service/prompt/azure_typespec_authoring/qa.md | Makes SDK breaking-change detection mandatory and includes the breaking-patterns blob document. |
| tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/TypeSpec/TypeSpecAuthoringTool.cs | Adds --target-branch and sends a local TypeSpec diff to the KB request as additional info. |
| tools/azsdk-cli/Azure.Sdk.Tools.Cli/Helpers/GitHelper.cs | Adds GetDiffAsync helper used to retrieve the diff for the authoring tool. |
| eng/common/knowledge/sdk-breaking-patterns.md | New knowledge-base content: breaking change patterns + mitigations (used for RAG). |
| .github/skills/azure-typespec-author/SKILL.md | Updates the skill workflow to require surfacing SDK breaking-change warnings/mitigations from the plan output. |
| - Exact kind of changes to make (operations/models/decorators/versioning) | ||
| - Expected impact (breaking vs non-breaking) | ||
| - Diff outline (high level, no code): | ||
| - SDK Breaking Changes and Mitigation (REQUIRED if SDK breaking changes are detected in **DK Breaking Change Detection Process**): |
There was a problem hiding this comment.
This typo issue should be solved
|
The following pipelines have been queued for testing: |
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>
|
The following pipelines have been queued for testing: |
|
The following pipelines have been queued for testing: |
| return "failed to download the blob", err | ||
| } | ||
| } else { | ||
| return "No blob path found", err |
There was a problem hiding this comment.
Since err is null here, "No blob path found" will be included in the prompt. Should we change it to return "", fmt.Errorf("failed to download blob %q: %w", blobName, err)?
There was a problem hiding this comment.
Could you help update the changelog and version for chat bot backend?
|
Closing this PR, in lieu of #15248 (comment) |
Summary
Extends the TypeSpec authoring plan tool (
azsdk_typespec_generate_authoring_plan) to detect SDK breaking changes during TypeSpec authoring — before merge.Today: Write TypeSpec → merge → SDK generation fails or produces breaking changes → weeks of back-and-forth
After: Write TypeSpec → authoring plan warns about SDK impact → dev applies
client.tspmitigations in the same sessionWhat This PR Does
New:
eng/common/knowledge/sdk-breaking-patterns.md(539 lines)client.tspmitigations with concrete code examplesUpdated:
TypeSpecAuthoringTool.cs(+1 line)Updated: Knowledge sync config + QA bot prompts
knowledge-config.json: Registerssdk-breaking-patterns.mdfor KB indexingqa.md/prompt.go: Updated system prompts to include SDK breaking change contextHow It Works
The authoring plan tool already sends one HTTP POST to the KB service (
IAzureSdkKnowledgeBaseService). Breaking change patterns are delivered via server-side RAG retrieval — the KB service indexessdk-breaking-patterns.mdand retrieves relevant patterns when the user's request matches. Zero additional client-side calls. No SDK repo needed — works entirely from TypeSpec changes + patterns.This is Layer 1 (authoring-time detection) of a multi-layer approach:
azsdk_typespec_generate_authoring_planvia KB/RAGazsdk_sdk_breaking_change_detect(proposed)Layer 1 is proactive and free. Layer 3 is the comprehensive safety net. They complement each other.
Key Design Decisions
TryLoadBreakingPatternsAsyncapproach).client.tspmitigations are standard practice.Related