.Net: docs: Add ADR-0065 for LINQ-based ITextSearch filtering migration strategy #13335#3
Merged
alzarei merged 1 commit intofeature-text-search-linqfrom Nov 24, 2025
Conversation
Add comprehensive Architectural Decision Record documenting the dual interface pattern approach for migrating ITextSearch from clause-based filtering to LINQ expressions. Documents migration strategy, obsolete marking, and future removal path.
alzarei
added a commit
that referenced
this pull request
Feb 12, 2026
Fixes two moderate-severity bugs identified in Copilot review: 1. Bing: Query parameters now throw ArgumentException when negation (!= operator) is attempted, since Bing API doesn't support negation for query params (cc, count, offset, mkt, safeSearch, textFormat). Negation only works with advanced search operators (site, language, contains, loc, location). 2. Google: Removed incorrect MIME->filter mapping. Google's 'filter' parameter is for duplicate content filtering (0/1), not MIME types. Also removed mappings for non-existent properties (HL, GL, CR, LR). Resolves Copilot feedback Issues #3 and microsoft#4 on PR microsoft#13384.
alzarei
added a commit
that referenced
this pull request
Feb 13, 2026
…rave/Tavily TextSearch (microsoft#13541) # PR: Fix AOT Compatibility - Remove Expression.Compile() from Brave/Tavily TextSearch ## Motivation and Context This PR addresses **Critical Issue #1** from Copilot's code review feedback on PR microsoft#13384 (feature-text-search-linq → main). **Problem:** The `ExtractValue()` method in both `BraveTextSearch` and `TavilyTextSearch` uses `Expression.Lambda(expression).Compile().DynamicInvoke()` as a fallback case, which breaks NativeAOT compatibility. This directly contradicts the ADR-0065 architectural decision that states: > "Both interfaces are AOT-compatible with no `[RequiresDynamicCode]` attributes" **Impact:** - Breaks NativeAOT compilation scenarios - Violates Semantic Kernel's AOT compatibility requirements - Contradicts documented architectural decisions - Blocks deployment scenarios requiring AOT **Root Cause:** The issue was introduced in PR microsoft#13191 (Tavily/Brave connector modernization) without AOT consideration. ## Description Replaces the AOT-incompatible `Expression.Lambda(expression).Compile().DynamicInvoke()` fallback with a `NotSupportedException` that: - Maintains AOT compatibility (no dynamic code generation) - Provides clear error message explaining supported expression types - Includes expression type and value for debugging - Guides users to use only constant expressions and simple member access ## Changes Made ### Files Modified - `dotnet/src/Plugins/Plugins.Web/Brave/BraveTextSearch.cs` - `dotnet/src/Plugins/Plugins.Web/Tavily/TavilyTextSearch.cs` ### Before (AOT-incompatible): ```csharp private static object? ExtractValue(Expression expression) { return expression switch { ConstantExpression constant => constant.Value, MemberExpression member when member.Expression is ConstantExpression constantExpr => member.Member switch { System.Reflection.FieldInfo field => field.GetValue(constantExpr.Value), System.Reflection.PropertyInfo property => property.GetValue(constantExpr.Value), _ => null }, _ => Expression.Lambda(expression).Compile().DynamicInvoke() // ❌ BREAKS AOT }; } ``` ### After (AOT-compatible): ```csharp private static object? ExtractValue(Expression expression) { return expression switch { ConstantExpression constant => constant.Value, MemberExpression member when member.Expression is ConstantExpression constantExpr => member.Member switch { System.Reflection.FieldInfo field => field.GetValue(constantExpr.Value), System.Reflection.PropertyInfo property => property.GetValue(constantExpr.Value), _ => null }, _ => throw new NotSupportedException( $"Unable to extract value from expression of type '{expression.GetType().Name}'. " + $"Only constant expressions and simple member access are supported for AOT compatibility. " + $"Expression: {expression}") }; } ``` ## Supported Expression Patterns ✅ **Works (AOT-compatible):** - Constant values: `page.Language == "en"` - Simple variables: `var lang = "en"; page.Language == lang` - Member access: `config.Language` (where `config` is a captured variable) ❌ **Not Supported (would require dynamic compilation):** - Computed values: `page.Language == someVariable` - Method calls: `page.Language == GetLanguage()` - Complex expressions: `page.Language == (isEnglish ? "en" : "fr")` Users encountering the exception should extract the value to a variable before using it in the filter expression. ## Testing - [x] Verified changes compile successfully - [x] Confirmed AOT compatibility (no `Expression.Compile()` usage) - [x] Exception message provides clear guidance - [ ] Unit tests pass (pending CI/CD validation) ## Related Issues/PRs - **Parent PR:** microsoft#13384 (feature-text-search-linq → main) - **Copilot Review:** Issue #1 - AOT Compatibility (Critical) - **Root Cause:** PR microsoft#13191 (Tavily/Brave modernization) - **ADR Reference:** docs/decisions/0065-linq-based-text-search-filtering.md (PR microsoft#13335) ## Contribution Checklist - [ ] The code builds clean without any errors or warnings - [x] Appropriate tests have been added (existing tests remain) - [x] Changes are documented as needed - [x] AOT compatibility verified ## Reviewer Notes @markwallace-microsoft, @westey-m This is the first of two critical fixes for PR microsoft#13384. The fix: 1. Eliminates all `Expression.Compile()` usage for AOT compatibility 2. Provides clear error messages for unsupported expression patterns 3. Aligns implementation with ADR-0065's AOT compatibility guarantee The exception should rarely be hit in practice since most filters use simple constants or variables (which are handled by the first two cases). Users with complex expressions will get immediate, clear feedback on how to fix their code. **Next Steps:** - Issue #2 (Breaking Change - Legacy Result Type) requires design decision - Issues #3-4 (Moderate bugs) can be addressed in follow-up PR --------- Co-authored-by: Alexander Zarei <alzarei@users.noreply.github.com>
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
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.
Replica of microsoft#13335