-
Notifications
You must be signed in to change notification settings - Fork 786
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
Add Aspire support to chat template #6102
base: main
Are you sure you want to change the base?
Conversation
}, | ||
"IsAspire": { | ||
"type": "computed", | ||
"value": "(UseAspire || VectorStore == \"qdrant\")" |
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.
(UseAspire || VectorStore == "qdrant")
This is because Qdrant is only supported in the Aspire variation of the template. If there's a way for the template engine to disallow certain combinations of options (I couldn't seem to find one), then that's an alternative to consider.
....Extensions.AI.Templates/src/ChatWithCustomData/ChatWithCustomData-CSharp.AppHost/Program.cs
Show resolved
Hide resolved
...AI.Templates/src/ChatWithCustomData/ChatWithCustomData-CSharp.Web/Directory.Build.targets.in
Show resolved
Hide resolved
...rc/ChatWithCustomData/ChatWithCustomData-CSharp.Web/Services/Ingestion/PDFDirectorySource.cs
Show resolved
Hide resolved
foreach (var clause in options?.Filter?.FilterClauses ?? []) | ||
if (options?.Filter is { } filter) | ||
{ | ||
if (clause is EqualToFilterClause equalClause) | ||
{ | ||
var propertyInfo = typeof(TRecord).GetProperty(equalClause.FieldName); | ||
filteredRecords = filteredRecords.Where(record => propertyInfo!.GetValue(record)!.Equals(equalClause.Value)); | ||
} | ||
else | ||
{ | ||
throw new NotSupportedException($"The provided filter clause type {clause.GetType().FullName} is not supported."); | ||
} | ||
filteredRecords = filteredRecords.AsQueryable().Where(filter); | ||
} | ||
|
||
var ranked = (from record in filteredRecords | ||
let candidateVector = _getVector(record) | ||
let similarity = TensorPrimitives.CosineSimilarity(candidateVector.Span, floatVector.Span) | ||
orderby similarity descending | ||
select (Record: record, Similarity: similarity)); | ||
var ranked = from record in filteredRecords | ||
let candidateVector = _getVector(record) | ||
let similarity = TensorPrimitives.CosineSimilarity(candidateVector.Span, floatVector.Span) | ||
orderby similarity descending | ||
select (Record: record, Similarity: similarity); |
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.
These changes are all reacting to updating the Microsoft.SemanticKernel
version.
...lates/Microsoft.Extensions.AI.Templates/src/ChatWithCustomData/ChatWithCustomData-CSharp.sln
Outdated
Show resolved
Hide resolved
I'm going to undraft this PR so it can get some eyes. Note that there are some known issues I still need to address:
|
...ates/Microsoft.Extensions.AI.Templates/src/ChatWithCustomData/.template.config/template.json
Outdated
Show resolved
Hide resolved
…tWithCustomData/.template.config/template.json Co-authored-by: Steve Sanderson <[email protected]>
....Extensions.AI.Templates/src/ChatWithCustomData/ChatWithCustomData-CSharp.AppHost/Program.cs
Outdated
Show resolved
Hide resolved
"UseAspire": { | ||
"type": "parameter", | ||
"displayName": "Use Aspire orchestration", | ||
"datatype": "bool", | ||
"defaultValue": "false", | ||
"description": "Create the project as a distributed application using .NET Aspire." | ||
}, |
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 is resulting in -Us
as the short switch, but single-character switches are more standard, and it highlights that --UseManagedIdentity
managed to squat on -U
. I don't see a way to prevent a short name version of the option name from being recognized, so our possible approaches are limited.
@jmatthiesen @MackinnonBuck What would you think about having -U, --Use
where multiple values can be specified with allowMultipleValues
? Such as --Use Aspire --Use ManagedIdentity
(which would then also respect -U aspire,managedidentity
.
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 agree we should find an alternative here.
The UseManagedIdentity
option is currently disabled when the Aspire option is specified, so specifying both options isn't currently a scenario.
I wonder if we should do an audit of all the short/long names in general. For example, the Blazor Web App template has names like -au
for authentication, -uld
for "Use LocalDB", etc.
Maybe we have something like:
-p, --provider AI service provider
-v, --vector-store Vector store
-mi, --managed-identity Use managed identity
-a, --aspire Use Aspire
...
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.
Updated to use the option names listed directly above.
var credential = new ApiKeyCredential(builder.Configuration["GITHUB_MODELS_TOKEN"] ?? throw new InvalidOperationException("Missing configuration: GITHUB_MODELS_TOKEN. See the README for details.")); | ||
var openAIOptions = new OpenAIClientOptions() | ||
{ | ||
Endpoint = new Uri("https://models.inference.ai.azure.com") | ||
}; | ||
|
||
var ghModelsClient = new OpenAIClient(credential, openAIOptions); | ||
var chatClient = ghModelsClient.AsChatClient("gpt-4o-mini"); | ||
var embeddingGenerator = ghModelsClient.AsEmbeddingGenerator("text-embedding-3-small"); |
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.
Why not use the OpenAI Aspire package for this?
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 - updated to use the Aspire package for GitHub models.
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 looked for this README at the root of the solution rather than in the Web project. I'm not sure if others will have that same expectation though.
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.
Yeah, I'm preparing an Aspire variation of the README. Will push soon 🙂
|
||
```sh | ||
cd <<your-project-directory>> | ||
dotnet user-secrets set GitHubModels:Token YOUR-TOKEN |
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 wasn't sure which project folder to run this from. I either chose poorly, or this didn't flow through as expected because the dashboard told me the value was missing.
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 new README will explain how to configure things.
...ates/Microsoft.Extensions.AI.Templates/src/ChatWithCustomData/.template.config/template.json
Outdated
Show resolved
Hide resolved
eng/Versions.props
Outdated
<MicrosoftExtensionsServiceDiscoveryVersion>9.1.0</MicrosoftExtensionsServiceDiscoveryVersion> | ||
<MicrosoftSemanticKernelConnectorsAzureAISearchVersion>1.41.0-preview</MicrosoftSemanticKernelConnectorsAzureAISearchVersion> | ||
<MicrosoftSemanticKernelConnectorsQdrantVersion>1.41.0-preview</MicrosoftSemanticKernelConnectorsQdrantVersion> | ||
<MicrosoftSemanticKernelCoreVersion>1.41.0</MicrosoftSemanticKernelCoreVersion> | ||
<OllamaSharpVersion>5.1.5</OllamaSharpVersion> | ||
<OpenAIVersion>2.2.0-beta.1</OpenAIVersion> |
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 ended up with both 2.2.0-beta.3 and 2.2.0-beta.1 in my solution, which gave me an error that it was being downgraded from .3 to .1, and I had to force bump it to .3.
I presume when building with reference to the publicly available MEAI.OpenAI version, it would be OK because both would reference beta.1, but when using the just-built packages, it fails.
…tWithCustomData/.template.config/template.json Co-authored-by: Jeff Handley <[email protected]>
Adds an Aspire option to the AI Chat Web template.
All the existing AI service providers and vector databases are supported, and a new Qdrant option is now enabled when using Aspire.
Fixes #6052
Microsoft Reviewers: Open in CodeFlow