diff --git a/.autover/changes/fix-memory-history-ordering.json b/.autover/changes/fix-memory-history-ordering.json new file mode 100644 index 0000000..5dcc8ef --- /dev/null +++ b/.autover/changes/fix-memory-history-ordering.json @@ -0,0 +1,18 @@ +{ + "Projects": [ + { + "Name": "AWS.AgentCore.Hosting", + "Type": "Patch", + "ChangelogMessages": [ + "Fixed AgentCoreMemoryProvider replaying conversation history in reverse (newest-first) order. The AgentCore Memory ListEvents API returns events newest-first; the provider now sorts them ascending by EventTimestamp so chat history is presented oldest-first and multi-turn follow-ups bind to the most recent turn." + ] + }, + { + "Name": "AWS.AgentCore.Testing", + "Type": "Patch", + "ChangelogMessages": [ + "Updated the in-memory Memory emulator (InMemoryEventStore.ListEvents) to return events newest-first, matching the ordering of the real Amazon Bedrock AgentCore Memory ListEvents API so local tests exercise the same ordering behavior as production." + ] + } + ] +} diff --git a/src/AWS.AgentCore.Hosting/AgentCoreMemoryProvider.cs b/src/AWS.AgentCore.Hosting/AgentCoreMemoryProvider.cs index 90a10fa..37e53df 100644 --- a/src/AWS.AgentCore.Hosting/AgentCoreMemoryProvider.cs +++ b/src/AWS.AgentCore.Hosting/AgentCoreMemoryProvider.cs @@ -138,7 +138,7 @@ private async Task> LoadHistoryAsync( AgentCoreMetrics.RecordMemoryLoad(); - var messages = new List(); + var events = new List(); var request = new ListEventsRequest { @@ -153,16 +153,26 @@ private async Task> LoadHistoryAsync( { await foreach (var evt in memoryClient.Paginators.ListEvents(request).Events.WithCancellation(cancellationToken)) { - if (TryConvertEventToChatMessage(evt, out var chatMessage)) - { - messages.Add(chatMessage); - } + events.Add(evt); } } - catch (Exception ex) when (messages.Count > 0) + catch (Exception ex) when (events.Count > 0) { - // Partial pagination failure — return what we have - logger.LogWarning(ex, "Error during pagination. Returning {Count} messages loaded so far.", messages.Count); + // Partial pagination failure — proceed with what we have. + logger.LogWarning(ex, "Error during pagination. Proceeding with {Count} events loaded so far.", events.Count); + } + + // ListEvents returns events newest-first, but chat history must be presented + // to the model oldest-first (chronological order) so multi-turn follow-ups + // (e.g. "prices please") bind to the most recent turn rather than a stale one. + // OrderBy is a stable sort, so events sharing a timestamp keep their relative order. + var messages = new List(events.Count); + foreach (var evt in events.OrderBy(e => e.EventTimestamp)) + { + if (TryConvertEventToChatMessage(evt, out var chatMessage)) + { + messages.Add(chatMessage); + } } return messages; diff --git a/src/AWS.AgentCore.Testing/Emulators/Memory/InMemoryEventStore.cs b/src/AWS.AgentCore.Testing/Emulators/Memory/InMemoryEventStore.cs index 75ba86d..4ca1581 100644 --- a/src/AWS.AgentCore.Testing/Emulators/Memory/InMemoryEventStore.cs +++ b/src/AWS.AgentCore.Testing/Emulators/Memory/InMemoryEventStore.cs @@ -48,7 +48,9 @@ public CreateEventApiResponse CreateEvent(string memoryId, CreateEventApiRequest /// /// Lists events filtered by memoryId/actorId/sessionId with pagination and optional payload inclusion. - /// Returns events in chronological order. + /// Returns events newest-first (most recent EventTimestamp first), matching the + /// ordering of the real Amazon Bedrock AgentCore Memory ListEvents API. Consumers + /// that need chronological order must sort ascending themselves. /// /// Thrown when the nextToken is malformed or not a valid pagination token. public ListEventsApiResponse ListEvents( @@ -67,7 +69,8 @@ public ListEventsApiResponse ListEvents( List snapshot; lock (events) { - snapshot = events.OrderBy(e => e.EventTimestamp).ToList(); + // Newest-first, matching the real AgentCore Memory ListEvents API. + snapshot = events.OrderByDescending(e => e.EventTimestamp).ToList(); } var page = snapshot diff --git a/test/AWS.AgentCore.Hosting.UnitTests/AgentCoreMemoryProviderPropertyTests.cs b/test/AWS.AgentCore.Hosting.UnitTests/AgentCoreMemoryProviderPropertyTests.cs index 9f6b397..93eafe9 100644 --- a/test/AWS.AgentCore.Hosting.UnitTests/AgentCoreMemoryProviderPropertyTests.cs +++ b/test/AWS.AgentCore.Hosting.UnitTests/AgentCoreMemoryProviderPropertyTests.cs @@ -221,6 +221,69 @@ public async Task Pagination_FetchesAllEventsInOrder(PositiveInt eventCountWrapp } } + // ────────────────────────────────────────────────────────────────── + // Regression: History is returned oldest-first (chronological order) + // The real AgentCore Memory ListEvents API returns events newest-first. + // The provider must sort ascending by EventTimestamp so multi-turn + // follow-ups bind to the most recent turn, not a stale one. + // ────────────────────────────────────────────────────────────────── + + [Fact] + public async Task ProvideChatHistory_WhenServiceReturnsNewestFirst_ReturnsOldestFirst() + { + var baseTime = new DateTime(2024, 1, 1, 0, 0, 0, DateTimeKind.Utc); + + // Chronological conversation: index i is older than index i+1. + var chronological = new[] + { + (Role.USER, "top 3 dinners for a rainy Thursday?", baseTime.AddMinutes(0)), + (Role.ASSISTANT, "here are 3 dinners", baseTime.AddMinutes(1)), + (Role.USER, "what candy for movie night?", baseTime.AddMinutes(2)), + (Role.ASSISTANT, "here are 3 candies", baseTime.AddMinutes(3)), + }; + + // The service returns events newest-first — reverse the chronological order. + var newestFirst = chronological.Reverse().Select(t => new Event + { + EventTimestamp = t.Item3, + Payload = + [ + new PayloadType + { + Conversational = new Conversational + { + Role = t.Item1, + Content = new Content { Text = t.Item2 } + } + } + ] + }).ToList(); + + var mockPaginator = new Mock(); + mockPaginator.Setup(p => p.Events).Returns(new TestPaginatedEnumerable(newestFirst)); + + var mockPaginatorFactory = new Mock(); + mockPaginatorFactory.Setup(f => f.ListEvents(It.IsAny())).Returns(mockPaginator.Object); + + var mockClient = new Mock(); + mockClient.Setup(c => c.Paginators).Returns(mockPaginatorFactory.Object); + + var options = new AgentCoreOptions { MemoryId = "test-memory" }; + var provider = new AgentCoreMemoryProvider(options, NullLogger.Instance, mockClient.Object); + + var session = CreateSessionWithRuntimeContext("test-session"); + var context = CreateInvokingContext(provider, session); + + var messages = (await InvokeProvideChatHistoryAsync(provider, context)).ToList(); + + // History must be oldest-first so the latest turn sits closest to the new prompt. + Assert.Equal(chronological.Length, messages.Count); + for (int i = 0; i < chronological.Length; i++) + { + Assert.Equal(chronological[i].Item2, messages[i].Text); + } + } + // ────────────────────────────────────────────────────────────────── // Property 4: Errors Never Propagate to Caller // For any exception thrown by the Memory client during ListEvents or diff --git a/test/AWS.AgentCore.Testing.UnitTests/InMemoryEventStorePropertyTests.cs b/test/AWS.AgentCore.Testing.UnitTests/InMemoryEventStorePropertyTests.cs index b43142e..83064b5 100644 --- a/test/AWS.AgentCore.Testing.UnitTests/InMemoryEventStorePropertyTests.cs +++ b/test/AWS.AgentCore.Testing.UnitTests/InMemoryEventStorePropertyTests.cs @@ -94,15 +94,16 @@ public bool RoundTrip_PreservesRoleTextAndTimestamp( } // ────────────────────────────────────────────────────────────────── - // Property 2: Memory Store Chronological Ordering + // Property 2: Memory Store Newest-First Ordering // For any set of N events stored with distinct timestamps (in arbitrary // insertion order) for the same MemoryId/SessionId/ActorId, ListEvents - // should return all N events sorted by EventTimestamp in ascending order. + // should return all N events sorted by EventTimestamp in descending order + // (newest-first), matching the real AgentCore Memory ListEvents API. // **Validates: Requirements 3.3** // ────────────────────────────────────────────────────────────────── [Property(MaxTest = 20)] - public bool ChronologicalOrdering_ListEventsReturnsSortedByTimestamp(PositiveInt countWrapper) + public bool NewestFirstOrdering_ListEventsReturnsSortedByTimestampDescending(PositiveInt countWrapper) { // Cap N to a reasonable size for test performance var n = Math.Min(countWrapper.Get, 50); @@ -153,10 +154,10 @@ public bool ChronologicalOrdering_ListEventsReturnsSortedByTimestamp(PositiveInt if (listResponse.Events.Count != n) return false; - // Verify events are sorted by EventTimestamp ascending + // Verify events are sorted by EventTimestamp descending (newest-first) for (int i = 0; i < listResponse.Events.Count - 1; i++) { - if (listResponse.Events[i].EventTimestamp >= listResponse.Events[i + 1].EventTimestamp) + if (listResponse.Events[i].EventTimestamp <= listResponse.Events[i + 1].EventTimestamp) return false; } @@ -168,7 +169,7 @@ public bool ChronologicalOrdering_ListEventsReturnsSortedByTimestamp(PositiveInt // For any set of N events stored for the same MemoryId/SessionId/ActorId // where N exceeds the page size, iterating through all pages using // NextToken should yield exactly N events with no duplicates and no gaps, - // in chronological order. + // in newest-first order. // **Validates: Requirements 3.7** // ────────────────────────────────────────────────────────────────── @@ -238,10 +239,10 @@ public bool PaginationCompleteness_AllEventsReturnedWithNoDuplicatesNoGaps(Posit if (distinctIds != n) return false; - // Verify chronological order (no gaps — events are sorted ascending by timestamp) + // Verify newest-first order (no gaps — events are sorted descending by timestamp) for (int i = 0; i < allEvents.Count - 1; i++) { - if (allEvents[i].EventTimestamp >= allEvents[i + 1].EventTimestamp) + if (allEvents[i].EventTimestamp <= allEvents[i + 1].EventTimestamp) return false; } @@ -439,7 +440,9 @@ public bool IncludePayloads_ControlsResponseContent(PositiveInt countWrapper, bo return false; } - // Verify payloads present when includePayloads=true + // Verify payloads present when includePayloads=true. + // Events were stored with ascending timestamps (Message 0..n-1) but are + // returned newest-first, so Events[i] corresponds to Message (n-1-i). for (int i = 0; i < n; i++) { var payload = withPayloads.Events[i].Payload; @@ -448,7 +451,7 @@ public bool IncludePayloads_ControlsResponseContent(PositiveInt countWrapper, bo // Verify the payload content is intact var text = payload[0].Conversational?.Content?.Text; - if (text != $"Message {i}") + if (text != $"Message {n - 1 - i}") return false; } diff --git a/test/AWS.AgentCore.Testing.UnitTests/InMemoryEventStoreTests.cs b/test/AWS.AgentCore.Testing.UnitTests/InMemoryEventStoreTests.cs index d9ed166..fdf67d0 100644 --- a/test/AWS.AgentCore.Testing.UnitTests/InMemoryEventStoreTests.cs +++ b/test/AWS.AgentCore.Testing.UnitTests/InMemoryEventStoreTests.cs @@ -196,11 +196,13 @@ public void ListEvents_FiltersCorrectly_CombinedFilters() } // ────────────────────────────────────────────────────────────────── - // ListEvents_ReturnsChronologicalOrder + // ListEvents_ReturnsNewestFirst + // Matches the real AgentCore Memory ListEvents API, which returns events + // in reverse chronological order (most recent first). // ────────────────────────────────────────────────────────────────── [Fact] - public void ListEvents_ReturnsChronologicalOrder() + public void ListEvents_ReturnsNewestFirst() { var store = new InMemoryEventStore(); @@ -217,23 +219,23 @@ public void ListEvents_ReturnsChronologicalOrder() includePayloads: true, maxResults: null, nextToken: null); Assert.Equal(3, response.Events.Count); - Assert.Equal("First", response.Events[0].Payload![0].Conversational!.Content!.Text); + Assert.Equal("Third", response.Events[0].Payload![0].Conversational!.Content!.Text); Assert.Equal("Second", response.Events[1].Payload![0].Conversational!.Content!.Text); - Assert.Equal("Third", response.Events[2].Payload![0].Conversational!.Content!.Text); + Assert.Equal("First", response.Events[2].Payload![0].Conversational!.Content!.Text); - // Also verify timestamps are in ascending order - Assert.True(response.Events[0].EventTimestamp < response.Events[1].EventTimestamp); - Assert.True(response.Events[1].EventTimestamp < response.Events[2].EventTimestamp); + // Also verify timestamps are in descending order (newest-first) + Assert.True(response.Events[0].EventTimestamp > response.Events[1].EventTimestamp); + Assert.True(response.Events[1].EventTimestamp > response.Events[2].EventTimestamp); } [Fact] - public void ListEvents_ReturnsChronologicalOrder_WithManyEvents() + public void ListEvents_ReturnsNewestFirst_WithManyEvents() { var store = new InMemoryEventStore(); var baseTime = new DateTime(2024, 1, 1, 0, 0, 0, DateTimeKind.Utc); - // Insert 10 events in reverse chronological order - for (int i = 9; i >= 0; i--) + // Insert 10 events in chronological order + for (int i = 0; i < 10; i++) { store.CreateEvent("memory-1", CreateRequest( timestamp: baseTime.AddMinutes(i), @@ -247,8 +249,8 @@ public void ListEvents_ReturnsChronologicalOrder_WithManyEvents() for (int i = 0; i < response.Events.Count - 1; i++) { - Assert.True(response.Events[i].EventTimestamp <= response.Events[i + 1].EventTimestamp, - $"Event at index {i} has timestamp {response.Events[i].EventTimestamp} which is after event at index {i + 1} with timestamp {response.Events[i + 1].EventTimestamp}"); + Assert.True(response.Events[i].EventTimestamp >= response.Events[i + 1].EventTimestamp, + $"Event at index {i} has timestamp {response.Events[i].EventTimestamp} which is before event at index {i + 1} with timestamp {response.Events[i + 1].EventTimestamp}"); } } @@ -368,11 +370,11 @@ public void Pagination_IterateAll_NoDuplicatesNoGaps() var uniqueIds = allEvents.Select(e => e.EventId).Distinct().ToList(); Assert.Equal(totalEvents, uniqueIds.Count); - // Verify chronological order is maintained across pages + // Verify newest-first order is maintained across pages for (int i = 0; i < allEvents.Count - 1; i++) { - Assert.True(allEvents[i].EventTimestamp <= allEvents[i + 1].EventTimestamp, - $"Event at index {i} has timestamp {allEvents[i].EventTimestamp} which is after event at index {i + 1} with timestamp {allEvents[i + 1].EventTimestamp}"); + Assert.True(allEvents[i].EventTimestamp >= allEvents[i + 1].EventTimestamp, + $"Event at index {i} has timestamp {allEvents[i].EventTimestamp} which is before event at index {i + 1} with timestamp {allEvents[i + 1].EventTimestamp}"); } }