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

TypeSpec for Azure AI Inference #28438

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

Conversation

dargilco
Copy link
Member

@dargilco dargilco commented Mar 25, 2024

Data Plane API - Pull Request

Azure AI Studio now hosts a catalog of Large Language Models (LLM) for chat completion, embeddings and image generation, as two offerings called Model-as-a-Service (MaaS, aka "Pay as you go") and Model-as-a-Platform (MaaP, aka "Real-time endpoint"). This TypeSpec defines the common REST API for endpoints hosting these models. It is checked in here to facilitate auto-geration of the client libraries (Python, JS and C# at the moment). Other programing languages will follow. The service team uses Python code which at run-time auto-generates a Swagger file for the service. They do not use the TypeSpec files in this PR. But I am verifying that their Swagger and the one created by this TypeSpec are equivalent, and the emitted client libraries work properly when connecting to this service.

For more information on the catalog see Explore the model catalog in Azure AI Studio.

This TypeSpec is a subset of the Azure OpenAI Inference TypeSpec and additional changes. We use the same model and property names for now, but that may be subject to change. It implements four routes /chat/completions, /embeddings, /image-generation/ and /info.

Is this review for (select one):

  • a private preview
  • a public preview
  • GA release

@dargilco dargilco requested a review from a team as a code owner March 25, 2024 21:13
@dargilco dargilco requested review from vicancy and bexxx and removed request for a team March 25, 2024 21:13
Copy link

openapi-pipeline-app bot commented Mar 25, 2024

Next Steps to Merge

Next steps that must be taken to merge this PR:
  • ❌ This is the public specs repo main branch which is not intended for iterative development.
    You must acknowledge that you understand that after this PR is merged, you won't be able to stop your changes from being published to Azure customers.
    If this is what you intend, add PublishToCustomers label to your PR.
    Otherwise, retarget this PR onto a feature branch, i.e. with prefix release- (see aka.ms/azsdk/api-versions#release--branches).
  • ❌ Your PR requires an API stewardship board review as it introduces a new API version (label: new-api-version). Schedule the review by following aka.ms/azsdk/onboarding/restapischedule.
  • ❌ The required check named TypeSpec Validation has failed. Refer to the check in the PR's 'Checks' tab for details on how to fix it and consult the aka.ms/ci-fix guide

Copy link

openapi-pipeline-app bot commented Mar 25, 2024

Swagger Validation Report

️️✔️BreakingChange succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️LintDiff: 12 Warnings warning [Detail]
Compared specs (v2.2.2) new version base version
package-2024-05-01-preview package-2024-05-01-preview(dda6da8) default(main)

[must fix]The following errors/warnings are introduced by current PR:

Rule Message Related RPC [For API reviewers]
⚠️ SecurityDefinitionDescription Security definition should have a description.
Location: ModelClient/preview/2024-05-01-preview/openapi.json#L29
⚠️ OperationId OperationId should be of the form 'Noun_Verb'
Location: ModelClient/preview/2024-05-01-preview/openapi.json#L42
⚠️ ParameterDescription Parameter should have a description.
Location: ModelClient/preview/2024-05-01-preview/openapi.json#L82
⚠️ EnumInsteadOfBoolean Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum.
Location: ModelClient/preview/2024-05-01-preview/openapi.json#L106
⚠️ PropertyType Property should have a defined type.
Location: ModelClient/preview/2024-05-01-preview/openapi.json#L161
⚠️ OperationId OperationId should be of the form 'Noun_Verb'
Location: ModelClient/preview/2024-05-01-preview/openapi.json#L213
⚠️ ParameterDescription Parameter should have a description.
Location: ModelClient/preview/2024-05-01-preview/openapi.json#L253
⚠️ OperationId OperationId should be of the form 'Noun_Verb'
Location: ModelClient/preview/2024-05-01-preview/openapi.json#L319
⚠️ ParameterDescription Parameter should have a description.
Location: ModelClient/preview/2024-05-01-preview/openapi.json#L359
⚠️ OperationId OperationId should be of the form 'Noun_Verb'
Location: ModelClient/preview/2024-05-01-preview/openapi.json#L426
⚠️ PropertyType Property should have a defined type.
Location: ModelClient/preview/2024-05-01-preview/openapi.json#L950
⚠️ PropertyType Property should have a defined type.
Location: ModelClient/preview/2024-05-01-preview/openapi.json#L1289
️️✔️Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️️✔️ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️️✔️PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
️️✔️Automated merging requirements met succeeded [Detail] [Expand]
Posted by Swagger Pipeline | How to fix these errors?

Copy link

openapi-pipeline-app bot commented Mar 25, 2024

Swagger Generation Artifacts

️️✔️ApiDocPreview succeeded [Detail] [Expand]
 Please click here to preview with your @microsoft account. 
️⚠️ azure-sdk-for-python warning [Detail]
    For more instructions, please refer to the FAQ .
  • ⚠️Warning in generating from 821752a02b00a60a2fb347197c1b9a8cb6c4b8e2. SDK Automation 14.0.0
    command	sh scripts/automation_init.sh ../azure-sdk-for-python_tmp/initInput.json ../azure-sdk-for-python_tmp/initOutput.json
    cmderr	[automation_init.sh] W: Target Packages (main/binary-amd64/Packages) is configured multiple times in /etc/apt/sources.list.d/azure-cli.list:1 and /etc/apt/sources.list.d/azure-cli.sources:1
    cmderr	[automation_init.sh] W: Target Packages (main/binary-all/Packages) is configured multiple times in /etc/apt/sources.list.d/azure-cli.list:1 and /etc/apt/sources.list.d/azure-cli.sources:1
    cmderr	[automation_init.sh] W: Target Translations (main/i18n/Translation-en) is configured multiple times in /etc/apt/sources.list.d/azure-cli.list:1 and /etc/apt/sources.list.d/azure-cli.sources:1
    cmderr	[automation_init.sh] W: Target CNF (main/cnf/Commands-amd64) is configured multiple times in /etc/apt/sources.list.d/azure-cli.list:1 and /etc/apt/sources.list.d/azure-cli.sources:1
    cmderr	[automation_init.sh] W: Target CNF (main/cnf/Commands-all) is configured multiple times in /etc/apt/sources.list.d/azure-cli.list:1 and /etc/apt/sources.list.d/azure-cli.sources:1
    cmderr	[automation_init.sh] W: Target Packages (main/binary-amd64/Packages) is configured multiple times in /etc/apt/sources.list.d/azure-cli.list:1 and /etc/apt/sources.list.d/azure-cli.sources:1
    cmderr	[automation_init.sh] W: Target Packages (main/binary-all/Packages) is configured multiple times in /etc/apt/sources.list.d/azure-cli.list:1 and /etc/apt/sources.list.d/azure-cli.sources:1
    cmderr	[automation_init.sh] W: Target Translations (main/i18n/Translation-en) is configured multiple times in /etc/apt/sources.list.d/azure-cli.list:1 and /etc/apt/sources.list.d/azure-cli.sources:1
    cmderr	[automation_init.sh] W: Target CNF (main/cnf/Commands-amd64) is configured multiple times in /etc/apt/sources.list.d/azure-cli.list:1 and /etc/apt/sources.list.d/azure-cli.sources:1
    cmderr	[automation_init.sh] W: Target CNF (main/cnf/Commands-all) is configured multiple times in /etc/apt/sources.list.d/azure-cli.list:1 and /etc/apt/sources.list.d/azure-cli.sources:1
    cmderr	[automation_init.sh] WARNING: Skipping azure-nspkg as it is not installed.
    cmderr	[automation_init.sh] npm notice
    cmderr	[automation_init.sh] npm notice New minor version of npm available! 10.7.0 -> 10.8.2
    cmderr	[automation_init.sh] npm notice Changelog: https://github.com/npm/cli/releases/tag/v10.8.2
    cmderr	[automation_init.sh] npm notice To update run: npm install -g [email protected]
    cmderr	[automation_init.sh] npm notice
    warn		Warning: azure-sdk-for-python cannot be found in specification/ai/data-plane/AI.Model/readme.md. This SDK will be skipped from SDK generation. Please add the right config to the readme file according to this guidance https://github.com/Azure/azure-rest-api-specs/blob/main/documentation/code-gen/configure-go-sdk.md#swagger-to-sdk.
    command	sh scripts/automation_generate.sh ../azure-sdk-for-python_tmp/generateInput.json ../azure-sdk-for-python_tmp/generateOutput.json
  • ️✔️azure-ai-inference [Preview SDK Changes]
    info	[Changelog] data-plan skip changelog generation temporarily
Posted by Swagger Pipeline | How to fix these errors?

Copy link
Contributor

Hi, @dargilco. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

@microsoft-github-policy-service microsoft-github-policy-service bot added the no-recent-activity There has been no recent activity on this issue. label Sep 30, 2024
Copy link
Contributor

Hi, @dargilco. The PR will be closed since the PR has no update for 28 days. If you still need the PR review to proceed, please reopen it and @ mention PR assignee.

Comment on lines +18 to +60
@doc("""
Gets chat completions for the provided chat messages.
Completions support a wide variety of tasks and generate text that continues from or "completes"
provided prompt data. The method makes a REST API call to the `/chat/completions` route
on the given endpoint.
""")
@actionSeparator("/")
@route("chat/completions")
op getChatCompletions is Azure.Core.RpcOperation<
{
...ChatCompletionsOptions;
...AdditionalRequestHeaders;
},
ChatCompletions
>;

@doc("""
Return the embedding vectors for given text prompts.
The method makes a REST API call to the `/embeddings` route on the given endpoint.
""")
@actionSeparator("/")
@route("embeddings")
op getEmbeddings is Azure.Core.RpcOperation<
{
...EmbeddingsOptions;
...AdditionalRequestHeaders;
},
EmbeddingsResult
>;

@doc("""
Return the embedding vectors for given images.
The method makes a REST API call to the `/images/embeddings` route on the given endpoint.
""")
@actionSeparator("/")
@route("images/embeddings")
op getImageEmbeddings is Azure.Core.RpcOperation<
{
...ImageEmbeddingsOptions;
...AdditionalRequestHeaders;
},
EmbeddingsResult
>;
Copy link
Member

@weidongxu-microsoft weidongxu-microsoft Oct 16, 2024

Choose a reason for hiding this comment

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

@dargilco

Please check if you can put the ###Options (also make it a model instead of alias) with the explicit @body, e.g.

op getChatCompletions is Azure.Core.RpcOperation<
  {
    @body body: ChatCompletionsOptions;
    ...AdditionalRequestHeaders;
  },
  ChatCompletions
>;

The reason is that without @body, typespec would let SDK emitter spread the body properties to method signature. (see spread).

However, SDK emitter cannot handle the ...Record<unknown> in the ###Options in above "spread" scenario. -- ...Record<unknown> means any key-value can be set to the request body, but that won't work if the request body properties now on the method signature.

Therefore, this is going to cause problem to SDK emitters, and the output code would be incorrect.

Copy link
Member

@weidongxu-microsoft weidongxu-microsoft Oct 23, 2024

Choose a reason for hiding this comment

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

Basically, the request body can have properties

messages
frequency_penalty
stream
...
model
<any undocumented properties, as the Record>

But after the spread in method (the way of writing the alias ChatCompletionsOptions in getChatCompletions without @body), the SDK method signature would become
getChatCompletions(messages, frequency_penalty, stream, ..., model) without a way to specify undocumented properties.

@dargilco dargilco removed the no-recent-activity There has been no recent activity on this issue. label Oct 23, 2024
@dargilco
Copy link
Member Author

Assigned to @dargilco

@dargilco
Copy link
Member Author

Thank you for your comment @weidongxu-microsoft , I will look into this when I get back to working on this SDK, as part of releasing another beta version.

Copy link
Contributor

Hi, @dargilco. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

@microsoft-github-policy-service microsoft-github-policy-service bot added the no-recent-activity There has been no recent activity on this issue. label Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants