diff --git a/src/Infrastructure/BotSharp.Abstraction/Conversations/Settings/ConversationSetting.cs b/src/Infrastructure/BotSharp.Abstraction/Conversations/Settings/ConversationSetting.cs index 018f6086c..b656af2b2 100644 --- a/src/Infrastructure/BotSharp.Abstraction/Conversations/Settings/ConversationSetting.cs +++ b/src/Infrastructure/BotSharp.Abstraction/Conversations/Settings/ConversationSetting.cs @@ -13,10 +13,26 @@ public class ConversationSetting public bool EnableStateLog { get; set; } public bool EnableTranslationMemory { get; set; } public CleanConversationSetting CleanSetting { get; set; } = new(); + public ToolResultTrimSetting ToolResultTrim { get; set; } = new(); public RateLimitSetting RateLimit { get; set; } = new(); public FileSelectSetting? FileSelect { get; set; } } +/// +/// Caps what an older turn's tool result costs in the prompt. The turn that ran the tool always +/// sees it whole; only what history replays is shortened. +/// +public class ToolResultTrimSetting +{ + public bool Enable { get; set; } = true; + + /// How many of the most recent turns keep their tool results verbatim. + public int KeepTurns { get; set; } = 2; + + /// A result longer than this, and older than , is shortened. + public int MaxLength { get; set; } = 500; +} + public class CleanConversationSetting { public bool Enable { get; set; } diff --git a/src/Infrastructure/BotSharp.Core/Conversations/ConversationPlugin.cs b/src/Infrastructure/BotSharp.Core/Conversations/ConversationPlugin.cs index c8f3ef1c2..93c2bed55 100644 --- a/src/Infrastructure/BotSharp.Core/Conversations/ConversationPlugin.cs +++ b/src/Infrastructure/BotSharp.Core/Conversations/ConversationPlugin.cs @@ -8,6 +8,7 @@ using BotSharp.Abstraction.Settings; using BotSharp.Abstraction.Templating; using BotSharp.Core.Coding; +using BotSharp.Core.Conversations.Hooks; using BotSharp.Core.Instructs; using BotSharp.Core.MessageHub; using BotSharp.Core.MessageHub.Observers; @@ -72,6 +73,8 @@ public void RegisterDI(IServiceCollection services, IConfiguration config) services.AddScoped(); services.AddScoped(); + + services.AddScoped(); } public bool AttachMenu(List menu) diff --git a/src/Infrastructure/BotSharp.Core/Conversations/Hooks/ToolResultTrimHook.cs b/src/Infrastructure/BotSharp.Core/Conversations/Hooks/ToolResultTrimHook.cs new file mode 100644 index 000000000..3cfc92d9b --- /dev/null +++ b/src/Infrastructure/BotSharp.Core/Conversations/Hooks/ToolResultTrimHook.cs @@ -0,0 +1,115 @@ +using System.Text.RegularExpressions; +using BotSharp.Abstraction.Conversations.Settings; + +namespace BotSharp.Core.Conversations.Hooks; + +/// +/// Shortens the tool results an older turn replays into the prompt. +/// +/// +/// It runs where history is loaded, which is exactly the turn boundary: the messages a turn +/// produces are appended to the list in memory and never pass through here, so the turn that ran +/// a tool always reads its result whole. Only Content is touched -- the call itself stays +/// intact, so a later turn can still tell that the function already ran, and storage keeps the +/// full text either way. +/// +public class ToolResultTrimHook : ConversationHookBase +{ + /// The opening of a structured document: a brace or bracket that starts one, not a word in prose. + private static readonly Regex StructuredStart = new(@"[{\[]\s*[""{\[\d-]", RegexOptions.Compiled); + + private readonly IServiceProvider _services; + private readonly ConversationSetting _settings; + private readonly ILogger _logger; + + public ToolResultTrimHook( + IServiceProvider services, + ConversationSetting settings, + ILogger logger) + { + _services = services; + _settings = settings; + _logger = logger; + } + + public override string SelfId => string.Empty; + + public override Task OnDialogsLoaded(List dialogs) + { + var setting = _settings.ToolResultTrim; + if (!setting.Enable || dialogs.IsNullOrEmpty()) + { + return base.OnDialogsLoaded(dialogs); + } + + // A turn is a message id: everything a turn derives carries the id of the message that + // started it. The current one is skipped as well, because a function that reloads the + // history mid-turn would otherwise be handed a shortened copy of what it just produced. + var currentMessageId = _services.GetRequiredService().MessageId; + var recentTurns = dialogs + .Select(x => x.MessageId) + .Where(x => !string.IsNullOrEmpty(x)) + .Distinct() + .TakeLast(Math.Max(setting.KeepTurns, 0)) + .ToHashSet(); + + var trimmed = 0; + var saved = 0; + + foreach (var dialog in dialogs) + { + if (dialog.Role != AgentRole.Function + || dialog.MessageId == currentMessageId + || recentTurns.Contains(dialog.MessageId) + || (dialog.Content?.Length ?? 0) <= setting.MaxLength) + { + continue; + } + + var before = dialog.Content.Length; + dialog.Content = Shorten(dialog, setting.MaxLength); + + trimmed++; + saved += before - dialog.Content.Length; + } + + if (trimmed > 0) + { + _logger.LogInformation( + "[ToolResultTrim] {Count} tool result(s) older than {KeepTurns} turn(s) shortened, {Saved} characters kept out of the prompt.", + trimmed, setting.KeepTurns, saved); + } + + return base.OnDialogsLoaded(dialogs); + } + + /// + /// Keeps the head of a rendered result, whose useful part comes first, and replaces a + /// structured one outright: half a JSON document is not something a model can read, and it + /// cannot tell that the half it got is not the whole. + /// + /// + /// The cut goes wherever comes first: where a structured value begins, or the length cap. A + /// tool is free to answer with a sentence and then a document -- an MCP server returning + /// several text blocks does exactly that -- and cutting inside the document would leave a + /// model something it cannot read and cannot tell is incomplete, while cutting in front of it + /// keeps the sentence that introduced it. A lone brace in prose is not a document: what is + /// looked for is a brace or bracket that opens a value. + /// + private static string Shorten(RoleDialogModel dialog, int maxLength) + { + var content = dialog.Content; + var structure = StructuredStart.Match(content); + var cut = structure.Success ? Math.Min(structure.Index, maxLength) : maxLength; + + // The call itself sits in the message right before this one, so the note does not name it + // again -- what a later turn cannot work out on its own is that something was dropped. + if (cut == 0) + { + return $"[{content.Length} chars omitted; call again for detail]"; + } + + return content[..cut].TrimEnd() + + $"\r\n... [{content.Length - cut} chars omitted; call again for detail]"; + } +} diff --git a/src/Infrastructure/BotSharp.Core/Routing/RoutingService.GetConversationContent.cs b/src/Infrastructure/BotSharp.Core/Routing/RoutingService.GetConversationContent.cs index 256f5d065..b1d40f23d 100644 --- a/src/Infrastructure/BotSharp.Core/Routing/RoutingService.GetConversationContent.cs +++ b/src/Infrastructure/BotSharp.Core/Routing/RoutingService.GetConversationContent.cs @@ -5,28 +5,35 @@ public partial class RoutingService public async Task GetConversationContent(List dialogs, int maxDialogCount = 100) { var agentService = _services.GetRequiredService(); - var conversation = ""; - var conversationDialogs = dialogs.Where(x => !x.ExcludeFromContext).TakeLast(maxDialogCount).ToList(); + var conversation = new StringBuilder(); + // A tool result from an earlier turn says nothing about which agent should answer now. + var conversationDialogs = dialogs + .Where(x => !x.ExcludeFromContext) + .Where(x => x.Role != AgentRole.Function || x.MessageId == Context.MessageId) + .TakeLast(maxDialogCount) + .ToList(); foreach (var dialog in conversationDialogs) { - var role = dialog.Role; - if (role != AgentRole.User) + var agent = dialog.Role == AgentRole.User ? null : await agentService.GetAgent(dialog.CurrentAgentId); + var name = agent?.Name ?? dialog.Role; + + if (dialog.Role == AgentRole.User) { - var agent = await agentService.GetAgent(dialog.CurrentAgentId); - role = agent.Name; + // What the user said can arrive as a postback payload rather than as text + conversation.Append($"{name}: {dialog.LlmContent}\r\n"); } - - if (role == AgentRole.User) + else if (dialog.Role == AgentRole.Function) { - conversation += $"{role}: {dialog.Payload ?? dialog.Content}\r\n"; + // A tool result is not something the agent said, so name the call it answers + conversation.Append($"{name}: Call function {dialog.FunctionName}({dialog.FunctionArgs}) => {dialog.Content}\r\n"); } else { // Assistant reply doesn't need help with payload - conversation += $"{role}: {dialog.Content}\r\n"; + conversation.Append($"{name}: {dialog.Content}\r\n"); } } - return conversation; + return conversation.ToString(); } } diff --git a/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeAgent.cs b/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeAgent.cs index 39be4646d..35f3049b3 100644 --- a/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeAgent.cs +++ b/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeAgent.cs @@ -133,11 +133,10 @@ private async Task InvokeFunction( } else { - // The function wrote the reply itself. The call is still worth a record, but kept out - // of context, since the assistant message that follows carries the same text. - var record = RoleDialogModel.From(message, role: AgentRole.Function); - record.ExcludeFromContext = true; - await Persist(record); + // The function wrote the reply itself, and the assistant message that follows repeats + // its text. The call is recorded anyway, and stays in context: it is the only thing + // telling a later turn that this function already ran. + await Persist(RoleDialogModel.From(message, role: AgentRole.Function)); var msg = RoleDialogModel.From(message, role: AgentRole.Assistant, diff --git a/tests/BotSharp.Core.UnitTests/Conversations/ToolResultTrimHookTests.cs b/tests/BotSharp.Core.UnitTests/Conversations/ToolResultTrimHookTests.cs new file mode 100644 index 000000000..78f0c6362 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/Conversations/ToolResultTrimHookTests.cs @@ -0,0 +1,233 @@ +using BotSharp.Abstraction.Agents.Enums; +using BotSharp.Abstraction.Conversations.Models; +using BotSharp.Abstraction.Conversations.Settings; +using BotSharp.Abstraction.Routing; +using BotSharp.Core.Conversations.Hooks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace BotSharp.Core.UnitTests.Conversations; + +/// +/// A tool result is worth its tokens to the turn that asked for it and rarely worth them again. +/// This hook is where that is acted on, and these tests pin the two things it must never do: +/// touch what the current turn is about to read, and lose the call itself -- an agent that cannot +/// see it already ran a function runs it again. +/// +public class ToolResultTrimHookTests +{ + private const string OldTurn = "turn-1"; + private const string MiddleTurn = "turn-2"; + private const string RecentTurn = "turn-3"; + private const string CurrentTurn = "turn-4"; + + private static ToolResultTrimHook BuildHook(ConversationSetting settings, string currentMessageId) + { + var context = new Mock(); + context.SetupGet(x => x.MessageId).Returns(currentMessageId); + + var services = new ServiceCollection(); + services.AddSingleton(context.Object); + + return new ToolResultTrimHook( + services.BuildServiceProvider(), + settings, + NullLogger.Instance); + } + + private static ConversationSetting Settings(bool enable = true, int keepTurns = 2, int maxLength = 500) + => new() + { + ToolResultTrim = new ToolResultTrimSetting + { + Enable = enable, + KeepTurns = keepTurns, + MaxLength = maxLength + } + }; + + private static RoleDialogModel Tool(string messageId, string content, string function = "read_work_order") + => new(AgentRole.Function, content) + { + MessageId = messageId, + FunctionName = function, + ToolCallId = $"call_{messageId}", + FunctionArgs = "{\"wo_num\":\"A123\"}" + }; + + private static string Rendered(int length) + => "WO Num: A1234567\r\n" + new string('x', length - 18); + + private static string Json(int length) + => "{\"wo_num\":\"A1234567\",\"pad\":\"" + new string('x', length - 30) + "\"}"; + + [Fact] + public async Task Leaves_the_turn_that_is_running_untouched() + { + var dialogs = new List + { + Tool(OldTurn, Rendered(4000)), + Tool(MiddleTurn, Rendered(4000)), + Tool(RecentTurn, Rendered(4000)), + Tool(CurrentTurn, Rendered(4000)) + }; + + // keepTurns 0, so only "this is the turn in flight" can protect the last one. + await BuildHook(Settings(keepTurns: 0), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.Equal(4000, dialogs[3].Content.Length); + Assert.All(dialogs.Take(3), x => Assert.True(x.Content.Length < 4000)); + } + + [Fact] + public async Task Keeps_the_most_recent_turns_whole_and_shortens_what_is_older() + { + var dialogs = new List + { + new(AgentRole.User, "how is my work order?") { MessageId = OldTurn }, + Tool(OldTurn, Rendered(4000)), + Tool(MiddleTurn, Rendered(4000)), + Tool(RecentTurn, Rendered(4000)) + }; + + await BuildHook(Settings(keepTurns: 2), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.True(dialogs[1].Content.Length < 4000); + Assert.Equal(4000, dialogs[2].Content.Length); + Assert.Equal(4000, dialogs[3].Content.Length); + } + + [Fact] + public async Task Keeps_the_call_even_when_the_result_goes() + { + var dialogs = new List { Tool(OldTurn, Json(4000)) }; + + await BuildHook(Settings(keepTurns: 0), CurrentTurn).OnDialogsLoaded(dialogs); + + var stored = dialogs[0]; + Assert.Equal(AgentRole.Function, stored.Role); + Assert.Equal("read_work_order", stored.FunctionName); + Assert.Equal($"call_{OldTurn}", stored.ToolCallId); + Assert.Equal("{\"wo_num\":\"A123\"}", stored.FunctionArgs); + } + + [Fact] + public async Task Replaces_a_structured_result_rather_than_cutting_it_in_half() + { + var dialogs = new List { Tool(OldTurn, Json(4000)) }; + + await BuildHook(Settings(keepTurns: 0), CurrentTurn).OnDialogsLoaded(dialogs); + + // Half a JSON document reads as a whole one to a model that cannot see where it was cut. + Assert.DoesNotContain("{\"wo_num\"", dialogs[0].Content); + Assert.Equal("[4000 chars omitted; call again for detail]", dialogs[0].Content); + } + + [Fact] + public async Task Keeps_the_head_of_a_rendered_result_and_says_what_was_dropped() + { + var dialogs = new List { Tool(OldTurn, Rendered(4000)) }; + + await BuildHook(Settings(keepTurns: 0, maxLength: 500), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.StartsWith("WO Num: A1234567", dialogs[0].Content); + Assert.Contains("3500 chars omitted", dialogs[0].Content); + } + + [Fact] + public async Task Keeps_the_sentence_that_introduces_a_document_and_drops_the_document() + { + // What an MCP server answers with is several text blocks joined together, so a document + // can sit behind a line of prose. The prose is what a later turn can still use. + var dialogs = new List + { + Tool(OldTurn, "Found 3 work orders:\r\n" + Json(4000)) + }; + + await BuildHook(Settings(keepTurns: 0, maxLength: 500), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.StartsWith("Found 3 work orders:", dialogs[0].Content); + Assert.DoesNotContain("\"wo_num\"", dialogs[0].Content); + Assert.Contains("chars omitted", dialogs[0].Content); + } + + [Fact] + public async Task Keeps_prose_whose_document_begins_past_the_cut() + { + var dialogs = new List + { + Tool(OldTurn, Rendered(900) + Json(3000)) + }; + + await BuildHook(Settings(keepTurns: 0, maxLength: 500), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.StartsWith("WO Num: A1234567", dialogs[0].Content); + Assert.DoesNotContain("\"wo_num\"", dialogs[0].Content); + } + + [Theory] + [InlineData("{\"wo_num\": \"A123\", ")] // object + [InlineData("[{\"wo_num\": \"A123\"}, ")] // array of objects + [InlineData("[\"A123\", \"A456\", ")] // array of strings + [InlineData("[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ")] // array of numbers + public async Task Treats_an_opening_value_as_structured(string opening) + { + var dialogs = new List { Tool(OldTurn, opening + new string('x', 4000)) }; + + await BuildHook(Settings(keepTurns: 0, maxLength: 500), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.Matches(@"^\[\d+ chars omitted; call again for detail\]$", dialogs[0].Content); + } + + [Fact] + public async Task Does_not_mistake_a_brace_in_a_sentence_for_a_document() + { + // Prose is allowed to contain a brace. Replacing the whole result over one would throw + // away a head that was perfectly safe to keep. + var dialogs = new List + { + Tool(OldTurn, "Use the {name} placeholder in the template. " + new string('x', 4000)) + }; + + await BuildHook(Settings(keepTurns: 0, maxLength: 500), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.StartsWith("Use the {name} placeholder", dialogs[0].Content); + Assert.Contains("chars omitted", dialogs[0].Content); + } + + [Fact] + public async Task Leaves_a_short_result_alone() + { + var dialogs = new List { Tool(OldTurn, Rendered(400)) }; + + await BuildHook(Settings(keepTurns: 0), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.Equal(400, dialogs[0].Content.Length); + } + + [Fact] + public async Task Does_nothing_at_all_when_switched_off() + { + var dialogs = new List { Tool(OldTurn, Json(4000)) }; + + await BuildHook(Settings(enable: false, keepTurns: 0), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.Equal(4000, dialogs[0].Content.Length); + } + + [Fact] + public async Task Leaves_every_role_but_function_alone() + { + var dialogs = new List + { + new(AgentRole.User, Rendered(4000)) { MessageId = OldTurn }, + new(AgentRole.Assistant, Rendered(4000)) { MessageId = OldTurn } + }; + + await BuildHook(Settings(keepTurns: 0), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.All(dialogs, x => Assert.Equal(4000, x.Content.Length)); + } +}