Skip to content

Closes #31 Support oneOf that contains primitive types and objects #32

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

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

walsha2
Copy link
Contributor

@walsha2 walsha2 commented Nov 2, 2023

@davidmigloz this is a little tricky, how does one discern between the following hypothetical example where there are no union keys between objects:

function_call:
  description: Controls how the model calls functions. 
  oneOf:
    - type: string
      enum: [none, auto]
      title: ChatCompletionFunctionCallMode
    - $ref: "#/components/schemas/ChatCompletionFunctionCallOption"
    - type: object
      additionalParameters: true

Im not a fan of this hypothetical API definition, but it still allowed by the OpenAPI spec.

ChatCompletionFunctionCallOption and the other object cannot be differentiated from Map<String,dynamic> for deserialization logic... just one use case that came to mind. The solution in this PR accounts for that should it arise by having a smart sort order and try/catch when attempting to deserialize a known object union. Best solution I could conjure without making things more difficult than necessary.

Result

For your use case referenced in #31 this would be the result. I am not really sure of a better way to shorten the constructor for ChatCompletionFunctionCall.chatCompletionFunctionCallOption and ensure it is unique. Open to suggestions.

If this is good enough for you, we can merge it in. Will wait for confirmation or any follow-up suggestions.

CreateChatCompletionRequest c;

c = CreateChatCompletionRequest(
  messages: [],
  model: CreateChatCompletionRequestModel.string('custom-model'),
  functionCall: ChatCompletionFunctionCall.enumeration(
    ChatCompletionFunctionCallMode.auto,
  ),
);

print(c.toJson());
print(CreateChatCompletionRequest.fromJson(c.toJson()));

c = CreateChatCompletionRequest(
  messages: [],
  model: CreateChatCompletionRequestModel.string('custom-model'),
  functionCall: ChatCompletionFunctionCall.chatCompletionFunctionCallOption(
    ChatCompletionFunctionCallOption(name: 'option'),
  ),
);

print(c.toJson());
print(CreateChatCompletionRequest.fromJson(c.toJson()));
// ==========================================
// CLASS: ChatCompletionFunctionCall
// ==========================================

/// Controls how the model calls functions.
@freezed
sealed class ChatCompletionFunctionCall with _$ChatCompletionFunctionCall {
  const ChatCompletionFunctionCall._();

  const factory ChatCompletionFunctionCall.enumeration(
    ChatCompletionFunctionCallMode value,
  ) = _UnionChatCompletionFunctionCallEnum;

  const factory ChatCompletionFunctionCall.chatCompletionFunctionCallOption(
    ChatCompletionFunctionCallOption value,
  ) = _UnionChatCompletionFunctionCallChatCompletionFunctionCallOption;

  /// Object construction from a JSON representation
  factory ChatCompletionFunctionCall.fromJson(Map<String, dynamic> json) =>
      _$ChatCompletionFunctionCallFromJson(json);
}

/// Custom JSON converter for [ChatCompletionFunctionCall]
class _ChatCompletionFunctionCallConverter
    implements JsonConverter<ChatCompletionFunctionCall?, Object?> {
  const _ChatCompletionFunctionCallConverter();

  @override
  ChatCompletionFunctionCall? fromJson(Object? data) {
    if (data == null) {
      return null;
    }
    if (data is String &&
        _$ChatCompletionFunctionCallModeEnumMap.values.contains(data)) {
      return ChatCompletionFunctionCall.enumeration(
        _$ChatCompletionFunctionCallModeEnumMap.keys.elementAt(
          _$ChatCompletionFunctionCallModeEnumMap.values.toList().indexOf(data),
        ),
      );
    }
    if (data is Map<String, dynamic>) {
      try {
        return ChatCompletionFunctionCall.chatCompletionFunctionCallOption(
          ChatCompletionFunctionCallOption.fromJson(data),
        );
      } catch (e) {}
    }
    throw Exception(
      'Unexpected value for ChatCompletionFunctionCall: $data',
    );
  }

  @override
  Object? toJson(ChatCompletionFunctionCall? data) {
    return switch (data) {
      _UnionChatCompletionFunctionCallEnum(value: final v) =>
        _$ChatCompletionFunctionCallModeEnumMap[v]!,
      _UnionChatCompletionFunctionCallChatCompletionFunctionCallOption(
        value: final v
      ) =>
        v.toJson(),
      null => null,
    };
  }
}

@davidmigloz
Copy link
Contributor

Thanks again for the fast implementation. lgtm!

I think the ordered try-catch approach is a good solution. Anyway, OpenAPI doesn't explicitly specify how conflicts should be handled, so returning the first schema that matches sounds the most reasonable way to go.

Regarding the factory constructors names, I think it's good enough.

I had in my mind a feature request to allow to specify custom factory names, which (together with the already implemented titles) would be the ultimate piece for full customization support.

My idea was to use Specification Extensions to define the factory name in the schema (e.g. using something like x-factoryName):

function_call:
  title: ChatCompletionFunctionCall
  description: Controls how the model calls functions.
  oneOf:
    - type: string
      title: ChatCompletionFunctionCallMode
      x-factoryName: mode
      enum: [none, auto]
    - $ref: "#/components/schemas/ChatCompletionFunctionCallOption"
ChatCompletionFunctionCallOption:
  type: object
  x-factoryName: functionCallOption
  properties:
    name:
      type: string
      description: The name of the function to call.
  required:
    - name

(but that's out of scope for this PR, we can move it to a separate issue)

@walsha2
Copy link
Contributor Author

walsha2 commented Nov 2, 2023

My idea was to use Specification Extensions to define the factory name in the schema (e.g. using something like x-factoryName):

Yea this package does not introduce any sort of package specific customization like that. The idea was to just be compatible with any API schema out of the box, without introducing any extra dependency or custom definitions.

@davidmigloz alternatively, we can do it at the code generation level with an onUnionConstructorName function that would allow a user to override the constructor of a specific union. Feel free to open an issue and we can move that chat there.

Thanks again for the fast implementation. lgtm!

Will cut a new release with these changes.

@walsha2 walsha2 merged commit a924f3e into main Nov 2, 2023
@walsha2 walsha2 deleted the 31-primitive-object branch November 2, 2023 17:21
@walsha2
Copy link
Contributor Author

walsha2 commented Nov 2, 2023

openapi_spec (v0.7.3) - Published.

@davidmigloz
Copy link
Contributor

I've published the first version of the OpenAI client generated with this package: openai_dart 🚀

Pretty happy with the result! (the OpenAI spec was a good test for the generator, it has all kinds of edge cases 😄)

There are a couple of areas where we could still improve the generator:

  • Streaming support: if you check the implementation I'm subclassing the generated client to add streaming support manually. It may be possible to generate that code as well. The only piece that you cannot generate automatically from the spec is the StreamTransformer which could be provided as a ClientGeneratorOptions.
  • Multipart requests: although the client already supports multipart requests, with the current implementation I wasn't able to generate the necessary code to consume some of the multipart endpoints from OpenAI (e.g. the audio endpoints. I may spend some time in the future adding support for this.

@walsha2
Copy link
Contributor Author

walsha2 commented Nov 2, 2023

OpenAI spec was a good test for the generator

100%. Adding it as another test to exercise the generators has been a great addition.

Will open a couple issues from the comments above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants