diff --git a/backend/src/Taskdeck.Application/Services/ChatService.cs b/backend/src/Taskdeck.Application/Services/ChatService.cs index 346f04193..eb9d8109f 100644 --- a/backend/src/Taskdeck.Application/Services/ChatService.cs +++ b/backend/src/Taskdeck.Application/Services/ChatService.cs @@ -1462,10 +1462,7 @@ private static ChatSessionDto MapSessionToDto(ChatSession session) session.Status, session.CreatedAt, session.UpdatedAt, - // The UI and recovery logic consume this as a turn transcript. EF does not guarantee - // Include collection order, so return the causal creation order explicitly. session.Messages - .OrderBy(message => message.CreatedAt) .Select(MapMessageToDto) .ToList() ); diff --git a/backend/src/Taskdeck.Domain/Entities/ChatSession.cs b/backend/src/Taskdeck.Domain/Entities/ChatSession.cs index 31b1019fc..b67dce8e3 100644 --- a/backend/src/Taskdeck.Domain/Entities/ChatSession.cs +++ b/backend/src/Taskdeck.Domain/Entities/ChatSession.cs @@ -11,7 +11,14 @@ public class ChatSession : Entity public ChatSessionStatus Status { get; private set; } private readonly List _messages = new(); - public IReadOnlyList Messages => _messages.AsReadOnly(); + // EF Core populates the backing field during relationship fixup and does not promise + // collection order. Expose a fresh, immutable transcript snapshot in chronological order, with + // Id as a deterministic tie-break, so every consumer observes the same history. + public IReadOnlyList Messages => _messages + .OrderBy(message => message.CreatedAt) + .ThenBy(message => message.Id) + .ToList() + .AsReadOnly(); private ChatSession() { } // EF Core diff --git a/backend/tests/Taskdeck.Api.Tests/ChatSessionRepositoryConcurrencyTests.cs b/backend/tests/Taskdeck.Api.Tests/ChatSessionRepositoryConcurrencyTests.cs index 9179438ef..f9bc1dacd 100644 --- a/backend/tests/Taskdeck.Api.Tests/ChatSessionRepositoryConcurrencyTests.cs +++ b/backend/tests/Taskdeck.Api.Tests/ChatSessionRepositoryConcurrencyTests.cs @@ -1,6 +1,7 @@ using FluentAssertions; using Microsoft.EntityFrameworkCore; using Taskdeck.Api.Tests.Support; +using Taskdeck.Domain.Common; using Taskdeck.Domain.Entities; using Taskdeck.Infrastructure.Persistence; using Taskdeck.Infrastructure.Repositories; @@ -70,6 +71,64 @@ public async Task TryBindBoardAsync_ConcurrentSameBoard_LoserRereadsAuthoritativ "the CAS loser must not resolve an idempotent same-board race from its stale tracked entity"); } + [Fact] + public async Task GetByIdWithMessagesAsync_ShouldKeepTrackedNavigationChronological_AfterDescendingFixup() + { + var options = new DbContextOptionsBuilder() + .UseSqlite(TestSqlite.ConnectionString(_dbPath)) + .Options; + + var userId = Guid.NewGuid(); + var sessionId = Guid.NewGuid(); + await using (var seedDb = new TaskdeckDbContext(options)) + { + await seedDb.Database.MigrateAsync(); + var user = new User( + $"chat-order-{Guid.NewGuid():N}"[..20], + $"chat-order-{Guid.NewGuid():N}@example.com", + "hash"); + var session = new ChatSession(user.Id, "Ordered history"); + typeof(Entity).GetProperty(nameof(Entity.Id))!.SetValue(session, sessionId); + var oldest = new ChatMessage(session.Id, ChatMessageRole.User, "Original instruction"); + var clarification = new ChatMessage( + session.Id, + ChatMessageRole.Assistant, + "What should I call it?", + "clarification"); + var answer = new ChatMessage(session.Id, ChatMessageRole.User, "Ship notes"); + var baseTime = DateTimeOffset.UtcNow.AddMinutes(-3); + SetCreatedAt(oldest, baseTime); + SetCreatedAt(clarification, baseTime.AddMinutes(1)); + SetCreatedAt(answer, baseTime.AddMinutes(2)); + session.AddMessage(oldest); + session.AddMessage(clarification); + session.AddMessage(answer); + seedDb.AddRange(user, session); + await seedDb.SaveChangesAsync(); + userId = user.Id; + } + + await using var db = new TaskdeckDbContext(options); + var trackedSession = await db.ChatSessions.FindAsync(sessionId); + trackedSession.Should().NotBeNull(); + trackedSession!.UserId.Should().Be(userId); + + // SQLite cannot translate DateTimeOffset ordering. Use the persisted column directly to + // reproduce descending materialization and let EF relationship fixup populate the tracked + // session navigation in that order. + await db.ChatMessages + .FromSqlInterpolated($"SELECT * FROM ChatMessages WHERE SessionId = {sessionId} ORDER BY CreatedAt DESC") + .LoadAsync(); + + trackedSession.Messages.Select(message => message.Content).Should().Equal( + "Original instruction", + "What should I call it?", + "Ship notes"); + } + + private static void SetCreatedAt(Entity entity, DateTimeOffset timestamp) + => typeof(Entity).GetProperty(nameof(Entity.CreatedAt))!.SetValue(entity, timestamp); + public void Dispose() { try diff --git a/backend/tests/Taskdeck.Application.Tests/Services/ChatServiceClarificationTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/ChatServiceClarificationTests.cs index a6e523c35..154322cc7 100644 --- a/backend/tests/Taskdeck.Application.Tests/Services/ChatServiceClarificationTests.cs +++ b/backend/tests/Taskdeck.Application.Tests/Services/ChatServiceClarificationTests.cs @@ -250,6 +250,78 @@ public async Task SendMessage_ShouldAttemptOriginalIntentWithPlainClarificationA result.Value.ProposalId.Should().Be(proposalId); } + [Fact] + public async Task SendMessage_ShouldRecoverOriginalIntent_WhenPersistedHistoryArrivesScrambled() + { + var userId = Guid.NewGuid(); + var boardId = Guid.NewGuid(); + var proposalId = Guid.NewGuid(); + var session = new ChatSession(userId, "Scrambled clarification", boardId); + var original = new ChatMessage( + session.Id, + ChatMessageRole.User, + "create card for the release follow-up"); + var clarification = new ChatMessage( + session.Id, + ChatMessageRole.Assistant, + "What should the card be called?", + "clarification"); + var baseTime = DateTimeOffset.UtcNow.AddMinutes(-2); + SetCreatedAt(original, baseTime); + SetCreatedAt(clarification, baseTime.AddMinutes(1)); + + // EF navigation fixup can expose persisted rows in a different order than creation time. + session.AddMessage(clarification); + session.AddMessage(original); + _chatSessionRepoMock + .Setup(r => r.GetByIdWithMessagesAsync(session.Id, default)) + .ReturnsAsync(session); + _plannerMock + .Setup(planner => planner.ParseInstructionAsync( + It.Is(instruction => + instruction.Contains("create card for the release follow-up") && + instruction.Contains("Clarification answer: Ship notes")), + userId, + boardId, + It.IsAny(), + ProposalSourceType.Chat, + session.Id.ToString(), + It.IsAny())) + .ReturnsAsync(Result.Success(new ProposalDto( + proposalId, + ProposalSourceType.Chat, + null, + boardId, + userId, + ProposalStatus.PendingReview, + RiskLevel.Low, + "Create release follow-up", + null, + null, + DateTimeOffset.UtcNow, + DateTimeOffset.UtcNow, + DateTime.UtcNow.AddHours(1), + null, + null, + null, + null, + "corr", + new List()))); + + var result = await _service.SendMessageAsync( + session.Id, + userId, + new SendChatMessageDto("Ship notes"), + default); + + result.IsSuccess.Should().BeTrue(); + result.Value.MessageType.Should().Be("proposal-reference"); + result.Value.ProposalId.Should().Be(proposalId); + } + + private static void SetCreatedAt(Entity entity, DateTimeOffset timestamp) + => typeof(Entity).GetProperty(nameof(Entity.CreatedAt))!.SetValue(entity, timestamp); + [Fact] public async Task MockProvider_ShouldReturnClarification_ForAmbiguousInput() { diff --git a/backend/tests/Taskdeck.Application.Tests/Services/ChatServiceTests.cs b/backend/tests/Taskdeck.Application.Tests/Services/ChatServiceTests.cs index 461cdb7cb..5d14bd52d 100644 --- a/backend/tests/Taskdeck.Application.Tests/Services/ChatServiceTests.cs +++ b/backend/tests/Taskdeck.Application.Tests/Services/ChatServiceTests.cs @@ -1086,6 +1086,50 @@ public async Task StreamResponseAsync_ShouldPassServerDerivedAttributionToProvid capturedRequest.Attribution.CorrelationId.Should().NotBeNullOrWhiteSpace(); } + [Fact] + public async Task StreamResponseAsync_ShouldSendChronologicalHistory_WhenTrackedMessagesArriveScrambled() + { + var userId = Guid.NewGuid(); + var session = new ChatSession(userId, "Scrambled stream history", Guid.NewGuid()); + var original = new ChatMessage( + session.Id, + ChatMessageRole.User, + "create card for the release follow-up"); + var clarification = new ChatMessage( + session.Id, + ChatMessageRole.Assistant, + "What should the card be called?", + "clarification"); + var answer = new ChatMessage(session.Id, ChatMessageRole.User, "Ship notes"); + var baseTime = DateTimeOffset.UtcNow.AddMinutes(-3); + SetCreatedAt(original, baseTime); + SetCreatedAt(clarification, baseTime.AddMinutes(1)); + SetCreatedAt(answer, baseTime.AddMinutes(2)); + + session.AddMessage(answer); + session.AddMessage(clarification); + session.AddMessage(original); + ChatCompletionRequest? capturedRequest = null; + _chatSessionRepoMock + .Setup(r => r.GetByIdWithMessagesAsync(session.Id, default)) + .ReturnsAsync(session); + _llmProviderMock + .Setup(p => p.StreamAsync(It.IsAny(), default)) + .Returns((ChatCompletionRequest request, CancellationToken _) => + { + capturedRequest = request; + return StreamEvents(); + }); + + await foreach (var _ in _service.StreamResponseAsync(session.Id, userId, default)) { } + + capturedRequest.Should().NotBeNull(); + capturedRequest!.Messages.Select(message => message.Content).Should().Equal( + "create card for the release follow-up", + "What should the card be called?", + "Ship notes"); + } + [Fact] public async Task GetProviderHealthAsync_ShouldSurfaceProviderStatus() { @@ -3393,6 +3437,9 @@ private static async IAsyncEnumerable StreamEvents() await Task.CompletedTask; } + private static void SetCreatedAt(Entity entity, DateTimeOffset timestamp) + => typeof(Entity).GetProperty(nameof(Entity.CreatedAt))!.SetValue(entity, timestamp); + private static async IAsyncEnumerable StreamEventsWithUsage() { yield return new LlmTokenEvent("hello", false); diff --git a/backend/tests/Taskdeck.Domain.Tests/Entities/ChatSessionStateMachineTests.cs b/backend/tests/Taskdeck.Domain.Tests/Entities/ChatSessionStateMachineTests.cs index ecca63d27..c59a21f3f 100644 --- a/backend/tests/Taskdeck.Domain.Tests/Entities/ChatSessionStateMachineTests.cs +++ b/backend/tests/Taskdeck.Domain.Tests/Entities/ChatSessionStateMachineTests.cs @@ -1,4 +1,5 @@ using FluentAssertions; +using Taskdeck.Domain.Common; using Taskdeck.Domain.Entities; using Taskdeck.Domain.Exceptions; using Xunit; @@ -291,6 +292,36 @@ public void Active_AddMultipleMessages_PreservesOrder() session.Messages[1].Content.Should().Be("Second"); } + [Fact] + public void Messages_SortsByCreatedAtThenId_WhenTrackedCollectionIsScrambled() + { + var session = CreateActiveSession(); + var baseTime = new DateTimeOffset(2026, 9, 9, 12, 0, 0, TimeSpan.Zero); + var oldest = new ChatMessage(session.Id, ChatMessageRole.User, "Oldest", "text"); + var tieLaterId = new ChatMessage(session.Id, ChatMessageRole.Assistant, "Tie later", "text"); + var newest = new ChatMessage(session.Id, ChatMessageRole.User, "Newest", "text"); + var tieEarlierId = new ChatMessage(session.Id, ChatMessageRole.Assistant, "Tie earlier", "text"); + + SetId(oldest, Guid.Parse("00000000-0000-0000-0000-000000000004")); + SetId(tieLaterId, Guid.Parse("00000000-0000-0000-0000-000000000003")); + SetId(newest, Guid.Parse("00000000-0000-0000-0000-000000000001")); + SetId(tieEarlierId, Guid.Parse("00000000-0000-0000-0000-000000000002")); + SetCreatedAt(oldest, baseTime); + SetCreatedAt(tieLaterId, baseTime.AddMinutes(1)); + SetCreatedAt(newest, baseTime.AddMinutes(2)); + SetCreatedAt(tieEarlierId, baseTime.AddMinutes(1)); + + // Simulate a provider/ORM collection whose materialization order is unrelated to the + // transcript's causal order, including a same-timestamp tie. + session.AddMessage(newest); + session.AddMessage(tieLaterId); + session.AddMessage(oldest); + session.AddMessage(tieEarlierId); + + session.Messages.Select(message => message.Content).Should().Equal( + "Oldest", "Tie earlier", "Tie later", "Newest"); + } + [Fact] public void Archived_AddMessage_Throws() { @@ -356,5 +387,11 @@ public void UpdateTitle_WorksOnArchivedSession() session.Title.Should().Be("Archived title update"); } + private static void SetId(Entity entity, Guid id) + => typeof(Entity).GetProperty(nameof(Entity.Id))!.SetValue(entity, id); + + private static void SetCreatedAt(Entity entity, DateTimeOffset timestamp) + => typeof(Entity).GetProperty(nameof(Entity.CreatedAt))!.SetValue(entity, timestamp); + #endregion }