[OpenAI] Whisper TSP definition for AOAI#25513
Closed
jpalvarezl wants to merge 36 commits intomainfrom
Closed
Conversation
Next Steps to MergeNext steps that must be taken to merge this PR:
|
Swagger Validation Report
|
| compared swaggers (via Oad v0.10.4)] | new version | base version |
|---|---|---|
| generated.json | 2023-06-01-preview(fb4f558) | 2023-06-01-preview(main) |
| generated.json | 2023-07-01-preview(fb4f558) | 2023-07-01-preview(main) |
| generated.json | 2023-08-01-preview(fb4f558) | 2023-08-01-preview(main) |
| generated.json | 2022-12-01(fb4f558) | 2022-12-01(main) |
| generated.json | 2023-05-15(fb4f558) | 2023-05-15(main) |
️️✔️Breaking Change(Cross-Version) succeeded [Detail] [Expand]
There are no breaking changes.
| compared swaggers (via Oad v0.10.4)] | new version | base version |
|---|---|---|
| generated.json | 2023-09-01-preview(fb4f558) | 2023-05-15(main) |
| generated.json | 2023-09-01-preview(fb4f558) | 2023-08-01-preview(main) |
️️✔️CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️LintDiff: 21 Warnings warning [Detail]
| compared tags (via openapi-validator v2.1.4) | new version | base version |
|---|---|---|
| release_2022_12_01_autogen | release_2022_12_01_autogen(fb4f558) | release_2022_12_01_autogen(main) |
| release_2023_05_15_autogen | release_2023_05_15_autogen(fb4f558) | release_2023_05_15_autogen(main) |
| release_2023_06_01_preview_autogen | release_2023_06_01_preview_autogen(fb4f558) | release_2023_06_01_preview_autogen(main) |
| release_2023_07_01_preview_autogen | release_2023_07_01_preview_autogen(fb4f558) | release_2023_07_01_preview_autogen(main) |
| release_2023_08_01_preview_autogen | release_2023_08_01_preview_autogen(fb4f558) | release_2023_08_01_preview_autogen(main) |
[must fix]The following errors/warnings are introduced by current PR:
| Rule | Message | Related RPC [For API reviewers] |
|---|---|---|
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-06-01-preview/generated.json#L63 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-06-01-preview/generated.json#L109 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-06-01-preview/generated.json#L155 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-06-01-preview/generated.json#L201 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-06-01-preview/generated.json#L248 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-07-01-preview/generated.json#L63 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-07-01-preview/generated.json#L109 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-07-01-preview/generated.json#L155 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-07-01-preview/generated.json#L201 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-07-01-preview/generated.json#L248 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-08-01-preview/generated.json#L63 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-08-01-preview/generated.json#L109 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-08-01-preview/generated.json#L155 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-08-01-preview/generated.json#L201 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-08-01-preview/generated.json#L247 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/preview/2023-08-01-preview/generated.json#L294 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/stable/2022-12-01/generated.json#L63 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/stable/2022-12-01/generated.json#L109 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/stable/2023-05-15/generated.json#L63 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/stable/2023-05-15/generated.json#L109 |
||
| Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Location: inference/stable/2023-05-15/generated.json#L155 |
The following errors/warnings exist before current PR submission:
Only 25 items are listed, please refer to log for more details.
️❌Avocado: 30 Errors, 0 Warnings failed [Detail]
Only 25 items are listed, please refer to log for more details.
️️✔️SwaggerAPIView succeeded [Detail] [Expand]
️️✔️TypeSpecAPIView succeeded [Detail] [Expand]
️❌ModelValidation: 29 Errors, 0 Warnings failed [Detail]
Only 25 items are listed, please refer to log for more details.
️️✔️SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️❌PrettierCheck: 2 Errors, 0 Warnings failed [Detail]
| Rule | Message |
|---|---|
|
Code style issues found path: OpenAI.Inference/examples/2023-09-01-preview/transcription.json |
|
Code style issues found path: preview/2023-09-01-preview/examples/transcription.json |
️️✔️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]
Swagger Generation Artifacts
|
Generated ApiView
|
trrwilson
reviewed
Aug 29, 2023
trrwilson
reviewed
Aug 29, 2023
trrwilson
reviewed
Aug 29, 2023
specification/cognitiveservices/OpenAI.Inference/models/transcription.create.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 29, 2023
specification/cognitiveservices/OpenAI.Inference/models/transcription.create.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 29, 2023
specification/cognitiveservices/OpenAI.Inference/models/transcription.create.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 29, 2023
specification/cognitiveservices/OpenAI.Inference/models/transcription.create.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 29, 2023
specification/cognitiveservices/OpenAI.Inference/models/transcription.create.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 29, 2023
specification/cognitiveservices/OpenAI.Inference/models/transcription.create.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 29, 2023
specification/cognitiveservices/OpenAI.Inference/models/translation.create.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 29, 2023
specification/cognitiveservices/OpenAI.Inference/models/translation.create.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 29, 2023
specification/cognitiveservices/OpenAI.Inference/models/translation.create.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 30, 2023
specification/cognitiveservices/OpenAI.Inference/models/transcription.create.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 30, 2023
specification/cognitiveservices/OpenAI.Inference/models/transcription.create.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 30, 2023
specification/cognitiveservices/OpenAI.Inference/models/translation.create.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 30, 2023
specification/cognitiveservices/OpenAI.Inference/models/whisper.common.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Aug 30, 2023
specification/cognitiveservices/OpenAI.Inference/models/whisper.common.tsp
Outdated
Show resolved
Hide resolved
trrwilson
reviewed
Sep 7, 2023
|
|
||
| @doc("The content length of the operation. This needs to be provided by the caller.") | ||
| @header("content-length") | ||
| contentLength: int64; |
Member
There was a problem hiding this comment.
This ends up emitting a bit strangely (at least in .NET) with the generated method signatures requiring contentLength as a parameter -- the actual content length is dependent on the serialization of the (peer) options instance as well as the internal details on how multipart is serialized, so I don't believe that parameter makes sense.
We should check with the TypeSpec experts on the intended way to make "must be internally populated headers" show up -- if the convention is that these get promoted into a method-level property, this seems odd.
Concretely, I'm seeing .NET emit signatures like this, and I don't know how a caller (internal or not) would ever be able to use the contentLength parameter:
internal virtual async Task<Response<AudioTranscriptionSimpleJson>> GetAudioTranscriptionSimpleJsonAsync(
string deploymentId,
long contentLength,
AudioTranscriptionOptionsSimpleJson audioTranscriptionOptionsSimpleJson,
CancellationToken cancellationToken = default)
Member
Author
There was a problem hiding this comment.
The sames is true for Java, but for me this is a feature. I can then provide the contentLength at the protocol method level. I am expecting for all these methods to be private from the perspective of the user. We'd then have a single public entry point from the users perspective. contentLength wouldn't be visible to them AFAIU.
Although, maybe I am not understanding your concern here. I agree, let's open this up to the meeting on Monday or Wednesday :)
2 tasks
trrwilson
reviewed
Sep 7, 2023
| @@ -31,6 +31,17 @@ Requesting format 'json' will result on only the 'text' field being set. | |||
| For more output data use 'verbose_json. | |||
| """) | |||
| model AudioTranscriptionOptionsCommon { | |||
Member
There was a problem hiding this comment.
Given we're going for accuracy, we should likely add "model" to this and hide it (hide it at least until we incorporate the feedback to get rid of model/deployment as an operation parameter from the customer perspective)
deyaaeldeen
reviewed
Sep 13, 2023
|
|
||
| @doc("Average log probability.") | ||
| @projectedName("json", "avg_logprob") | ||
| averageLogProb: float32; |
Member
There was a problem hiding this comment.
shouldn't this be avgLogprob? If we need a different name for a specific language, we can add it as a projectedName for that language?
microsoft-github-policy-service
bot
added
no-recent-activity
and removed
no-recent-activity
labels
Member
Author
|
Introduced the refactor on a different PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remaining TODOs:
translateandtranscribeendpoints