Skip to content

Commit d1f2733

Browse files
alzareialzarei
committed
.NET: Implement Option 3 dual interface for Issue #10456
Add ITextSearch<TRecord> generic interface with LINQ filtering while maintaining existing ITextSearch non-generic interface for backward compatibility. ## Background: Architectural Decision (Issue #10456) Three options considered: Option 1 (Native LINQ): Replace TextSearchFilter with Expression<Func<T, bool>> - Breaking change: requires user migration - Best long-term architecture Option 2 (Translation Layer): Convert TextSearchFilter to LINQ internally - Breaking change: RequiresDynamicCode propagates through API surface - Reflection overhead, AOT incompatible Option 3 (Dual Interface): Add ITextSearch<TRecord> alongside ITextSearch - No breaking changes - Maintains AOT compatibility - Uses obsolete VectorSearchFilter in legacy path (temporary during transition) ## Implementation ### Generic Interface - ITextSearch<TRecord> with 3 methods accepting TextSearchOptions<TRecord> - TextSearchOptions<TRecord> with Expression<Func<TRecord, bool>>? Filter - Explicit interface implementation in VectorStoreTextSearch<TRecord> ### Dual-Path Architecture Two independent code paths, no translation layer: Legacy path (non-generic): - ITextSearch with TextSearchOptions and TextSearchFilter (clause-based) - Uses VectorSearchOptions.OldFilter (obsolete) with pragma warning suppression - No dynamic code, AOT compatible - 10 existing tests unchanged Modern path (generic): - ITextSearch<TRecord> with TextSearchOptions<TRecord> and Expression filter - Uses VectorSearchOptions.Filter (LINQ native, not obsolete) - No dynamic code, AOT compatible - 7 new tests ## Changes - Added ITextSearch<TRecord> interface and TextSearchOptions<TRecord> class - Implemented dual interface in VectorStoreTextSearch<TRecord> - Deleted ~150 lines of Option 2 translation layer code - Removed all RequiresDynamicCode attributes - Removed DynamicallyAccessedMemberTypes.PublicMethods from 5 locations: - VectorStoreTextSearch.cs - TextSearchServiceCollectionExtensions.cs (3 methods) - TextSearchKernelBuilderExtensions.cs (1 method) - Deleted 7 Option 2 translation tests - Added 7 LINQ filtering tests - Added DataModelWithTags to test base - Reverted Program.cs to original state ## Files Changed 8 files, +144 insertions, -395 deletions - ITextSearch.cs - TextSearchOptions.cs (added generic class) - VectorStoreTextSearch.cs (removed translation layer, added dual interface) - TextSearchServiceCollectionExtensions.cs (removed PublicMethods annotation) - TextSearchKernelBuilderExtensions.cs (removed PublicMethods annotation) - VectorStoreTextSearchTestBase.cs (added DataModelWithTags) - VectorStoreTextSearchTests.cs (removed 7 tests, added 7 tests) - Program.cs in AotTests (removed suppression, restored tests)
1 parent a802eaa commit d1f2733

File tree

8 files changed

+143
-396
lines changed

8 files changed

+143
-396
lines changed

‎dotnet/src/SemanticKernel.Abstractions/Data/TextSearch/ITextSearch.cs‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ namespace Microsoft.SemanticKernel.Data;
88

