fix(http-client-csharp): restore historical bases over mapped ancestors by live1206 · Pull Request #12310 · microsoft/typespec

Wei Hu (live1206) · 2026-10-20T09:23:42Z

Summary

Fixes #12304 (the original BotData existing-mapped-base scenario).

Restore a compatible historical framework base over an existing mapped CLR ancestor, reconciling child-owned and ancestor-owned inherited properties without changing input ownership or shared providers.
Keep canonicalization and constructor traversal consistent with the model-local projection. Preserve source-owner vetoes, custom property/field authority, and type/accessor/collision/ambiguity safeguards.
Retain the historical BotData location constructor through compatible inherited-property assignment, preserving parameter types/defaults and rejecting unsafe conversions or inaccessible setters.
Preserve projected child properties during deserialization. Getter-only collection population requires an explicit downstream mutability contract; unknown interfaces and read-only wrappers retain the current base.

Scope after the split

This PR does not introduce a framework constructor-default approval API. The separate Network proposal #12311 is withdrawn. The latest plan is an SDK-owned legacy constructor with existing CodeGenSuppress, plus a narrowly scoped relaxation of the custom-constructor restoration guard here if it remains small. That relaxation is not implemented yet; the flat-base and real-SDK validation prerequisites still apply.

BotData’s inherited-property constructor repair is included: restoring its base alone does not retain its historical public location overload.

Change-specific validation

The original BotData regression remains intact: historical base, inherited Location/Tags ownership, and the public non-nullable location constructor.
Controls cover nonempty-base canonicalization, duplicate ancestor initialization, input/shared-provider isolation, source-owner vetoes in both cache orders, custom property/field aliases, and inherited parameter/default/setter safety.
Added compiled scalar/dictionary/list JSON round trips, read-only-wrapper and unapproved-interface fallback controls, virtual-event override/hiding controls, and an emitted constructor baseline including inherited backing-field exclusion.
Latest local validation: 148 core + 75 ClientModel focused checks, 3,058/3,058 core Generator tests, and 2,648/2,648 ClientModel tests passed, with no skipped tests. CI for 4cb8aac5e passed, including Windows/Linux C# tests and regeneration checks (build 7013300).

No real SDK regeneration, GA ApiCompat run, or inherited ARM-envelope JSON-roundtrip validation is claimed. The separate flat-base feature remains #12303.

- by copilot

…ctors

pkg-pr-new · 2026-10-20T09:26:00Z

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@12310

commit: ac1cc78

github-actions · 2026-10-20T09:27:24Z

No changes needing a change description found.

…to follow-up

…-restoration

Copilot

🟡 Changes recommended

The synthetic full constructor can omit inherited Location, causing deserialization to leave the property at its default value.

3 open issues

What changed in this PR

This PR addresses #12304 by extending C# client-generator compatibility handling to restore historical framework bases over existing mapped ancestors, including BotData’s inherited properties and public location constructor. Flat-model restoration and framework-default constructor approval remain outside this PR’s scope.

Changes:

Adds model-local inherited-property projections while preserving input ownership and shared providers.
Updates base resolution, canonical properties, and constructor traversal to use those projections.
Adds regression coverage for compatibility safeguards, customization authority, and historical constructor metadata.

Paths are relative to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/.

File	Description
`test/Providers/TestData/MappedFrameworkBaseRestorationTests/RestoresMoreDerivedFrameworkBaseWithChildOwnedOptionalProperties/BotData.cs`	Supplies the historical BotData contract fixture.
`test/Providers/MappedModelBaseUpgradeTests.cs`	Tests ancestor-member preservation and hiding checks.
`test/Providers/MappedModelBaseUpgradeRegressionTests.cs`	Covers projection, customization, and mapping ambiguity.
`test/Providers/MappedFrameworkBaseRestorationTests.cs`	Covers BotData restoration, ownership isolation, and vetoes.
`test/Providers/InheritedFrameworkConstructorTests.cs`	Tests historical parameter, default, and setter safety.
`src/TypeFactory.cs`	Adds cache-aware property lookup without caching new conversions.
`src/Providers/SystemObjectModelProvider.cs`	Distinguishes upgrade projections and adjusts constructor traversal.
`src/Providers/ModelProvider.cs`	Restores constructors through compatible inherited-property assignments.
`src/Providers/MappedModelBaseUpgrade.cs`	Implements local projection and ancestor compatibility checks.
`src/Providers/MappedModelBaseCompatibility.cs`	Applies projection-aware collision and displaced-base checks.
`src/Providers/CanonicalTypeProvider.cs`	Includes projected input properties in canonicalization.
`src/Providers/BackCompatModelBaseResolver.cs`	Creates upgrade projections during historical-base resolution.

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

