You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[http-client-csharp] Fix external model serialization ownership and factory discriminators - #12343
Allow an explicit [CodeGenMember] property on the first generated subclass to replace a TypeSpec property added to an externally mapped base.
Preserve its inherited serialization-constructor slot and metadata instead of introducing a second deserialization branch/constructor argument.
Give the explicit replacement ownership of its JSON key, excluding that key from generated raw and typed additional-data writers while preserving unrelated data and existing read-only/wire-format rules.
Recover discriminator metadata for matching handwritten serialization-constructor parameters. Known-variant factories keep their fixed discriminator; unknown variants remain flexible.
Root causes and supported scope
The canonical customization view only considered locally declared TypeSpec properties. A proxy for an inherited external-base property therefore became a separate member, while the external writer could not serialize the newly added property. A proxy backed by additional data could also write the same JSON key twice.
Handwritten constructor parameters do not carry generated PropertyProvider metadata. Additionally, an external extensible discriminator struct's runtime/symbol representations differ in generator-only enum annotations, so strict CSharpType.Equals cannot establish CLR identity. Factory matching now uses the original parameter name, namespace, CLR type-name identity and parameter modifiers before restoring discriminator metadata.
This does not invent external CLR members, constructors, additional-data storage or patch behavior. Consumers must explicitly bind extension properties and supply constructors that call the actual accessible external constructor when its shape differs. Wire-name aliases alone are not silently deduplicated, and properties already owned by the external writer must not be claimed again.
Validation
Added regressions and observed failures before the corresponding fixes, including actual duplicate JSON keys and the external extensible-struct factory gap.
Complete emitted models, serialization partials, factories, helpers and MRW-generated context compile and execute. Eight runtime combinations cover JSON/wire formats, raw/typed bags and read-only properties, with unique-key and read/write/read assertions through base JSON and persistable interfaces. Negative alias cases and compiled known/unknown factory cases are included.
Core generator: 2,944 passed; final factory rerun: 86 passed.
ClientModel: 2,519 passed; Input: 304 passed.
Emitter: 384 passed, 2 skipped.
Full eng/scripts/Generate.ps1 regeneration succeeded with no generated-file changes.
Full generator solution build, emitter build and API extraction passed; repository formatting, package-scoped oxlint --type-aware --deny-warnings, and npm run cop passed.
Windows validation used short-path mappings and an artifact-only MSBuild target to avoid existing MAX_PATH limits when copying source fixtures. The package's stale npm run lint references unavailable ESLint; the repository-standard oxlint command above passed instead. No repository/toolchain configuration was changed for these workarounds.
The coordinating Azure SDK investigation also generated and compiled an actual OpenAI 2.14-derived fixture using these shared DLLs: both keys occur once in J/W through derived/base interfaces, values round-trip, and the concrete factory fixes its discriminator. All 315 Azure.Generator tests passed. The independent downstream guard is Azure/azure-sdk-for-net#64105, not a consuming generator release.
SDK integration
Consume the updated shared generator assemblies, bind proxies using [CodeGenMember("ResponseId")] / [CodeGenMember("AgentReference")], and regenerate. Retain the single original constructor argument for each property; remove duplicate proxy-specific arguments. Keep necessary custom constructor/storage logic; CodeGenSerialization.SerializationName can complement the binding but does not replace it.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Normalized-name bindings can lose serialization metadata, and required replacements can produce duplicate constructor parameters in generated grandchildren.
This PR updates the C# client generator so explicit custom properties can own serialization of properties added to externally mapped bases, while model factories preserve discriminator behavior.
Changes:
Bind external-base replacements to inherited metadata and constructor slots, and exclude their JSON keys from generated additional-data writers.
Recover discriminator metadata for handwritten constructors, keeping known variants fixed and unknown variants flexible.
Add compilation and round-trip regressions and document customization boundaries.
This prefilter rejects normalized bindings such as [CodeGenMember("IPFoo")] for ipFoo and [CodeGenMember("CreatedOn")] for a date-time creationDate property, leaving the external-property proxy without inherited serialization metadata or constructor-slot replacement. Include the non-exact-name normalization already used by this class at lines 41–44 and covered by ModelCustomizationTests.CustomCodeReplacesAcronymNormalizedProperty and CustomCodeReplacesDateNormalizedProperty.
Preserve IEnumerable parameters for initialization constructors
For a required list replacement, this substitutes IList<T> for the inherited initialization constructor's IEnumerable<T> parameter, so callers can no longer pass arbitrary enumerable sequences. Preserve ToPublicInputParameter() for initialization constructors, as AddInitializationParameterForCtor does at ModelProvider.cs:1858 and ModelProviderTests.ValidateListParameterHandlingInConstructor verifies; keep AsParameter for serialization constructors.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Shared constructor metadata and serialization ownership changes across external inheritance boundaries warrant final human compatibility review despite the extensive regressions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
microsoft-github-policy-service
Bot
added
the
emitter:client:csharp
Summary
[CodeGenMember]property on the first generated subclass to replace a TypeSpec property added to an externally mapped base.Root causes and supported scope
The canonical customization view only considered locally declared TypeSpec properties. A proxy for an inherited external-base property therefore became a separate member, while the external writer could not serialize the newly added property. A proxy backed by additional data could also write the same JSON key twice.
Handwritten constructor parameters do not carry generated
PropertyProvidermetadata. Additionally, an external extensible discriminator struct's runtime/symbol representations differ in generator-only enum annotations, so strictCSharpType.Equalscannot establish CLR identity. Factory matching now uses the original parameter name, namespace, CLR type-name identity and parameter modifiers before restoring discriminator metadata.This does not invent external CLR members, constructors, additional-data storage or patch behavior. Consumers must explicitly bind extension properties and supply constructors that call the actual accessible external constructor when its shape differs. Wire-name aliases alone are not silently deduplicated, and properties already owned by the external writer must not be claimed again.
Validation
eng/scripts/Generate.ps1regeneration succeeded with no generated-file changes.oxlint --type-aware --deny-warnings, andnpm run coppassed.Windows validation used short-path mappings and an artifact-only MSBuild target to avoid existing MAX_PATH limits when copying source fixtures. The package's stale
npm run lintreferences unavailable ESLint; the repository-standard oxlint command above passed instead. No repository/toolchain configuration was changed for these workarounds.The coordinating Azure SDK investigation also generated and compiled an actual OpenAI 2.14-derived fixture using these shared DLLs: both keys occur once in J/W through derived/base interfaces, values round-trip, and the concrete factory fixes its discriminator. All 315 Azure.Generator tests passed. The independent downstream guard is Azure/azure-sdk-for-net#64105, not a consuming generator release.
SDK integration
Consume the updated shared generator assemblies, bind proxies using
[CodeGenMember("ResponseId")]/[CodeGenMember("AgentReference")], and regenerate. Retain the single original constructor argument for each property; remove duplicate proxy-specific arguments. Keep necessary custom constructor/storage logic;CodeGenSerialization.SerializationNamecan complement the binding but does not replace it.--generated by Copilot