Kept the unsent message with its chat (#1049)

app/Tests/Chat/ChatDraftAutosaveTests.cs

@@ -0,0 +1,192 @@
+using AIStudio.Chat;
+
+using Microsoft.Extensions.Logging.Abstractions;
+
+namespace AIStudio.Tests.Chat;
+
+/// <summary>
+/// Checks when a chat gets stored with the message the user has not sent yet.
+/// </summary>
+/// <remarks>
+/// Storing writes the whole conversation, so it waits until the user paused, and it happens once more
+/// when the user leaves the chat. A chat being answered is stored by its AI job instead, and a chat which
+/// is not on the disk is left to whoever creates it: when it is gone, it was deleted.
+/// </remarks>
+[TestFixture]
+public sealed class ChatDraftAutosaveTests
+{
+    private static readonly TimeSpan SHORT_DELAY = TimeSpan.FromMilliseconds(50);
+    private static readonly TimeSpan LONG_DELAY = TimeSpan.FromHours(1);
+    private static readonly TimeSpan WAIT_LIMIT = TimeSpan.FromSeconds(10);
+
+    [Test]
+    public async Task ManyChangesInARowAreStoredOnce()
+    {
+        var storage = new FakeStorage();
+        await using var autosave = storage.CreateAutosave(SHORT_DELAY);
+        var chat = new ChatThread { ChatId = Guid.NewGuid() };
+
+        for (var keystroke = 0; keystroke < 5; keystroke++)
+            autosave.Schedule(chat);
+
+        await storage.WaitForStoreAsync();
+        await Task.Delay(SHORT_DELAY * 4);
+
+        Assert.That(storage.StoredChats, Has.Count.EqualTo(1), "Every change starts the waiting anew, so typing a word writes the chat once, not once per letter.");
+    }
+
+    [Test]
+    public async Task LeavingTheChatStoresItRightAway()
+    {
+        var storage = new FakeStorage();
+        await using var autosave = storage.CreateAutosave(LONG_DELAY);
+        var chat = new ChatThread { ChatId = Guid.NewGuid() };
+
+        autosave.Schedule(chat);
+        await autosave.FlushAsync();
+
+        Assert.That(storage.StoredChats, Is.EqualTo(new[] { chat }), "The chat being left has to be on the disk before the next one opens.");
+    }
+
+    [Test]
+    public async Task AChatBeingAnsweredIsStoredOnceTheAnswerIsDone()
+    {
+        var storage = new FakeStorage { IsBusy = true };
+        await using var autosave = storage.CreateAutosave(SHORT_DELAY);
+        var chat = new ChatThread { ChatId = Guid.NewGuid() };
+
+        autosave.Schedule(chat);
+        await Task.Delay(SHORT_DELAY * 4);
+        var storedWhileBusy = storage.StoredChats.Count;
+
+        storage.IsBusy = false;
+        await storage.WaitForStoreAsync();
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(storedWhileBusy, Is.Zero, "The AI job changes the conversation meanwhile, so storing it from elsewhere could read it in the middle of a change.");
+            Assert.That(storage.StoredChats, Is.EqualTo(new[] { chat }), "Once the answer is done, the change must still reach the disk.");
+        });
+    }
+
+    [Test]
+    public async Task LeavingAChatBeingAnsweredLeavesItToTheJob()
+    {
+        var storage = new FakeStorage { IsBusy = true };
+        await using var autosave = storage.CreateAutosave(LONG_DELAY);
+
+        autosave.Schedule(new ChatThread { ChatId = Guid.NewGuid() });
+        await autosave.FlushAsync();
+
+        Assert.That(storage.StoredChats, Is.Empty, "The AI job stores the chat once more when the answer is done, together with the draft it shares.");
+    }
+
+    [Test]
+    public async Task AChatWhichIsNotOnTheDiskIsNeverStored()
+    {
+        var storage = new FakeStorage { IsStored = false };
+        await using var autosave = storage.CreateAutosave(SHORT_DELAY);
+
+        autosave.Schedule(new ChatThread { ChatId = Guid.NewGuid() });
+        await Task.Delay(SHORT_DELAY * 4);
+        await autosave.FlushAsync();
+
+        Assert.That(storage.StoredChats, Is.Empty, "A chat gone from the disk was deleted, and storing it would bring it back.");
+    }
+
+    [Test]
+    public async Task TheNewestCopyOfTheChatIsStored()
+    {
+        var storage = new FakeStorage();
+        await using var autosave = storage.CreateAutosave(LONG_DELAY);
+        var chat = new ChatThread { ChatId = Guid.NewGuid() };
+
+        autosave.Schedule(chat);
+
+        // What the chat does when the user picks another profile:
+        var withOtherProfile = chat with { SelectedProfile = Guid.NewGuid().ToString() };
+        autosave.Follow(withOtherProfile);
+        await autosave.FlushAsync();
+
+        Assert.That(storage.StoredChats.Single(), Is.SameAs(withOtherProfile), "Storing the former copy would bring the former profile back.");
+    }
+
+    [Test]
+    public async Task AChangeInAnotherChatStoresThePreviousOne()
+    {
+        var storage = new FakeStorage();
+        await using var autosave = storage.CreateAutosave(LONG_DELAY);
+        var previousChat = new ChatThread { ChatId = Guid.NewGuid() };
+
+        autosave.Schedule(previousChat);
+        autosave.Schedule(new ChatThread { ChatId = Guid.NewGuid() });
+        await storage.WaitForStoreAsync();
+
+        Assert.That(storage.StoredChats.Single(), Is.SameAs(previousChat), "The change in one chat must not cost the other its draft.");
+    }
+
+    [Test]
+    public async Task DisposingStoresWhatIsPending()
+    {
+        var storage = new FakeStorage();
+        var autosave = storage.CreateAutosave(LONG_DELAY);
+        var chat = new ChatThread { ChatId = Guid.NewGuid() };
+
+        autosave.Schedule(chat);
+        await autosave.DisposeAsync();
+        autosave.Schedule(new ChatThread { ChatId = Guid.NewGuid() });
+        await autosave.FlushAsync();
+
+        Assert.That(storage.StoredChats, Is.EqualTo(new[] { chat }), "Leaving the chat page disposes the chat, which counts as leaving the chat. What comes later belongs to no chat on the screen.");
+    }
+
+    /// <summary>
+    /// Stands in for the disk and for the AI jobs, and records which chats were stored.
+    /// </summary>
+    private sealed class FakeStorage
+    {
+        private readonly Lock storedLock = new();
+        private readonly List<ChatThread> storedChats = [];
+        private readonly SemaphoreSlim storeSignal = new(0);
+        private volatile bool isBusy;
+        private volatile bool isStored = true;
+
+        public bool IsBusy
+        {
+            get => this.isBusy;
+            set => this.isBusy = value;
+        }
+
+        public bool IsStored
+        {
+            get => this.isStored;
+            init => this.isStored = value;
+        }
+
+        public IReadOnlyList<ChatThread> StoredChats
+        {
+            get
+            {
+                lock (this.storedLock)
+                    return [..this.storedChats];
+            }
+        }
+
+        public ChatDraftAutosave CreateAutosave(TimeSpan delay) => new(delay, _ => this.IsStored, _ => this.IsBusy, this.StoreAsync, NullLogger.Instance);
+
+        public async Task WaitForStoreAsync()
+        {
+            if (!await this.storeSignal.WaitAsync(WAIT_LIMIT))
+                Assert.Fail("The chat was never stored.");
+        }
+
+        private Task<bool> StoreAsync(ChatThread chat)
+        {
+            lock (this.storedLock)
+                this.storedChats.Add(chat);
+
+            this.storeSignal.Release();
+            return Task.FromResult(true);
+        }
+    }
+}

app/Tests/Chat/ChatDraftTests.cs

@@ -0,0 +1,194 @@
+using System.Text.Json;
+
+using AIStudio.Chat;
+using AIStudio.Tools;
+
+namespace AIStudio.Tests.Chat;
+
+/// <summary>
+/// Checks the draft of a chat, the message the user has not sent yet.
+/// </summary>
+/// <remarks>
+/// The draft is stored with its chat, so the message is still there when the user comes back to it.
+/// Two parties change the draft: the user in the composer, and the media transcription service,
+/// which adds a finished transcript before the composer shows it. That is why a transcript only
+/// leaves the draft when the user removes it, or when the message is sent.
+/// </remarks>
+[TestFixture]
+public sealed class ChatDraftTests
+{
+    private static readonly FileAttachment AGENDA = new(FileAttachmentType.DOCUMENT, "agenda.pdf", "/documents/agenda.pdf", 1024);
+    private static readonly FileAttachment MINUTES = new(FileAttachmentType.DOCUMENT, "minutes.pdf", "/documents/minutes.pdf", 2048);
+    private static readonly FileAttachmentImage CHART = new("chart.png", "/documents/chart.png", 4096);
+    private static readonly ManagedTranscriptAttachment TRANSCRIPT = new("meeting-transcript-0001.md", "/data/tempChats/meeting-transcript-0001.md", 512, "meeting.mp4", false);
+    private static readonly ManagedTranscriptAttachment STAGED_TRANSCRIPT = new("4f1c.md", "/data/media-staging/4f1c.md", 256, "kickoff.mp4", true);
+
+    [Test]
+    public void MirroringTheComposerKeepsATranscriptItDoesNotShowYet()
+    {
+        var draft = new ChatDraft();
+        draft.AddAttachments([AGENDA]);
+
+        // The transcription service finished, and the composer has not shown the transcript yet:
+        draft.AddAttachments([TRANSCRIPT]);
+
+        // Meanwhile, the user replaced the agenda by the minutes:
+        draft.MirrorAttachments([MINUTES]);
+
+        Assert.That(draft.FileAttachments, Is.EqualTo(new[] { MINUTES, TRANSCRIPT }), "The composer decides on the other attachments, but a transcript it does not show yet has to stay, or its file is left without an owner.");
+    }
+
+    [Test]
+    public void AnAttachmentIsAddedOnlyOnce()
+    {
+        var draft = new ChatDraft();
+
+        var added = draft.AddAttachments([TRANSCRIPT]);
+        var addedAgain = draft.AddAttachments([TRANSCRIPT with { FileSizeBytes = 1024 }]);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(added, Is.True);
+            Assert.That(addedAgain, Is.False, "The same file counts once, even in a newer state.");
+            Assert.That(draft.FileAttachments, Is.EqualTo(new FileAttachment[] { TRANSCRIPT }), "The composer would show the transcript twice otherwise.");
+        });
+    }
+
+    [Test]
+    public void RemovingAnAttachmentFindsItByItsPath()
+    {
+        var draft = new ChatDraft();
+        draft.AddAttachments([AGENDA, TRANSCRIPT]);
+
+        // The composer holds the transcript as it normalized it, with the current size of its file:
+        draft.RemoveAttachments([TRANSCRIPT with { FileSizeBytes = 1024 }]);
+
+        Assert.That(draft.FileAttachments, Is.EqualTo(new[] { AGENDA }), "The transcript the user removed must not come back when the chat is opened again.");
+    }
+
+    [Test]
+    public void ReplacingTheAttachmentsKeepsTheirKind()
+    {
+        var draft = new ChatDraft();
+        draft.AddAttachments([AGENDA, TRANSCRIPT]);
+
+        draft.ReplaceAttachments(attachment => attachment with { FilePath = $"/moved{attachment.FilePath}" });
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(draft.FileAttachments.Select(attachment => attachment.FilePath), Is.EqualTo(new[] { "/moved/documents/agenda.pdf", "/moved/data/tempChats/meeting-transcript-0001.md" }));
+            Assert.That(draft.MediaTranscripts, Has.Count.EqualTo(1), "A transcript stays a transcript, so removing it from the composer still deletes its file.");
+        });
+    }
+
+    [Test]
+    public void EditingAMessageKeepsTheTranscriptsOfTheDraft()
+    {
+        var draft = new ChatDraft();
+        draft.SetText("An older draft");
+        draft.AddAttachments([AGENDA, TRANSCRIPT]);
+
+        draft.RestoreFromMessage(new ContentText { Text = "Summarize the minutes", FileAttachments = [MINUTES] });
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(draft.Text, Is.EqualTo("Summarize the minutes"), "The message to edit replaces the older draft.");
+            Assert.That(draft.FileAttachments, Is.EqualTo(new[] { MINUTES, TRANSCRIPT }), "The draft is the only owner of its transcripts, so dropping them would leave their files behind.");
+        });
+    }
+
+    [Test]
+    public void SendingEmptiesTheDraft()
+    {
+        var draft = new ChatDraft();
+        draft.SetText("Summarize the meeting");
+        draft.AddAttachments([TRANSCRIPT]);
+
+        draft.Clear();
+
+        Assert.That(draft.IsEmpty, Is.True, "The attachments belong to the sent message now, and a transcript has only one owner.");
+    }
+
+    [Test]
+    public void TheDraftKeepsTheKindOfEveryAttachmentWhenStored()
+    {
+        var thread = new ChatThread { ChatId = Guid.NewGuid(), Name = "Meeting notes" };
+        thread.Draft.SetText("Summarize the meeting");
+        thread.Draft.AddAttachments([AGENDA, CHART, STAGED_TRANSCRIPT]);
+
+        var loaded = JsonSerializer.Deserialize<ChatThread>(JsonSerializer.Serialize(thread, WorkspaceBehaviour.JSON_OPTIONS), WorkspaceBehaviour.JSON_OPTIONS)!;
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(loaded.Draft.Text, Is.EqualTo("Summarize the meeting"));
+            Assert.That(loaded.Draft.FileAttachments, Is.EqualTo(new[] { AGENDA, CHART, STAGED_TRANSCRIPT }), "A transcript loaded as a plain document would no longer be managed, and its file would never be deleted.");
+            Assert.That(loaded.Draft.MediaTranscripts.Single().IsStaged, Is.True, "A staged transcript has to move into the chat directory once the chat is stored.");
+        });
+    }
+
+    [Test]
+    public void AChatOfTheFormerVersionBringsItsPendingTranscriptsIntoTheDraft()
+    {
+        // How v26.9.1 stored a chat with a transcript which was not sent yet, without a type discriminator:
+        const string CHAT_WITH_PENDING_TRANSCRIPTS = """
+                                                     {
+                                                       "chat_id": "2b7c9e1f-4a6d-4c8b-9e3f-1d5a7c9b2e4f",
+                                                       "pending_media_transcripts": [
+                                                         {
+                                                           "original_file_name": "meeting.mp4",
+                                                           "is_staged": false,
+                                                           "type": "DOCUMENT",
+                                                           "file_name": "meeting-transcript-0001.md",
+                                                           "file_path": "/data/tempChats/meeting-transcript-0001.md",
+                                                           "file_size_bytes": 512,
+                                                           "is_forbidden": false,
+                                                           "is_valid": true,
+                                                           "is_image": false,
+                                                           "file_path_as_url": "/local/file?path=%2Fdata%2FtempChats%2Fmeeting-transcript-0001.md",
+                                                           "exists": true
+                                                         }
+                                                       ],
+                                                       "name": "Meeting notes",
+                                                       "blocks": []
+                                                     }
+                                                     """;
+
+        var loaded = JsonSerializer.Deserialize<ChatThread>(CHAT_WITH_PENDING_TRANSCRIPTS, WorkspaceBehaviour.JSON_OPTIONS)!;
+        var stored = JsonSerializer.Serialize(loaded, WorkspaceBehaviour.JSON_OPTIONS);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(loaded.Draft.FileAttachments, Is.EqualTo(new FileAttachment[] { TRANSCRIPT }), "Opening the chat has to bring the transcript back into the composer.");
+            Assert.That(stored, Does.Not.Contain("pending_media_transcripts"), "The draft holds the transcript now, so storing the chat must not write the former list again.");
+            Assert.That(stored, Does.Contain("\"draft\""));
+        });
+    }
+
+    [Test]
+    public void AChatWithoutADraftOpensWithAnEmptyOne()
+    {
+        const string CHAT_WITHOUT_DRAFT = """
+                                          {
+                                            "chat_id": "2b7c9e1f-4a6d-4c8b-9e3f-1d5a7c9b2e4f",
+                                            "name": "An older chat",
+                                            "blocks": []
+                                          }
+                                          """;
+
+        var loaded = JsonSerializer.Deserialize<ChatThread>(CHAT_WITHOUT_DRAFT, WorkspaceBehaviour.JSON_OPTIONS);
+
+        Assert.That(loaded?.Draft.IsEmpty, Is.True);
+    }
+
+    [Test]
+    public void ACopyOfTheSameChatSharesTheDraft()
+    {
+        var thread = new ChatThread { ChatId = Guid.NewGuid() };
+
+        // What the chat does when the user picks another profile:
+        var withOtherProfile = thread with { SelectedProfile = Guid.NewGuid().ToString() };
+        withOtherProfile.Draft.SetText("Summarize the meeting");
+
+        Assert.That(thread.Draft.Text, Is.EqualTo("Summarize the meeting"), "Both are the same chat, and the transcription service may still hold the former one.");
+    }
+}

app/Tests/Chat/ChatThreadEditTests.cs

@@ -63,16 +63,17 @@ public sealed class ChatThreadEditTests
         var composer = new ChatComposerState();
 
         // What the chat does when the user stops the answer and edits the question:
-        composer.RestoreFromTextBlock((ContentText)question.Content!);
         thread.RemoveForEditing(question.Content!);
         thread.Remove(answer.Content!);
+        composer.RestoreFromDraft(thread.Draft);
 
         Assert.Multiple(() =>
         {
             Assert.That(thread.Blocks, Is.Empty, "Both the question and the stopped answer leave the chat.");
             Assert.That(File.Exists(transcript.FilePath), Is.True, "The composer still carries the transcript, so its file has to stay.");
             Assert.That(composer.FileAttachments.Single().Exists, Is.True, "The edited message goes out with the transcript, not with a reference to a deleted file.");
-            Assert.That(thread.PendingMediaTranscripts.Select(x => x.FilePath), Is.EqualTo(new[] { transcript.FilePath }), "The draft owns the transcript now: opening the chat again brings it back, and removing it from the composer deletes the file.");
+            Assert.That(thread.Draft.MediaTranscripts.Select(x => x.FilePath), Is.EqualTo(new[] { transcript.FilePath }), "The draft owns the transcript now: opening the chat again brings it back, and removing it from the composer deletes the file.");
+            Assert.That(thread.Draft.Text, Is.EqualTo("Summarize the meeting"), "The message being edited is the draft now, its text included.");
         });
     }
 
