.Net: Net: Address roji/westey review feedback round 2 on text search connectors (#10456)#13611
Merged
roji merged 5 commits intomicrosoft:feature-text-search-linqfrom Mar 2, 2026
Conversation
- Add [Obsolete] to FilterClause, EqualToFilterClause, AnyTagEqualToFilterClause - Add [Obsolete] to non-generic TextSearchOptions and TextSearchFilter - Refactor Google connector: merge 8 methods into 3 using pattern matching - Refactor Bing/Brave/Tavily: pattern matching, remove MemoryExtensions - Add explicit error for collection Contains (Enumerable.Contains) in Google/Bing - Handle null-guard patterns (x != null) in Google expression processing - Remove dead code (MapGoogleFilterToProperty) - Consolidate pragma CS0618 to file-level in all affected files - ExtractFiltersFromLegacy: throw on unsupported FilterClause types
…ptions/TextSearchFilter
7d5f751 to
45e852b
Compare
roji
requested changes
Mar 2, 2026
| @@ -1,5 +1,7 @@ | |||
| // Copyright (c) Microsoft. All rights reserved. | |||
|
|
|||
| #pragma warning disable CS0618 // Obsolete ITextSearch, TextSearchOptions, TextSearchFilter, FilterClause - backward compatibility | |||
Member
There was a problem hiding this comment.
Ideally these would still be around the code using the obsolete legacy filtering stuff rather than over the entire file... Like I'd group all the legacy APIs in a #region and apply the suppression there.
Or is it too much somehow?
Contributor
Author
There was a problem hiding this comment.
Thanks! Yes, I like the suggested style too. Will revise shortly.
Member
There was a problem hiding this comment.
@alzarei I'm still seeing a file-scoped suppression here - the point is to more narrowly scope the suppression so we don't accidentally use another unrelated obsolete API etc.
| /// A <see cref="FilterClause"/> is used to request that the underlying search service should | ||
| /// filter search results based on the specified criteria. | ||
| /// </remarks> | ||
| [Obsolete("Use LINQ expressions via TextSearchOptions<TRecord>.Filter instead. This type will be removed in a future version.")] |
Member
There was a problem hiding this comment.
The obsoletion message is incorrect - we're in MEVD, so we shouldn't be mentioning TextSearchOptions
- Scope file-level #pragma warning disable CS0618 to only surround code that references obsolete types, leaving LINQ-based methods clean - Fix FilterClause/EqualToFilterClause/AnyTagEqualToFilterClause [Obsolete] messages to reference VectorSearchOptions<TRecord>.Filter (these types live in VectorData.Abstractions, not TextSearch)
westey-m
approved these changes
Mar 2, 2026
Per reviewer feedback, replace file-level #pragma warning disable CS0618 with surgically scoped pragmas around only the code that references obsolete types (ITextSearch, TextSearchOptions, TextSearchFilter, FilterClause, ITextEmbeddingGenerationService). - 4 connectors (Bing/Brave/Google/Tavily): scope to class declaration, legacy ITextSearch methods region, and ExtractFiltersFromLegacy - VectorStoreTextSearch: scope to class declaration, obsolete constructors, legacy methods region, legacy ExecuteVectorSearchAsync, _textEmbeddingGeneration field, and BuildFilterExpression - VectorStoreTextSearchTests: scope to legacy test methods only, leaving LINQ-based generic interface tests clean
westey-m
approved these changes
Mar 2, 2026
…ecuteVectorSearchCoreAsync
roji
approved these changes
Mar 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
.Net: Address roji/westey review feedback round 2 on text search connectors (#10456)
Motivation and Context
Addresses second round of review feedback from @roji and @westey-m on PR #13384 (
feature-text-search-linq). This round focuses on code quality improvements: adding[Obsolete]attributes to legacy types, consolidating expression processing with pattern matching, and removing dead code.Issue: #10456
Description
Obsolete attributes
Marked the following legacy types as
[Obsolete]to signal the transition to LINQ-based filtering:FilterClause,EqualToFilterClause,AnyTagEqualToFilterClause(VectorData.Abstractions) — base filter clause hierarchy replaced by LINQ expressionsTextSearchFilter(SemanticKernel.Abstractions) — replaced byTextSearchOptions<TRecord>.FilterTextSearchOptions(non-generic,SemanticKernel.Abstractions) — replaced byTextSearchOptions<TRecord>Google connector refactoring
Consolidated 8 expression-processing methods into 3 using C# pattern matching:
ProcessFilterNode: Recursive switch-based handler for AND, equality, inequality, string Contains, null-guard, collection Contains (error), and negation patternsProcessNegatedFilterNode: Handles!(expr)patterns, mapping to Google'sexcludeTermsMapPropertyToGoogleFilter: Converted to expression-bodied memberTryProcessSingleExpression,TryProcessEqualityExpression,TryProcessInequalityExpression,TryProcessContainsExpression,TryProcessNotExpression,CollectAndCombineFilters,IsMemoryExtensionsContains,MapGoogleFilterToProperty(dead code)Bing/Brave/Tavily connector cleanup
switchexpressions replacingif-chainsMemoryExtensionstype checks — replaced withObject: nullfor static method detectionError handling improvements
NotSupportedExceptionwith actionable guidance whencollection.Contains(page.Property)is used (unsupported by these APIs)ExtractFiltersFromLegacy: All four connectors now throw on unsupportedFilterClausesubtypes instead of silently skippingpage.Property != nullcomparisons (silently skipped)Pragma consolidation
#pragma warning disable/restore CS0618pairs with single file-level pragmas across 22 files (library, tests, samples, integration tests)AgentWithTextSearchProvider.cswhere a local#pragma warning restore CS0618was re-enabling the warning mid-methodFiles Changed
Commit 1: Core changes (15 files, +198/-445)
FilterClause.cs[Obsolete]EqualToFilterClause.cs[Obsolete]AnyTagEqualToFilterClause.cs[Obsolete]TextSearchFilter.cs[Obsolete], file-level pragmaTextSearchOptions.cs[Obsolete]on non-generic classGoogleTextSearch.csBingTextSearch.csMemoryExtensions, collection Contains errorBraveTextSearch.csMemoryExtensions, file-level pragmaTavilyTextSearch.csMemoryExtensions, file-level pragmaVectorStoreTextSearch.csTextSearchExtensions.csTextSearchProviderOptions.csMockTextSearch.cs(UnitTests)MockTextSearch.cs(AotTests)VectorStoreTextSearchTests.csCommit 2: Pragma fixes for samples/tests (7 files, +16/-1)
Bing_RagWithTextSearch.cs#pragma warning disable CS0618Bing_FunctionCallingWithTextSearch.csBing_TextSearch.csGoogle_TextSearch.csTavily_TextSearch.csStep3_Search_With_FunctionCalling.csAgentWithTextSearchProvider.csContribution Checklist
Validation
dotnet format --verify-no-changes)dotnet build SK-dotnet.slnx -c Release --warnaserror --no-incremental)dotnet publish -f net10.0)