From ac9f471992d230bd9557ed69ce8454cd4502e701 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Thu, 5 Mar 2026 15:32:39 +0000 Subject: [PATCH 1/3] Fix filter combine logic for ChatHistoryMemoryProvider --- .../Memory/ChatHistoryMemoryProvider.cs | 51 +++++++++---- .../Memory/ChatHistoryMemoryProviderTests.cs | 71 +++++++++++++++++++ 2 files changed, 108 insertions(+), 14 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/Memory/ChatHistoryMemoryProvider.cs b/dotnet/src/Microsoft.Agents.AI/Memory/ChatHistoryMemoryProvider.cs index 80d5e1144f..46a3ee3e61 100644 --- a/dotnet/src/Microsoft.Agents.AI/Memory/ChatHistoryMemoryProvider.cs +++ b/dotnet/src/Microsoft.Agents.AI/Memory/ChatHistoryMemoryProvider.cs @@ -350,36 +350,38 @@ private async Task SearchTextAsync(string userQuestion, ChatHistoryMemor string? userId = searchScope.UserId; string? sessionId = searchScope.SessionId; - Expression, bool>>? filter = null; + // Build a combined filter using a single shared parameter to avoid expression tree + // scoping issues when multiple filters are combined with AndAlso. + var parameter = Expression.Parameter(typeof(Dictionary), "x"); + Expression? filterBody = null; + if (applicationId != null) { - filter = x => (string?)x[ApplicationIdField] == applicationId; + filterBody = RebindFilterBody(x => (string?)x[ApplicationIdField] == applicationId, parameter); } if (agentId != null) { - Expression, bool>> agentIdFilter = x => (string?)x[AgentIdField] == agentId; - filter = filter == null ? agentIdFilter : Expression.Lambda, bool>>( - Expression.AndAlso(filter.Body, agentIdFilter.Body), - filter.Parameters); + var body = RebindFilterBody(x => (string?)x[AgentIdField] == agentId, parameter); + filterBody = filterBody == null ? body : Expression.AndAlso(filterBody, body); } if (userId != null) { - Expression, bool>> userIdFilter = x => (string?)x[UserIdField] == userId; - filter = filter == null ? userIdFilter : Expression.Lambda, bool>>( - Expression.AndAlso(filter.Body, userIdFilter.Body), - filter.Parameters); + var body = RebindFilterBody(x => (string?)x[UserIdField] == userId, parameter); + filterBody = filterBody == null ? body : Expression.AndAlso(filterBody, body); } if (sessionId != null) { - Expression, bool>> sessionIdFilter = x => (string?)x[SessionIdField] == sessionId; - filter = filter == null ? sessionIdFilter : Expression.Lambda, bool>>( - Expression.AndAlso(filter.Body, sessionIdFilter.Body), - filter.Parameters); + var body = RebindFilterBody(x => (string?)x[SessionIdField] == sessionId, parameter); + filterBody = filterBody == null ? body : Expression.AndAlso(filterBody, body); } + Expression, bool>>? filter = filterBody != null + ? Expression.Lambda, bool>>(filterBody, parameter) + : null; + // Use search to find relevant messages var searchResults = collection.SearchAsync( queryText, @@ -467,6 +469,27 @@ public void Dispose() private string? SanitizeLogData(string? data) => this._enableSensitiveTelemetryData ? data : ""; + /// + /// Rebinds a filter expression's body to use the specified shared parameter, + /// replacing the original lambda parameter so that multiple filters can be safely + /// combined with . + /// + private static Expression RebindFilterBody( + Expression, bool>> filter, + ParameterExpression sharedParameter) + { + return new ParameterReplacer(filter.Parameters[0], sharedParameter).Visit(filter.Body); + } + + /// + /// An that replaces one with another. + /// + private sealed class ParameterReplacer(ParameterExpression original, ParameterExpression replacement) : ExpressionVisitor + { + protected override Expression VisitParameter(ParameterExpression node) + => node == original ? replacement : base.VisitParameter(node); + } + /// /// Represents the state of a stored in the . /// diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs index 5211fa0956..b89b86f4d0 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs @@ -454,6 +454,77 @@ public async Task InvokedAsync_CreatesFilter_WhenSearchScopeProvidedAsync() Times.Once); } + [Fact] + public async Task InvokedAsync_CombinedFilterCanBeCompiled_WhenMultipleScopeFiltersProvidedAsync() + { + // Arrange + // This test reproduces a bug where combining multiple scope filters + // (e.g. userId + sessionId) produces an expression tree with dangling + // ParameterExpression references that fails at compile time. + var providerOptions = new ChatHistoryMemoryProviderOptions + { + SearchTime = ChatHistoryMemoryProviderOptions.SearchBehavior.BeforeAIInvoke, + MaxResults = 2, + ContextPrompt = "Here is the relevant chat history:\n" + }; + + var searchScope = new ChatHistoryMemoryProviderScope + { + ApplicationId = "app1", + AgentId = "agent1", + SessionId = "session1", + UserId = "user1" + }; + + System.Linq.Expressions.Expression, bool>>? capturedFilter = null; + + this._vectorStoreCollectionMock + .Setup(c => c.SearchAsync( + It.IsAny(), + It.IsAny(), + It.IsAny>>(), + It.IsAny())) + .Callback((string query, int maxResults, VectorSearchOptions> options, CancellationToken ct) => + capturedFilter = options.Filter) + .Returns(ToAsyncEnumerableAsync(new List>>())); + + var provider = new ChatHistoryMemoryProvider( + this._vectorStoreMock.Object, + TestCollectionName, + 1, + _ => new ChatHistoryMemoryProvider.State(searchScope, searchScope), + options: providerOptions); + + var requestMsg = new ChatMessage(ChatRole.User, "requesting relevant history"); + var invokingContext = new AIContextProvider.InvokingContext(s_mockAgent, new TestAgentSession(), new AIContext { Messages = new List { requestMsg } }); + + // Act + await provider.InvokingAsync(invokingContext, CancellationToken.None); + + // Assert - The filter must be compilable and executable without expression tree scoping errors + Assert.NotNull(capturedFilter); + var compiledFilter = capturedFilter!.Compile(); + + var matchingRecord = new Dictionary + { + ["ApplicationId"] = "app1", + ["AgentId"] = "agent1", + ["SessionId"] = "session1", + ["UserId"] = "user1" + }; + + var nonMatchingRecord = new Dictionary + { + ["ApplicationId"] = "app1", + ["AgentId"] = "agent1", + ["SessionId"] = "other-session", + ["UserId"] = "user1" + }; + + Assert.True(compiledFilter(matchingRecord)); + Assert.False(compiledFilter(nonMatchingRecord)); + } + [Theory] [InlineData(false, false, 2)] [InlineData(true, false, 2)] From 2f2b0d21e6027abf18c10934bd5bdc8cd3546151 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Thu, 5 Mar 2026 17:19:12 +0000 Subject: [PATCH 2/3] Replace var with explicit types in filter building code and test 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> --- .../Memory/ChatHistoryMemoryProvider.cs | 8 ++++---- .../Memory/ChatHistoryMemoryProviderTests.cs | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/Memory/ChatHistoryMemoryProvider.cs b/dotnet/src/Microsoft.Agents.AI/Memory/ChatHistoryMemoryProvider.cs index 46a3ee3e61..0cc35fe85e 100644 --- a/dotnet/src/Microsoft.Agents.AI/Memory/ChatHistoryMemoryProvider.cs +++ b/dotnet/src/Microsoft.Agents.AI/Memory/ChatHistoryMemoryProvider.cs @@ -352,7 +352,7 @@ private async Task SearchTextAsync(string userQuestion, ChatHistoryMemor // Build a combined filter using a single shared parameter to avoid expression tree // scoping issues when multiple filters are combined with AndAlso. - var parameter = Expression.Parameter(typeof(Dictionary), "x"); + ParameterExpression parameter = Expression.Parameter(typeof(Dictionary), "x"); Expression? filterBody = null; if (applicationId != null) @@ -362,19 +362,19 @@ private async Task SearchTextAsync(string userQuestion, ChatHistoryMemor if (agentId != null) { - var body = RebindFilterBody(x => (string?)x[AgentIdField] == agentId, parameter); + Expression body = RebindFilterBody(x => (string?)x[AgentIdField] == agentId, parameter); filterBody = filterBody == null ? body : Expression.AndAlso(filterBody, body); } if (userId != null) { - var body = RebindFilterBody(x => (string?)x[UserIdField] == userId, parameter); + Expression body = RebindFilterBody(x => (string?)x[UserIdField] == userId, parameter); filterBody = filterBody == null ? body : Expression.AndAlso(filterBody, body); } if (sessionId != null) { - var body = RebindFilterBody(x => (string?)x[SessionIdField] == sessionId, parameter); + Expression body = RebindFilterBody(x => (string?)x[SessionIdField] == sessionId, parameter); filterBody = filterBody == null ? body : Expression.AndAlso(filterBody, body); } diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs index b89b86f4d0..46d29438cb 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs @@ -461,14 +461,14 @@ public async Task InvokedAsync_CombinedFilterCanBeCompiled_WhenMultipleScopeFilt // This test reproduces a bug where combining multiple scope filters // (e.g. userId + sessionId) produces an expression tree with dangling // ParameterExpression references that fails at compile time. - var providerOptions = new ChatHistoryMemoryProviderOptions + ChatHistoryMemoryProviderOptions providerOptions = new ChatHistoryMemoryProviderOptions { SearchTime = ChatHistoryMemoryProviderOptions.SearchBehavior.BeforeAIInvoke, MaxResults = 2, ContextPrompt = "Here is the relevant chat history:\n" }; - var searchScope = new ChatHistoryMemoryProviderScope + ChatHistoryMemoryProviderScope searchScope = new ChatHistoryMemoryProviderScope { ApplicationId = "app1", AgentId = "agent1", @@ -488,24 +488,24 @@ public async Task InvokedAsync_CombinedFilterCanBeCompiled_WhenMultipleScopeFilt capturedFilter = options.Filter) .Returns(ToAsyncEnumerableAsync(new List>>())); - var provider = new ChatHistoryMemoryProvider( + ChatHistoryMemoryProvider provider = new ChatHistoryMemoryProvider( this._vectorStoreMock.Object, TestCollectionName, 1, _ => new ChatHistoryMemoryProvider.State(searchScope, searchScope), options: providerOptions); - var requestMsg = new ChatMessage(ChatRole.User, "requesting relevant history"); - var invokingContext = new AIContextProvider.InvokingContext(s_mockAgent, new TestAgentSession(), new AIContext { Messages = new List { requestMsg } }); + ChatMessage requestMsg = new ChatMessage(ChatRole.User, "requesting relevant history"); + AIContextProvider.InvokingContext invokingContext = new AIContextProvider.InvokingContext(s_mockAgent, new TestAgentSession(), new AIContext { Messages = new List { requestMsg } }); // Act await provider.InvokingAsync(invokingContext, CancellationToken.None); // Assert - The filter must be compilable and executable without expression tree scoping errors Assert.NotNull(capturedFilter); - var compiledFilter = capturedFilter!.Compile(); + Func, bool> compiledFilter = capturedFilter!.Compile(); - var matchingRecord = new Dictionary + Dictionary matchingRecord = new Dictionary { ["ApplicationId"] = "app1", ["AgentId"] = "agent1", @@ -513,7 +513,7 @@ public async Task InvokedAsync_CombinedFilterCanBeCompiled_WhenMultipleScopeFilt ["UserId"] = "user1" }; - var nonMatchingRecord = new Dictionary + Dictionary nonMatchingRecord = new Dictionary { ["ApplicationId"] = "app1", ["AgentId"] = "agent1", From 665b812827a65e3c70266d1a4229d9ccf93f3e1b Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Thu, 5 Mar 2026 17:30:04 +0000 Subject: [PATCH 3/3] Fix style issues --- .../Memory/ChatHistoryMemoryProviderTests.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs index 46d29438cb..35c7f780b4 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/Memory/ChatHistoryMemoryProviderTests.cs @@ -461,14 +461,14 @@ public async Task InvokedAsync_CombinedFilterCanBeCompiled_WhenMultipleScopeFilt // This test reproduces a bug where combining multiple scope filters // (e.g. userId + sessionId) produces an expression tree with dangling // ParameterExpression references that fails at compile time. - ChatHistoryMemoryProviderOptions providerOptions = new ChatHistoryMemoryProviderOptions + ChatHistoryMemoryProviderOptions providerOptions = new() { SearchTime = ChatHistoryMemoryProviderOptions.SearchBehavior.BeforeAIInvoke, MaxResults = 2, ContextPrompt = "Here is the relevant chat history:\n" }; - ChatHistoryMemoryProviderScope searchScope = new ChatHistoryMemoryProviderScope + ChatHistoryMemoryProviderScope searchScope = new() { ApplicationId = "app1", AgentId = "agent1", @@ -488,15 +488,15 @@ public async Task InvokedAsync_CombinedFilterCanBeCompiled_WhenMultipleScopeFilt capturedFilter = options.Filter) .Returns(ToAsyncEnumerableAsync(new List>>())); - ChatHistoryMemoryProvider provider = new ChatHistoryMemoryProvider( + ChatHistoryMemoryProvider provider = new( this._vectorStoreMock.Object, TestCollectionName, 1, _ => new ChatHistoryMemoryProvider.State(searchScope, searchScope), options: providerOptions); - ChatMessage requestMsg = new ChatMessage(ChatRole.User, "requesting relevant history"); - AIContextProvider.InvokingContext invokingContext = new AIContextProvider.InvokingContext(s_mockAgent, new TestAgentSession(), new AIContext { Messages = new List { requestMsg } }); + ChatMessage requestMsg = new(ChatRole.User, "requesting relevant history"); + AIContextProvider.InvokingContext invokingContext = new(s_mockAgent, new TestAgentSession(), new AIContext { Messages = new List { requestMsg } }); // Act await provider.InvokingAsync(invokingContext, CancellationToken.None); @@ -505,7 +505,7 @@ public async Task InvokedAsync_CombinedFilterCanBeCompiled_WhenMultipleScopeFilt Assert.NotNull(capturedFilter); Func, bool> compiledFilter = capturedFilter!.Compile(); - Dictionary matchingRecord = new Dictionary + Dictionary matchingRecord = new() { ["ApplicationId"] = "app1", ["AgentId"] = "agent1", @@ -513,7 +513,7 @@ public async Task InvokedAsync_CombinedFilterCanBeCompiled_WhenMultipleScopeFilt ["UserId"] = "user1" }; - Dictionary nonMatchingRecord = new Dictionary + Dictionary nonMatchingRecord = new() { ["ApplicationId"] = "app1", ["AgentId"] = "agent1",