Unconsumed ancestor-owned properties can be omitted from deserialization, silently losing JSON values, while getter-only collections with unknown mutability can throw during restoration.

2 open issues

2 resolved since last review

_{Review effort: Balanced}

…ract

Copilot

🔵 Needs a closer look

Restoration can silently discard projected ancestor property values during deserialization and turn null wire collections into empty collections.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Reject nullable wire collections during restoration

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/SystemObjectModelProvider.cs:116

Checking CLR nullability does not reject nullable wire collections: ApplyCurrentInputMetadata replaces Type with the historical non-nullable type while retaining WireInfo.IsNullable. With an approved getter-only Tags and nullable input, {"tags":null} therefore passes restoration, but ModelProvider.cs:1996 skips population and leaves an empty dictionary; reject nullable wire collections here and add a null-payload fallback control.

2 resolved since last review

_{Review effort: Balanced}

Wei Hu (live1206) · 2026-10-29T07:44:25Z

regen: Azure/azure-sdk-for-net#64320

Jorge Rangel (jorgerangel-msft)

Two deserialization cases need follow-up.

…lues

Copilot

🔵 Needs a closer look

Human assessment is needed to weigh an unpublished, nit-level finding about C# test inputs conflicting with repository guidance.

No open issues 🎉

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Human assessment is needed because projected-property restoration can discard source-only wire-name overrides and silently change the JSON contract.

No open issues 🎉

_{Review effort: Balanced}

Wei Hu (live1206) · 2026-10-30T08:35:56Z

latest regen: Azure/azure-sdk-for-net#64353

Jorge Rangel (jorgerangel-msft) · 2026-10-30T17:24:48Z


-        public CanonicalTypeProvider(TypeProvider generatedTypeProvider, InputType? inputType)
+        public CanonicalTypeProvider(TypeProvider generatedTypeProvider, InputType? inputType,
+            IReadOnlyList<InputModelProperty>? inputProperties = null)


imo this is still confusing. Can we guarantee that inputProperties will originate from the inputType ? Or are we allowing inputProperties to possibly originate from another input type ? Previously, we knew the properties always came from the inputmodel..

Clarified in ac1cc78. The normal constructor still takes properties from inputType. Mapped views now use a separate CreateForPropertyProjection factory whose contract explicitly allows properties from a descendant or multiple models in its hierarchy, retaining their original input owners. The model argument supplies model-level metadata, not property provenance. Canonicalization and input-ownership regressions pass.

- by copilot

Jorge Rangel (jorgerangel-msft) · 2026-10-30T17:25:32Z

+            if (model.InputModel.BaseModel is not { } inputBase ||
+                !IsDerivedFromCurrentMappedBase(model, candidate.SystemType) ||
+                CodeModelGenerator.Instance.TypeFactory.CreateModel(inputBase) is not SystemObjectModelProvider current ||
+                !PreservesFrameworkMembers(current.SystemType.FrameworkType, candidate.SystemType.FrameworkType) ||
+                candidate.GetAdditionalSerializationProperties().Any(property =>
+                    property.IsInitOnly || !ModelProvider.HasAccessibleInheritedSetter(property) &&
+                        !candidate.CanPopulateInheritedCollection(property)))
+            {
+                return false;
+            }


this is really difficult to follow...

Simplified in ac1cc78. Historical matching is now a shared named helper, and source-owner/cache/customization validation is isolated in PreservesSourceOwnerProperty. The hierarchy loop now handles only traversal, matching, and duplicate tracking. Existing mapped-base regressions pass.

- by copilot

Jorge Rangel (jorgerangel-msft) · 2026-10-30T17:26:25Z

+                            ? inheritedProperty.Invoke("Add", item.Property("Key"), item.Property("Value"))
+                            : inheritedProperty.Invoke("Add", item)).Terminate());


nit: can we please try and use Snippets here ?

Updated in ac1cc78 to use DictionarySnippets.Add and ListSnippets.Add on the typed inherited-property expression. There is no existing Clear snippet, so that invocation uses nameof(ICollection<object>.Clear) rather than a string literal. The compiled dictionary/list payload round-trip regressions pass.

- by copilot

Jorge Rangel (jorgerangel-msft) · 2026-10-30T17:29:42Z

+        /// <param name="mappedBase">The mapped framework base containing the property.</param>
+        /// <param name="property">The inherited property to populate during deserialization.</param>
+        /// <returns>True only when the framework contract guarantees safe collection population.</returns>
+        protected virtual bool IsLastContractModelBaseCollectionMutableCore(CSharpType mappedBase, PropertyProvider property)


