### Motivation and Context
<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
1. Why is this change required?
2. What problem does it solve?
3. What scenario does it contribute to?
4. If it fixes an open issue, please link to the issue here.
-->
### Description
Improve Milvus filtering to allow more id types to be supported
### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
### Motivation and Context
All .NET MEVD vector store providers have been migrated to the
[CommunityToolkit/AI
repository](https://github.com/CommunityToolkit/AI). This removes the
now-redundant source code and tests from this repo and leaves
discoverable redirects in their place.
### Description
**Removed source + tests** for 12 providers (Chroma and Milvus kept
unchanged):
- **AzureAISearch, CosmosMongoDB, CosmosNoSql, InMemory, PgVector,
Qdrant, Redis, SqlServer, SqliteVec, Weaviate** → moved to
[CommunityToolkit/AI/MEVD/src/](https://github.com/CommunityToolkit/AI/tree/main/MEVD/src)
- **MongoDB** → moved to
[mongodb/mongo-mevd-provider](https://github.com/mongodb/mongo-mevd-provider)
- **Pinecone** → removed; Pinecone archived their [.NET
SDK](https://github.com/pinecone-io/pinecone-dotnet-client), no longer
supported
- **Common** → removed (was only shared by SqlServer/SqliteVec/PgVector,
all gone)
**Added `README.md`** in each removed provider directory pointing to the
new upstream location (or explaining Pinecone deprecation).
**Removed all associated test projects** including the shared
`VectorData.ConformanceTests` infrastructure (no remaining consumers
after provider removal).
**Updated `dotnet/MEVD.slnx`** — now contains only Chroma and Milvus.
### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
---------
Co-authored-by: copilot-swe-agent[bot] <[email protected]>
### Motivation and Context
- The MEVD packages are not shipped from this repo anymore.
### Description
- Moving the packages out of the main SK solution
- Update all projects to use MEVD.Abstractions package directly
- Revert some previous code changes to make the MEVD implementations
compatible with the last MEVD.Abstractions package released from this
repo.
### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
In the SQL Server MEVD provider, table/schema/column names embedded
inside SQL string literals (`N'...'`) were not escaping single quotes.
This could produce broken SQL when names contain single quotes.
For example, a table named `it's` would generate:
```sql
IF OBJECT_ID(N'[dbo].[it's]', N'U') IS NULL
```
which is invalid SQL. The fix produces:
```sql
IF OBJECT_ID(N'[dbo].[it''s]', N'U') IS NULL
```
### Changes
Added `AppendTableNameInsideLiteral` and `AppendIdentifierInsideLiteral`
helper methods that escape single quotes by doubling them, and updated
the 5 call sites where identifiers appear inside string literals:
- `OBJECT_ID` in `CreateTable` (ifNotExists check)
- `OBJECT_ID` in full-text index PK lookup
- Dynamic SQL table name, column names, and catalog name for full-text
indexes
### Tests
Added unit tests for `AppendTableNameInsideLiteral`,
`AppendIdentifierInsideLiteral`, and a
`CreateTable_WithSingleQuoteInName` integration test. All 283 tests pass
(unit + conformance).
Co-authored-by: Copilot <[email protected]>
Updates the SQL Server vector store connector to support the latest
Azure SQL VECTOR_SEARCH() features, matching what was done in EF Core
(dotnet/efcore#38075).
## Changes
### VECTOR_SEARCH syntax update
- Replace deprecated `TOP_N` parameter with `SELECT TOP(N) WITH
APPROXIMATE`
- Enable iterative WHERE predicates are now applied during the vector
search process, not after retrievalfiltering
- Support `skip` via subquery wrapping (`TOP` and `OFFSET/FETCH` cannot
coexist in the same SELECT)
### Hybrid search
- Same VECTOR_SEARCH syntax changes in the semantic search CTE
- Filter support added in the vector search CTE (iterative filtering)
### Azure SQL runtime detection
- DiskAnn vector indexes and VECTOR_SEARCH require Azure SQL Database or
SQL database in Microsoft Fabric
- Added runtime check via `SERVERPROPERTY('EngineEdition')` at table
creation and query time
- Throws informative `NotSupportedException` when used against on-prem
SQL Server
- Result is cached per collection instance
### Test infrastructure
- Deleted `SqlServerDiskAnnVectorSearchTests.cs` (read-only table
workaround no longer tables with latest vector indexes now support full
DML)needed
- Created `SqlServerConnectionStringRequiredAttribute` to gate DiskAnn
tests on Azure SQL connection string availability
- Rewrote DiskAnn conformance test to handle 100-row minimum requirement
for vector index creation
- Updated all command builder unit tests for new SQL syntax
### Kept
- `PREVIEW_FEATURES` command in `CreateTable` (needed for future on-prem
SQL Server support)
---------
Co-authored-by: Copilot <[email protected]>
### Motivation and Context
`VectorStoreKeyAttribute.IsAutoGenerated` was typed as `bool?`, which C#
does not permit as a compile-time attribute argument. This made the
following valid-looking code fail to compile:
```csharp
[VectorStoreKey(StorageName = "name", IsAutoGenerated = true)]
public Guid Key { get; set; }
```
### Description
- **`VectorStoreKeyAttribute.IsAutoGenerated`**: Changed from `public
bool?` to a `public bool` property. Its setter stores the value into a
new `internal bool? IsAutoGeneratedNullable` backing property. The
getter returns `IsAutoGeneratedNullable.GetValueOrDefault()` rather than
throwing; an inline comment explains this is a workaround for the C#
compiler restriction that prevents `Nullable<bool>` from being used as a
compile-time attribute argument. The `bool?` backing preserves the "not
explicitly set" vs. "explicitly set" distinction the framework relies
on.
- **`CollectionModelBuilder`**: Updated to read
`keyAttribute.IsAutoGeneratedNullable` (the `bool?` backing) instead of
the former `bool?` public property, keeping the existing `??
SupportsKeyAutoGeneration(...)` fallback behaviour intact.
- **`KernelFunctionFromMethod`**: Fixed a pre-existing operator
precedence bug in a pattern match expression (`value is not JsonElement
or JsonDocument or JsonNode`) that caused CS9336 redundant-pattern
errors on the .NET 10 compiler. Added parentheses to correctly express
the intended negation of all three types: `value is not (JsonElement or
JsonDocument or JsonNode)`.
- **`CollectionModelBuilderTests`**: Added four new unit tests covering
all `IsAutoGenerated` attribute scenarios: explicitly set to `true`,
explicitly set to `false`, omitted on a `Guid` key (fallback to
`SupportsKeyAutoGeneration` returns `true`), and omitted on an `int` key
(fallback returns `false`).
```csharp
// Now compiles and works correctly
[VectorStoreKey(IsAutoGenerated = true)]
public Guid Key { get; set; }
[VectorStoreKey(IsAutoGenerated = false)]
public Guid Key { get; set; }
[VectorStoreKey] // IsAutoGeneratedNullable remains null, fallback to SupportsKeyAutoGeneration()
public Guid Key { get; set; }
```
### Contribution Checklist
- [x] The code builds clean without any errors or warnings
- [ ] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
<!-- START COPILOT ORIGINAL PROMPT -->
<details>
<summary>Original prompt</summary>
>
> ----
>
> *This section details on the original issue you should resolve*
>
> <issue_title>[MEVD] Can't set IsAutoGenerated property of
VectorStoreKeyAttribute at compile time</issue_title>
> <issue_description>The following code fails to compile:
>
> ```csharp
> [VectorStoreKey(StorageName = "name", IsAutoGenerated = true)]
> public Guid Key { get; set; }
> ```
>
> Because the `IsAutoGenerated` is `bool?`:
>
>
https://github.com/microsoft/semantic-kernel/blob/082e28e52ec2b45ad8cf287176d844150617e8b5/dotnet/src/VectorData/VectorData.Abstractions/RecordAttributes/VectorStoreKeyAttribute.cs#L30
>
> Which in C# maps to `System.Nullable<bool>` (a struct) and can't be
set at compile time.
>
> Could we make it just a `bool` like what it's mapped to at the end?
>
>
https://github.com/microsoft/semantic-kernel/blob/082e28e52ec2b45ad8cf287176d844150617e8b5/dotnet/src/VectorData/VectorData.Abstractions/ProviderServices/KeyPropertyModel.cs#L18
>
> Context:
https://github.com/dotnet/extensions/pull/7423/changes#r2974567816
>
> cc @roji </issue_description>
>
> <agent_instructions>1. Make
[this](https://github.com/microsoft/semantic-kernel/blob/082e28e52ec2b45ad8cf287176d844150617e8b5/dotnet/src/VectorData/VectorData.Abstractions/RecordAttributes/VectorStoreKeyAttribute.cs#L30)
property be a `bool` rather than `bool?` property.
> 2. Implement the getter and setter for it.
> 3. The setter should set an internal `bool?` property that should be
used instead of the public property everywhere in the repo.
[Example]https://github.com/microsoft/semantic-kernel/blob/082e28e52ec2b45ad8cf287176d844150617e8b5/dotnet/src/VectorData/VectorData.Abstractions/ProviderServices/CollectionModelBuilder.cs#L162)
> 3. Proof of concept code that I've used to verify it's going to get us
what we need:
>
> ```csharp
> using System.Reflection;
>
> namespace ConsoleApp15
> {
> public class Program
> {
> static void Main()
> {
> TestAttribute setToTrue =
typeof(Program).GetMethod(nameof(SetToTrue))!.GetCustomAttribute<TestAttribute>()!;
> Console.WriteLine($"IsAutoGenerated_Core for [Test(IsAutoGenerated =
true)]: {setToTrue.IsAutoGenerated_Core}");
>
> TestAttribute setToFalse =
typeof(Program).GetMethod(nameof(SetToFalse))!.GetCustomAttribute<TestAttribute>()!;
> Console.WriteLine($"IsAutoGenerated_Core for [Test(IsAutoGenerated =
false)]: {setToFalse.IsAutoGenerated_Core}");
>
> TestAttribute notSet =
typeof(Program).GetMethod(nameof(NotSet))!.GetCustomAttribute<TestAttribute>()!;
> Console.WriteLine($"IsAutoGenerated_Core for [Test]:
{notSet.IsAutoGenerated_Core}");
> }
>
> [Test(IsAutoGenerated = true)]
> public static void SetToTrue() { }
>
> [Test(IsAutoGenerated = false)]
> public static void SetToFalse() { }
>
> [Test]
> public static void NotSet() { }
> }
>
> [AttributeUsage(AttributeTargets.All)]
> public class TestAttribute : Attribute
> {
> public bool IsAutoGenerated
> {
> get => throw new InvalidOperationException(); // provided to satisfy
compiler requirements
> set => IsAutoGenerated_Core = value;
> }
>
> internal bool? IsAutoGenerated_Core { get; set; }
> }
> }
> ```</agent_instructions>
>
> ## Comments on the Issue (you are @copilot in this section)
>
> <comments>
> </comments>
>
</details>
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixesmicrosoft/semantic-kernel#13697
<!-- START COPILOT CODING AGENT TIPS -->
---
💬 Send tasks to Copilot coding agent from
[Slack](https://gh.io/cca-slack-docs) and
[Teams](https://gh.io/cca-teams-docs) to turn conversations into code.
Copilot posts an update in your thread when it's finished.
---------
Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: adamsitnik <[email protected]>
Removes all legacy filtering logic from MEVD, leaving only (obsoleted)
FilterClause and subclasses, which are still needed by ITextSearch (see
conversation in
https://github.com/microsoft/semantic-kernel/pull/13384#discussion_r2867207653).
As suggested by @westey-m let's do an SK release before merging this
(see above).
Closes#10456
/cc @alzarei
### Motivation and Context
With Qdrant server 1.17 Embeddings stopped being returned.
Fixes#13587
### Description
The `VectorOutput.Data` property became deprecated in `Qdrant.Client`
1.17. The alternative is to use `VectorOutput.Dense.Data` instead or
`VectorOutput.GetDenseVector()` (from `Qdrant.Client` 1.16.0 or later).
Since the `.Dense` already exyists in `Qdrant.Client` 1.15.1 that is
used by the current connector, I used these. The `.GetDenseVector()`
also returns nullable `DenseVector`, so I was unsure how to handle in
context of Semantic Kernel, as as far as I see it Semantic Kernel only
supports Dense Vectors (the other two are `.Sparse` for sparse vectors
and `.MultiDense`).
So this may need special handling in future to throw an exception, when
the server returns one of the other two vector types and not dense
vectors.
### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
I'm currently unable (or rather I don't know how) to run the local
integration/unit tests against the qdrant database. But I validated the
change in my personal project mentioned in #13587 by directly
referencing the connector in my project and it correctly returned the
vectors on the sample code where it failed previously
```csharp
VectorStoreCollection<ulong, VectorModels.Project> projects = await GetProjectsCollection();
VectorModels.Project? project = await projects.GetAsync((ulong)projectId, new()
{
IncludeVectors = true
});
// project at this poind hat 0 sized Embedding property
if (project is null)
{
return;
}
await projects.UpsertAsync(project);
```
---------
Co-authored-by: westey <[email protected]>
We have the model building option SupportsMultipleKeys to allow multiple
(composite) keys, but no provider uses them at this point (note that
Cosmos NoSQL has a sort of composite key - document + partition key, but
does not go through this support in any case).
Removing this - we can always add it when we actually need to support it
(and providers aren't blocked from implementing composite keys if they
really want to).
This is a sizable cleanup/redo of large chunks of the Cosmos NoSQL
provider; I've broken it down to three commits for easier reviewing.
* Cosmos NoSQL collections can now have either string/Guid as their key,
or CosmosNoSqlKey. In the former case, this configures the partition key
to be the same as the string/Guid document key - this is a sensible
default that's common (and encouraged) in Cosmos. All other scenarios
require CosmosNoSqlKey:
* A single partition key that isn't the document ID
* A hierarchical partition key (more than one property)
* No partition key (legacy/discouraged)
* Note that CosmosNoSqlKey is only passed to GetAsync and DeleteAsync;
on the user's .NET record type, the properties are there as usual (e.g.
a string document Id property, an int partition key property). This
introduces a discrepancy for the first time between the collection's
TKey (CosmosNoSqlKey) and its actual key property on the .NET type (a
string); this made some things trickier, but is the correct way to do
things for composite key cases (we'd do the same if we implement
composite keys for relational providers).
* Added support for Cosmos hierarchical partition keys (2-3 properties
instead of just one).
* GetAsync no longer does a SQL query with a WHERE clause, but rather an
efficient point read via ReadItem. The multiple-key overload does
ReadManyItemsAsync, which again is far more efficient.
* UpdateAsync and DeleteAsync now correctly supply the partition key in
the call's request options, again making it much more efficient.
* Various other cleanup and fixes all around.
DateTime now maps to the PG timestamptz type by default, which
represents a UTC timestamp; this aligns with Npgsql and EF Core.
However, now that we have property annotations, I've also added a new
`WithStoreType()` that allows the user to specify a database type
explicitly. For now, this is only supported for setting `timestamp with
time zone` (short: `timestamp`), which is the other PG date/time type,
for unspecified timestamps (timezone is unknown or implicitly assumed).
This allows users who do want to store non-UTC timestamp to do so.
Closes#10641
For providers which don't natively support score threshold, I've gone
with applying the filter client-side - the other option would be to
throw.
* In EF we're quite strict on not bringing back database data in order
to client-evaluate; in other words, if the user wants to load database
rows from the database and then perform some post-filtering, they need
to write that explicitly in code, to avoid scenarios where tons of data
gets unintentionally transferred.
* In MEVD we already implement Skip in various providers by bringing
back the entire matching resultset and then filtering client-side. This
is potentially much worse than filtering for score threshold
client-side, as the amount of extra data brought back by threshold
post-filtering is limited to top (k), whereas with Skip/pagination
queries that amount of data grows with each page.
* Ultimately, MEVD is simpler and more high-level, and is also
explicitly designed for layers to be composed on top (e.g. MEDI). So
there does seem to be more value in features "just working" across
providers.
Closes#9566
* Adds IsAutoGenerated to key property definition and attribute.
* When auto-generation is on, saving a record where the key property is
the default (empty GUID, zero integer) triggers the auto-generation.
It's still possible to set the key property to a non-default value,
overriding the auto-generation.
* Auto-generation can happen in the database (mainly on the relational
databases), or on the cilent side (Guid.NewGuid()).
* IsAutoGenerated is null by default, meaning that the provider decides
whether to turn on autogeneration by default, based on the property
type. This means that GUID properties are always auto-generated by
default (if the key property is left unassigned), and int/long
properties are auto-generated on relational providers. This aligns with
the behavior in Entity Framework.
* All providers support GUID generation, by doing it on the client
before sending the record (Guid.NewGuid()). Some providers have special
behavior, e.g. PG generates UUIDv7 and SQL Server generates sequential
IDs in the database (much better for indexing). Universal GUID
auto-generation makes things much simpler for upper layers, specifically
Microsoft.Extensions.DataIngestion.
Closes#11485
And allow pointing to an external database via environment variable on
SQL Server and PostgreSQL for faster iteration.
Test-only change.
---------
Co-authored-by: Mark Wallace <[email protected]>
### Motivation and Context
Switches the MongoDB Driver to the latest 3.5 release which had a number
of breaking changes.
This affects both the MongoDB vector data project and the CosmosMongoDB
project.
This fixes#11652 and likely addresses #12707 and partly #10291.
### Description
Switches the MongoDB driver to 3.5. Part of the breaking changes is that
GUIDs in BSON no longer have a default storage format specified due to
the need to switch from the C#-only format to the cross-MongoDB-driver
standard format. Setting this is achieved in this PR by way of both a
convention for the registry based mode and an alternative to
BsonValue.Create in the scenarios where we don't have access to
conventions/serialization such as key creation and using the dynamic
mapper.
### Contribution Checklist
- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
cc @roji
---------
Co-authored-by: Arthur Vickers <[email protected]>
Co-authored-by: Mark Wallace <[email protected]>
Co-authored-by: westey <[email protected]>
Co-authored-by: Roger Barreto <[email protected]>
Co-authored-by: Shay Rojansky <[email protected]>
This PR fixes a bug reported offline by @jeffhandley
```log
fail: Microsoft.Extensions.DataIngestion.IngestionPipeline[6]
An error occurred while ingesting document 'Example_GPS_Watch.md'.
System.InvalidCastException: Unable to cast object of type 'System.Guid' to type 'System.String'.
at System.Linq.Enumerable.CastICollectionIterator`1.MoveNext()
at Azure.Search.Documents.Models.IndexDocumentsBatch.Delete(String keyName, IEnumerable`1 keyValues)
at Azure.Search.Documents.SearchClient.DeleteDocumentsAsync(String keyName, IEnumerable`1 keyValues, IndexDocumentsOptions options, CancellationToken cancellationToken)
at Microsoft.Extensions.VectorData.VectorStoreErrorHandler.RunOperationAsync[TResult,TException](VectorStoreCollectionMetadata metadata, String operationName, Func`1 operation)
at Microsoft.Extensions.DataIngestion.VectorStoreWriter`1.WriteAsync(IAsyncEnumerable`1 chunks, CancellationToken cancellationToken)
at Microsoft.Extensions.DataIngestion.IngestionPipeline`1.IngestAsync(IngestionDocument document, Activity parentActivity, CancellationToken cancellationToken)
at Microsoft.Extensions.DataIngestion.IngestionPipeline`1.ProcessAsync(IEnumerable`1 files, Activity rootActivity, CancellationToken cancellationToken)+MoveNext()
```
---------
Co-authored-by: Shay Rojansky <[email protected]>
Another batch of test cleanup work - this almost concludes what I have
planned for the tests.
* We no longer create any collections with GUID names; all collections
have a manually-defined name, and are deleted when a test starts
running. This ensures that no collections are left behind if a test
crashes or is stopped in the middle, and makes it easier to examine the
database.
* Removed global SimpleRecord and SimpleFixture, each test suite now has
its own record and fixture.
* Cleaned up and renamed DistanceFunctionTests to only focus on testing
the different distance functions, and added corresponding tests
elsewhere.
* Added IndexKindTests to specifically cover different index types.
The main motivation here was to ensure that our infrastructure and tests
properly support binary embeddings (and more in general, non-float/half
ones).
Closes#13321
* Added new KeyTypeTests to cover provider key support, alongside the
existing DataTypeTests and EmbeddingTypeTests; moved all three into
TypeTests folder.
* As discussed offline, this adds Guid-as-string key support for all
providers. From research this seems to be very standard practice across
a wide range of databases: JSON-based databases generally encode various
types (e.g. DateTime) as string, and Guids are standard there. Sqlite is
similar, with Microsoft.Data.Sqlite supporting it natively. This notably
allows upper layers to assume that they can always use Guid, which helps
MEDI ([see this
issue](https://github.com/dotnet/extensions/issues/6973)).
* Per-provider notes
* Azure AI Search: string only supported, added Guid-as-string as an
option
([docs](https://learn.microsoft.com/en-us/rest/api/searchservice/data-type-map-for-indexers-in-azure-search#bkmk_sql_search)
encouraging mapping Guid to strings)
* MongoDB/CosmosMongoDB: Added int and long support (Guid was already
supported).
* CosmosNoSql: Added Guid-as-string (standard practice)
* Pinecone: Added Guid-as-string
* Redis: Added Guid-as-string
* SQL Server: removed DateTime and `byte[]` as key types (as above) and
added Guid.
* SQLite: The Microsoft.Data.Sqlite (the low-level ADO.NET provider
we're built on top of) supports GUIDs as TEXT; so it's really not us
deciding to support - it's the lower-level driver. In addition, we
support mapping boolean to INTEGER (1/0), which seems similar. Added
support as data property as well, not just key.
Closes#11784Closes#12182
This is an (almost) entirely test-only cleanup (I plan to do a bit more
later).
* Remove BasicQueryTests, rename BasicFilterTests to FilterTests and
move out to the root
* BasicFilterTests and BasicQueryTests both run the same filter
scenarios, the former via SearchAsync and the latter via filtering
GetAsync. Although in theory, some filter query construct could work in
one but not in the other, in practice no such example exists in the 12
providers we have (all databases use the same filter language/engine for
both APIs. So we had needless duplication for all the various test
exceptions/customizations, which had to be overridden in exactly the
same way in both test suites.
* At least for now, decided to remove BasicQueryTests as it adds
maintenance burden without any real coverage; coverage (included missing
coverage) specifically for filtered GetAsync was added separately (e.g.
we weren't testing ordering, skip/take with filtered get).
* If we do want to add back support for all query constructs (I'd want
to have a good reason to do so), I'd do that within FilterTests: just
like each scenario already exercises both static and dynamic models, it
could run GetAsync as well.
* Renamed CollectionConformanceTests to CollecitonManagementTests and
moved it out to the root.
* In general, this PR removes "Conformance" from the test type names.
* Big cleanup/refactor of all the so-called CRUD tests.
* We now have a ModelTests folder with a test suite for each different
model; BasicModelTests covers Get/Upsert/Delete (and their batching
variants) for the basic scenario (strongly-typed model with data
properties and a single vector property). Alongside that we have
DynamicModelTests, NoDataModelTests, NoVectorModelTests (I'll be added
MultipleVectorsModelTests).
* These tests no longer use random GUIDs for the collection name
(difficult to debug, the collection never gets cleaned up if the test
crashes/is stopped in the middle), but rather has fixed names per test
suite.
* The collection is created once per test suite, but cleared and
reseeded between each test run, providing full isolation (previously we
had no isolation, and documented each record in the seed data as
belonging to this scenario or that - which is brittle).
* Many other small improvements and cleanup.
Part of #12507
Update SqlClient to 6.1.2 and use the new `SqlVector<float>` type, which
allows efficient binary encoding of vector data (rather than the
previous JSON-basd transfer).
Closes#12220
/cc @yorek
This makes the provider filters compatible with C# 14 (see #12504, and
https://github.com/dotnet/runtime/issues/109757 on the runtime side).
Note that this changes the entire repo to build with dotnet SDK 10.0
(rc2) - that's probably a good idea regardless to test with the latest,
ensure compatibility etc. (but if this is problematic, let me know -
it's only truly necessary for test coverage and can be reverted).
I'm not happy to have the implementation duplicated across the
implementations: we have FilterTranslationPreprocessor for common LINQ
filter preprocessing, and that would be the ideal place for this.
However, rewriting MemoryExtensions.Contains to Enumerable.Contains
would require doing MethodInfo.MakeGenericMethod which is incompatible
with NativeAOT. I considered have a fake, non-invocable MethodInfo
implementation to represent the closed generic MethodInfo to
Enumerable.Contains (MethodInfo and Type are abstract, so we can create
custom implementations compatible with NativeAOT), but that's quite
involved/complicated. The better story here is probably to have a base
class implementation for the provider filter processors, which
recognizes specific patterns and dispatches to abstract handlers which
specific implementations override; it would take care of recognizing the
various Contains variants.
Note also that we already have duplication around Enumerable.Contains
and List.Contains (the instance method), so this just adds another
variant (for now). I'll think about cleaning this up.
Fixes#12504
### Motivation and Context
When using negative dot product scoring, the more negative the value,
the closer the vector is to the query vector.
### Description
Fixed the sorting used for negative dot product distance calculations
### Contribution Checklist
- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
---------
Co-authored-by: westey <[email protected]>
### Motivation and Context
To fix unreliable MongoDB Test Container start-up.
Fixes#13025
### Description
TestContainers.NET is, by default checking, for the string `Waiting for
connections` to appear TWICE in the MongoDB logs before deciding the
container is available. While this can happen quite often it doesn't
**always** happen and I believe this is a mistake (introduced in a large
PR).
TestContainers clients for Go and Rust only check the message appears
once (and TestContainers for Java has a different mechanism altogether).
This introduces an alternative waiting mechanism until I can get the fix
shipped upstream that only checks the message appears once.
Note: This PR does not yet enable the MongoDB tests as I think there are
some other intermittent issues relating to the use of lock/counting
mechanism that sometimes responds with a very different error.
### Contribution Checklist
- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
### Motivation and Context
- Fixes#12826
### Description
- Fixing bug where serialized data includes updated internal storage
type but deserialization did not.
### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
### Motivation and Context
We are running the SK tests in our
https://github.com/mongodb-labs/ai-ml-pipeline-testing repo and would
like to also be able to run them against live MongoDB Atlas clusters for
further verification rather than just the Docker image.
### Description
The PR adjusts the vector data tests (only as per @roji recommendation)
so that if an environment variable named `MONGODB__CONNECTIONURL` exists
then that is used to connect to MongoDB rather than using TestContainers
to fire up a Docker image of MongoDB Atlas.
Additionally to improve reliability it was found that we need to
increase the number of attempts to wait for the data to be ready after
creating an index. (Bumped from 20 to 200). We're going to look at
adding index status helper methods in the C# Driver going forward for a
better experience.
Once this is done we should be able to fully enable the new tests on our
repo and diagnose any further instabilities and then be able to submit a
PR to fully-enable the image-based tests here.
### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
### Motivation, Context and Description
This PR prepares the SK codebase for a future update of the A2A agent to
the new A2A .NET SDK and establishes the foundation for easy adoption of
the System.Linq.AsyncEnumerable package when it released. This will
completely eliminate the need for the AsyncEnumerable polyfills that SK
currently uses.
As part of this preparation, this PR stops using of the
System.Linq.Async package and replaces all SK calls to its APIs with
either equivalent calls to the AsyncEnumerable polyfills or in-place
implementations. This prevents "The call is ambiguous between..." name
conflicts that would occur when the AsyncEnumerable APIs from the A2A
.NET SDK's transitive dependency (System.Linq.AsyncEnumerable) become
available to the Agents.UnitTests project, which already contains
equivalent APIs from either AsyncEnumerable polyfills and/or the
System.Linq.Async package brought in from other projects that have the
Agents.UnitTests project specified as "InternalVisibleTo".
As a result, almost all non-production code (tests and samples) uses
AsyncEnumerable APIs from the preview version of the
System.Linq.AsyncEnumerable package, while production code uses either
AsyncEnumerable APIs from polyfills or uses in-place implementations.
I noticed that actual test suite implementation is a bit spotty across
providers; so this adds a test suite which tests that all other test
suites are implemented. We have this in the EF tests as well, also as a
way of letting provider maintainers know when a new test suite is
introduced which they probably want to implement. Of course, providers
can explicitly ignore specific test suites which don't make sense for
them.
Part of #10194
westey