-
Notifications
You must be signed in to change notification settings - Fork 5k
Port Microsoft.Extensions.AI.AzureAIInference to Azure.AI.Inference #50097
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
base: main
Are you sure you want to change the base?
Conversation
Thank you for your contribution @jozkee! We will review the pull request and get back to you soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR ports Microsoft.Extensions.AI.AzureAIInference into the Azure.AI.Inference namespace and updates associated tests and clients. Key changes include:
- Adding test utilities (e.g. VerbatimHttpHandler) and comprehensive tests for image and text embedding generators.
- Updating project files with new package references and embedded resources.
- Implementing new client wrappers and extension methods to expose endpoints and generate embeddings via ImageEmbeddingsClient, EmbeddingsClient, and ChatCompletionsClient.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
sdk/ai/Azure.AI.Inference/tests/Utilities/VerbatimHttpHandler.cs | Added a custom HttpMessageHandler for request/response validation in tests. |
sdk/ai/Azure.AI.Inference/tests/AzureAIInferenceImageEmbeddingGeneratorTests.cs | Introduced tests for image embedding generation and validated response metadata. |
sdk/ai/Azure.AI.Inference/tests/AzureAIInferenceEmbeddingGeneratorTests.cs | Added tests for text embedding generation including usage metrics validation. |
sdk/ai/Azure.AI.Inference/tests/Azure.AI.Inference.Tests.csproj | Updated project references and embedded resource configuration. |
sdk/ai/Azure.AI.Inference/src/Customized/*.cs | Added clients, extension methods, and generator implementations to wrap Azure.AI.Inference functionality. |
eng/Packages.Data.props | Updated package references for integrations with AI and caching abstractions. |
|
||
foreach (Embedding<float> e in response) | ||
{ | ||
Assert.That(e.ModelId, Is.EqualTo("embed-v4.0")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test constructs the generator with a default model ID of 'embed-v-4-0' yet later asserts the produced ModelId is 'embed-v4.0'. If this conversion is intentional, please document the transformation for clarity; otherwise, consider aligning the model naming consistently.
Copilot uses AI. Check for mistakes.
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
eng/Packages.Data.props
Outdated
@@ -104,6 +104,7 @@ | |||
<PackageReference Update="Microsoft.Bcl.AsyncInterfaces" Version="8.0.0" /> | |||
<PackageReference Update="Microsoft.CSharp" Version="4.7.0" /> | |||
<PackageReference Update="Microsoft.Extensions.Logging.Abstractions" Version="8.0.3"/> | |||
<PackageReference Update="Microsoft.Extensions.AI.Abstractions" Version="9.5.0-preview.1.25262.9"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The repository is intentionally snapped to the 8.x line of dependencies, per guidelines. We also avoid preview dependencies as the central version unless there is no stable alternative, as stable versions of Azure SDK packages cannot have beta/preview dependencies.
Packages shipped from here should be following that pattern unless there's a special circumstance. So that we can determine the best way forward, can you please help me understand:
-
Have each of these new packages been approved as dependencies by @KrzysztofCwalina and @christothes?
-
Is there a reason why you're unable to use the 8.x line for them? If so, please share the scenarios which are blocked.
-
Are there stable versions that could replace the preview versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A dependency to an GA version have been approved. Not preview.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should have been 9.5.0 (rather than 9.5.0-preview.1.25262.9), which is GA. @jozkee, mind fixing it?
It's not 8.x because an 8.x doesn't exist. But the package's assets don't reference anything newer than 8:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsquire / @KrzysztofCwalina / @stephentoub - I apologize; this was my communication error. I said to @jozkee that this was good to go ahead and push today, but of course we should have just waited a few hours until the stable version of Microsoft.Extensions.AI was published and initiate the PR based on that.
Since @jozkee is away from work Friday, I updated the PR to:
- Use Microsoft.Extensions.AI and Microsoft.Extensions.AI.Abstractions 9.5.0 stable
- Use Microsoft.Extensions.Caching.Memory 8.0.1 per @jsquire's comment that we should baseline to 8.0.x versions
- Incorporated the last change we made in dotnet/extensions to use RawRepresentationFactory on embedding generators
- Made a couple remaining
Assert
coding style changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @jeffhandley. We'll also need to isolate the 9.x dependencies in a conditional block specific to this library. We don't want them to appear as approved dependencies for other libraries in the repository. An example (and a good place to add it) is around here: https://github.com/Azure/azure-sdk-for-net/blob/main/eng/Packages.Data.props#L199
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pointer on that @jsquire. I just pushed a commit that uses a conditional <PackageReference />
. For the test reference to Microsoft.Extensions.AI
, I didn't see any project-specific package referenced defined, so I left a comment on the line instead.
…tionFactory on embedding generators.
Code was originally located in
https://github.com/dotnet/extensions/tree/749caedb584a20811c5ebe1b00db7763fbcaac93/src/Libraries/Microsoft.Extensions.AI.AzureAIInference
@stephentoub @jeffhandley