can we at least make this internal if there's no usage outside of mtg ?

Changed in ac1cc78. Removed the protected core hook and made IsLastContractModelBaseCollectionMutable internal virtual, updating the test setups accordingly. No production override was found in this checkout. The collection round-trip and unsafe-collection fallback regressions pass.

- by copilot

Jorge Rangel (jorgerangel-msft) · 2026-10-30T17:32:33Z

        internal PropertyProvider? CreateUncachedProperty(InputProperty property, TypeProvider enclosingType)
            => CreatePropertyCore(property, enclosingType);

+        // Honor cached results without caching new conversions, which may be reentrant.
+        internal PropertyProvider? GetProperty(InputProperty property, TypeProvider enclosingType)
+            => PropertyCache.TryGetValue(property, out var cached) ? cached : CreateUncachedProperty(property, enclosingType);


as I'm reviewing this, I'm wondering why do we need CreateUncachedProperty and this method? we could just make PropertyCache internal since we are the only users. Mutating it should still only happen in the typefactory, but I don't see an issue with reading the dictionary directly in mtg

Updated in ac1cc78: PropertyCache is exposed internally as an IReadOnlyDictionary, with writes remaining inside TypeFactory, and the redundant GetProperty wrapper is removed. CreateUncachedProperty remains necessary to dispatch through protected factory overrides and visitors for a specific owner without reading/populating the input-keyed cache during projection or reentrant validation. Both cache-order regressions pass.

- by copilot

Jorge Rangel (jorgerangel-msft)

The earlier ancestor-deserialization and nullable-collection findings are addressed. One remaining correctness concern and one nonblocking simplification below.

Jorge Rangel (jorgerangel-msft) · 2026-10-30T17:33:17Z

+                        !(matchedProperties ??= []).Add(matches[0]) ||
+                        factory.GetProperty(property, owner) is not { } current ||
+                        owner.IsPropertySuppressed(current) ||
+                        current.Name != matches[0].Name || HasCustomAlias(owner, property, current.Name, matches[0].Name))


[P2] Preserve source-owner visitor changes

PreVisitProperty can modify the property (LibraryVisitor.cs:121–128), but this only validates its name: a source-owner-specific change to Location’s type or setter still passes, then disappears when ApplyCurrentInputMetadata recreates it under the framework owner. Reject source-owner customizations the projection cannot preserve, with regression coverage for both cache orders.

Fixed in ac1cc78. Restoration now rejects source-owner CLR-shape changes that framework projection would discard, comparing the visited property against an unvisited source-owner property. Added ten cases covering type, setter presence/accessibility, member accessibility, and init-only changes in both cache orders; all ten failed before the fix and pass afterward. Both full unit suites pass: 3,069 core + 2,651 ClientModel tests, with no skips. CI for the new commit is still outstanding, so this thread remains unresolved.

- by copilot

Jorge Rangel (jorgerangel-msft) · 2026-10-30T17:33:17Z

