diff --git a/app/MindWork AI Studio/Components/DirectChatLauncherForm.razor b/app/MindWork AI Studio/Components/DirectChatLauncherForm.razor index af72de38..2ccad984 100644 --- a/app/MindWork AI Studio/Components/DirectChatLauncherForm.razor +++ b/app/MindWork AI Studio/Components/DirectChatLauncherForm.razor @@ -51,4 +51,20 @@ } - \ No newline at end of file +@* A chat template wins over what is chosen here, so the form says so before somebody picks + something which would never take effect: *@ +@if (this.SelectedChatTemplate.DataSourceOptions is not null) +{ + + @T("The chosen chat template brings data sources of its own, and those win over a selection made here. Only a template can also leave the choice of sources to the AI, which is why it decides this on its own.") + +} + + + +@if (this.SelectedChatTemplate.ToolIds is not null) +{ + + @T("The chosen chat template brings tools of its own, and those win over a selection made here.") + +} \ No newline at end of file diff --git a/app/MindWork AI Studio/Components/DirectChatLauncherForm.razor.cs b/app/MindWork AI Studio/Components/DirectChatLauncherForm.razor.cs index bdd363ac..902d39ee 100644 --- a/app/MindWork AI Studio/Components/DirectChatLauncherForm.razor.cs +++ b/app/MindWork AI Studio/Components/DirectChatLauncherForm.razor.cs @@ -1,3 +1,5 @@ +using AIStudio.Settings; + using Microsoft.AspNetCore.Components; namespace AIStudio.Components; @@ -81,6 +83,19 @@ public partial class DirectChatLauncherForm : MSGComponentBase /// private bool OpensTemporaryChat => string.IsNullOrWhiteSpace(this.WorkspaceName); + /// + /// The chat template the launcher would open its chat with, as far as it is known here. + /// + /// + /// With "use chat default" chosen, this is whichever template the chat options name right now, + /// and that may well be another one by the time somebody opens the launcher. The form therefore + /// only says what such a template brings along instead of disabling the fields below it: a field + /// which locks itself behind the user's back is worse than a sentence explaining the situation. + /// + private ChatTemplate SelectedChatTemplate => string.IsNullOrWhiteSpace(this.ChatTemplateId) + ? this.SettingsManager.GetPreselectedChatTemplate(Tools.Components.CHAT) + : this.SettingsManager.GetChatTemplateById(this.ChatTemplateId); + private IReadOnlyList availableWorkspaces = []; private static readonly Dictionary USER_INPUT_ATTRIBUTES = new(); diff --git a/app/MindWork AI Studio/Settings/ChatTemplate.cs b/app/MindWork AI Studio/Settings/ChatTemplate.cs index 12532743..afd114ef 100644 --- a/app/MindWork AI Studio/Settings/ChatTemplate.cs +++ b/app/MindWork AI Studio/Settings/ChatTemplate.cs @@ -102,10 +102,49 @@ public record ChatTemplate( { if(this.Num == uint.MaxValue) return string.Empty; - + return this.SystemPrompt; } + /// + /// Decides whose tools a chat started by a launcher begins with. + /// + /// + /// A launcher may name tools itself and may choose a chat template which names tools as well. + /// When both do, the template wins as a whole — the same rule as for the data sources, so that + /// nobody has to remember two of them. + /// + /// The chat template the launcher opens its chat with. + /// The tools the launcher names itself, or null when it names none. + /// The tools to start with — null when neither says anything, which leaves the chat default in place — and whether the launcher's own choice was dropped for it. + public static (IReadOnlyCollection? ToolIds, bool LauncherChoiceDropped) ChooseToolIds(ChatTemplate chatTemplate, IReadOnlyCollection? launcherToolIds) + { + if (chatTemplate.ToolIds is not { } templateToolIds) + return (launcherToolIds, false); + + return (templateToolIds, launcherToolIds is not null); + } + + /// + /// Decides whose data source options a chat started by a launcher begins with. + /// + /// + /// The two sides are not equally expressive: a launcher can only ever say "these sources, picked + /// by hand", while a chat template carries the whole options and can also say "let an agent pick + /// them for each message". Mixing them field by field would produce something neither of them + /// asked for, so the template wins as a whole. + /// + /// The chat template the launcher opens its chat with. + /// The options built from the data sources the launcher names, or null when it names none. + /// The options to start with — null when neither says anything, which leaves the chat default in place — and whether the launcher's own choice was dropped for them. + public static (DataSourceOptions? Options, bool LauncherChoiceDropped) ChooseDataSourceOptions(ChatTemplate chatTemplate, DataSourceOptions? launcherOptions) + { + if (chatTemplate.DataSourceOptions is not { } templateOptions) + return (launcherOptions, false); + + return (templateOptions.CreateCopy(), launcherOptions is not null); + } + public static bool TryParseChatTemplateTable(int idx, LuaTable table, Guid configPluginId, string pluginPath, out ConfigurationBaseObject template) { template = NO_CHAT_TEMPLATE; diff --git a/app/MindWork AI Studio/Tools/Services/DirectChatService.cs b/app/MindWork AI Studio/Tools/Services/DirectChatService.cs index b89f3cea..0bbd5ffe 100644 --- a/app/MindWork AI Studio/Tools/Services/DirectChatService.cs +++ b/app/MindWork AI Studio/Tools/Services/DirectChatService.cs @@ -45,7 +45,7 @@ public sealed class DirectChatService(SettingsManager settingsManager, DataSourc profile = Profile.NO_PROFILE; } - var dataSourceOptionsResult = await this.ResolveDataSourceOptionsAsync(providerResult.Provider, launchConfiguration.DataSourceIds); + var dataSourceOptionsResult = await this.ResolveDataSourceOptionsAsync(assistantPlugin, providerResult.Provider, chatTemplate, launchConfiguration.DataSourceIds); var dataSourceOptions = dataSourceOptionsResult.Options; if (dataSourceOptions is null) return new(null, dataSourceOptionsResult.ErrorMessage); @@ -75,15 +75,21 @@ public sealed class DirectChatService(SettingsManager settingsManager, DataSourc } } + var toolChoice = ChatTemplate.ChooseToolIds(chatTemplate, launchConfiguration.ToolIds); + if (toolChoice.LauncherChoiceDropped) + logger.LogWarning( + "Assistant plugin '{PluginName}' selects the tools '{LauncherToolIds}', but its chat template '{ChatTemplateName}' names tools of its own. The chat starts with the tools of that template.", + assistantPlugin.Name, string.Join(", ", launchConfiguration.ToolIds!), chatTemplate.GetSafeName()); + // - // Only the tools the user could have switched on themselves. A launcher may name one whose + // Only the tools the user could have switched on themselves. Either side may name one whose // settings are incomplete — an unconfigured web search, say — and starting the chat with it // enabled would show a state the user cannot produce by hand and cannot fix from the chat. // Null keeps the chat's own defaults, which is what a launcher without tools wants. // - var selectedToolIds = launchConfiguration.ToolIds is null + var selectedToolIds = toolChoice.ToolIds is null ? null - : await toolRegistry.FilterSelectableToolIdsAsync(Components.CHAT, launchConfiguration.ToolIds); + : await toolRegistry.FilterSelectableToolIdsAsync(Components.CHAT, toolChoice.ToolIds); var chatThread = new ChatThread { @@ -101,7 +107,12 @@ public sealed class DirectChatService(SettingsManager settingsManager, DataSourc Blocks = chatTemplate == ChatTemplate.NO_CHAT_TEMPLATE ? [] : chatTemplate.ExampleConversation.Select(block => block.DeepClone()).ToList(), }; - return new(new(chatThread, ApplySelectedChatTemplateToComposer: true, PreserveDataSourceOptions: launchConfiguration.DataSourceIds is not null), string.Empty); + // + // Whoever decided these options — the chat template or the launcher — decided them for this + // chat. Without saying so, the chat page would replace them with the chat defaults again: + // + var dataSourcesWereChosen = chatTemplate.DataSourceOptions is not null || launchConfiguration.DataSourceIds is not null; + return new(new(chatThread, ApplySelectedChatTemplateToComposer: true, PreserveDataSourceOptions: dataSourcesWereChosen), string.Empty); } private (ProviderSettings Provider, bool IsExplicit, string ErrorMessage) ResolveProvider(Guid? providerId) @@ -167,64 +178,119 @@ public sealed class DirectChatService(SettingsManager settingsManager, DataSourc : new(chatTemplate, string.Empty); } - private async Task<(DataSourceOptions? Options, string ErrorMessage)> ResolveDataSourceOptionsAsync(ProviderSettings provider, IReadOnlyList? dataSourceIds) + private async Task<(DataSourceOptions? Options, string ErrorMessage)> ResolveDataSourceOptionsAsync(PluginAssistants assistantPlugin, ProviderSettings provider, ChatTemplate chatTemplate, IReadOnlyList? launcherDataSourceIds) { - if (dataSourceIds is null) + // + // The launcher names data sources as plain IDs, and the options around them are always the + // same ones. Building them here turns its choice into the same kind of thing the chat + // template carries, which is what lets one rule decide between the two. + // + DataSourceOptions? launcherOptions = null; + if (launcherDataSourceIds is not null) + { + var standardOptions = settingsManager.ConfigurationData.Chat.PreselectedDataSourceOptions; + launcherOptions = new DataSourceOptions + { + DisableDataSources = false, + AutomaticDataSourceSelection = false, + AutomaticValidation = standardOptions.AutomaticValidation, + PreselectedDataSourceIds = launcherDataSourceIds.Select(dataSourceId => dataSourceId.ToString()).ToList(), + }; + } + + var optionsChoice = ChatTemplate.ChooseDataSourceOptions(chatTemplate, launcherOptions); + if (optionsChoice.LauncherChoiceDropped) + logger.LogWarning( + "Assistant plugin '{PluginName}' selects the data sources '{LauncherDataSourceIds}', but its chat template '{ChatTemplateName}' brings data source options of its own. The chat starts with the data sources of that template.", + assistantPlugin.Name, string.Join(", ", launcherDataSourceIds!), chatTemplate.GetSafeName()); + + // Neither side says anything, so the chat starts the way it would start on its own: + if (optionsChoice.Options is not { } chosenOptions) return new(settingsManager.ConfigurationData.Chat.PreselectedDataSourceOptions.CreateCopy(), string.Empty); + return await this.CheckChosenDataSourcesAsync(provider, chosenOptions, chatTemplate.DataSourceOptions is null ? null : chatTemplate); + } + + /// + /// Checks that the chosen data sources exist and may be used with the provider of the chat. + /// + /// + /// Opening a launcher is one click, so a source which is gone or not permitted has to be said + /// out loud instead of being dropped quietly: nobody would see what the chat is missing. Which + /// of the two sides chose the sources changes nothing but the wording — and that wording is the + /// only place where the user learns which of them to go and fix. + /// + /// The provider the launched chat runs with. + /// The options the chat is about to start with. + /// The chat template the options came from, or null when the launcher named the sources itself. + /// The checked options, or null and a message saying why no chat was created. + private async Task<(DataSourceOptions? Options, string ErrorMessage)> CheckChosenDataSourcesAsync(ProviderSettings provider, DataSourceOptions chosenOptions, ChatTemplate? originChatTemplate) + { + // + // There is nothing to check when data sources are switched off, and nothing to check either + // when an agent picks them: that choice is made per message in the chat, exactly as it is + // for a chat template the user picks by hand. + // + if (chosenOptions.DisableDataSources || chosenOptions.AutomaticDataSourceSelection || chosenOptions.PreselectedDataSourceIds.Count == 0) + return new(chosenOptions, string.Empty); + // // Deciding which data sources are permitted needs an effective provider. Without one, // the check below would report every requested source as unavailable, which would hide // the actual cause from the user: // if (provider == ProviderSettings.NONE) - return new(null, TB("The assistant chat launcher selects data sources, but no provider is available for chats. Please choose a default provider for chats first. No chat was created.")); + return new(null, originChatTemplate is null + ? TB("The assistant chat launcher selects data sources, but no provider is available for chats. Please choose a default provider for chats first. No chat was created.") + : string.Format(TB("The chat template '{0}' selects data sources, but no provider is available for chats. Please choose a default provider for chats first. No chat was created."), originChatTemplate.GetSafeName())); - var requestedDataSources = new List(dataSourceIds.Count); - foreach (var dataSourceId in dataSourceIds) + var requestedDataSources = new List(chosenOptions.PreselectedDataSourceIds.Count); + foreach (var dataSourceId in chosenOptions.PreselectedDataSourceIds) { // Data sources have no lookup helper in the settings manager, so we match their ids // the same way the rest of the app does: - var dataSourceIdText = dataSourceId.ToString(); var dataSource = settingsManager.ConfigurationData.DataSources.FirstOrDefault(candidate => - string.Equals(candidate.Id, dataSourceIdText, StringComparison.OrdinalIgnoreCase)); + string.Equals(candidate.Id, dataSourceId, StringComparison.OrdinalIgnoreCase)); if (dataSource is null) - return new(null, string.Format(TB("The assistant chat launcher references data source '{0}', but that data source does not exist."), dataSourceId)); + return new(null, originChatTemplate is null + ? string.Format(TB("The assistant chat launcher references data source '{0}', but that data source does not exist."), dataSourceId) + : string.Format(TB("The chat template '{0}' references data source '{1}', but that data source does not exist."), originChatTemplate.GetSafeName(), dataSourceId)); requestedDataSources.Add(dataSource); } // - // The options the launched chat will run under. We build them here already, because the - // data-source check depends on them: they decide which agent providers take part, and an - // agent with too little confidence makes a data source unavailable. + // The IDs are written back from the sources they resolved to: one of them may be spelled in + // another case than the source itself, and the chat matches its preselection literally. // - var standardOptions = settingsManager.ConfigurationData.Chat.PreselectedDataSourceOptions; - var launchedDataSourceOptions = new DataSourceOptions - { - DisableDataSources = false, - AutomaticDataSourceSelection = false, - AutomaticValidation = standardOptions.AutomaticValidation, - PreselectedDataSourceIds = requestedDataSources.Select(source => source.Id).ToList(), - }; + chosenOptions.PreselectedDataSourceIds = requestedDataSources.Select(source => source.Id).ToList(); IReadOnlyList availableDataSources; try { - availableDataSources = await dataSourceService.GetAllowedDataSources(provider, launchedDataSourceOptions, requestedDataSources); + // + // The options the launched chat will run under are what this check runs against: they + // decide which agent providers take part, and an agent with too little confidence makes + // a data source unavailable. + // + availableDataSources = await dataSourceService.GetAllowedDataSources(provider, chosenOptions, requestedDataSources); } catch (Exception exception) { - logger.LogError(exception, "The data sources configured by an assistant chat launcher could not be checked."); - return new(null, TB("The data sources selected by the assistant chat launcher could not be checked. No chat was created.")); + logger.LogError(exception, "The data sources an assistant chat launcher would start its chat with could not be checked."); + return new(null, originChatTemplate is null + ? TB("The data sources selected by the assistant chat launcher could not be checked. No chat was created.") + : string.Format(TB("The data sources selected by the chat template '{0}' could not be checked. No chat was created."), originChatTemplate.GetSafeName())); } var availableSelectedIds = availableDataSources.Select(source => source.Id).ToHashSet(StringComparer.OrdinalIgnoreCase); var unavailableDataSources = requestedDataSources.Where(source => !availableSelectedIds.Contains(source.Id)).Select(source => source.Name).ToList(); if (unavailableDataSources.Count > 0) - return new(null, string.Format(TB("The following data sources selected by the assistant chat launcher are currently unavailable or not permitted for the selected provider: {0}"), string.Join(", ", unavailableDataSources))); + return new(null, originChatTemplate is null + ? string.Format(TB("The following data sources selected by the assistant chat launcher are currently unavailable or not permitted for the selected provider: {0}"), string.Join(", ", unavailableDataSources)) + : string.Format(TB("The following data sources selected by the chat template '{0}' are currently unavailable or not permitted for the selected provider: {1}"), originChatTemplate.GetSafeName(), string.Join(", ", unavailableDataSources))); - return new(launchedDataSourceOptions, string.Empty); + return new(chosenOptions, string.Empty); } } \ No newline at end of file diff --git a/app/Tests/Settings/ChatTemplatePrecedenceTests.cs b/app/Tests/Settings/ChatTemplatePrecedenceTests.cs new file mode 100644 index 00000000..282b53c6 --- /dev/null +++ b/app/Tests/Settings/ChatTemplatePrecedenceTests.cs @@ -0,0 +1,165 @@ +using AIStudio.Settings; +using AIStudio.Settings.DataModel; + +namespace AIStudio.Tests.Settings; + +/// +/// Checks who decides the tools and data sources when a direct chat launcher and the chat template +/// it opens its chat with both name some. +/// +/// +/// Either side can be filled in without knowing about the other, so all four combinations happen. +/// The rule is deliberately the same for tools and for data sources: the chat template wins as a +/// whole, because it is the only one of the two which can also leave the choice of sources to an +/// agent, and a field-by-field mix of both would be something neither of them asked for. +/// +[TestFixture] +public sealed class ChatTemplatePrecedenceTests +{ + [Test] + public void WhenNeitherSideSaysAnythingTheChatDefaultsStay() + { + var toolChoice = ChatTemplate.ChooseToolIds(NewTemplate(), null); + var optionsChoice = ChatTemplate.ChooseDataSourceOptions(NewTemplate(), null); + + Assert.Multiple(() => + { + Assert.That(toolChoice.ToolIds, Is.Null, "Nobody named any tool, so the chat has to keep using the tools of its own default."); + Assert.That(toolChoice.LauncherChoiceDropped, Is.False); + Assert.That(optionsChoice.Options, Is.Null, "Nobody named any data source, so the chat has to keep using its own default options."); + Assert.That(optionsChoice.LauncherChoiceDropped, Is.False); + }); + } + + [Test] + public void ALauncherAloneDecidesForItself() + { + var template = NewTemplate(); + var launcherOptions = NewLauncherOptions("11111111-1111-1111-1111-111111111111"); + + var toolChoice = ChatTemplate.ChooseToolIds(template, new[] { "web_search" }); + var optionsChoice = ChatTemplate.ChooseDataSourceOptions(template, launcherOptions); + + Assert.Multiple(() => + { + Assert.That(toolChoice.ToolIds, Is.EquivalentTo(new[] { "web_search" })); + Assert.That(toolChoice.LauncherChoiceDropped, Is.False, "Nothing was dropped here, so nothing may be reported as dropped either."); + Assert.That(optionsChoice.Options, Is.SameAs(launcherOptions)); + Assert.That(optionsChoice.LauncherChoiceDropped, Is.False); + }); + } + + [Test] + public void ATemplateAloneDecidesForItself() + { + var template = NewTemplate() with + { + ToolIds = ["read_web_page"], + DataSourceOptions = new() { DisableDataSources = false, PreselectedDataSourceIds = ["22222222-2222-2222-2222-222222222222"] }, + }; + + var toolChoice = ChatTemplate.ChooseToolIds(template, null); + var optionsChoice = ChatTemplate.ChooseDataSourceOptions(template, null); + + Assert.Multiple(() => + { + Assert.That(toolChoice.ToolIds, Is.EquivalentTo(new[] { "read_web_page" })); + Assert.That(toolChoice.LauncherChoiceDropped, Is.False); + Assert.That(optionsChoice.Options!.PreselectedDataSourceIds, Is.EqualTo(new[] { "22222222-2222-2222-2222-222222222222" })); + Assert.That(optionsChoice.LauncherChoiceDropped, Is.False); + }); + } + + [Test] + public void WhenBothSidesSpeakTheTemplateWinsAndTheLossIsReported() + { + var template = NewTemplate() with + { + ToolIds = ["read_web_page"], + DataSourceOptions = new() { DisableDataSources = false, PreselectedDataSourceIds = ["22222222-2222-2222-2222-222222222222"] }, + }; + + var toolChoice = ChatTemplate.ChooseToolIds(template, new[] { "web_search" }); + var optionsChoice = ChatTemplate.ChooseDataSourceOptions(template, NewLauncherOptions("11111111-1111-1111-1111-111111111111")); + + Assert.Multiple(() => + { + Assert.That(toolChoice.ToolIds, Is.EquivalentTo(new[] { "read_web_page" })); + Assert.That(toolChoice.LauncherChoiceDropped, Is.True, "The tools of the launcher are gone, and only this flag can make the log say so."); + Assert.That(optionsChoice.Options!.PreselectedDataSourceIds, Is.EqualTo(new[] { "22222222-2222-2222-2222-222222222222" })); + Assert.That(optionsChoice.LauncherChoiceDropped, Is.True); + }); + } + + [Test] + public void ATemplateWhichWantsNoToolsWinsJustTheSame() + { + var template = NewTemplate() with { ToolIds = [] }; + + var toolChoice = ChatTemplate.ChooseToolIds(template, new[] { "web_search" }); + + Assert.Multiple(() => + { + Assert.That(toolChoice.ToolIds, Is.Empty, "An empty selection is the statement that this template wants no tools, which is as much of a statement as naming one."); + Assert.That(toolChoice.LauncherChoiceDropped, Is.True); + }); + } + + [Test] + public void TheAgenticSelectionOfATemplateSurvivesALauncherWithItsOwnSources() + { + var template = NewTemplate() with + { + DataSourceOptions = new() { DisableDataSources = false, AutomaticDataSourceSelection = true, PreselectedDataSourceIds = [] }, + }; + + var optionsChoice = ChatTemplate.ChooseDataSourceOptions(template, NewLauncherOptions("11111111-1111-1111-1111-111111111111")); + + Assert.Multiple(() => + { + Assert.That(optionsChoice.Options!.AutomaticDataSourceSelection, Is.True, "Letting an agent pick the sources is the one thing a launcher cannot express, so it is exactly what must not be overwritten by one."); + Assert.That(optionsChoice.Options.PreselectedDataSourceIds, Is.Empty); + Assert.That(optionsChoice.LauncherChoiceDropped, Is.True); + }); + } + + [Test] + public void TheChosenOptionsAreACopyRatherThanTheOnesOfTheTemplate() + { + var template = NewTemplate() with + { + DataSourceOptions = new() { DisableDataSources = false, PreselectedDataSourceIds = ["22222222-2222-2222-2222-222222222222"] }, + }; + + var optionsChoice = ChatTemplate.ChooseDataSourceOptions(template, null); + optionsChoice.Options!.PreselectedDataSourceIds.Clear(); + + Assert.That(template.DataSourceOptions!.PreselectedDataSourceIds, Is.Not.Empty, "The launched chat goes on to change these options, and the template is a setting of the user which must not change with it."); + } + + /// + /// A template which says nothing about tools or data sources. + /// + private static ChatTemplate NewTemplate() => new() + { + Num = 1, + Id = "33333333-3333-3333-3333-333333333333", + Name = "Intranet Research", + SystemPrompt = "You are a research assistant.", + }; + + /// + /// The options a launcher which names data sources ends up with. + /// + /// + /// A launcher has no switches of its own: it names sources, and the rest is always this. Which + /// is the whole reason the chat template wins whenever both of them speak. + /// + private static DataSourceOptions NewLauncherOptions(params string[] dataSourceIds) => new() + { + DisableDataSources = false, + AutomaticDataSourceSelection = false, + AutomaticValidation = false, + PreselectedDataSourceIds = [..dataSourceIds], + }; +} \ No newline at end of file