Skip to content

.NET: Fix filter combine logic for ChatHistoryMemoryProvider#4501

Merged
westey-m merged 4 commits intomicrosoft:mainfrom
westey-m:chathistorymemoryprovider-filter-combine-fix
Mar 6, 2026
Merged

.NET: Fix filter combine logic for ChatHistoryMemoryProvider#4501
westey-m merged 4 commits intomicrosoft:mainfrom
westey-m:chathistorymemoryprovider-filter-combine-fix

Conversation

@westey-m
Copy link
Contributor

@westey-m westey-m commented Mar 5, 2026

Motivation and Context

Fixes #4459

Description

  • Fix bug where each and-also part of the expression had a different parameter for the dictionary, while one is required.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the Contribution Guidelines
  • All unit tests pass, and I have added new tests where possible
  • Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

Copilot AI review requested due to automatic review settings March 5, 2026 15:41
@github-actions github-actions bot changed the title Fix filter combine logic for ChatHistoryMemoryProvider .NET: Fix filter combine logic for ChatHistoryMemoryProvider Mar 5, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a bug in ChatHistoryMemoryProvider where combining multiple scope filters (ApplicationId/AgentId/UserId/SessionId) could produce an invalid expression tree due to mismatched ParameterExpression instances, breaking compilation/translation in some vector store connectors.

Changes:

  • Reworked filter construction to use a single shared ParameterExpression, rebinding each filter body before combining with Expression.AndAlso.
  • Added a unit test that captures the generated filter and verifies it can be compiled and executed when multiple scope fields are set.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
dotnet/src/Microsoft.Agents.AI/Memory/ChatHistoryMemoryProvider.cs Builds combined scope filters using a shared parameter and introduces a small expression visitor to rebind parameters safely.
dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs Adds a regression test ensuring combined filters are compilable/executable when multiple scope filters are present.

westey-m and others added 3 commits March 5, 2026 17:19
Address PR review nit: use explicit types instead of var for better
readability in the filter-building logic and the new combined filter
compilation test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@westey-m westey-m added this pull request to the merge queue Mar 6, 2026
Merged via the queue into microsoft:main with commit f7e4143 Mar 6, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

.NET: [Bug]: ChatHistoryMemoryProvider.SearchChatHistoryAsync return error when calling collection.SearchAsync(

5 participants