Skip to content

Commit 78d5aed

Browse files
committed
fix: skip local history after service conversation id
1 parent 41a9c54 commit 78d5aed

2 files changed

Lines changed: 17 additions & 15 deletions

File tree

dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ internal async Task NotifyProvidersOfNewMessagesAsync(
478478
ChatOptions? chatOptions,
479479
CancellationToken cancellationToken)
480480
{
481-
ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(chatOptions);
481+
ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(session, chatOptions);
482482

483483
if (chatHistoryProvider is not null)
484484
{
@@ -510,7 +510,7 @@ internal async Task NotifyProvidersOfFailureAsync(
510510
ChatOptions? chatOptions,
511511
CancellationToken cancellationToken)
512512
{
513-
ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(chatOptions);
513+
ChatHistoryProvider? chatHistoryProvider = this.ResolveChatHistoryProvider(session, chatOptions);
514514

515515
if (chatHistoryProvider is not null)
516516
{
@@ -980,19 +980,19 @@ private void WarnOnMissingPerServiceCallChatHistoryPersistingChatClient()
980980
}
981981
}
982982

983-
private ChatHistoryProvider? ResolveChatHistoryProvider(ChatOptions? chatOptions)
983+
private ChatHistoryProvider? ResolveChatHistoryProvider(ChatClientAgentSession session, ChatOptions? chatOptions)
984984
{
985-
ChatHistoryProvider? provider =
986-
chatOptions?.ConversationId is null || IsAGUIProviderName(this._agentMetadata.ProviderName)
987-
? this.ChatHistoryProvider
988-
: null;
985+
bool aguiProvider = IsAGUIProviderName(this._agentMetadata.ProviderName);
986+
bool serviceStoresHistory = !this.RequiresPerServiceCallChatHistoryPersistence
987+
&& !aguiProvider
988+
&& (!string.IsNullOrWhiteSpace(session.ConversationId)
989+
|| !string.IsNullOrWhiteSpace(chatOptions?.ConversationId));
990+
ChatHistoryProvider? provider = serviceStoresHistory ? null : this.ChatHistoryProvider;
989991

990992
// If someone provided an override ChatHistoryProvider via AdditionalProperties, we should use that instead.
991993
if (chatOptions?.AdditionalProperties?.TryGetValue(out ChatHistoryProvider? overrideProvider) is true)
992994
{
993-
if (!IsAGUIProviderName(this._agentMetadata.ProviderName) &&
994-
this._agentOptions?.ThrowOnChatHistoryProviderConflict is true &&
995-
string.IsNullOrWhiteSpace(chatOptions?.ConversationId) is false)
995+
if (this._agentOptions?.ThrowOnChatHistoryProviderConflict is true && serviceStoresHistory)
996996
{
997997
throw new InvalidOperationException(
998998
$"Only {nameof(ChatClientAgentSession.ConversationId)} or {nameof(this.ChatHistoryProvider)} may be used, but not both. The current {nameof(ChatClientAgentSession)} has a {nameof(ChatClientAgentSession.ConversationId)} indicating server-side chat history management, but an override {nameof(this.ChatHistoryProvider)} was provided via {nameof(AgentRunOptions.AdditionalProperties)}.");
@@ -1030,7 +1030,7 @@ internal async Task<IEnumerable<ChatMessage>> LoadChatHistoryAsync(
10301030
ChatOptions? chatOptions,
10311031
CancellationToken cancellationToken)
10321032
{
1033-
var chatHistoryProvider = this.ResolveChatHistoryProvider(chatOptions);
1033+
var chatHistoryProvider = this.ResolveChatHistoryProvider(session, chatOptions);
10341034
if (chatHistoryProvider is null)
10351035
{
10361036
return messages;

dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatHistoryManagementTests.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -385,11 +385,11 @@ public async Task RunAsync_Throws_WhenThrowEnabledRegardlessOfClearSettingAsync(
385385
}
386386

387387
/// <summary>
388-
/// Verify that RunAsync does not throw when no ChatHistoryProvider is configured on options,
389-
/// even if the service returns a conversation id (default InMemoryChatHistoryProvider is used but not from options).
388+
/// Verify that RunAsync does not persist to the default InMemoryChatHistoryProvider when
389+
/// the service returns a conversation id.
390390
/// </summary>
391391
[Fact]
392-
public async Task RunAsync_DoesNotThrow_WhenNoChatHistoryProviderInOptionsAndConversationIdReturnedAsync()
392+
public async Task RunAsync_DoesNotUseDefaultInMemoryChatHistoryProvider_WhenConversationIdReturnedAsync()
393393
{
394394
// Arrange
395395
Mock<IChatClient> mockService = new();
@@ -407,8 +407,10 @@ public async Task RunAsync_DoesNotThrow_WhenNoChatHistoryProviderInOptionsAndCon
407407
ChatClientAgentSession? session = await agent.CreateSessionAsync() as ChatClientAgentSession;
408408
await agent.RunAsync([new(ChatRole.User, "test")], session);
409409

410-
// Assert - no exception, session gets the conversation id
410+
// Assert
411411
Assert.Equal("ConvId", session!.ConversationId);
412+
var inMemoryProvider = Assert.IsType<InMemoryChatHistoryProvider>(agent.ChatHistoryProvider);
413+
Assert.Empty(inMemoryProvider.GetMessages(session));
412414
}
413415

414416
#endregion

0 commit comments

Comments
 (0)