Skip to content

Use deterministic ordering for C# derived-model references - #11950

Open
JoshLove-msft with Copilot wants to merge 3 commits into
mainfrom
copilot/use-deterministic-ordering
Open

Use deterministic ordering for C# derived-model references#11950
JoshLove-msft with Copilot wants to merge 3 commits into
mainfrom
copilot/use-deterministic-ordering

Conversation

Copilot AI commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Derived-model references in C# documentation inherit HashSet enumeration order, risking unintended changes on regeneration.

  • Ordering: Sort the deduplicated models by name using StringComparer.Ordinal for culture-independent output.
  • Coverage: Add regression cases for reversed input order, deduplication, and ordinal sorting; update abstract and non-abstract base-model documentation expectations.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Co-authored-by: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com>
@microsoft-github-policy-service microsoft-github-policy-service Bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Sep 11, 2026
Copilot AI changed the title [WIP] Ensure deterministic ordering in abstract models reference docs Use deterministic ordering for C# derived-model references Sep 11, 2026
Copilot AI requested a review from JoshLove-msft September 11, 2026 23:39
@JoshLove-msft
JoshLove-msft marked this pull request as ready for review September 11, 2026 23:43
Copilot AI lite review requested due to automatic review settings September 11, 2026 23:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Add a fully qualified-name tie-breaker for same-named models to ensure ordering is deterministic.

Pull request overview

Updates C# derived-model documentation to use deterministic ordering and adds regression coverage.

Changes:

  • Sorts derived models with ordinal comparison.
  • Adds ordering and deduplication tests.
  • Updates documentation expectations.
File summaries
File Reviewed change
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs Adds ordering and deduplication coverage.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/DiscriminatorTests.cs Updates documentation expectations.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs Applies deterministic derived-model sorting.
Review details

Suppressed comments (1)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs:134

  • This only orders by the simple generated name, so two valid derived models with the same name in different namespaces compare equal. OrderBy preserves the HashSet order for equal keys, leaving the fully qualified <see> references nondeterministic; add a ThenBy on m.Type.FullyQualifiedName (the value emitted in the documentation).
            return [.. derivedModels.OrderBy(m => m.Name, StringComparer.Ordinal)];
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@JoshLove-msft

Copy link
Copy Markdown
Contributor

Copilot you need to regenerate

Co-authored-by: JoshLove-msft <54595583+JoshLove-msft@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 12, 2026 04:29

Copilot AI commented Sep 12, 2026

Copy link
Copy Markdown
Contributor Author

Copilot you need to regenerate

Regenerated all C# libraries in 510e491. Three generated sample files changed, and repeat generation produced identical output. The build and all runnable emitter/.NET tests passed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review details

Suppressed comments (1)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs:134

  • Sorting only by the short Name does not make this fully deterministic: derived models can share a C# name when they come from different namespaces, while the documentation emits Type.FullyQualifiedName. For equal names, OrderBy preserves the HashSet iteration order, so reversed input can still reorder the references; add an ordinal tie-breaker on the fully qualified name (and cover that case).
            return [.. derivedModels.OrderBy(m => m.Name, StringComparer.Ordinal)];
  • Files reviewed: 3/6 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use deterministic ordering in abstract models reference docs

3 participants