diff --git a/app/MindWork AI Studio/Settings/ProviderCapabilityOverrides.cs b/app/MindWork AI Studio/Settings/ProviderCapabilityOverrides.cs index 4c001f1d..598e1cd3 100644 --- a/app/MindWork AI Studio/Settings/ProviderCapabilityOverrides.cs +++ b/app/MindWork AI Studio/Settings/ProviderCapabilityOverrides.cs @@ -304,16 +304,16 @@ public sealed record ProviderCapabilityOverrides /// Works out how a model reasons, out of what the rules say and what a person said. /// /// - /// This replaces thirty lines which repaired states that could not exist -- a model both always + /// This replaced thirty lines which repaired states that cannot exist -- a model both always /// reasoning and reasoning on request -- by an answer which cannot be in two of them at once. - /// The expert dialog writes all three words together, so the five combinations it produces are - /// answered exactly as they are today. + /// The expert dialog writes all three words together, and every combination it produces means + /// exactly what it meant before. /// - /// One thing changes, and it is a defect going away. A word nobody said anything about used to - /// destroy the answer: a provider carrying any override at all, say tool calling turned off, lost - /// "reasoning on by default" on the way through, because the old repair took the word away unless - /// "reasoning on request" stood next to it -- which no rule ever states. Here a "no" only takes - /// away what it names. + /// One thing did change, and it is a defect going away. A word nobody said anything about used + /// to destroy the answer: a provider carrying any override at all, say tool calling turned off, + /// lost "reasoning on by default" on the way through, because the repair took the word away + /// unless "reasoning on request" stood next to it -- which no rule ever states. Here a "no" only + /// takes away what it names. /// /// How the rules say the model reasons. /// How it reasons after the overrides. @@ -340,53 +340,6 @@ public sealed record ProviderCapabilityOverrides }; } - public List ApplyTo(IEnumerable automaticCapabilities) - { - var mergedCapabilities = automaticCapabilities.Distinct().ToList(); - foreach (var capability in SUPPORTED_CAPABILITIES) - { - var overrideValue = this.GetOverride(capability); - if (overrideValue == true && !mergedCapabilities.Contains(capability)) - mergedCapabilities.Add(capability); - else if (overrideValue == false) - mergedCapabilities.Remove(capability); - } - - this.NormalizeReasoningCapabilities(mergedCapabilities); - return mergedCapabilities; - } - - private void NormalizeReasoningCapabilities(List capabilities) - { - if (this.AlwaysReasoning == true || - this.AlwaysReasoning is not false && - this.OptionalReasoning is not true && - this.ReasoningByDefault is not true && - capabilities.Contains(Capability.ALWAYS_REASONING)) - { - capabilities.Remove(Capability.OPTIONAL_REASONING); - capabilities.Remove(Capability.REASONING_BY_DEFAULT); - return; - } - - if (this.AlwaysReasoning == false || - this.OptionalReasoning == true || - this.ReasoningByDefault == true) - capabilities.Remove(Capability.ALWAYS_REASONING); - - if (this.OptionalReasoning == false) - { - capabilities.Remove(Capability.REASONING_BY_DEFAULT); - return; - } - - if (this.ReasoningByDefault == true && !capabilities.Contains(Capability.OPTIONAL_REASONING)) - capabilities.Add(Capability.OPTIONAL_REASONING); - - if (!capabilities.Contains(Capability.OPTIONAL_REASONING)) - capabilities.Remove(Capability.REASONING_BY_DEFAULT); - } - public string ExportAsLuaTable(string indentation) { if (!this.HasOverrides) diff --git a/app/Tests/Settings/ProviderCapabilityOverridesTests.cs b/app/Tests/Settings/ProviderCapabilityOverridesTests.cs index bfab7428..c6d38ed2 100644 --- a/app/Tests/Settings/ProviderCapabilityOverridesTests.cs +++ b/app/Tests/Settings/ProviderCapabilityOverridesTests.cs @@ -9,14 +9,19 @@ namespace AIStudio.Tests.Settings; /// /// /// The expert dialog writes the three reasoning words together, in five combinations. Those five -/// are the whole surface the app produces, so they are the ones held against the code being -/// replaced: every one of them has to come out of the new resolution exactly as it comes out of the -/// old repair today. +/// are the whole surface the app produces, so each of them is stated below with the one thing it +/// means -- whatever the rules said about the model, because that is what choosing from a list of +/// five does. /// -/// A configuration plugin can write the three words one at a time, and there the two differ on -/// purpose. The old repair took a word away unless another one stood next to it, so an override -/// about something else destroyed an answer nobody had touched. Those cases are stated below, one -/// by one, with what they answer now. +/// These used to be measured against the repair they replaced, by running both and comparing. That +/// comparison is gone with the repair itself: keeping a dead implementation alive so a test can ask +/// it questions makes the test the only reason it still exists, and the next reader cannot tell +/// which of the two is the real one. What it guaranteed is written out instead. +/// +/// A configuration plugin can write the three words one at a time, and there the two differed on +/// purpose. The repair took a word away unless another one stood next to it, so an override about +/// something else destroyed an answer nobody had touched. Those cases are stated below, one by one, +/// with what they answer now. /// [TestFixture] public sealed class ProviderCapabilityOverridesTests @@ -24,55 +29,53 @@ public sealed class ProviderCapabilityOverridesTests private static readonly ReasoningSupport[] EVERY_STATE = [ReasoningSupport.NONE, ReasoningSupport.OPTIONAL, ReasoningSupport.ON_BY_DEFAULT, ReasoningSupport.ALWAYS]; /// - /// The five combinations the expert dialog writes, in the order its list shows them. + /// The four combinations the expert dialog writes, in the order its list shows them, and the + /// one state each of them means. /// /// - /// "Automatic" is not among them. It means the person said nothing, and a provider carrying - /// nothing but nothing is saved without an override record at all, so it never reaches here -- - /// which is exactly why the defect below went unnoticed for so long: it needed a second, - /// unrelated switch to become visible. + /// "Automatic" is the fifth choice and is not among them. It means the person said nothing, and + /// a provider carrying nothing but nothing is saved without an override record at all, so it + /// never reaches here -- which is exactly why the defect below went unnoticed for so long: it + /// needed a second, unrelated switch to become visible. /// - private static readonly ProviderCapabilityOverrides[] WHAT_THE_DIALOG_WRITES = + private static readonly (ProviderCapabilityOverrides Overrides, ReasoningSupport Means)[] WHAT_THE_DIALOG_WRITES = [ - new() { AlwaysReasoning = false, OptionalReasoning = false, ReasoningByDefault = false }, - new() { AlwaysReasoning = false, OptionalReasoning = true, ReasoningByDefault = false }, - new() { AlwaysReasoning = false, OptionalReasoning = true, ReasoningByDefault = true }, - new() { AlwaysReasoning = true, OptionalReasoning = false, ReasoningByDefault = false }, + (new() { AlwaysReasoning = false, OptionalReasoning = false, ReasoningByDefault = false }, ReasoningSupport.NONE), + (new() { AlwaysReasoning = false, OptionalReasoning = true, ReasoningByDefault = false }, ReasoningSupport.OPTIONAL), + (new() { AlwaysReasoning = false, OptionalReasoning = true, ReasoningByDefault = true }, ReasoningSupport.ON_BY_DEFAULT), + (new() { AlwaysReasoning = true, OptionalReasoning = false, ReasoningByDefault = false }, ReasoningSupport.ALWAYS), ]; [Test] - public void EveryChoiceTheExpertDialogOffersMeansTheSameAsItDoesToday() + public void EveryChoiceTheExpertDialogOffersMeansOneStateAndNothingElse() { + // + // Whatever the rules said about the model is beside the point here: somebody picked one of + // five entries from a list, and each entry says outright how this model reasons. That is + // also what makes these four the cheapest guard there is against somebody rearranging the + // resolution below them. + // Assert.Multiple(() => { - foreach (var overrides in WHAT_THE_DIALOG_WRITES) + foreach (var (overrides, means) in WHAT_THE_DIALOG_WRITES) foreach (var stated in EVERY_STATE) - { - var rebuilt = overrides.ApplyTo(ProfileWhichReasons(stated)).Reasoning; - var today = ReasoningOf(overrides.ApplyTo(CapabilitiesWhichReason(stated))); - - Assert.That(rebuilt, Is.EqualTo(today), $"A model which reasons {stated}, with {Describe(overrides)}."); - } + Assert.That(overrides.ApplyTo(ProfileWhichReasons(stated)).Reasoning, Is.EqualTo(means), $"A model which reasons {stated}, with {Describe(overrides)}."); }); } [Test] - public void AnOverrideAboutSomethingElseNoLongerTakesTheThinkingAway() + public void AnOverrideAboutSomethingElseLeavesTheThinkingAlone() { // - // The defect this replaces. Turning tool calling off said nothing about reasoning, and yet + // The defect this replaced. Turning tool calling off said nothing about reasoning, and yet // a model which thinks unless asked not to came out of it as a model which never thinks -- // because the repair kept "on by default" only where "on request" stood next to it, which - // no rule has ever stated. + // no rule has ever stated. It cannot come back through this door: the answer is one value + // now, and the combination the repair existed for cannot be written down any more. // var overrides = new ProviderCapabilityOverrides { FunctionCalling = false }; - var thinker = ProfileWhichReasons(ReasoningSupport.ON_BY_DEFAULT); - Assert.Multiple(() => - { - Assert.That(overrides.ApplyTo(thinker).Reasoning, Is.EqualTo(ReasoningSupport.ON_BY_DEFAULT)); - Assert.That(ReasoningOf(overrides.ApplyTo(CapabilitiesWhichReason(ReasoningSupport.ON_BY_DEFAULT))), Is.EqualTo(ReasoningSupport.NONE), "Which is what it used to answer, and the reason this test exists."); - }); + Assert.That(overrides.ApplyTo(ProfileWhichReasons(ReasoningSupport.ON_BY_DEFAULT)).Reasoning, Is.EqualTo(ReasoningSupport.ON_BY_DEFAULT)); } [TestCase(ReasoningSupport.ALWAYS, ReasoningSupport.ALWAYS, Description = "Saying it is not on by default says nothing about a model which cannot turn it off.")] @@ -127,50 +130,5 @@ public sealed class ProviderCapabilityOverridesTests Reasoning = reasoning, }; - /// - /// The same model, written the way the rules being replaced answer. - /// - /// How the model reasons. - /// The capabilities, with the one word which stands for that state. - private static List CapabilitiesWhichReason(ReasoningSupport reasoning) - { - List capabilities = [Capability.TEXT_INPUT, Capability.TEXT_OUTPUT]; - switch (reasoning) - { - case ReasoningSupport.OPTIONAL: - capabilities.Add(Capability.OPTIONAL_REASONING); - break; - - case ReasoningSupport.ON_BY_DEFAULT: - capabilities.Add(Capability.REASONING_BY_DEFAULT); - break; - - case ReasoningSupport.ALWAYS: - capabilities.Add(Capability.ALWAYS_REASONING); - break; - } - - return capabilities; - } - - /// - /// Which state a list of capabilities stands for, read the way the app reads it today. - /// - /// The capabilities. - /// The state they stand for. - private static ReasoningSupport ReasoningOf(List capabilities) - { - if (capabilities.Contains(Capability.ALWAYS_REASONING)) - return ReasoningSupport.ALWAYS; - - if (capabilities.Contains(Capability.REASONING_BY_DEFAULT)) - return ReasoningSupport.ON_BY_DEFAULT; - - if (capabilities.Contains(Capability.OPTIONAL_REASONING)) - return ReasoningSupport.OPTIONAL; - - return ReasoningSupport.NONE; - } - private static string Describe(ProviderCapabilityOverrides overrides) => $"always={overrides.AlwaysReasoning?.ToString() ?? "auto"}, optional={overrides.OptionalReasoning?.ToString() ?? "auto"}, byDefault={overrides.ReasoningByDefault?.ToString() ?? "auto"}"; } \ No newline at end of file