Skip to content

Conversation

@alzarei
Copy link

@alzarei alzarei commented Nov 5, 2025

Add ADR-0065: LINQ-Based Filtering for ITextSearch Interface

Summary

This PR adds a comprehensive Architectural Decision Record (ADR) documenting the decision to migrate ITextSearch from clause-based filtering to LINQ-based filtering using a dual interface pattern.

Context

Issue: #10456

The existing ITextSearch interface uses TextSearchFilter (clause-based approach) which:

  • Creates runtime errors from property name typos
  • Lacks compile-time type safety
  • Requires conversion to obsolete VectorSearchFilter APIs internally
  • Provides no IntelliSense support

Decision

Chosen Approach: Dual Interface Pattern (Option 3)

  • NEW: ITextSearch<TRecord> with LINQ filtering (Expression<Func<TRecord, bool>>)
  • LEGACY: ITextSearch marked [Obsolete] for backward compatibility
  • Both interfaces coexist temporarily during migration

Key Architectural Points

Migration Strategy (Temporary by Design)

  1. Phase 1 (Current): Both interfaces coexist - zero breaking changes
  2. Phase 2 (Future Major Version): Increase deprecation warnings
  3. Phase 3 (Future Major Version): Remove obsolete interface entirely

Why Mark as [Obsolete] Now

  • Prevents new code from adopting legacy patterns
  • Signals future removal to ecosystem
  • Gives 1-2 major versions for migration
  • Follows .NET best practices for API evolution

Implementation Patterns Documented

Pattern A: Direct LINQ Passthrough (VectorStoreTextSearch)

  • Two independent code paths
  • No conversion overhead
  • Native LINQ support in vector stores

Pattern B: LINQ-to-Legacy Conversion (Bing, Google, Tavily, Brave)

  • Converts LINQ expressions to legacy format
  • Reuses existing API implementations
  • Expression tree analysis

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:

This ADR serves as:

  • Permanent documentation of the architectural decision
  • Reference for future maintainers
  • Explanation of the temporary dual interface state
  • Migration guide for ecosystem consumers

Checklist

  • ADR follows template structure
  • All required sections included
  • Deciders listed as required reviewers
  • Status set to accepted
  • Date reflects decision finalization
  • File naming follows convention: 0065-linq-based-text-search-filtering.md
  • Located in correct directory: dotnet/docs/decisions/

Target Branch

  • Base: feature-text-search-linq
  • Reason: ADR documents the architectural decisions implemented in this feature branch

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.
@markwallace-microsoft markwallace-microsoft added .NET Issue or Pull requests regarding .NET code documentation labels Nov 5, 2025
@github-actions github-actions bot changed the title docs: Add ADR-0065 for LINQ-based ITextSearch filtering migration strategy .Net: docs: Add ADR-0065 for LINQ-based ITextSearch filtering migration strategy Nov 5, 2025
@alzarei alzarei marked this pull request as ready for review November 5, 2025 08:54
@alzarei alzarei requested a review from a team as a code owner November 5, 2025 08:54
- 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

**Alternatives Considered**:

1. **Move to Plugins.Web**: Impossible due to internal constructor
Copy link
Author

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:

#13194 (comment)

…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation .NET Issue or Pull requests regarding .NET code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants