Skip to content

Replica: .Net: feat: Modernize samples and documentation for ITextSearch<TRecord> interface (microsoft#10456) #13194#2

Merged
alzarei merged 1 commit intofeature-text-search-linqfrom
feature-text-search-linq-pr6
Nov 24, 2025
Merged

Replica: .Net: feat: Modernize samples and documentation for ITextSearch<TRecord> interface (microsoft#10456) #13194#2
alzarei merged 1 commit intofeature-text-search-linqfrom
feature-text-search-linq-pr6

Conversation

@alzarei
Copy link
Owner

@alzarei alzarei commented Nov 24, 2025

Replica of microsoft#13194 (comment) in the Microsoft Repo

…ples

Add comprehensive LINQ filtering demonstrations for the new ITextSearch<TRecord> interface to showcase type-safe search capabilities across multiple connectors.

**GettingStartedWithTextSearch (Step1_Web_Search.cs):**
- Add BingSearchWithLinqFilteringAsync() demonstrating ITextSearch<BingWebPage> with Language and FamilyFriendly filters
- Add GoogleSearchWithLinqFilteringAsync() demonstrating ITextSearch<GoogleWebPage> with Title.Contains() and DisplayLink.EndsWith() filters
- Use interface type declaration (ITextSearch<TRecord>) to showcase the new generic interface
- Include CA1859 pragma suppression with explanation (sample intentionally demonstrates interface usage)

**Concepts (Bing_TextSearch.cs):**
- Add UsingBingTextSearchWithLinqFilteringAsync() with 4 progressive LINQ filter examples
- Demonstrate Language equality, FamilyFriendly boolean filtering, compound AND filters, and complex multi-property filters
- Show both SearchAsync() and GetSearchResultsAsync() usage patterns

**Concepts (Tavily_TextSearch.cs):**
- Add UsingTavilyTextSearchWithLinqFilteringAsync() with 4 progressive LINQ filter examples
- Demonstrate Title.Contains() filtering, compound filters with exclusion (!Contains), full result retrieval with filtering, and complex multi-property filters
- Showcase null-safety patterns with Title != null checks

**Technical Notes:**
- Uses ITextSearch<TRecord> interface type to avoid method ambiguity with legacy ITextSearch
- Generic methods are explicitly implemented, requiring interface type or casting
- All examples include proper null checks for nullable properties
- Maintains backward compatibility with existing simple search examples

Addresses microsoft#10456
@alzarei alzarei merged commit 9604b91 into feature-text-search-linq Nov 24, 2025
1 of 7 checks passed
alzarei added a commit that referenced this pull request Feb 12, 2026
Reverts GetSearchResultsAsync to return full BraveWebResult instead of limited BraveWebPage for backward compatibility.

The legacy ITextSearch interface (non-generic) previously returned full BraveWebResult objects with all fields (Age, PageAge, Breaking, Video, MetaUrl, Thumbnail, Source, etc.). The recent change to return BraveWebPage with only Title, Url, and Description was a breaking change for existing consumers.

This fix restores backward compatibility by returning the complete BraveWebResult object.

Resolves Copilot feedback Issue #2 on PR microsoft#13384.
alzarei added a commit that referenced this pull request Feb 13, 2026
…3542)

# PR: Revert Brave Legacy Interface Breaking Change

## Motivation and Context

This PR addresses **High Priority Issue 2** from Copilot's code review
feedback on PR microsoft#13384 (feature-text-search-linq → main).

**Problem:**
The legacy `ITextSearch` interface (non-generic, deprecated) for
`BraveTextSearch` introduced a breaking change by returning
`BraveWebPage` objects instead of `BraveWebResult` objects in the
`GetSearchResultsAsync()` method.

This issue was introduced in PR microsoft#13191 when `BraveWebPage` was created
for the modern generic interface. The `GetResultsAsObjectAsync()` method
was inadvertently changed to return the new type instead of maintaining
compatibility with the legacy API contract.

## Testing

- [ ] Verified changes compile successfully
- [x] Confirmed backward compatibility restored
- [x] Full `BraveWebResult` object returned with all fields
- [ ] Unit tests pass (pending CI/CD validation)

## Related Issues/PRs

- **Parent PR:** microsoft#13384 (feature-text-search-linq → main)
- **Copilot Review:** Issue #2 - Breaking Change (High Priority)
- **Root Cause:** PR microsoft#13191 introduced `BraveWebPage` for modern generic
interface but inadvertently changed legacy interface to return new type
instead of preserving `BraveWebResult`
- **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] Backward compatibility maintained for legacy interface
- [x] Changes are documented as needed
- [x] Migration path to modern interface documented

---------

Co-authored-by: Alexander Zarei <alzarei@users.noreply.github.com>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant