-
Couldn't load subscription status.
- Fork 287
Add AI support to the Microsoft.Testing.Platform
#6777
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
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.
Any tests possible for this?
src/Platform/Microsoft.Testing.Platform.AI/IChatClientFactory.cs
Outdated
Show resolved
Hide resolved
...m/Microsoft.Testing.Extensions.AzureFoundry/Microsoft.Testing.Extensions.AzureFoundry.csproj
Show resolved
Hide resolved
src/Platform/Microsoft.Testing.Extensions.AzFoundry/OpenAIChatClientFactory.cs
Outdated
Show resolved
Hide resolved
src/Platform/Microsoft.Testing.Platform.AI/Microsoft.Testing.Platform.AI.csproj
Show resolved
Hide resolved
Co-authored-by: Youssef Victor <youssefvictor00@gmail.com>
Co-authored-by: Youssef Victor <youssefvictor00@gmail.com>
At the moment nope it's experimental for now...I need to play a bit with the usage to understand, I would not ship the core ai and the extension lib for now, the plumbing is internal and should not affect the normal run if not registered. We can try to add some real test when complete...but not sure if we will be able to test the AI interaction, but let's see maybe we will do it in some way. |
|
Should we avoid publishing it publicly for now until we implement something that uses it and we have more confidence of the design? Maybe we should set |
wonder if we can ship it inside not official nuget feed but in the dogfooding testing one so I can pick it up to do some experiment outside the testfx repo. We can always delist there in case. |
|
I'm fine publishing to test-tools feed and not nuget.org. But I want to ensure we don't end up publishing to nuget.org accidentally. I think setting |
Done! |
src/Platform/Microsoft.Testing.Extensions.AzureFoundry/OpenAIChatClientProvider.cs
Outdated
Show resolved
Hide resolved
src/Platform/Microsoft.Testing.Platform.AI/Microsoft.Testing.Platform.AI.csproj
Outdated
Show resolved
Hide resolved
…mplementation - Make AzureOpenAIChatClientProvider internal - Add public AddAzureOpenAIChatClientProvider extension method - Extract IEnvironment service and store in field instead of keeping IServiceProvider - Remove unnecessary InternalsVisibleTo from Microsoft.Testing.Platform.AI - Update Playground sample to use new extension method
…nAIChatClientProvider
...m/Microsoft.Testing.Extensions.AzureFoundry/Microsoft.Testing.Extensions.AzureFoundry.csproj
Show resolved
Hide resolved
...m/Microsoft.Testing.Extensions.AzureFoundry/Microsoft.Testing.Extensions.AzureFoundry.csproj
Show resolved
Hide resolved
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.
Playground project is now run during tests and so it fails
| /// Gets a value indicating whether the chat client supports tool calling | ||
| /// (e.g., MCP tools or functions). | ||
| /// </summary> | ||
| bool SupportsToolCalling { get; } |
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.
Maybe HasToolCapability
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.
in LLM space the feature is called "tool calling", tool capability looks pretty generic imo
| void AddChatClientProvider(Func<IServiceProvider, object> chatClientProvider); | ||
| void RegisterChatClientProvider(ServiceProvider serviceProvider); |
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.
It's not easy to grasp difference between APIs here.
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.
Propose a name pls it's all internal, register is "to register to the service provider" looks clear to me, but open to different naming
| <data name="AzureOpenAIEndpointNotSet" xml:space="preserve"> | ||
| <value>AZURE_OPENAI_ENDPOINT environment variable is not set.</value> | ||
| </data> | ||
| <data name="AzureOpenAIDeploymentNameNotSet" xml:space="preserve"> | ||
| <value>AZURE_OPENAI_DEPLOYMENT_NAME environment variable is not set.</value> | ||
| </data> | ||
| <data name="AzureOpenAIApiKeyNotSet" xml:space="preserve"> | ||
| <value>AZURE_OPENAI_API_KEY environment variable is not set.</value> | ||
| </data> |
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.
Maybe reverse order in the message and create a single resx entry: Environment variable '{0}' is not set
| !string.IsNullOrEmpty(_environment.GetEnvironmentVariable("AZURE_OPENAI_ENDPOINT")) && | ||
| !string.IsNullOrEmpty(_environment.GetEnvironmentVariable("AZURE_OPENAI_DEPLOYMENT_NAME")) && | ||
| !string.IsNullOrEmpty(_environment.GetEnvironmentVariable("AZURE_OPENAI_API_KEY")); |
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.
Use the string helper (not sure to recall the name in testfx repo).
| /// Initializes a new instance of the <see cref="AzureOpenAIChatClientProvider"/> class. | ||
| /// </summary> | ||
| /// <param name="serviceProvider">The service provider.</param> | ||
| internal AzureOpenAIChatClientProvider(IServiceProvider serviceProvider) |
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.
I would take IEnvironment here. It's easier to grasp what the service needs.
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.
It's all internal but it's fine I'll update the ctor
| /// <summary> | ||
| /// Provider interface for creating and configuring chat clients. | ||
| /// </summary> | ||
| public interface IChatClientProvider |
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.
Shall we make it Experimental to give us time to refine?
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.
We won't ship in public for now, I'll anyway add experimental.
Add AI support to the
Microsoft.Testing.Platform