diff --git a/app/MindWork AI Studio/Tools/ToolCallingSystem/ToolCallingImplementations/WebSearch/WebSearchTool.cs b/app/MindWork AI Studio/Tools/ToolCallingSystem/ToolCallingImplementations/WebSearch/WebSearchTool.cs index 63772cd5..d252965b 100644 --- a/app/MindWork AI Studio/Tools/ToolCallingSystem/ToolCallingImplementations/WebSearch/WebSearchTool.cs +++ b/app/MindWork AI Studio/Tools/ToolCallingSystem/ToolCallingImplementations/WebSearch/WebSearchTool.cs @@ -97,6 +97,21 @@ public sealed class WebSearchTool(IEnumerable backends, WebPa private const string TIME_RANGE_MONTH = "month"; private const string TIME_RANGE_YEAR = "year"; + /// + /// The time ranges a search can be restricted to. + /// + /// + /// Only those which every service with a time filter understands: SearXNG documents day, month, + /// and year, while Tavily knows a week as well. The schema offers exactly these and the reader + /// checks against them, so the two cannot drift apart. + /// + private static readonly string[] TIME_RANGES = [TIME_RANGE_DAY, TIME_RANGE_MONTH, TIME_RANGE_YEAR]; + + /// + /// How much of a wrongly passed argument an error message repeats back to the model. + /// + private const int MAX_ARGUMENT_ECHO_LENGTH = 40; + public string ImplementationKey => ToolSelectionRules.WEB_SEARCH_TOOL_ID; /// @@ -118,7 +133,7 @@ public sealed class WebSearchTool(IEnumerable backends, WebPa Parameters = ToolParameterSchemaBuilder.Create() .RequiredString(QUERY_ARGUMENT, "The search query.") .OptionalString(LANGUAGE_ARGUMENT, "Optional IETF language tag restricting the search to one language, such as 'de-DE', 'en-US', or 'all' for no restriction. Leave it out to search in the language configured for this tool. Do not pass a language name such as 'German': search engines expect the tag and silently return nothing for anything else.") - .OptionalEnum(TIME_RANGE_ARGUMENT, "Optional time range filter for the search.", TIME_RANGE_DAY, TIME_RANGE_MONTH, TIME_RANGE_YEAR) + .OptionalEnum(TIME_RANGE_ARGUMENT, "Optional time range filter for the search.", TIME_RANGES) .OptionalInteger(PAGE_ARGUMENT, "Optional search result page number starting at 1.") .OptionalInteger(LIMIT_ARGUMENT, $"Optional maximum number of ranked result pages to retrieve and return. The hard maximum is {MAX_RESULTS}.") .Build(), @@ -482,14 +497,11 @@ public sealed class WebSearchTool(IEnumerable backends, WebPa public async Task ExecuteAsync(JsonElement arguments, ToolExecutionContext context, CancellationToken token = default) { - var query = ReadRequiredString(arguments, QUERY_ARGUMENT); - var language = ReadOptionalString(arguments, LANGUAGE_ARGUMENT); - var timeRange = ReadOptionalString(arguments, TIME_RANGE_ARGUMENT); - var page = ReadOptionalPositiveInt(arguments, PAGE_ARGUMENT); - var requestedLimit = ReadOptionalPositiveInt(arguments, LIMIT_ARGUMENT); - - if (timeRange is not null && timeRange is not (TIME_RANGE_DAY or TIME_RANGE_MONTH or TIME_RANGE_YEAR)) - throw new ArgumentException($"Invalid time_range '{timeRange}'."); + var query = ReadQuery(arguments); + var language = ReadLanguage(arguments); + var timeRange = ReadTimeRange(arguments); + var page = ReadPage(arguments); + var requestedLimit = ReadLimit(arguments); language = string.IsNullOrWhiteSpace(language) ? context.SettingsValues.GetValueOrDefault(DEFAULT_LANGUAGE_SETTING) : language; var safeSearch = ReadSafeSearchPolicy(context.SettingsValues); @@ -807,42 +819,99 @@ public sealed class WebSearchTool(IEnumerable backends, WebPa return result; } - private static string ReadRequiredString(JsonElement arguments, string propertyName) + /// + /// Reads the search query, the one argument the model always has to pass. + /// + internal static string ReadQuery(JsonElement arguments) { - var value = ReadOptionalString(arguments, propertyName); - if (string.IsNullOrWhiteSpace(value)) - throw new ArgumentException($"Missing required argument '{propertyName}'."); + var query = ReadOptionalString(arguments, QUERY_ARGUMENT, whenLeftOut: null); + if (string.IsNullOrWhiteSpace(query)) + throw new ArgumentException($"Missing required argument '{QUERY_ARGUMENT}'."); - return value; + return query; } - private static string? ReadOptionalString(JsonElement arguments, string propertyName) + /// + /// Reads the language tag the model asked for, or null for the configured language. + /// + internal static string? ReadLanguage(JsonElement arguments) => ReadOptionalString(arguments, LANGUAGE_ARGUMENT, "to use the configured language"); + + /// + /// Reads the time range the model asked for, or null for no restriction. + /// + internal static string? ReadTimeRange(JsonElement arguments) { - if (!arguments.TryGetProperty(propertyName, out var value)) + if (!TryGetArgument(arguments, TIME_RANGE_ARGUMENT, out var value)) return null; - return value.ValueKind switch - { - JsonValueKind.Null => null, - JsonValueKind.String => value.GetString()?.Trim(), - _ => throw new ArgumentException($"Argument '{propertyName}' must be a string."), - }; + var timeRange = value.ValueKind is JsonValueKind.String ? value.GetString()?.Trim() : null; + if (timeRange is null || !TIME_RANGES.Contains(timeRange, StringComparer.Ordinal)) + throw InvalidArgument(TIME_RANGE_ARGUMENT, value, $"one of {string.Join(", ", TIME_RANGES)}", "to search without a time restriction"); + + return timeRange; } - private static int? ReadOptionalPositiveInt(JsonElement arguments, string propertyName) + /// + /// Reads the result page the model asked for, or null for the first one. + /// + internal static int? ReadPage(JsonElement arguments) => ReadOptionalPositiveInt(arguments, PAGE_ARGUMENT, "to get the first page"); + + /// + /// Reads how many results the model asked for, or null for the configured number. + /// + internal static int? ReadLimit(JsonElement arguments) => ReadOptionalPositiveInt(arguments, LIMIT_ARGUMENT, "to get as many results as configured"); + + /// + /// Looks up an argument, treating null the same as leaving it out. + /// + private static bool TryGetArgument(JsonElement arguments, string propertyName, out JsonElement value) => + arguments.TryGetProperty(propertyName, out value) && value.ValueKind is not JsonValueKind.Null; + + private static string? ReadOptionalString(JsonElement arguments, string propertyName, string? whenLeftOut) { - if (!arguments.TryGetProperty(propertyName, out var value)) + if (!TryGetArgument(arguments, propertyName, out var value)) return null; - if (value.ValueKind is JsonValueKind.Null) + if (value.ValueKind is not JsonValueKind.String) + throw InvalidArgument(propertyName, value, "a string", whenLeftOut); + + return value.GetString()?.Trim(); + } + + private static int? ReadOptionalPositiveInt(JsonElement arguments, string propertyName, string whenLeftOut) + { + if (!TryGetArgument(arguments, propertyName, out var value)) return null; if (value.ValueKind is not JsonValueKind.Number || !value.TryGetInt32(out var intValue) || intValue <= 0) - throw new ArgumentException($"Argument '{propertyName}' must be a positive integer."); + throw InvalidArgument(propertyName, value, "a positive integer", whenLeftOut); return intValue; } + /// + /// Builds the error a model gets for an argument it passed wrongly. + /// + /// + /// The model reads this and tries again, so it says what arrived, what would have been right, + /// and, for an optional argument, that leaving it out is always an option. A model which + /// believes the argument has to be there otherwise keeps trying placeholders, and every attempt + /// costs one of the tool calls an answer may make. + /// + /// The argument. + /// What the model passed, as it arrived. + /// What the argument must be, completing "must be ...". + /// What happens without the argument, completing "Leave it out ...", or null for a required one. + private static ArgumentException InvalidArgument(string propertyName, JsonElement value, string expectation, string? whenLeftOut) + { + var receivedValue = value.GetRawText(); + if (receivedValue.Length > MAX_ARGUMENT_ECHO_LENGTH) + receivedValue = $"{receivedValue[..MAX_ARGUMENT_ECHO_LENGTH]}..."; + + var message = $"Argument '{propertyName}' must be {expectation}, but was {receivedValue}."; + return new ArgumentException(whenLeftOut is null ? message : $"{message} Leave it out {whenLeftOut}."); + } + private static string FormatQueryForLog(string query) { var singleLineQuery = query diff --git a/app/Tests/Tools/ToolCalling/WebSearchToolArgumentTests.cs b/app/Tests/Tools/ToolCalling/WebSearchToolArgumentTests.cs new file mode 100644 index 00000000..02f8ca19 --- /dev/null +++ b/app/Tests/Tools/ToolCalling/WebSearchToolArgumentTests.cs @@ -0,0 +1,157 @@ +using System.Text.Json; + +using AIStudio.Tools.ToolCallingSystem.ToolCallingImplementations.WebSearch; + +namespace AIStudio.Tests.Tools.ToolCalling; + +/// +/// Checks how the web search reads the arguments a model passes, and what it says about wrong ones. +/// +/// +/// A wrong argument is refused rather than guessed at: a placeholder such as 0 is not a page, and +/// quietly reading it as "no page" would do something the model did not ask for. What makes the +/// refusal work is its message. The model reads it and tries again, so it has to say what arrived, +/// what would have been right, and that an optional argument may simply be left out. A model +/// which believes it has to pass one otherwise keeps trying placeholders, and every attempt costs +/// one of the tool calls an answer may make. +/// +[TestFixture] +public sealed class WebSearchToolArgumentTests +{ + [Test] + public void AnArgumentLeftOutIsNotSet() + { + var arguments = Arguments("""{"query":"weather"}"""); + + Assert.Multiple(() => + { + Assert.That(WebSearchTool.ReadLanguage(arguments), Is.Null); + Assert.That(WebSearchTool.ReadTimeRange(arguments), Is.Null); + Assert.That(WebSearchTool.ReadPage(arguments), Is.Null); + Assert.That(WebSearchTool.ReadLimit(arguments), Is.Null); + }); + } + + [Test] + public void ANullArgumentIsTheSameAsOneLeftOut() + { + var arguments = Arguments("""{"query":"weather","language":null,"time_range":null,"page":null,"limit":null}"""); + + Assert.Multiple(() => + { + Assert.That(WebSearchTool.ReadLanguage(arguments), Is.Null); + Assert.That(WebSearchTool.ReadTimeRange(arguments), Is.Null); + Assert.That(WebSearchTool.ReadPage(arguments), Is.Null); + Assert.That(WebSearchTool.ReadLimit(arguments), Is.Null); + }); + } + + [Test] + public void ValidArgumentsComeThrough() + { + var arguments = Arguments("""{"query":" weather ","language":"de-DE","time_range":"day","page":2,"limit":7}"""); + + Assert.Multiple(() => + { + Assert.That(WebSearchTool.ReadQuery(arguments), Is.EqualTo("weather")); + Assert.That(WebSearchTool.ReadLanguage(arguments), Is.EqualTo("de-DE")); + Assert.That(WebSearchTool.ReadTimeRange(arguments), Is.EqualTo("day")); + Assert.That(WebSearchTool.ReadPage(arguments), Is.EqualTo(2)); + Assert.That(WebSearchTool.ReadLimit(arguments), Is.EqualTo(7)); + }); + } + + [TestCase("0")] + [TestCase("-1")] + [TestCase("2.5")] + [TestCase("\"5\"")] + [TestCase("true")] + public void AWrongPageIsRefusedWithWhatArrived(string value) + { + var message = Refusal(() => WebSearchTool.ReadPage(Arguments($$"""{"query":"weather","page":{{value}}}"""))); + + Assert.Multiple(() => + { + Assert.That(message, Does.Contain("'page'").And.Contain("a positive integer")); + Assert.That(message, Does.Contain($"but was {value}."), "Without the value, the model cannot tell which of its arguments the tool means."); + Assert.That(message, Does.Contain("Leave it out"), "The way out a model needs when it believes the argument is required."); + }); + } + + [TestCase("0")] + [TestCase("-1")] + [TestCase("\"5\"")] + public void AWrongLimitIsRefusedWithWhatArrived(string value) + { + var message = Refusal(() => WebSearchTool.ReadLimit(Arguments($$"""{"query":"weather","limit":{{value}}}"""))); + + Assert.Multiple(() => + { + Assert.That(message, Does.Contain("'limit'").And.Contain("a positive integer")); + Assert.That(message, Does.Contain($"but was {value}.")); + Assert.That(message, Does.Contain("Leave it out")); + }); + } + + [TestCase("\"\"")] + [TestCase("\"Day\"")] + [TestCase("\"week\"")] + [TestCase("5")] + public void AWrongTimeRangeIsRefusedWithTheValuesThatWouldDo(string value) + { + var message = Refusal(() => WebSearchTool.ReadTimeRange(Arguments($$"""{"query":"weather","time_range":{{value}}}"""))); + + Assert.Multiple(() => + { + Assert.That(message, Does.Contain("'time_range'").And.Contain("one of day, month, year")); + Assert.That(message, Does.Contain($"but was {value}.")); + Assert.That(message, Does.Contain("Leave it out")); + }); + } + + [Test] + public void ALanguageWhichIsNoStringIsRefused() + { + var message = Refusal(() => WebSearchTool.ReadLanguage(Arguments("""{"query":"weather","language":5}"""))); + Assert.That(message, Does.Contain("'language'").And.Contain("but was 5.").And.Contain("Leave it out")); + } + + [Test] + public void AMissingQueryIsRefused() + { + var message = Refusal(() => WebSearchTool.ReadQuery(Arguments("""{"page":1}"""))); + Assert.That(message, Does.Contain("Missing required argument 'query'")); + } + + [Test] + public void AQueryWhichIsNoStringIsRefusedWithoutOfferingToLeaveItOut() + { + var message = Refusal(() => WebSearchTool.ReadQuery(Arguments("""{"query":42}"""))); + + Assert.Multiple(() => + { + Assert.That(message, Does.Contain("'query'").And.Contain("but was 42.")); + Assert.That(message, Does.Not.Contain("Leave it out"), "The query is required, so leaving it out is no way out."); + }); + } + + [Test] + public void ALongValueIsShortenedInTheMessage() + { + var longValue = new string('x', 500); + var message = Refusal(() => WebSearchTool.ReadTimeRange(Arguments($$"""{"query":"weather","time_range":"{{longValue}}"}"""))); + + Assert.Multiple(() => + { + Assert.That(message, Does.Not.Contain(longValue), "The model sent the value itself; repeating all of it back only costs tokens."); + Assert.That(message, Does.Contain("...")); + }); + } + + private static JsonElement Arguments(string json) => JsonSerializer.Deserialize(json); + + /// + /// Runs a reader which has to refuse its argument and returns what it said. + /// + private static string Refusal(TestDelegate read) => Assert.Throws(read)!.Message; +} \ No newline at end of file