From dbac4eab56559294922746d031908e708effbe4a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 19:26:08 +0000 Subject: [PATCH] Add hosting executor invariants and required test coverage Agent-Logs-Url: https://github.com/microsoft/agent-framework/sessions/9241f85a-8657-4979-8d64-0736dc6e7ebb Co-authored-by: lokitoth <6936551+lokitoth@users.noreply.github.com> --- docs/MessageMerger-EdgeCases-TestCoverage.md | 177 ++++++++++++++++++- 1 file changed, 174 insertions(+), 3 deletions(-) diff --git a/docs/MessageMerger-EdgeCases-TestCoverage.md b/docs/MessageMerger-EdgeCases-TestCoverage.md index eda1cfa21c..4456574f8e 100644 --- a/docs/MessageMerger-EdgeCases-TestCoverage.md +++ b/docs/MessageMerger-EdgeCases-TestCoverage.md @@ -1,10 +1,54 @@ -# MessageMerger Edge Cases and Test Coverage +# Agent Hosting Executor Invariants and Test Coverage -This document identifies edge cases in `MessageMerger` and recommends additional test coverage. +This document defines the **invariants** that Agent hosting executors must maintain, identifies edge cases in `MessageMerger`, and recommends additional test coverage. + +## Hosting Executor Invariants + +Agent hosting executors (e.g., `HostedAgentResponseExecutor`, `AIAgentResponseExecutor`, `WorkflowOrchestrator`) **must** maintain the following invariants: + +### Invariant 1: Single ResponseId per Turn + +**For a given "turn", only one ResponseId is emitted.** + +- If the underlying Agent does not provide a `ResponseId`, the hosting executor **must** assign one. +- All `AgentResponseUpdate` items emitted during a single turn must share the same `ResponseId`. +- This ensures clients can reliably group streaming updates into a single logical response. + +**Relevant Code:** +- `IdGenerator` creates a `ResponseId` in `InMemoryResponsesService.InitializeResponse()` +- `AgentInvocationContext.ResponseId` provides the single ResponseId for the turn +- `ToStreamingResponseAsync()` uses `context.ResponseId` for the `Response` object + +### Invariant 2: Output Order Preservation for Untimestamped Messages + +**For a given turn, all messages without `CreatedAt` must be merged in output (arrival) order.** + +- When `AgentResponseUpdate` items lack a `CreatedAt` timestamp, their relative order must be preserved based on arrival sequence. +- The current `MessageMerger.CompareByDateTimeOffset()` falls back to index comparison when timestamps are missing or equal, which satisfies this invariant. +- Edge case: Mixed timestamped/untimestamped messages require careful handling to avoid non-transitive sort issues. + +**Relevant Code:** +- `MessageMerger.CompareByDateTimeOffset()` uses index as tiebreaker +- `ResponseMergeState.AddUpdate()` preserves insertion order in lists + +### Invariant 3: Agent Message Grouping (No Interleaving) + +**For multi-agent systems speaking concurrently, a given agent's response messages must all be grouped together (not interleaved with other agents' messages).** + +- When multiple agents emit responses concurrently (e.g., in handoff scenarios), messages from each agent must remain contiguous. +- The merged output should group messages by agent, not interleave them arbitrarily. +- This is critical for maintaining conversation coherence in multi-agent orchestration. + +**Relevant Code:** +- `MessageMerger` groups by `ResponseId`, then by `MessageId` +- Multi-agent scenarios may emit different `ResponseId` values per agent +- `ComputeMerged()` processes responses in dictionary iteration order (first-seen order) + +--- ## Overview -`MessageMerger` (`dotnet/src/Microsoft.Agents.AI.Hosting.AzureFunctions/MessageMerger.cs`) handles merging streaming `AgentResponseUpdate` messages into a final `AgentResponse`. It groups updates by `ResponseId`, then by `MessageId`, sorts messages by `CreatedAt` timestamp, and produces a consolidated response. +`MessageMerger` (`dotnet/src/Microsoft.Agents.AI.Workflows/MessageMerger.cs`) handles merging streaming `AgentResponseUpdate` messages into a final `AgentResponse`. It groups updates by `ResponseId`, then by `MessageId`, sorts messages by `CreatedAt` timestamp, and produces a consolidated response. --- @@ -211,3 +255,130 @@ if (update.CreatedAt.HasValue) createdTimes.Add(update.CreatedAt.Value); | Low | 1 | MessageId ordering | **Recommendation:** Add at minimum the 2 high-priority tests and 2 medium-priority tests (interleaved ResponseIds, dangling metadata) to ensure `MessageMerger` behaves correctly in production streaming scenarios. + +--- + +## Required Tests for Hosting Executor Invariants + +### Invariant 1: Single ResponseId per Turn + +```csharp +[Fact] +public async Task HostingExecutor_AssignsSingleResponseId_WhenAgentProvidesNone() +{ + // Arrange: Agent that emits updates without ResponseId + // Act: Execute through hosting executor + // Assert: All emitted updates have the same ResponseId assigned by executor +} + +[Fact] +public async Task HostingExecutor_PreservesAgentResponseId_WhenProvided() +{ + // Arrange: Agent that emits updates with a consistent ResponseId + // Act: Execute through hosting executor + // Assert: ResponseId from agent is preserved (or overridden if executor policy requires) +} + +[Fact] +public async Task HostingExecutor_RejectsMultipleResponseIds_InSingleTurn() +{ + // Arrange: Agent that incorrectly emits updates with different ResponseIds + // Act: Execute through hosting executor + // Assert: Executor normalizes to single ResponseId or throws validation error +} +``` + +### Invariant 2: Output Order Preservation + +```csharp +[Fact] +public void MessageMerger_PreservesInsertionOrder_WhenNoTimestamps() +{ + // Arrange: Multiple updates without CreatedAt, in specific order A, B, C + // Act: Merge + // Assert: Output order is A, B, C +} + +[Fact] +public void MessageMerger_PreservesInsertionOrder_WhenMixedTimestamps() +{ + // Arrange: Updates where some have CreatedAt and some don't + // Act: Merge + // Assert: Untimestamped updates maintain relative order among themselves +} + +[Fact] +public void MessageMerger_StableSort_WithThreeOrMoreMixedTimestampMessages() +{ + // Arrange: 3+ messages with mixed null/non-null CreatedAt values + // Act: Merge multiple times + // Assert: Result is deterministic and consistent across runs +} +``` + +### Invariant 3: Agent Message Grouping + +```csharp +[Fact] +public void MessageMerger_GroupsMessagesByAgent_InMultiAgentScenario() +{ + // Arrange: Interleaved updates from Agent1 and Agent2 + // A1-msg1, A2-msg1, A1-msg2, A2-msg2 + // Act: Merge + // Assert: Output groups Agent1 messages together, Agent2 messages together + // Either [A1-msg1, A1-msg2, A2-msg1, A2-msg2] or [A2-msg1, A2-msg2, A1-msg1, A1-msg2] +} + +[Fact] +public void MessageMerger_MaintainsAgentGrouping_WithDifferentResponseIds() +{ + // Arrange: Agent1 uses ResponseId=R1, Agent2 uses ResponseId=R2 + // Act: Merge with primaryResponseId + // Assert: Messages from each agent are contiguous, not interleaved +} + +[Fact] +public async Task HostingExecutor_HandoffScenario_MaintainsMessageCoherence() +{ + // Arrange: Agent1 hands off to Agent2 mid-conversation + // Act: Execute through hosting executor + // Assert: Agent1's messages appear before Agent2's messages (no interleaving) +} + +[Fact] +public void MessageMerger_PreservesAgentMessageOrder_WithConcurrentAgents() +{ + // Arrange: Two agents emitting messages concurrently with timestamps + // A1 at T1, A2 at T2, A1 at T3, A2 at T4 (where T1 < T2 < T3 < T4) + // Act: Merge + // Assert: Agent grouping is maintained, not sorted purely by timestamp +} +``` + +--- + +## Implementation Recommendations + +### For Invariant 1 (Single ResponseId) + +1. **Hosting Executor Layer**: The `AgentInvocationContext` already generates a `ResponseId`. Ensure this is consistently applied to all outgoing `AgentResponseUpdate` items. + +2. **Validation**: Consider adding runtime validation that throws if an agent emits conflicting `ResponseId` values during a single turn. + +3. **Code Location**: `ToStreamingResponseAsync()` in `AgentResponseUpdateExtensions.cs` should ensure the `context.ResponseId` is used consistently. + +### For Invariant 2 (Output Order) + +1. **Index Tracking**: Store original insertion index alongside each update to ensure stable sorting. + +2. **Code Location**: `MessageMerger.ResponseMergeState.AddUpdate()` should track insertion order explicitly. + +3. **Sort Stability**: Replace the current `OrderBy` with a stable sort that uses insertion index as the final tiebreaker. + +### For Invariant 3 (Agent Grouping) + +1. **Agent-First Grouping**: Modify `ComputeMerged()` to group by `AgentId` before processing. + +2. **Deterministic Order**: Define explicit ordering rules (e.g., first-seen agent order, or alphabetical by AgentId). + +3. **Code Location**: `MessageMerger.ComputeMerged()` needs logic to ensure agent messages are contiguous.