99
/// <summary>
1010
/// Interface for text based search queries with type-safe LINQ filtering for use with Semantic Kernel prompts and automatic function calling.
11+
/// This generic interface supports LINQ-based filtering through <see cref="TextSearchOptions{TRecord}"/> for type-safe queries.
1112
/// </summary>
1213
/// <typeparam name="TRecord">The type of record being searched.</typeparam>
1314
[Experimental("SKEXP0001")]
@@ -19,10 +20,6 @@ public interface ITextSearch<TRecord>
1920
/// <param name="query">What to search for.</param>
2021
/// <param name="searchOptions">Options used when executing a text search.</param>
2122
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests. The default is <see cref="CancellationToken.None"/>.</param>
22-
/// <remarks>
23-
/// Dynamic code generation is only required when using AnyTagEqualTo filter operations that generate LINQ Contains expressions.
24-
/// Simple equality filtering is AOT-compatible.
25-
/// </remarks>
2623
Task<KernelSearchResults<string>> SearchAsync(
2724
string query,
2825
TextSearchOptions<TRecord>? searchOptions = null,
@@ -34,10 +31,6 @@ Task<KernelSearchResults<string>> SearchAsync(
3431
/// <param name="query">What to search for.</param>
3532
/// <param name="searchOptions">Options used when executing a text search.</param>
3633
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests. The default is <see cref="CancellationToken.None"/>.</param>
37-
/// <remarks>
38-
/// Dynamic code generation is only required when using AnyTagEqualTo filter operations that generate LINQ Contains expressions.
39-
/// Simple equality filtering is AOT-compatible.
40-
/// </remarks>
4134
Task<KernelSearchResults<TextSearchResult>> GetTextSearchResultsAsync(
4235
string query,
4336
TextSearchOptions<TRecord>? searchOptions = null,
@@ -49,10 +42,6 @@ Task<KernelSearchResults<TextSearchResult>> GetTextSearchResultsAsync(
4942
/// <param name="query">What to search for.</param>
5043
/// <param name="searchOptions">Options used when executing a text search.</param>
5144
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests. The default is <see cref="CancellationToken.None"/>.</param>
52-
/// <remarks>
53-
/// Dynamic code generation is only required when using AnyTagEqualTo filter operations that generate LINQ Contains expressions.
54-
/// Simple equality filtering is AOT-compatible.
55-
/// </remarks>
5645
Task<KernelSearchResults<object>> GetSearchResultsAsync(
5746
string query,
5847
TextSearchOptions<TRecord>? searchOptions = null,
@@ -61,6 +50,7 @@ Task<KernelSearchResults<object>> GetSearchResultsAsync(
6150

6251
/// <summary>
6352
/// Interface for text based search queries for use with Semantic Kernel prompts and automatic function calling.
53+
/// This non-generic interface uses legacy <see cref="TextSearchFilter"/> for backward compatibility.
6454
/// </summary>
6555
public interface ITextSearch
6656
{

‎dotnet/src/SemanticKernel.AotTests/Program.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// Copyright (c) Microsoft. All rights reserved.
22

3-
using System.Diagnostics.CodeAnalysis;
43
using SemanticKernel.AotTests.UnitTests.Core.Functions;
54
using SemanticKernel.AotTests.UnitTests.Core.Plugins;
65
using SemanticKernel.AotTests.UnitTests.Search;
@@ -20,7 +19,6 @@ private static async Task<int> Main(string[] args)
2019
return success ? 1 : 0;
2120
}
2221

23-
[UnconditionalSuppressMessage("AOT", "IL3050:Calling members annotated with 'RequiresDynamicCodeAttribute' may break functionality when AOT compiling.", Justification = "Test application intentionally tests dynamic code paths. VectorStoreTextSearch LINQ filtering requires reflection for dynamic expression building from runtime filter specifications.")]
2422
private static readonly Func<Task>[] s_unitTests =
2523
[
2624
// Tests for functions
@@ -59,7 +57,10 @@ private static async Task<int> Main(string[] args)
5957
KernelBuilderPluginsExtensionsTests.AddFromType,
6058
KernelBuilderPluginsExtensionsTests.AddFromObject,
6159

62-
// Tests for text search (VectorStoreTextSearch tests removed - incompatible with AOT due to RequiresDynamicCode for LINQ expressions)
60+
// Tests for text search
61+
VectorStoreTextSearchTests.GetTextSearchResultsAsync,
62+
VectorStoreTextSearchTests.AddVectorStoreTextSearch,
63+
6364
TextSearchExtensionsTests.CreateWithSearch,
6465
TextSearchExtensionsTests.CreateWithGetTextSearchResults,
6566
TextSearchExtensionsTests.CreateWithGetSearchResults,

‎dotnet/src/SemanticKernel.AotTests/UnitTests/Search/VectorStoreTextSearchTests.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// Copyright (c) Microsoft. All rights reserved.
22

3-
using System.Diagnostics.CodeAnalysis;
43
using Microsoft.Extensions.DependencyInjection;
54
using Microsoft.Extensions.VectorData;
65
using Microsoft.SemanticKernel;
@@ -11,7 +10,6 @@ namespace SemanticKernel.AotTests.UnitTests.Search;
1110

1211
internal sealed class VectorStoreTextSearchTests
1312
{
14-
[RequiresDynamicCode("Calls Microsoft.SemanticKernel.Data.VectorStoreTextSearch<TRecord>.GetTextSearchResultsAsync(String, TextSearchOptions, CancellationToken)")]
1513
public static async Task GetTextSearchResultsAsync()
1614
{
1715
// Arrange
@@ -39,7 +37,6 @@ public static async Task GetTextSearchResultsAsync()
3937
Assert.AreEqual("test-link", results[0].Link);
4038
}
4139

42-
[RequiresDynamicCode("Calls Microsoft.SemanticKernel.Data.VectorStoreTextSearch<TRecord>.GetTextSearchResultsAsync(String, TextSearchOptions, CancellationToken)")]
4340
public static async Task AddVectorStoreTextSearch()
4441
{
4542
// Arrange

‎dotnet/src/SemanticKernel.Core/Data/TextSearch/TextSearchKernelBuilderExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public static class TextSearchKernelBuilderExtensions
1818
/// <param name="resultMapper"><see cref="ITextSearchResultMapper" /> instance that can map a TRecord to a <see cref="TextSearchResult"/></param>
1919
/// <param name="options">Options used to construct an instance of <see cref="VectorStoreTextSearch{TRecord}"/></param>
2020
/// <param name="serviceId">An optional service id to use as the service key.</param>
21-
public static IKernelBuilder AddVectorStoreTextSearch<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicMethods)] TRecord>(
21+
public static IKernelBuilder AddVectorStoreTextSearch<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] TRecord>(
2222
this IKernelBuilder builder,
2323
ITextSearchStringMapper? stringMapper = null,
2424
ITextSearchResultMapper? resultMapper = null,

‎dotnet/src/SemanticKernel.Core/Data/TextSearch/TextSearchServiceCollectionExtensions.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public static class TextSearchServiceCollectionExtensions
2222
/// <param name="resultMapper"><see cref="ITextSearchResultMapper" /> instance that can map a TRecord to a <see cref="TextSearchResult"/></param>
2323
/// <param name="options">Options used to construct an instance of <see cref="VectorStoreTextSearch{TRecord}"/></param>
2424
/// <param name="serviceId">An optional service id to use as the service key.</param>
25-
public static IServiceCollection AddVectorStoreTextSearch<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicMethods)] TRecord>(
25+
public static IServiceCollection AddVectorStoreTextSearch<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] TRecord>(
2626
this IServiceCollection services,
2727
ITextSearchStringMapper? stringMapper = null,
2828
ITextSearchResultMapper? resultMapper = null,
@@ -58,7 +58,7 @@ public static class TextSearchServiceCollectionExtensions
5858
/// <param name="resultMapper"><see cref="ITextSearchResultMapper" /> instance that can map a TRecord to a <see cref="TextSearchResult"/></param>
5959
/// <param name="options">Options used to construct an instance of <see cref="VectorStoreTextSearch{TRecord}"/></param>
6060
/// <param name="serviceId">An optional service id to use as the service key.</param>
61-
public static IServiceCollection AddVectorStoreTextSearch<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicMethods)] TRecord>(
61+
public static IServiceCollection AddVectorStoreTextSearch<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] TRecord>(
6262
this IServiceCollection services,
6363
string vectorSearchableServiceId,
6464
ITextSearchStringMapper? stringMapper = null,
@@ -104,7 +104,7 @@ public static class TextSearchServiceCollectionExtensions
104104
/// <param name="options">Options used to construct an instance of <see cref="VectorStoreTextSearch{TRecord}"/></param>
105105
/// <param name="serviceId">An optional service id to use as the service key.</param>
106106
[Obsolete("Use the overload which doesn't accept a textEmbeddingGenerationServiceId, and configure an IEmbeddingGenerator instead with the collection represented by vectorSearchServiceId.")]
107-
public static IServiceCollection AddVectorStoreTextSearch<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicMethods)] TRecord>(
107+
public static IServiceCollection AddVectorStoreTextSearch<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties)] TRecord>(
108108
this IServiceCollection services,
109109
string vectorSearchServiceId,
110110
string textEmbeddingGenerationServiceId,

0 commit comments

Comments
 (0)