Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: getDiscrminator return undefined while TCGC can find the discriminator #3184

Closed
4 tasks done
Tracked by #4300
archerzz opened this issue Apr 17, 2024 · 8 comments
Closed
4 tasks done
Tracked by #4300
Assignees
Labels
bug Something isn't working needs-area needs-info Mark an issue that needs reply from the author or it will be closed automatically

Comments

@archerzz
Copy link
Member

Describe the bug

Check the TSP spec for OpenAI.Assitants: https://github.com/Azure/azure-rest-api-specs/blob/c4e661cdf92c8f579574008d0cd11874cc303da0/specification/ai/OpenAI.Assistants/runs/models.tsp#L171-L188

RequiredAction is the base model, and SubmitToolOutputsAction is the child model.

getDiscriminator() on RequiredAction will return undefined, while TCGC can find the discriminator.

image

Here modelType is the TCGC SdkModelType, and modelType.__raw is the compiler type.

Reproduction

Just try to parse the Tsp spec in the link above.

Checklist

@timotheeguerin
Copy link
Member

@archerzz is this an issue with tcgc or typespec I don't really get here?

@archerzz
Copy link
Member Author

@archerzz is this an issue with tcgc or typespec I don't really get here?

@timotheeguerin I think it's in typespec, because modelType (which is TCGC SDKModelType) has the correct discriminator property, but getDiscriminator() (which is typespec API) failed.

@timotheeguerin
Copy link
Member

how is tcgc getting it if the compiler is not returning it? can you make a smaller repro with exactly what is not working?

@archerzz
Copy link
Member Author

how is tcgc getting it if the compiler is not returning it? can you make a smaller repro with exactly what is not working?

I'll try, I actually just copy&paste those two models into a smaller tsp project, and I could not reproduce it.

Some interesting finding, it looks like RequiredAction is parsed as Union in typespec...
image

@archerzz
Copy link
Member Author

archerzz commented Apr 18, 2024

@tadelesh Is that possible that TCGC assigns a wrong instance to __raw? I don't think compiler will parse a model into a union.

@tadelesh
Copy link
Member

tadelesh commented Apr 18, 2024

See: https://github.com/Azure/azure-rest-api-specs/blob/c4e661cdf92c8f579574008d0cd11874cc303da0/specification/ai/OpenAI.Assistants/runs/models.tsp#L40.
The property type is a union of nullable type. For TCGC, we just return the type and set the property to nullable, but the raw will be the union type.

@timotheeguerin
Copy link
Member

so is the issue you are calling getDiscriminator on RequiredAction | null but should go down to RequiredAction.

Can this issue be closed?

@timotheeguerin timotheeguerin added the needs-info Mark an issue that needs reply from the author or it will be closed automatically label Apr 18, 2024
@archerzz
Copy link
Member Author

so is the issue you are calling getDiscriminator on RequiredAction | null but should go down to RequiredAction.

Can this issue be closed?

@timotheeguerin sure, thank you all for the investigation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-area needs-info Mark an issue that needs reply from the author or it will be closed automatically
Projects
None yet
Development

No branches or pull requests

3 participants