-
Notifications
You must be signed in to change notification settings - Fork 4.4k
.Net: docs: Add ADR-0065 for LINQ-based ITextSearch filtering migration strategy #13335
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
Open
alzarei
wants to merge
4
commits into
microsoft:feature-text-search-linq
Choose a base branch
from
alzarei:feature-text-search-linq-adr
base: feature-text-search-linq
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
.Net: docs: Add ADR-0065 for LINQ-based ITextSearch filtering migration strategy #13335
alzarei
wants to merge
4
commits into
microsoft:feature-text-search-linq
from
alzarei:feature-text-search-linq-adr
+706
−0
Conversation
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
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.
- Add 'Text Search Contains() Support Implementation' subsection - Documents query enhancement pattern for Brave/Tavily - Explains SearchQueryFilterClause design decision - Lists alternatives and consequences - Add 'SearchQueryFilterClause Public Visibility' subsection - Documents public API decision and rationale - Explains FilterClause internal constructor constraint - Lists alternatives considered (move to Plugins.Web, InternalsVisibleTo, etc.) These implementation decisions are integral to the overall LINQ-based text search filtering architecture and belong within the main ADR. microsoft#10456
alzarei
commented
Nov 16, 2025
|
|
||
| **Alternatives Considered**: | ||
|
|
||
| 1. **Move to Plugins.Web**: Impossible due to internal constructor |
Author
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.
Update per latest discussion, and changes:
…terClause decision in ADR-0065 Documents the architectural role of FilterClause as the common translation layer and updates the SearchQueryFilterClause visibility decision to reflect the implemented approach. Changes: - Added FilterClause architecture section with diagrams showing dual path convergence - Updated SearchQueryFilterClause decision: Made FilterClause constructor public and moved SearchQueryFilterClause to Plugins.Web (internal) - Clarified SearchQueryFilterClause is LINQ-only (never created by users) - Documented trade-off: Opening FilterClause to external inheritance to minimize MEVD API surface - Explained why FilterClause stays after legacy removal (LINQ translator needs it) - Distinguished user-facing APIs (removed in Phase 3) from internal translation layer (stays) - Added cross-reference between architecture explanation and decision rationale sections This captures the architectural discussion and final decision from PR microsoft#13191 review with @westey-m.
Changed FilterClause constructor from public to protected, which is the proper approach for abstract base classes. Changes: - Updated 'SearchQueryFilterClause Placement Decision' section (renamed from 'Public Visibility') - Simplified rationale: protected constructor allows inheritance while preventing instantiation - Removed mention of CA1012 suppression (no longer needed with protected) - Removed 'trade-off' language since protected is the standard practice - Updated cross-reference note to use new section name This reflects the final implementation using protected constructor as suggested in PR review.
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.
Add ADR-0065: LINQ-Based Filtering for ITextSearch Interface
Summary
This PR adds a comprehensive Architectural Decision Record (ADR) documenting the decision to migrate
ITextSearchfrom clause-based filtering to LINQ-based filtering using a dual interface pattern.Context
Issue: #10456
The existing
ITextSearchinterface usesTextSearchFilter(clause-based approach) which:VectorSearchFilterAPIs internallyDecision
Chosen Approach: Dual Interface Pattern (Option 3)
ITextSearch<TRecord>with LINQ filtering (Expression<Func<TRecord, bool>>)ITextSearchmarked[Obsolete]for backward compatibilityKey Architectural Points
Migration Strategy (Temporary by Design)
Why Mark as
[Obsolete]NowImplementation Patterns Documented
Pattern A: Direct LINQ Passthrough (VectorStoreTextSearch)
Pattern B: LINQ-to-Legacy Conversion (Bing, Google, Tavily, Brave)
Benefits
✅ Zero breaking changes - existing code continues working
✅ Type safety - compile-time validation and IntelliSense
✅ AOT compatible - no
[RequiresDynamicCode]attributes✅ Clear migration path - deprecation warnings guide users
✅ Future-ready - establishes path for technical debt removal
Related PRs
This ADR documents the architectural decision behind:
Review Notes
Per ADR process requirements:
accepted(implementation already complete)This ADR serves as:
Checklist
accepted0065-linq-based-text-search-filtering.mddotnet/docs/decisions/Target Branch
feature-text-search-linq