Skip to content

.NET: Fix filter combine logic for ChatHistoryMemoryProvider#4501

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

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

Conversation

@westey-m

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

Copy link
Copy Markdown
Contributor

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
@markwallace-microsoft markwallace-microsoft added the .NET Usage: [Issues, PRs], Target: .Net label Mar 5, 2026
@github-actions github-actions Bot changed the title Fix filter combine logic for ChatHistoryMemoryProvider .NET: Fix filter combine logic for ChatHistoryMemoryProvider Mar 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread dotnet/src/Microsoft.Agents.AI/Memory/ChatHistoryMemoryProvider.cs Outdated
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

.NET Usage: [Issues, PRs], Target: .Net

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