+                foreach (var property in input.Properties)
+                {
+                    var matches = historicalProperties.Where(previous =>
+                        factory.IsLastContractModelBasePropertyCompatible(candidateType, property, previous)).ToArray();


Nonblocking: consolidate duplicated matching rules

The property matching, ambiguity detection, and historical wire-metadata checks duplicate BackCompatModelBaseResolver.cs:111–125, leaving two implementations of the same compatibility rules to maintain. Extract those shared checks into a small helper, keeping hierarchy traversal and source-owner validation specific to each path.

Consolidated in ac1cc78. Both paths now use TryMatchHistoricalProperty for compatibility matching, ambiguity/identity detection, and known historical wire-metadata conflicts. Hierarchy traversal, duplicate ownership, and source-owner validation remain specific to each path. Both restoration paths' existing regressions pass.

- by copilot

Copilot

🟡 Changes recommended

The internal opt-in cannot be overridden by downstream TypeFactory subclasses in other assemblies, preventing approval of getter-only collection mappings and causing their base upgrades to be rejected.

1 open issue

Add a protected virtual hook for downstream collection approval · New

_{Review effort: Balanced}

Wei Hu (live1206) · 2026-10-31T12:49:38Z

+        internal virtual bool IsLastContractModelBaseCollectionMutable(CSharpType mappedBase, PropertyProvider property)
+            => false;


This recommendation conflicts with the earlier request to make this hook internal when there is no usage outside MTG: #12310 (comment) . The latest change followed that request; no production override was found in this checkout. A protected Core hook would allow non-friend downstream assemblies to approve known getter-only collection contracts, but whether we need that extension point is still an API-scope decision. Leaving the visibility decision open for now; no code change is being made in response to this comment yet.

- by copilot

fix(http-client-csharp): preserve mapped bases and historical constru…

8b734a0

…ctors

microsoft-github-policy-service Bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Oct 20, 2026

refactor(http-client-csharp): split Network constructor defaulting in…

5b58ac2

…to follow-up

Wei Hu (live1206) mentioned this pull request Oct 20, 2026

fix(http-client-csharp): preserve constructors with approved framework defaults #12311

Closed

Wei Hu (live1206) changed the title ~~fix(http-client-csharp): preserve mapped bases and historical constructors~~ fix(http-client-csharp): restore historical bases over mapped ancestors Oct 20, 2026

Wei Hu (live1206) mentioned this pull request Oct 20, 2026

[MPG] Clean up simple resource-data base customizations Azure/azure-sdk-for-net#63992

Draft

Wei Hu (live1206) added 3 commits October 28, 2026 09:42

Merge main into fix/12304-mapped-base-restoration

9db8af5

Merge remote-tracking branch 'origin/main' into fix/12304-mapped-base…

32dfe97

…-restoration

fix(http-client-csharp): preserve mapped-base source authority

da50561

Wei Hu (live1206) marked this pull request as ready for review October 29, 2026 02:33

Copilot AI balanced review requested due to automatic review settings October 29, 2026 02:33

Wei Hu (live1206) requested review from JoshLove-msft, Alan Zimmer (alzimmermsft), Jorge Rangel (jorgerangel-msft), Jose Arriaga Maldonado (joseharriaga) and m-nash as code owners October 29, 2026 02:33

Copilot started reviewing on behalf of Wei Hu (live1206) October 29, 2026 02:33

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

fix(http-client-csharp): preserve projected property deserialization

eda63ce

Copilot AI balanced review requested due to automatic review settings October 29, 2026 03:14

Copilot started reviewing on behalf of Wei Hu (live1206) October 29, 2026 03:15

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

Comment thread ...ent-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/SystemObjectModelProvider.cs Resolved

fix(http-client-csharp): require inherited collection mutability cont…

4cb8aac

…ract

Copilot AI balanced review requested due to automatic review settings October 29, 2026 03:59

Copilot started reviewing on behalf of Wei Hu (live1206) October 29, 2026 04:00

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

Jorge Rangel (jorgerangel-msft) reviewed Oct 29, 2026

View reviewed changes

Comment thread ...ent-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/SystemObjectModelProvider.cs Resolved

Comment thread ...ent-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/SystemObjectModelProvider.cs Resolved

fix(http-client-csharp): preserve projected ancestor serialization va…

ea08f1a

…lues

Copilot AI balanced review requested due to automatic review settings October 30, 2026 00:50

Copilot started reviewing on behalf of Wei Hu (live1206) October 30, 2026 00:50

View session

Copilot AI reviewed Oct 30, 2026

View reviewed changes

test(http-client-csharp): fix spelling in ancestor regression name

7bb10ba

Copilot AI balanced review requested due to automatic review settings October 30, 2026 01:13

Copilot started reviewing on behalf of Wei Hu (live1206) October 30, 2026 01:13

View session

Copilot AI reviewed Oct 30, 2026

View reviewed changes

Arthur Ma (ArthurMa1978) approved these changes Oct 30, 2026

View reviewed changes

Jorge Rangel (jorgerangel-msft) reviewed Oct 30, 2026

View reviewed changes

fix(http-client-csharp): preserve source-owner property customizations

ac1cc78

Copilot AI balanced review requested due to automatic review settings October 31, 2026 12:14

Copilot started reviewing on behalf of Wei Hu (live1206) October 31, 2026 12:15

View session

Copilot AI reviewed Oct 31, 2026

View reviewed changes

		? inheritedProperty.Invoke("Add", item.Property("Key"), item.Property("Value"))
		: inheritedProperty.Invoke("Add", item)).Terminate());

		internal virtual bool IsLastContractModelBaseCollectionMutable(CSharpType mappedBase, PropertyProvider property)
		=> false;

Conversation

Wei Hu (live1206) commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Scope after the split

Change-specific validation

Uh oh!

pkg-pr-new Bot commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Wei Hu (live1206) commented Oct 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jorge Rangel (jorgerangel-msft) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Wei Hu (live1206) commented Oct 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jorge Rangel (jorgerangel-msft) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Wei Hu (live1206) commented Oct 20, 2026 •

edited

Loading

pkg-pr-new Bot commented Oct 20, 2026 •

edited

Loading