mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
fix: JSON Serialization issue with MultiPartyConversation (#5653)
When MultiPartyConversation gets saved during checkpointing, the data for the chat history is not persisted, resulting in failures to deserialize after. The fix is to make the history visible to the source generated serialization code.
This commit is contained in:
committed by
GitHub
Unverified
parent
e9a6d43237
commit
9f3f7fd03b
@@ -36,7 +36,7 @@ internal sealed class HandoffEndExecutor(bool returnToPrevious) : Executor(Execu
|
||||
sharedState.PreviousAgentId = handoff.PreviousAgentId;
|
||||
}
|
||||
|
||||
await context.YieldOutputAsync(sharedState.Conversation.CloneAllMessages(), cancellationToken).ConfigureAwait(false);
|
||||
await context.YieldOutputAsync(sharedState.Conversation.CloneHistory(), cancellationToken).ConfigureAwait(false);
|
||||
|
||||
return sharedState;
|
||||
}, context, cancellationToken).ConfigureAwait(false);
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// Copyright (c) Microsoft. All rights reserved.
|
||||
|
||||
using System.Collections.Generic;
|
||||
using System.Text.Json.Serialization;
|
||||
using System.Threading;
|
||||
using System.Threading.Tasks;
|
||||
using Microsoft.Extensions.AI;
|
||||
@@ -23,7 +24,20 @@ internal static class HandoffConstants
|
||||
|
||||
internal sealed class HandoffSharedState
|
||||
{
|
||||
public MultiPartyConversation Conversation { get; } = new();
|
||||
[JsonConstructor]
|
||||
internal HandoffSharedState(MultiPartyConversation conversation, string? previousAgentId)
|
||||
{
|
||||
this.Conversation = conversation;
|
||||
this.PreviousAgentId = previousAgentId;
|
||||
}
|
||||
|
||||
public HandoffSharedState()
|
||||
{
|
||||
this.Conversation = new([]);
|
||||
}
|
||||
|
||||
[JsonInclude]
|
||||
public MultiPartyConversation Conversation { get; internal set; }
|
||||
|
||||
public string? PreviousAgentId { get; set; }
|
||||
}
|
||||
|
||||
@@ -3,20 +3,33 @@
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Linq;
|
||||
using System.Text.Json.Serialization;
|
||||
using Microsoft.Extensions.AI;
|
||||
|
||||
namespace Microsoft.Agents.AI.Workflows.Specialized;
|
||||
|
||||
internal sealed class MultiPartyConversation
|
||||
{
|
||||
private readonly List<ChatMessage> _history = [];
|
||||
private readonly object _mutex = new();
|
||||
|
||||
public List<ChatMessage> CloneAllMessages()
|
||||
[JsonConstructor]
|
||||
internal MultiPartyConversation(List<ChatMessage> history)
|
||||
{
|
||||
this.History = history ?? [];
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// In order to support JSON serializaiton, this property must be internally visible. However, it should not be used
|
||||
/// in concurrent contexts without proper locking, as the underlying list is not thread safe.
|
||||
/// </summary>
|
||||
[JsonInclude]
|
||||
internal List<ChatMessage> History { get; }
|
||||
|
||||
public List<ChatMessage> CloneHistory()
|
||||
{
|
||||
lock (this._mutex)
|
||||
{
|
||||
return this._history.ToList();
|
||||
return this.History.ToList();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,23 +37,24 @@ internal sealed class MultiPartyConversation
|
||||
{
|
||||
lock (this._mutex)
|
||||
{
|
||||
int count = this._history.Count - bookmark;
|
||||
int count = this.History.Count - bookmark;
|
||||
if (count < 0)
|
||||
{
|
||||
throw new InvalidOperationException($"Bookmark value too large: {bookmark} vs count={count}");
|
||||
}
|
||||
|
||||
return (this._history.Skip(bookmark).ToArray(), this.CurrentBookmark);
|
||||
return (this.History.Skip(bookmark).ToArray(), this.CurrentBookmark);
|
||||
}
|
||||
}
|
||||
|
||||
private int CurrentBookmark => this._history.Count;
|
||||
[JsonIgnore]
|
||||
private int CurrentBookmark => this.History.Count;
|
||||
|
||||
public int AddMessages(IEnumerable<ChatMessage> messages)
|
||||
{
|
||||
lock (this._mutex)
|
||||
{
|
||||
this._history.AddRange(messages);
|
||||
this.History.AddRange(messages);
|
||||
return this.CurrentBookmark;
|
||||
}
|
||||
}
|
||||
@@ -49,7 +63,7 @@ internal sealed class MultiPartyConversation
|
||||
{
|
||||
lock (this._mutex)
|
||||
{
|
||||
this._history.Add(message);
|
||||
this.History.Add(message);
|
||||
return this.CurrentBookmark;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -95,6 +95,8 @@ internal static partial class WorkflowsJsonUtilities
|
||||
|
||||
// Built-in Executor State Types
|
||||
[JsonSerializable(typeof(AIAgentHostState))]
|
||||
[JsonSerializable(typeof(HandoffSharedState))]
|
||||
[JsonSerializable(typeof(HandoffAgentHostState))]
|
||||
|
||||
// Event Types
|
||||
//[JsonSerializable(typeof(WorkflowEvent))]
|
||||
|
||||
@@ -12,6 +12,7 @@ using System.Threading.Tasks;
|
||||
using FluentAssertions;
|
||||
using Microsoft.Agents.AI.Workflows.Checkpointing;
|
||||
using Microsoft.Agents.AI.Workflows.Execution;
|
||||
using Microsoft.Agents.AI.Workflows.Specialized;
|
||||
using Microsoft.Extensions.AI;
|
||||
|
||||
namespace Microsoft.Agents.AI.Workflows.UnitTests;
|
||||
@@ -722,6 +723,71 @@ public class JsonSerializationTests
|
||||
result.PendingRequests.Should().BeNull();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Test_HandoffSharedState_JsonRoundtrip_Empty()
|
||||
{
|
||||
// Arrange
|
||||
HandoffSharedState prototype = new();
|
||||
|
||||
// Act
|
||||
HandoffSharedState result = RunJsonRoundtrip(prototype);
|
||||
|
||||
// Assert
|
||||
result.PreviousAgentId.Should().Be(prototype.PreviousAgentId);
|
||||
result.Conversation.CloneHistory().Should().BeEquivalentTo(prototype.Conversation.CloneHistory());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Test_HandoffSharedState_JsonRoundtrip_WithConversation()
|
||||
{
|
||||
// Arrange
|
||||
HandoffSharedState prototype = new();
|
||||
prototype.Conversation.AddMessage(TestUserMessage);
|
||||
prototype.Conversation.AddMessage(new(ChatRole.Assistant, "Hi"));
|
||||
prototype.PreviousAgentId = "agent-123";
|
||||
|
||||
// Act
|
||||
HandoffSharedState result = RunJsonRoundtrip(prototype);
|
||||
|
||||
// Assert
|
||||
result.PreviousAgentId.Should().Be(prototype.PreviousAgentId);
|
||||
result.Conversation.CloneHistory().Should().BeEquivalentTo(prototype.Conversation.CloneHistory());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Test_HandoffAgentHostState_JsonRoundtrip_TakingTurn()
|
||||
{
|
||||
// Arrange
|
||||
HandoffState handoffState = new(new TurnToken(emitEvents: true),
|
||||
nameof(HandoffState.RequestedHandoffTargetAgentId),
|
||||
nameof(handoffState.PreviousAgentId));
|
||||
|
||||
HandoffAgentHostState prototype = new(handoffState, 42);
|
||||
|
||||
// Act
|
||||
HandoffAgentHostState result = RunJsonRoundtrip(prototype);
|
||||
|
||||
// Assert
|
||||
result.IncomingState.Should().BeEquivalentTo(prototype.IncomingState);
|
||||
result.ConversationBookmark.Should().Be(prototype.ConversationBookmark);
|
||||
result.IsTakingTurn.Should().Be(prototype.IsTakingTurn);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Test_HandoffAgentHostState_JsonRoundtrip_NotTakingTurn()
|
||||
{
|
||||
// Arrange
|
||||
HandoffAgentHostState prototype = new(null, 42);
|
||||
|
||||
// Act
|
||||
HandoffAgentHostState result = RunJsonRoundtrip(prototype);
|
||||
|
||||
// Assert
|
||||
result.IncomingState.Should().BeEquivalentTo(prototype.IncomingState);
|
||||
result.ConversationBookmark.Should().Be(prototype.ConversationBookmark);
|
||||
result.IsTakingTurn.Should().Be(prototype.IsTakingTurn);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verifies that the default behavior (without AllowOutOfOrderMetadataProperties) fails
|
||||
/// when $type metadata is not the first property, demonstrating the PostgreSQL jsonb issue.
|
||||
|
||||
Reference in New Issue
Block a user