Skip to content

[TCGC] Fix @override check regression#4730

Merged
tadelesh merged 4 commits into
mainfrom
fix-tcgc-regression
Jun 30, 2026
Merged

[TCGC] Fix @override check regression#4730
tadelesh merged 4 commits into
mainfrom
fix-tcgc-regression

Conversation

@tadelesh

@tadelesh tadelesh commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Problem

The @override @path-preservation check (added in #4644, reworked in #4693) produced false override-parameters-mismatch diagnostics on real ARM specs. Three distinct issues:

  1. [tcgc] fix regression in override-parameters-mismatch #4693 switched parameter matching to name-based, which over-relaxed the check.
  2. [tcgc] fix path in override #4644 compared parameters by sorted position; when an override adds/reorders parameters the index misaligns, flagging params that already carry @path (e.g. botservice channelName).
  3. Determining "is a path parameter" from the @path decorator in the type graph (isPathParam) is unreliable: a parameter can carry @path without being part of the realized route (e.g. an ARM key surfaced by a templated ArmProviderActionSync, as in cognitiveservices calculateModelCapacity), and conversely a @path nested inside a plain model or @bodyRoot IS realized.

Fix

  • Revert [tcgc] fix regression in override-parameters-mismatch #4693 to restore positional structural comparison of the parameter lists.
  • Resolve the original operation's realized HTTP route (getHttpOperation) to get its actual path parameters — the ground truth that handles both phantom @path (orphaned, not in route → not enforced) and nested @path (in route → enforced).
  • Match the corresponding override parameter by name (not position) and require it to be a path parameter (isPathParam). This removes the finishType workaround and the body-param heuristics entirely.

Result

  • All previously-failing ARM specs compile: storage, security, recoveryservicesbackup, quota, policyinsights, iothub, cognitiveservices, botservice, apimanagement, frontdoor, databoxedge. Overrides that genuinely dropped @path on a realized path param add @path (correct); phantom cases (cognitiveservices calculateModelCapacity name) need no spec change.
  • override.test.ts 16/16; full TCGC suite 1330 pass.

@tadelesh tadelesh added the int:azure-specs Run integration tests against azure-rest-api-specs label Jun 29, 2026
@microsoft-github-policy-service microsoft-github-policy-service Bot added the lib:tcgc Issues for @azure-tools/typespec-client-generator-core library label Jun 29, 2026
@pkg-pr-new

pkg-pr-new Bot commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@azure-tools/typespec-client-generator-core@4730

commit: 59b4ba5

@azure-sdk-automation

azure-sdk-automation Bot commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown

All changed packages have been documented.

  • ✅ @azure-tools/typespec-client-generator-core
Show changes

@azure-tools/typespec-client-generator-core - fix ✏️

Verify all path parameters are preserved in @override customizations. The check now resolves the original operation's realized HTTP route to determine its path parameters, instead of relying on the @path decorator in the type graph. This correctly handles parameters that carry @path but are not part of the realized route (for example an ARM key surfaced by a templated provider action) as well as @path parameters nested inside a plain model or @bodyRoot. The corresponding override parameter is matched by name rather than position so overrides that add or reorder parameters no longer produce false override-parameters-mismatch diagnostics.

@github-actions

github-actions Bot commented Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

⚡ Benchmark Results

⚠️ 16 metric(s) regressed above the +5% threshold:

Metric Baseline Current Change
total 🔴 581.1ms 🔴 654.1ms +12.6% 🔴
loader 🟢 154.2ms 🟡 220.0ms +42.7% 🔴
resolver 🟢 16.3ms 🟢 17.3ms +6.2% 🔴
linter 🟢 134.3ms 🟢 144.9ms +7.9% 🔴
 ↳ linter/@azure-tools/typespec-azure-core/no-header-explode 🟡 18.3ms 🔴 21.8ms +19.3% 🔴
 ↳ linter/@azure-tools/typespec-azure-core/no-query-explode 🟡 17.8ms 🔴 21.7ms +22.0% 🔴
 ↳ linter/@azure-tools/typespec-azure-core/no-response-body 🔴 22.7ms 🔴 26.0ms +14.2% 🔴
 ↳ linter/@azure-tools/typespec-azure-core/response-schema-problem 🔴 22.0ms 🔴 23.3ms +6.1% 🔴
 ↳ linter/@azure-tools/typespec-azure-resource-manager/lro-location-header 🟡 13.9ms 🟡 15.6ms +12.2% 🔴
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-response-body 🟡 19.8ms 🔴 22.3ms +12.6% 🔴
emit 🔴 5.28s 🔴 5.80s +9.8% 🔴
 ↳ emit/@azure-tools/typespec-autorest 🟢 156.1ms 🟢 168.2ms +7.8% 🔴
 ↳ emit/@azure-tools/typespec-python 🔴 4.04s 🔴 4.43s +9.8% 🔴
 ↳ emit/@typespec/http-client-js 🔴 906.8ms 🔴 1.05s +15.3% 🔴
 ↳ emit/@typespec/openapi3 🟢 140.9ms 🟢 150.4ms +6.8% 🔴
 ↳ emit/@typespec/openapi3/compute 🟢 122.8ms 🟢 132.9ms +8.2% 🔴
Full details – comparing 48f5a5f vs baseline 32ff9cc
Metric Baseline Current Change
total 🔴 581.1ms 🔴 654.1ms +12.6% 🔴
loader 🟢 154.2ms 🟡 220.0ms +42.7% 🔴
resolver 🟢 16.3ms 🟢 17.3ms +6.2% 🔴
checker 🟡 223.4ms 🟡 201.5ms -9.8% 🟢
validation 🟢 44.6ms 🟢 44.6ms +0.0%
 ↳ validation/@azure-tools/typespec-azure-core 🟢 6.2ms 🟢 7.1ms +15.1%
 ↳ validation/@typespec/http 🟢 5.4ms 🟢 5.8ms +8.5%
 ↳ validation/@typespec/rest 🟢 0.5ms 🟢 0.5ms +6.8%
 ↳ validation/@typespec/versioning 🔴 30.7ms 🔴 29.4ms -4.2%
 ↳ validation/compiler 🟢 1.5ms 🟢 1.4ms -7.4%
linter 🟢 134.3ms 🟢 144.9ms +7.9% 🔴
 ↳ linter/@azure-tools/typespec-azure-core/auth-required 🟢 0.0ms 🟢 0.0ms -0.4%
 ↳ linter/@azure-tools/typespec-azure-core/bad-record-type 🟢 0.2ms 🟢 0.2ms +1.7%
 ↳ linter/@azure-tools/typespec-azure-core/byos 🟢 6.2ms 🟢 6.3ms +2.0%
 ↳ linter/@azure-tools/typespec-azure-core/casing-style 🟢 0.6ms 🟢 0.6ms +8.0%
 ↳ linter/@azure-tools/typespec-azure-core/composition-over-inheritance 🟢 0.1ms 🟢 0.1ms -4.7%
 ↳ linter/@azure-tools/typespec-azure-core/documentation-required 🟢 0.8ms 🟢 0.8ms -0.1%
 ↳ linter/@azure-tools/typespec-azure-core/friendly-name 🟢 0.5ms 🟢 0.6ms +13.0%
 ↳ linter/@azure-tools/typespec-azure-core/key-visibility-required 🟢 0.2ms 🟢 0.2ms +4.0%
 ↳ linter/@azure-tools/typespec-azure-core/known-encoding 🟢 0.3ms 🟢 0.3ms +16.4%
 ↳ linter/@azure-tools/typespec-azure-core/long-running-polling-operation-required 🟢 0.3ms 🟢 0.3ms +13.8%
 ↳ linter/@azure-tools/typespec-azure-core/no-case-mismatch 🟢 0.2ms 🟢 0.2ms +5.4%
 ↳ linter/@azure-tools/typespec-azure-core/no-closed-literal-union 🟢 0.2ms 🟢 0.2ms +8.0%
 ↳ linter/@azure-tools/typespec-azure-core/no-enum 🟢 0.0ms 🟢 0.0ms -5.7%
 ↳ linter/@azure-tools/typespec-azure-core/no-error-status-codes 🟢 0.1ms 🟢 0.1ms +5.4%
 ↳ linter/@azure-tools/typespec-azure-core/no-explicit-routes-resource-ops 🟢 0.1ms 🟢 0.1ms -4.5%
 ↳ linter/@azure-tools/typespec-azure-core/no-format 🟢 0.5ms 🟢 0.6ms +14.3%
 ↳ linter/@azure-tools/typespec-azure-core/no-generic-numeric 🟢 0.4ms 🟢 0.4ms +1.6%
 ↳ linter/@azure-tools/typespec-azure-core/no-header-explode 🟡 18.3ms 🔴 21.8ms +19.3% 🔴
 ↳ linter/@azure-tools/typespec-azure-core/no-legacy-usage 🟢 1.1ms 🟢 1.2ms +3.6%
 ↳ linter/@azure-tools/typespec-azure-core/no-multiple-discriminator 🟢 0.1ms 🟢 0.1ms +17.5%
 ↳ linter/@azure-tools/typespec-azure-core/no-nullable 🟢 0.2ms 🟢 0.3ms +11.9%
 ↳ linter/@azure-tools/typespec-azure-core/no-offsetdatetime 🟢 1.2ms 🟢 1.2ms +3.3%
 ↳ linter/@azure-tools/typespec-azure-core/no-openapi 🟢 2.0ms 🟢 2.1ms +5.3%
 ↳ linter/@azure-tools/typespec-azure-core/no-private-usage 🟢 1.8ms 🟢 1.9ms +7.4%
 ↳ linter/@azure-tools/typespec-azure-core/no-query-explode 🟡 17.8ms 🔴 21.7ms +22.0% 🔴
 ↳ linter/@azure-tools/typespec-azure-core/no-response-body 🔴 22.7ms 🔴 26.0ms +14.2% 🔴
 ↳ linter/@azure-tools/typespec-azure-core/no-rest-library-interfaces 🟢 0.0ms 🟢 0.0ms -6.2%
 ↳ linter/@azure-tools/typespec-azure-core/no-route-parameter-name-mismatch 🟢 4.8ms 🟢 5.5ms +13.2%
 ↳ linter/@azure-tools/typespec-azure-core/no-rpc-path-params 🟢 0.2ms 🟢 0.2ms +22.0%
 ↳ linter/@azure-tools/typespec-azure-core/no-string-discriminator 🟢 0.0ms 🟢 0.0ms +6.0%
 ↳ linter/@azure-tools/typespec-azure-core/no-unknown 🟢 0.2ms 🟢 0.2ms +17.7%
 ↳ linter/@azure-tools/typespec-azure-core/no-unnamed-union 🟢 0.3ms 🟢 0.4ms +11.1%
 ↳ linter/@azure-tools/typespec-azure-core/operation-missing-api-version 🟢 0.2ms 🟢 0.2ms -4.9%
 ↳ linter/@azure-tools/typespec-azure-core/request-body-problem 🟢 0.3ms 🟢 0.3ms +3.5%
 ↳ linter/@azure-tools/typespec-azure-core/require-versioned 🟢 0.0ms 🟢 0.0ms -18.3%
 ↳ linter/@azure-tools/typespec-azure-core/response-schema-problem 🔴 22.0ms 🔴 23.3ms +6.1% 🔴
 ↳ linter/@azure-tools/typespec-azure-core/rpc-operation-request-body 🟢 0.3ms 🟢 0.3ms +18.7%
 ↳ linter/@azure-tools/typespec-azure-core/spread-discriminated-model 🟢 0.2ms 🟢 0.3ms +11.4%
 ↳ linter/@azure-tools/typespec-azure-core/use-standard-names 🟢 5.0ms 🟢 5.8ms +16.9%
 ↳ linter/@azure-tools/typespec-azure-core/use-standard-operations 🟢 0.1ms 🟢 0.1ms +17.4%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-agent-base-type-child-resources 🟢 4.0ms 🟢 4.3ms +8.5%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-agent-base-type-lifecycle-operations 🟢 0.0ms 🟢 0.0ms -19.4%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-common-types-version 🟢 4.3ms 🟢 4.3ms -1.2%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-custom-resource-no-key 🟢 0.1ms 🟢 0.1ms +16.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-custom-resource-usage-discourage 🟢 0.1ms 🟢 0.1ms +9.5%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-delete-operation-response-codes 🟢 1.1ms 🟢 1.2ms +7.8%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-no-path-casing-conflicts 🟢 4.7ms 🟢 4.6ms -1.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-no-record 🟢 0.3ms 🟢 0.4ms +3.6%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-post-operation-response-codes 🟢 0.4ms 🟢 0.5ms +13.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-put-operation-response-codes 🟢 0.0ms 🟢 0.0ms -15.9%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-action-no-segment 🟢 0.2ms 🟢 0.2ms +14.0%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-duplicate-property 🟢 0.1ms 🟢 0.1ms +17.8%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator 🟢 0.0ms 🟢 0.0ms -5.5%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-action-verb 🟢 0.1ms 🟢 0.1ms +1.3%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-envelope-property 🟢 0.1ms 🟢 0.1ms +0.5%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-invalid-version-format 🟢 0.0ms 🟢 0.0ms +8.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-key-invalid-chars 🟢 0.2ms 🟢 0.3ms +7.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-name-pattern 🟢 0.0ms 🟢 0.0ms -13.6%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-operation 🟢 0.2ms 🟢 0.2ms +0.0%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-operation-response 🟢 4.5ms 🟢 4.9ms +7.5%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-patch 🟢 0.3ms 🟢 0.3ms +7.2%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-path-segment-invalid-chars 🟢 0.2ms 🟢 0.2ms +2.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/arm-resource-provisioning-state 🟢 0.1ms 🟢 0.1ms +1.8%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/beyond-nesting-levels 🟢 0.1ms 🟢 0.1ms +3.5%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/empty-updateable-properties 🟢 0.1ms 🟢 0.2ms +3.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/improper-subscription-list-operation 🟢 0.0ms 🟢 0.0ms -1.9%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/lro-location-header 🟡 13.9ms 🟡 15.6ms +12.2% 🔴
 ↳ linter/@azure-tools/typespec-azure-resource-manager/missing-operations-endpoint 🟢 0.0ms 🟢 0.0ms +2.7%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers 🟢 0.3ms 🟢 0.3ms +2.6%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-empty-model 🟢 0.1ms 🟢 0.1ms +6.0%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-override-props 🟢 0.1ms 🟢 0.1ms -5.3%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-resource-delete-operation 🟢 0.2ms 🟢 0.2ms +28.4%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/no-response-body 🟡 19.8ms 🔴 22.3ms +12.6% 🔴
 ↳ linter/@azure-tools/typespec-azure-resource-manager/patch-envelope 🟢 0.1ms 🟢 0.2ms +14.2%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/resource-name 🟢 0.2ms 🟢 0.2ms +3.2%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/secret-prop 🟢 2.3ms 🟢 2.0ms -15.2%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/unsupported-type 🟢 0.4ms 🟢 0.4ms +11.5%
 ↳ linter/@azure-tools/typespec-azure-resource-manager/version-progression 🟢 0.0ms 🟢 0.0ms -10.9%
 ↳ linter/@azure-tools/typespec-client-generator-core/property-name-conflict 🟢 1.0ms 🟢 1.2ms +9.9%
 ↳ linter/@azure-tools/typespec-client-generator-core/require-client-suffix 🟢 0.2ms 🟢 0.2ms -5.0%
emit 🔴 5.28s 🔴 5.80s +9.8% 🔴
 ↳ emit/@azure-tools/typespec-autorest 🟢 156.1ms 🟢 168.2ms +7.8% 🔴
 ↳ emit/@azure-tools/typespec-python 🔴 4.04s 🔴 4.43s +9.8% 🔴
 ↳ emit/@typespec/http-client-js 🔴 906.8ms 🔴 1.05s +15.3% 🔴
 ↳ emit/@typespec/openapi3 🟢 140.9ms 🟢 150.4ms +6.8% 🔴
 ↳ emit/@typespec/openapi3/compute 🟢 122.8ms 🟢 132.9ms +8.2% 🔴
 ↳ emit/@typespec/openapi3/write 🟢 17.3ms 🟢 17.6ms +2.0%

Averaged across 3 specs (azure-arm-resource-manager, azure-core-dataplane, azure-full).
Threshold: changes > ±5% are highlighted.
🟢 Fast · 🟡 Moderate (stages >200ms, rules >10ms) · 🔴 Slow (stages >400ms, rules >20ms)

@azure-sdk-automation

azure-sdk-automation Bot commented Jun 29, 2026

Copy link
Copy Markdown

You can try these changes here

🛝 Playground 🌐 Website

@tadelesh tadelesh force-pushed the fix-tcgc-regression branch 5 times, most recently from 086f086 to b06babb Compare June 29, 2026 10:09
@tadelesh tadelesh force-pushed the fix-tcgc-regression branch 2 times, most recently from 78ff5d8 to cbb4acb Compare June 30, 2026 03:21
fix(tcgc): use realized HTTP path params for override @path check
219eff8 
Resolve the original operation's realized HTTP route to determine its path parameters instead of relying on the @path decorator in the type graph, and match the corresponding override parameter by name rather than position. This fixes false override-parameters-mismatch diagnostics for params that carry @path without being in the realized route (ARM templated provider actions) and for overrides that add or reorder parameters.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tadelesh tadelesh force-pushed the fix-tcgc-regression branch from cbb4acb to 219eff8 Compare June 30, 2026 03:44
@tadelesh tadelesh added this pull request to the merge queue Jun 30, 2026
@tadelesh tadelesh removed this pull request from the merge queue due to a manual request Jun 30, 2026
@tadelesh tadelesh added this pull request to the merge queue Jun 30, 2026
Merged via the queue into main with commit 1140c23 Jun 30, 2026
29 of 30 checks passed
@tadelesh tadelesh deleted the fix-tcgc-regression branch June 30, 2026 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timotheeguerin timotheeguerin timotheeguerin approved these changes
@lmazuel lmazuel Awaiting requested review from lmazuel lmazuel is a code owner
@m-nash m-nash Awaiting requested review from m-nash m-nash is a code owner
@iscai-msft iscai-msft Awaiting requested review from iscai-msft iscai-msft is a code owner
@srnagar srnagar Awaiting requested review from srnagar srnagar is a code owner
@joheredi joheredi Awaiting requested review from joheredi joheredi is a code owner
@bterlson bterlson Awaiting requested review from bterlson bterlson is a code owner
@markcowl markcowl Awaiting requested review from markcowl markcowl is a code owner

Assignees

No one assigned

Labels

int:azure-specs Run integration tests against azure-rest-api-specs lib:tcgc Issues for @azure-tools/typespec-client-generator-core library

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tadelesh @timotheeguerin