openai / openai-dotnet

The official .NET library for the OpenAI API
https://www.nuget.org/packages/OpenAI
MIT License
1.55k stars 165 forks source link

OpenAI.Assistants.AssistantResponseFormat is not deserializable #67

Open douglasware opened 5 months ago

douglasware commented 5 months ago

Maybe I misunderstand this property. I can set it and read it, but serializing with System.Text.Json doesn't yield a value for this property on serialize, and deserialize fails with:

System.NotSupportedException: Deserialization of types without a parameterless constructor, a singular parameterized constructor, or a parameterized constructor annotated with 'JsonConstructorAttribute' is not supported. Type 'OpenAI.Assistants.AssistantResponseFormat'.

As a simple property with three valid values, the implementation of this property is (to me) exotic and strange, why doesn't it use an enum or really any other normal .NET approach?

dspear commented 5 months ago

You don't create the object. Just use one of the existing predefined objects. In your case, just assign the static object AssistantResponseFormat.JsonObject.

douglasware commented 5 months ago

Sorry for not being clear. I’m trying to serialize and deserialize AssistantCreationOptions which doesn’t appear to be possible due to this property’s class definition.

trrwilson commented 5 months ago

Hello, @douglasware -- thanks for getting in touch.

System.ClientModel uses the ModelReaderWriter type (and a supporting IJsonModel<T> interface behind the scenes) to support serialization and deserialization; you can achieve equivalent dehydration/hydration with code like the following:

AssistantCreationOptions assistantOptions = new()
{
    Name = "Test Assistant",
    ResponseFormat = AssistantResponseFormat.JsonObject,
    Tools =
    {
        new FunctionToolDefinition("get_current_weather", "Gets the current weather at the user's location"),
    },
};
BinaryData serializedAssistantOptions = ModelReaderWriter.Write(assistantOptions);

AssistantCreationOptions deserializedAssistantOptions
    = ModelReaderWriter.Read<AssistantCreationOptions>(serializedAssistantOptions);

I'll follow up with the team on the inconsistency when using standard System.Text.Json patterns; it's certainly confusing as-is.

douglasware commented 5 months ago

Thanks! Some additional feedback, if you don't plan to support normal serialization of these objects, please be clear about it. If that is the case, I personally will keep using REST instead.

I'm trying to use this library with Durable Task Framework and this kind of thing that prevents crossing process boundaries is a deal-breaker.

It's not just json... lots of serialization bits require things like parameterless constructors. It is easier to use normal REST approaches to get this behavior than it is to create poco's and marshal it all by hand.

I haven't got into messages and threads yet, can I expect similar problems there?

llRandom commented 2 months ago

I have the same issue with ChatMessage model on ChatClient. I need to keep chat history so I serialize & deserialize List<ChatMessage>. ModelReaderWriter solves it but it can't handle List<T> because it doesn't implement IPersistableModel<T>. as a workaround I serialize each ChatMessage, build JsonArray and serialize it and deserialize each node of deserialized array to get List<ChatMessage>