Skip to content

Conversation

@alzarei
Copy link

@alzarei alzarei commented Oct 31, 2025

This PR enhances the type safety of the ITextSearch<TRecord> interface by changing the GetSearchResultsAsync method to return KernelSearchResults<TRecord> instead of KernelSearchResults<object>. This improvement eliminates the need for manual casting and provides better IntelliSense support for consumers.

Motivation and Context

The current implementation of ITextSearch<TRecord>.GetSearchResultsAsync returns KernelSearchResults<object>, which requires consumers to manually cast results to the expected type. This reduces type safety and degrades the developer experience by losing compile-time type checking and IntelliSense support.

This change aligns the return type with the generic type parameter TRecord, providing the expected strongly-typed results that users of a generic interface would anticipate.

Changes Made

Interface (ITextSearch.cs)

  • Changed ITextSearch<TRecord>.GetSearchResultsAsync return type from KernelSearchResults<object> to KernelSearchResults<TRecord>
  • Updated XML documentation to reflect strongly-typed return value
  • Legacy ITextSearch interface (non-generic) remains unchanged, continuing to return KernelSearchResults<object> for backward compatibility

Implementation (VectorStoreTextSearch.cs)

  • Added new GetResultsAsTRecordAsync helper method returning IAsyncEnumerable<TRecord>
  • Updated generic interface implementation to use the new strongly-typed helper
  • Retained GetResultsAsRecordAsync method for the legacy non-generic interface

Tests (VectorStoreTextSearchTests.cs)

  • Updated 3 unit tests to use strongly-typed DataModel or DataModelWithRawEmbedding instead of object
  • Improved test assertions to leverage direct property access without casting
  • All 19 tests pass successfully

Breaking Changes

Interface Change (Experimental API):

  • ITextSearch<TRecord>.GetSearchResultsAsync now returns KernelSearchResults<TRecord> instead of KernelSearchResults<object>
  • This interface is marked with [Experimental("SKEXP0001")], indicating that breaking changes are expected during the preview period
  • Legacy ITextSearch interface (non-generic) is unaffected and maintains full backward compatibility

Benefits

  • Improved Type Safety: Eliminates runtime casting errors by providing compile-time type checking
  • Enhanced Developer Experience: Full IntelliSense support for TRecord properties and methods
  • Cleaner Code: Consumers no longer need to cast results from object to the expected type
  • Consistent API Design: Generic interface now behaves as expected, returning strongly-typed results
  • Zero Impact on Legacy Code: Non-generic ITextSearch interface remains unchanged

Testing

  • All 19 existing unit tests pass
  • Updated tests demonstrate improved type safety with direct property access
  • Verified both generic and legacy interfaces work correctly
  • Confirmed zero breaking changes to non-generic ITextSearch consumers

Related Work

This PR is part of the Issue #10456 multi-PR chain for modernizing ITextSearch with LINQ-based filtering:

All PRs target the feature-text-search-linq branch for coordinated release.

Migration Guide for Consumers

Before (Previous API)

ITextSearch<DataModel> search = ...;
KernelSearchResults<object> results = await search.GetSearchResultsAsync("query", options);

foreach (var obj in results.Results)
{
    var record = (DataModel)obj;  // Manual cast required
    Console.WriteLine(record.Name);
}

After (Improved API)

ITextSearch<DataModel> search = ...;
KernelSearchResults<DataModel> results = await search.GetSearchResultsAsync("query", options);

foreach (var record in results.Results)  // Strongly typed!
{
    Console.WriteLine(record.Name);  // Direct property access with IntelliSense
}

Checklist

  • Changes build successfully
  • All unit tests pass (19/19)
  • XML documentation updated
  • Breaking change documented (experimental API only)
  • Legacy interface backward compatibility maintained
  • Code follows project coding standards

…rchResults<TRecord>

- Change interface return type from KernelSearchResults<object> to KernelSearchResults<TRecord>
- Update VectorStoreTextSearch implementation with new GetResultsAsTRecordAsync helper
- Keep GetResultsAsRecordAsync for legacy ITextSearch interface backward compatibility
- Update 3 unit tests to use strongly-typed DataModel instead of object

Benefits:
- Improved type safety - no more casting required
- Better IntelliSense and developer experience
- Zero breaking changes to legacy ITextSearch interface
- All 19 unit tests pass

This is Part 2.1 of the Issue microsoft#10456 multi-PR chain, refining the API .
@markwallace-microsoft markwallace-microsoft added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel kernel.core labels Oct 31, 2025
@github-actions github-actions bot changed the title .NET Improve type safety: Return TRecord instead of object in ITextSearch.GetSearchResultsAsync .Net Improve type safety: Return TRecord instead of object in ITextSearch.GetSearchResultsAsync Oct 31, 2025
@alzarei alzarei marked this pull request as ready for review November 2, 2025 21:57
@alzarei alzarei requested a review from a team as a code owner November 2, 2025 21:57
@alzarei
Copy link
Author

alzarei commented Nov 2, 2025

Thanks for the review, @westey-m. @markwallace-microsoft, could we go ahead and merge this? Deployment passed and it's been approved. We need to get this into other PRs as well. Appreciate it!

alzarei added a commit to alzarei/semantic-kernel that referenced this pull request Nov 3, 2025
…ts<BingWebPage> per PR microsoft#13318

- Change ITextSearch<BingWebPage>.GetSearchResultsAsync to return KernelSearchResults<BingWebPage> instead of <object>
- Add strongly-typed helper GetResultsAsBingWebPageAsync returning IAsyncEnumerable<BingWebPage>
- Update tests to expect strongly-typed results (no manual casting needed)
- Fix duplicate XML comment issue

This aligns with PR microsoft#13318 interface type safety improvements and eliminates wasteful casting.
alzarei added a commit to alzarei/semantic-kernel that referenced this pull request Nov 3, 2025
…Async

Changes:
- Update GoogleTextSearch.GetSearchResultsAsync to return KernelSearchResults<GoogleWebPage> instead of KernelSearchResults<object>
- Remove wasteful .Cast<object>() call
- Update test to use strongly-typed GoogleWebPage instead of casting from object

Benefits:
- Eliminates runtime casting overhead
- Provides compile-time type safety
- Improves IntelliSense for consumers

This change aligns with PR microsoft#13318 interface fix that changed ITextSearch<TRecord>.GetSearchResultsAsync to return KernelSearchResults<TRecord>.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kernel.core kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants