Fixed and improved the system prompt of chats started from assistants (#1172)

app/Tests/Assistants/ChatSystemPromptChoiceTests.cs

@@ -0,0 +1,49 @@
+using AIStudio.Assistants;
+using AIStudio.Chat;
+
+namespace AIStudio.Tests.Assistants;
+
+/// <summary>
+/// Checks which system prompt a chat gets for each choice an assistant can make.
+/// </summary>
+[TestFixture]
+public sealed class ChatSystemPromptChoiceTests
+{
+    private const string PROMPT_OF_ASSISTANT = "You are a translation engine. Only translate the text.";
+    private const string PROMPT_FOR_THE_CHAT = "You are an expert in German and answer questions about a translation.";
+
+    [Test]
+    public void TheDefaultChoiceGivesTheDefaultPromptOfAIStudio() => Assert.That(ChatSystemPromptChoice.AI_STUDIO_DEFAULT.CreatePrompt(PROMPT_OF_ASSISTANT), Is.EqualTo(SystemPrompts.DEFAULT));
+
+    [Test]
+    public void TheAssistantChoiceGivesThePromptOfTheAssistant() => Assert.That(ChatSystemPromptChoice.ASSISTANT.CreatePrompt(PROMPT_OF_ASSISTANT), Is.EqualTo(PROMPT_OF_ASSISTANT));
+
+    [Test]
+    public void ACustomChoiceGivesThePromptTheAssistantWroteForTheChat()
+    {
+        var choice = ChatSystemPromptChoice.Custom(() => PROMPT_FOR_THE_CHAT);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(choice.Source, Is.EqualTo(ChatSystemPromptSource.CUSTOM));
+            Assert.That(choice.CreatePrompt(PROMPT_OF_ASSISTANT), Is.EqualTo(PROMPT_FOR_THE_CHAT));
+        }
+    }
+
+    [Test]
+    public void ACustomPromptIsBuiltOnlyWhenTheChatNeedsIt()
+    {
+        var builds = 0;
+        var choice = ChatSystemPromptChoice.Custom(() =>
+        {
+            builds++;
+            return PROMPT_FOR_THE_CHAT;
+        });
+
+        Assert.That(choice.Source, Is.EqualTo(ChatSystemPromptSource.CUSTOM));
+        Assert.That(builds, Is.Zero, "Reading the choice must not build the prompt, which may need the state of the assistant.");
+
+        choice.CreatePrompt(PROMPT_OF_ASSISTANT);
+        Assert.That(builds, Is.EqualTo(1));
+    }
+}

app/Tests/Assistants/ChatSystemPromptOfAssistantsTests.cs

@@ -0,0 +1,105 @@
+using System.Reflection;
+using System.Runtime.CompilerServices;
+
+using AIStudio.Assistants;
+using AIStudio.Assistants.Agenda;
+using AIStudio.Assistants.BatchProcessing;
+using AIStudio.Assistants.BiasDay;
+using AIStudio.Assistants.Builder;
+using AIStudio.Assistants.Coding;
+using AIStudio.Assistants.DocumentAnalysis;
+using AIStudio.Assistants.Dynamic;
+using AIStudio.Assistants.EMail;
+using AIStudio.Assistants.ERI;
+using AIStudio.Assistants.GrammarSpelling;
+using AIStudio.Assistants.I18N;
+using AIStudio.Assistants.IconFinder;
+using AIStudio.Assistants.JobPosting;
+using AIStudio.Assistants.LegalCheck;
+using AIStudio.Assistants.MyTasks;
+using AIStudio.Assistants.PromptOptimizer;
+using AIStudio.Assistants.RewriteImprove;
+using AIStudio.Assistants.SlideBuilder;
+using AIStudio.Assistants.SWOTAnalysis;
+using AIStudio.Assistants.Synonym;
+using AIStudio.Assistants.TextSummarizer;
+using AIStudio.Assistants.Translation;
+using AIStudio.Tools.AssistantLaunch;
+
+namespace AIStudio.Tests.Assistants;
+
+/// <summary>
+/// Holds which system prompt a chat gets that continues the conversation of each assistant.
+/// </summary>
+/// <remarks>
+/// Every assistant decides this itself, and the compiler makes sure that each one does. This table keeps
+/// what was decided, so that a change shows up as a change of this table, not as a side effect of
+/// another change: in v26.4.1, moving the default prompt into the common path took the prompts of
+/// several assistants away without anybody noticing. A new assistant fails the coverage test until it
+/// is listed here. Assistant plugins choose in their Lua file, which their own tests cover.<br/><br/>
+///
+/// The assistants are created without running their constructors, because only their choice is read,
+/// and building a prompt of their own for the chat would need their state.
+/// </remarks>
+[TestFixture]
+public sealed class ChatSystemPromptOfAssistantsTests
+{
+    private static readonly Assembly APP = typeof(AssistantLaunchService).Assembly;
+
+    private static readonly Dictionary<Type, ChatSystemPromptSource> DECISIONS = new()
+    {
+        [typeof(AssistantAgenda)] = ChatSystemPromptSource.CUSTOM,
+        [typeof(AssistantBatchProcessing)] = ChatSystemPromptSource.AI_STUDIO_DEFAULT,
+        [typeof(BiasOfTheDayAssistant)] = ChatSystemPromptSource.ASSISTANT,
+        [typeof(AssistantBuilder)] = ChatSystemPromptSource.AI_STUDIO_DEFAULT,
+        [typeof(AssistantCoding)] = ChatSystemPromptSource.ASSISTANT,
+        [typeof(DocumentAnalysisAssistant)] = ChatSystemPromptSource.CUSTOM,
+        [typeof(AssistantEMail)] = ChatSystemPromptSource.CUSTOM,
+        [typeof(AssistantERI)] = ChatSystemPromptSource.ASSISTANT,
+        [typeof(AssistantGrammarSpelling)] = ChatSystemPromptSource.CUSTOM,
+        [typeof(AssistantI18N)] = ChatSystemPromptSource.AI_STUDIO_DEFAULT,
+        [typeof(AssistantIconFinder)] = ChatSystemPromptSource.ASSISTANT,
+        [typeof(AssistantJobPostings)] = ChatSystemPromptSource.CUSTOM,
+        [typeof(AssistantLegalCheck)] = ChatSystemPromptSource.ASSISTANT,
+        [typeof(AssistantMyTasks)] = ChatSystemPromptSource.ASSISTANT,
+        [typeof(AssistantPromptOptimizer)] = ChatSystemPromptSource.AI_STUDIO_DEFAULT,
+        [typeof(AssistantRewriteImprove)] = ChatSystemPromptSource.CUSTOM,
+        [typeof(AssistantSWOTAnalysis)] = ChatSystemPromptSource.CUSTOM,
+        [typeof(SlideAssistant)] = ChatSystemPromptSource.CUSTOM,
+        [typeof(AssistantSynonyms)] = ChatSystemPromptSource.CUSTOM,
+        [typeof(AssistantTextSummarizer)] = ChatSystemPromptSource.CUSTOM,
+        [typeof(AssistantTranslation)] = ChatSystemPromptSource.CUSTOM,
+    };
+
+    private static IEnumerable<Type> Assistants() => DECISIONS.Keys;
+
+    [TestCaseSource(nameof(Assistants))]
+    public void TheAssistantKeepsItsDecision(Type assistant) => Assert.That(SourceOf(assistant), Is.EqualTo(DECISIONS[assistant]));
+
+    [Test]
+    public void EveryAssistantIsListed()
+    {
+        var assistants = APP.GetTypes()
+            .Where(type => type is { IsAbstract: false, IsClass: true } && IsAssistant(type))
+            .Where(type => type != typeof(AssistantDynamic))
+            .ToList();
+
+        Assert.That(assistants, Is.EquivalentTo(DECISIONS.Keys), "Each assistant needs its decision in this table.");
+    }
+
+    private static ChatSystemPromptSource SourceOf(Type assistant)
+    {
+        var instance = RuntimeHelpers.GetUninitializedObject(assistant);
+        var property = assistant.GetProperty("ChatSystemPrompt", BindingFlags.Instance | BindingFlags.NonPublic)!;
+        return ((ChatSystemPromptChoice)property.GetValue(instance)!).Source;
+    }
+
+    private static bool IsAssistant(Type type)
+    {
+        for (var current = type.BaseType; current is not null; current = current.BaseType)
+            if (current.IsGenericType && current.GetGenericTypeDefinition() == typeof(AssistantBase<>))
+                return true;
+
+        return false;
+    }
+}

app/Tests/Assistants/SWOTAnalysis/SwotAnalysisSendToChatTests.cs

@@ -26,7 +26,6 @@ public sealed class SwotAnalysisSendToChatTests
             Assert.That(chatThread.Blocks[1].HideFromUser, Is.False);
             Assert.That(((ContentText)chatThread.Blocks[1].Content!).Text, Is.EqualTo("## SWOT Matrix"));
             Assert.That(chatThread.Blocks[1].Content, Is.Not.SameAs(result.Content));
-            Assert.That(chatThread.SystemPrompt, Is.EqualTo(SystemPrompts.DEFAULT));
         }
     }
 

app/Tests/Chat/ChatThreadSystemPromptTests.cs

@@ -1,5 +1,9 @@
+using System.Text.Json;
+
+using AIStudio.Assistants;
 using AIStudio.Chat;
 using AIStudio.Settings;
+using AIStudio.Tools;
 using AIStudio.Tools.Services;
 using AIStudio.Tools.ToolCallingSystem;
 
@@ -37,6 +41,42 @@ public sealed class ChatThreadSystemPromptTests
             Assert.That(prepared.Text, Is.EqualTo(BASE_PROMPT));
             Assert.That(prepared.Parts.Select(part => part.Kind), Is.EqualTo(new[] { SystemPromptPartKind.BASE_PROMPT }));
             Assert.That(prepared.SwitchedOffProfile, Is.EqualTo(Profile.NO_PROFILE));
+            Assert.That(prepared.Parts[0].Origin, Is.Null, "Only a chat which continues the conversation of an assistant names one.");
+        }
+    }
+
+    [Test]
+    public void TheBasePromptNamesTheAssistantWhichChoseIt()
+    {
+        using var rustService = new RustService("1", "unused");
+        var settings = Settings(rustService, allowProfileUsage: true);
+        var origin = new ChatSystemPromptOrigin { Source = ChatSystemPromptSource.CUSTOM, AssistantName = "Translation" };
+        var thread = Thread();
+        thread.SystemPromptOrigin = origin;
+
+        var prepared = thread.BuildSystemPrompt(settings);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(prepared.Parts.Single().Kind, Is.EqualTo(SystemPromptPartKind.BASE_PROMPT));
+            Assert.That(prepared.Parts.Single().Origin, Is.EqualTo(origin));
+            Assert.That(prepared.Text, Is.EqualTo(BASE_PROMPT), "Naming the assistant must not change the prompt.");
+        }
+    }
+
+    [Test]
+    public void AStoredChatKeepsWhichAssistantChoseItsPrompt()
+    {
+        var thread = Thread();
+        thread.SystemPromptOrigin = new ChatSystemPromptOrigin { Source = ChatSystemPromptSource.CUSTOM, AssistantName = "Translation" };
+
+        var json = JsonSerializer.Serialize(thread, WorkspaceBehaviour.JSON_OPTIONS);
+        var stored = JsonSerializer.Deserialize<ChatThread>(json, WorkspaceBehaviour.JSON_OPTIONS)!;
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(json, Does.Contain("\"CUSTOM\""), "The source is stored by its name, which assistant plugins use as well.");
+            Assert.That(stored.SystemPromptOrigin, Is.EqualTo(thread.SystemPromptOrigin));
         }
     }
 

app/Tests/Tools/PluginSystem/PluginAssistantsChatSystemPromptTests.cs

@@ -0,0 +1,120 @@
+using AIStudio.Assistants;
+using AIStudio.Tools.PluginSystem;
+using AIStudio.Tools.PluginSystem.Assistants;
+
+using Lua;
+
+namespace AIStudio.Tests.Tools.PluginSystem;
+
+/// <summary>
+/// Checks how an assistant plugin chooses the system prompt of a chat which continues its conversation.
+/// </summary>
+/// <remarks>
+/// The choice is optional, so plugins written before it existed keep loading, and their chats keep the
+/// default prompt of AI Studio. A choice which cannot work, though, keeps the plugin from loading: its
+/// author would otherwise only notice it in the chat, if at all.
+/// </remarks>
+[TestFixture]
+public sealed class PluginAssistantsChatSystemPromptTests
+{
+    private const string PROMPT_FOR_THE_CHAT = "You explain plain language and answer follow-up questions about the translation.";
+
+    [Test]
+    public async Task WithoutAChoiceTheChatGetsTheDefaultPrompt()
+    {
+        using var state = PluginFactory.CreateLuaState(null);
+        var table = await AssistantTableAsync(state, """["Title"] = "Plain language" """);
+
+        var isValid = PluginAssistants.TryReadChatSystemPrompt(table, out var choice, out var rawCustomPrompt, out var message);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(isValid, Is.True, message);
+            Assert.That(choice.Source, Is.EqualTo(ChatSystemPromptSource.AI_STUDIO_DEFAULT));
+            Assert.That(rawCustomPrompt, Is.Empty);
+        }
+    }
+
+    [TestCase("AI_STUDIO_DEFAULT", ChatSystemPromptSource.AI_STUDIO_DEFAULT)]
+    [TestCase("ASSISTANT", ChatSystemPromptSource.ASSISTANT)]
+    [TestCase("assistant", ChatSystemPromptSource.ASSISTANT)]
+    public async Task APluginChoosesTheDefaultPromptOrItsOwnSystemPrompt(string value, ChatSystemPromptSource expectedSource)
+    {
+        using var state = PluginFactory.CreateLuaState(null);
+        var table = await AssistantTableAsync(state, $"""["ChatSystemPrompt"] = "{value}" """);
+
+        var isValid = PluginAssistants.TryReadChatSystemPrompt(table, out var choice, out var rawCustomPrompt, out var message);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(isValid, Is.True, message);
+            Assert.That(choice.Source, Is.EqualTo(expectedSource));
+            Assert.That(rawCustomPrompt, Is.Empty);
+        }
+    }
+
+    [Test]
+    public async Task APromptForTheChatGetsTheSecurityPolicyOfThePlugin()
+    {
+        using var state = PluginFactory.CreateLuaState(null);
+        var table = await AssistantTableAsync(state, $"""["ChatSystemPrompt"] = "CUSTOM", ["CustomChatSystemPrompt"] = "  {PROMPT_FOR_THE_CHAT}  " """);
+
+        var isValid = PluginAssistants.TryReadChatSystemPrompt(table, out var choice, out var rawCustomPrompt, out var message);
+        var prompt = choice.CreatePrompt("The system prompt of the assistant.");
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(isValid, Is.True, message);
+            Assert.That(choice.Source, Is.EqualTo(ChatSystemPromptSource.CUSTOM));
+            Assert.That(rawCustomPrompt, Is.EqualTo(PROMPT_FOR_THE_CHAT), "The audit reads the prompt as the plugin states it.");
+            Assert.That(prompt, Does.Contain(PROMPT_FOR_THE_CHAT));
+            Assert.That(prompt, Does.StartWith("You are a secure assistant"), "The prompt for the chat must not escape the security policy every prompt of a plugin gets.");
+            Assert.That(prompt, Does.Contain("Security reminder"));
+        }
+    }
+
+    [TestCase("\"UNKNOWN\"")]
+    [TestCase("\"1\"")]
+    [TestCase("1")]
+    [TestCase("true")]
+    [TestCase("\"\"")]
+    public async Task AnInvalidChoiceKeepsThePluginFromLoading(string luaValue)
+    {
+        using var state = PluginFactory.CreateLuaState(null);
+        var table = await AssistantTableAsync(state, $"""["ChatSystemPrompt"] = {luaValue}""");
+
+        var isValid = PluginAssistants.TryReadChatSystemPrompt(table, out _, out _, out var message);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(isValid, Is.False);
+            Assert.That(message, Is.Not.Empty);
+        }
+    }
+
+    [TestCase("""["ChatSystemPrompt"] = "CUSTOM" """)]
+    [TestCase("""["ChatSystemPrompt"] = "CUSTOM", ["CustomChatSystemPrompt"] = "   " """)]
+    [TestCase("""["ChatSystemPrompt"] = "CUSTOM", ["CustomChatSystemPrompt"] = 42""")]
+    [TestCase("""["ChatSystemPrompt"] = "ASSISTANT", ["CustomChatSystemPrompt"] = "Answer follow-up questions." """)]
+    [TestCase("""["CustomChatSystemPrompt"] = "Answer follow-up questions." """)]
+    public async Task APromptForTheChatNeedsTheCustomChoiceAndViceVersa(string fields)
+    {
+        using var state = PluginFactory.CreateLuaState(null);
+        var table = await AssistantTableAsync(state, fields);
+
+        var isValid = PluginAssistants.TryReadChatSystemPrompt(table, out _, out _, out var message);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(isValid, Is.False);
+            Assert.That(message, Is.Not.Empty);
+        }
+    }
+
+    private static async Task<LuaTable> AssistantTableAsync(LuaState state, string fields)
+    {
+        var results = await state.DoStringAsync($"return {{ {fields} }}");
+        Assert.That(results[0].TryRead<LuaTable>(out var table), Is.True, "The test needs a Lua table.");
+        return table;
+    }
+}