diff --git a/src/Infrastructure/BotSharp.Core/Conversations/Hooks/ToolResultTrimHook.cs b/src/Infrastructure/BotSharp.Core/Conversations/Hooks/ToolResultTrimHook.cs index 3cfc92d9b..fb19df1a9 100644 --- a/src/Infrastructure/BotSharp.Core/Conversations/Hooks/ToolResultTrimHook.cs +++ b/src/Infrastructure/BotSharp.Core/Conversations/Hooks/ToolResultTrimHook.cs @@ -4,18 +4,16 @@ namespace BotSharp.Core.Conversations.Hooks; /// -/// Shortens the tool results an older turn replays into the prompt. +/// Shortens the tool results an older turn replays into the prompt. It runs where history is +/// loaded, so the turn that ran a tool still reads its result whole. Only Content is +/// touched: the call stays intact, and storage keeps the full text. /// -/// -/// 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. + /// Stands in for a result the assistant message of the same turn already carries. + private const string RepliedToUser = "[replied to the user]"; + + /// A brace or bracket that opens a value, as opposed to one in a sentence. private static readonly Regex StructuredStart = new(@"[{\[]\s*[""{\[\d-]", RegexOptions.Compiled); private readonly IServiceProvider _services; @@ -42,9 +40,8 @@ public override Task OnDialogsLoaded(List dialogs) 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. + // A turn is a message id. The current one is left alone for a function that reloads the + // history mid-turn. var currentMessageId = _services.GetRequiredService().MessageId; var recentTurns = dialogs .Select(x => x.MessageId) @@ -53,6 +50,11 @@ public override Task OnDialogsLoaded(List dialogs) .TakeLast(Math.Max(setting.KeepTurns, 0)) .ToHashSet(); + var replies = dialogs + .Where(x => x.Role == AgentRole.Assistant && !string.IsNullOrEmpty(x.Content)) + .Select(x => (x.MessageId, x.Content)) + .ToHashSet(); + var trimmed = 0; var saved = 0; @@ -60,14 +62,27 @@ public override Task OnDialogsLoaded(List dialogs) { if (dialog.Role != AgentRole.Function || dialog.MessageId == currentMessageId - || recentTurns.Contains(dialog.MessageId) - || (dialog.Content?.Length ?? 0) <= setting.MaxLength) + || string.IsNullOrEmpty(dialog.Content)) { continue; } var before = dialog.Content.Length; - dialog.Content = Shorten(dialog, setting.MaxLength); + + if (replies.Contains((dialog.MessageId, dialog.Content))) + { + // Repeating what the assistant message says word for word teaches the model to + // answer by echoing tool output. Nothing is lost, so recent turns are no exception. + dialog.Content = RepliedToUser; + } + else if (!recentTurns.Contains(dialog.MessageId) && before > setting.MaxLength) + { + dialog.Content = Shorten(dialog, setting.MaxLength); + } + else + { + continue; + } trimmed++; saved += before - dialog.Content.Length; @@ -76,34 +91,25 @@ public override Task OnDialogsLoaded(List dialogs) 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); + "[ToolResultTrim] {Count} tool result(s) shortened, {Saved} characters kept out of the prompt.", + trimmed, 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. + /// Cuts wherever comes first, a structured value or the length cap. Half a document is + /// something a model can neither read nor tell is incomplete, so one is replaced outright, + /// while the sentence that introduced it is worth keeping. /// - /// - /// 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. + // The call is named in the message right before this one, so the note does not repeat it. if (cut == 0) { return $"[{content.Length} chars omitted; call again for detail]"; diff --git a/tests/BotSharp.Core.UnitTests/Conversations/ToolResultTrimHookTests.cs b/tests/BotSharp.Core.UnitTests/Conversations/ToolResultTrimHookTests.cs index 78f0c6362..5754361f6 100644 --- a/tests/BotSharp.Core.UnitTests/Conversations/ToolResultTrimHookTests.cs +++ b/tests/BotSharp.Core.UnitTests/Conversations/ToolResultTrimHookTests.cs @@ -207,6 +207,72 @@ public async Task Leaves_a_short_result_alone() Assert.Equal(400, dialogs[0].Content.Length); } + [Fact] + public async Task Stands_in_for_a_result_the_assistant_message_repeats() + { + // A function that writes the user-facing reply itself has its text stored twice. Short + // enough to pass the length cap, and shown to the model as a tool result echoed word for + // word by the assistant -- which is what the model then learns to do. + const string reply = "Are you creating a duplicate work order or not?"; + var dialogs = new List + { + Tool(OldTurn, reply, function: "check_prerequisites"), + new(AgentRole.Assistant, reply) { MessageId = OldTurn } + }; + + await BuildHook(Settings(keepTurns: 0), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.Equal("[replied to the user]", dialogs[0].Content); + Assert.Equal("check_prerequisites", dialogs[0].FunctionName); + Assert.Equal(reply, dialogs[1].Content); + } + + [Fact] + public async Task Leaves_a_result_the_assistant_only_paraphrased() + { + var dialogs = new List + { + Tool(OldTurn, "Got location id 750229, resident id 1673151, continue current process."), + new(AgentRole.Assistant, "Got it. Can you tell me about the issue?") { MessageId = OldTurn } + }; + + await BuildHook(Settings(keepTurns: 0), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.StartsWith("Got location id 750229", dialogs[0].Content); + } + + [Fact] + public async Task Stands_in_even_for_a_turn_that_keeps_its_results() + { + // Shortening spares the recent turns because their detail may still be wanted. A repeat + // has no detail to spare: the assistant message beside it says the same thing. + const string reply = "Are you ready for some questions?"; + var dialogs = new List + { + Tool(RecentTurn, reply), + new(AgentRole.Assistant, reply) { MessageId = RecentTurn } + }; + + await BuildHook(Settings(keepTurns: 2), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.Equal("[replied to the user]", dialogs[0].Content); + } + + [Fact] + public async Task Does_not_match_the_same_text_from_another_turn() + { + const string reply = "Are you creating a duplicate work order or not?"; + var dialogs = new List + { + Tool(OldTurn, reply), + new(AgentRole.Assistant, reply) { MessageId = MiddleTurn } + }; + + await BuildHook(Settings(keepTurns: 0), CurrentTurn).OnDialogsLoaded(dialogs); + + Assert.Equal(reply, dialogs[0].Content); + } + [Fact] public async Task Does_nothing_at_all_when_switched_off() {