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

Update TypeSpec with latest features including Assistants #6

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

joseharriaga
Copy link

Added all the new features that have been introduced since the last update to the spec, including the Assistants API. This now matches OpenAI's OpenAPI spec here:
🔗 https://raw.githubusercontent.com/openai/openai-openapi/b648b7823135e6fa5148ac9a303c16fdad050da6/openapi.yaml

@joseharriaga joseharriaga changed the title Update TypeSpec Update TypeSpec with latest features including Assistants Feb 8, 2024
Comment on lines +37 to +44
):
| CreateTranscriptionResponse
| {
// TODO: Is this the appropriate way to describe the multiple possible response types?
@header contentType: "text/plain";
@body text: string;
}
| ErrorResponse;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bterlson: Is this the appropriate way to describe the different types of responses? For the record, OpenAI's OpenAPIv3 spec does not describe this behavior even though that is how the service works.

Comment on lines +124 to +126
@minValue(-9223372036854775808) // TODO: Min and max exceed the limits of safeint.
@maxValue(9223372036854775807)
seed?: safeint | null;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bterlson : OpenAI's OpenAPIv3 spec claims these are the min and max values for this property, but they exceed the limits of safeint, which makes me think they are wrong. What do you think?

Comment on lines +36 to +38
// TODO: The generated spec produces "additionalProperties: {}" for this instead of
// "additionalProperties: true". Are they equivalent?
model FunctionParameters is Record<unknown>;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bterlson: This is generated as "additionalProperties: {}" instead of "additionalProperties: true". Do you know if they equivalent?

Comment on lines +232 to +234
/** For now, this is always going to be an empty object. */
@extension("x-oaiTypeLabel", "map")
retrieval: { }; // TODO: Is this the appropriate way to represent an empty object?
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bterlson: Is this the appropriate way to represent the empty object?

Comment on lines +33 to +37
/**
* Sort order by the `created_at` timestamp of the objects. `asc` for ascending order and`desc`
* for descending order.
*/
@query order?: "asc" | "desc" = "desc";
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bterlson : I noticed that the generated OpenAPIv3 spec seems to have an issue when generating some enums. For example, this query parameter produces the following:

- name: order
  in: query
  required: false
  description: |-
    Sort order by the `created_at` timestamp of the objects. `asc` for ascending order and`desc`
    for descending order.
  schema:
    type: string
    enum:
      - asc
      - desc
      - desc
      - desc
      - desc
      - desc
      - desc
    default: desc

Please let me know where I can file a bug for this one. 🙂

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.

1 participant