[http-client-csharp] Model factories use intermediate discriminator for leaf models sharing an inherited discriminator

Aperta
  • Qualcosa non va?
#12,151 1 commento 2 reazioni 2 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

@JoshLove01 ci sta già lavorando.

Dal 12/10/2026.

  • #12157 di @copilot-swe-agent — chiusa senza merge

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
22/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
csharp
Ambito
devtools

Direzione di ricerca

Start in Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs at EnsureDiscriminatorValueExpression, which only searches the direct base's canonical properties, and in ModelFactoryProvider.cs at GetDiscriminatorExpression, which walks ancestors. Run the focused NUnit management-generator tests for the instant and range query factories. Done means the leaf factories pass their own discriminator value, separate-discriminator hierarchies such as root kind and nested nestedKind still pass, and customization mappings are preserved.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug emitter:client:csharp

Summary

@typespec/http-client-csharp selects an intermediate model's discriminator value in model factories when three inheritance levels share one discriminator property. The concrete leaf models have the correct DiscriminatorValue, but their DiscriminatorValueExpression is null. This needs property-specific discriminator resolution in the upstream generator rather than a generated-SDK patch.

Observed in Azure SDK regeneration PR https://github.com/Azure/azure-sdk-for-net/pull/63730, review comment https://github.com/Azure/azure-sdk-for-net/pull/63730#discussion_r4237098529.

Minimal generator-input reproduction

Using the existing management-generator NUnit infrastructure and upstream InputFactory/ModelFactoryProvider, construct:

var discriminatorEnum = InputFactory.StringEnum(
    "AlertsManagementType",
    [("PrometheusEnrichmentItem", "PrometheusEnrichmentItem"),
     ("PrometheusInstantQuery", "PrometheusInstantQuery"),
     ("PrometheusRangeQuery", "PrometheusRangeQuery")],
    access: "internal", isExtensible: true, clientNamespace: "Samples.Models");
var instantQuery = InputFactory.Model(
    "PrometheusInstantQuery", properties: [], discriminatedKind: "PrometheusInstantQuery");
var rangeQuery = InputFactory.Model(
    "PrometheusRangeQuery", properties: [], discriminatedKind: "PrometheusRangeQuery");
var prometheusItem = InputFactory.Model(
    "PrometheusEnrichmentItem", properties: [], discriminatedKind: "PrometheusEnrichmentItem",
    derivedModels: [instantQuery, rangeQuery]);
var enrichmentItem = InputFactory.Model(
    "AlertEnrichmentItem",
    properties: [InputFactory.Property("alertEnrichmentType", discriminatorEnum,
        isRequired: true, isDiscriminator: true)],
    derivedModels: [prometheusItem]);

The hierarchy has one discriminator property declared on the root; neither the intermediate nor leaf models declares another discriminator.

The unmodified upstream factory body, before management constructor repair, is:

return new PrometheusInstantQuery(
    AlertsManagementType.PrometheusEnrichmentItem,
    additionalBinaryDataProperties: null);

The range-query factory likewise uses PrometheusEnrichmentItem.

Expected: the respective AlertsManagementType.PrometheusInstantQuery / AlertsManagementType.PrometheusRangeQuery value.

Impact in the SDK

Both generated factory constructors forward the supplied discriminator unchanged. Serialization writes "type":"PrometheusEnrichmentItem", while polymorphic readers dispatch concrete queries only for "PrometheusInstantQuery" and "PrometheusRangeQuery". Reading through the base type falls through to UnknownPrometheusEnrichmentItem, losing the concrete query type.

This impact was validated by inspecting the generated constructor/writer/reader paths; no SDK runtime round-trip test was run for this report.

Root-cause evidence

The focused test reports:

Model DiscriminatorValue DiscriminatorValueExpression Declared discriminator properties
AlertEnrichmentItem null null AlertEnrichmentType
PrometheusEnrichmentItem PrometheusEnrichmentItem correct enum expression none
PrometheusInstantQuery PrometheusInstantQuery null none
PrometheusRangeQuery PrometheusRangeQuery null none

In the installed upstream generator:

  • ModelProvider.EnsureDiscriminatorValueExpression searches only BaseModelProvider.CanonicalView.Properties for the discriminator. The direct parent of each leaf declares no discriminator, so the leaf expression is null.
  • ModelFactoryProvider.GetDiscriminatorExpression recursively walks base models until model.BaseModelProvider.DiscriminatorProperty == property, then returns that level's expression. Here it returns the intermediate model's value rather than the leaf's.

Source locations (the same direct-parent/ancestor-selection logic is also present on current TypeSpec main):

Validation and provenance

  • Reproduced using @typespec/http-client-csharp 1.0.0-alpha.20260918.2, in Azure SDK for .NET main revision e9bdd3ebf344345a7ae2dd1828d0577d33fe1651.
  • Added four local NUnit cases: instant/range factories × visitor-time/write-time management repair. All four fail the expected leaf-discriminator assertion. The upstream bodies are already incorrect before either repair path.
  • A controlled comparison replacing only the management repair helper with its pre-PR #63719 revision, retaining identical current dependencies and test expectations, also fails all four cases: old repair emits typed default, current repair emits the intermediate value. This is not a full historical-checkout comparison.
  • Therefore https://github.com/Azure/azure-sdk-for-net/pull/63719 exposed the intermediate non-null value but did not introduce the underlying concrete-type failure.

Suggested direction and regression coverage

Resolve discriminator values for the specific property, distinguishing a shared discriminator across inheritance levels from separate discriminators declared at different levels. For the shared property, use the most-derived applicable value and the declaring property's enum type/mapping. Preserve customization mappings and unknown-model behavior.

Simply using the leaf's existing DiscriminatorValueExpression is insufficient because it is null here. Conversely, always using the leaf's value would break hierarchies with separate discriminators (e.g. root kind and intermediate nestedKind). Please cover both cases, including upstream factory generation and constructor initialization. Management constructor repair has analogous ancestor-selection logic and will need alignment once upstream resolution is corrected.

- by copilot

Lingua principale
Java
Stelle
5.9k
Fork
421
Merge medio
2g 3h
PR unite (30g)
176

Preparare l'ambiente

Apri in Codespaces

Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di microsoft/typespec

Tutte le issue di microsoft/typespec

Issue simili

Altre issue su Java

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.