@@ -82,16 +83,12 @@ public sealed class ChatThreadEditTests
         var chatId = Guid.NewGuid();
         var transcript = this.CreateTranscript(chatId, "meeting");
         var question = Block(ChatRole.USER, "Summarize the meeting", 1, transcript);
-        var thread = new ChatThread
-        {
-            ChatId = chatId,
-            PendingMediaTranscripts = [transcript],
-            Blocks = [question],
-        };
+        var thread = new ChatThread { ChatId = chatId, Blocks = [question] };
+        thread.Draft.AddAttachments([transcript]);
 
         thread.RemoveForEditing(question.Content!);
 
-        Assert.That(thread.PendingMediaTranscripts, Has.Count.EqualTo(1), "The composer would show the same transcript twice otherwise.");
+        Assert.That(thread.Draft.FileAttachments, Has.Count.EqualTo(1), "The composer would show the same transcript twice otherwise.");
     }
 
     [Test]
@@ -107,7 +104,7 @@ public sealed class ChatThreadEditTests
         Assert.Multiple(() =>
         {
             Assert.That(File.Exists(transcript.FilePath), Is.False, "Nothing refers to the transcript of a deleted message any longer, so its file has to go.");
-            Assert.That(thread.PendingMediaTranscripts, Is.Empty, "A deleted message hands nothing over to the draft.");
+            Assert.That(thread.Draft.IsEmpty, Is.True, "A deleted message hands nothing over to the draft.");
         });
     }
 
@@ -127,7 +124,7 @@ public sealed class ChatThreadEditTests
         Assert.Multiple(() =>
         {
             Assert.That(thread.Blocks, Has.Count.EqualTo(2), "The message to edit is not part of this chat, so no block goes.");
-            Assert.That(thread.PendingMediaTranscripts, Is.Empty, "A transcript of a message this chat does not hold never becomes part of its draft.");
+            Assert.That(thread.Draft.IsEmpty, Is.True, "A message this chat does not hold never becomes its draft.");
             Assert.That(File.Exists(transcript.FilePath), Is.True);
         });
     }