diff --git a/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeAgent.cs b/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeAgent.cs index 39be4646d..0b66f3272 100644 --- a/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeAgent.cs +++ b/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeAgent.cs @@ -32,7 +32,7 @@ public async Task InvokeAgent( model = agentSettings.LlmConfig.Model; } - var chatCompletion = CompletionProvider.GetChatCompletion(_services, + var chatCompletion = CompletionProvider.GetChatCompletion(_services, provider: provider, model: model); @@ -48,21 +48,10 @@ public async Task InvokeAgent( response = await chatCompletion.GetChatCompletions(agent, conversationDialogs); } - if (response.Role == AgentRole.Function && !string.IsNullOrEmpty(response.FunctionName)) + var toolCalls = GetToolCalls(response); + if (!toolCalls.IsNullOrEmpty()) { - message = RoleDialogModel.From(message, role: AgentRole.Function); - response.FunctionName = response.FunctionName.NormalizeFunctionName(); - message.ToolCallId = response.ToolCallId; - message.FunctionName = response.FunctionName; - message.FunctionArgs = response.FunctionArgs; - message.Thought = response.Thought != null ? new(response.Thought) : null; - message.MetaData = response.MetaData != null ? new(response.MetaData) : null; - message.Indication = response.Indication; - message.CurrentAgentId = agent.Id; - message.IsStreaming = response.IsStreaming; - message.MessageLabel = response.MessageLabel; - - await InvokeFunction(message, dialogs, options); + await InvokeToolCalls(toolCalls, response, message, agent, dialogs, options); } else { @@ -86,6 +75,118 @@ public async Task InvokeAgent( return true; } + /// + /// Every tool call one reply asked for, in the order the model produced them. + /// + /// + /// is where a provider reports the whole set. The + /// single FunctionName/FunctionArgs/ToolCallId fields beside it are a view of its first entry, + /// kept for callers that can only run one call, so a provider that fills only those is read + /// through the second branch and needs no change to work here. + /// + private static List GetToolCalls(RoleDialogModel response) + { + if (!response.ToolCalls.IsNullOrEmpty()) + { + // A call with no name is not something that can be dispatched. The single-call path + // has always guarded on the name being present, and the batch drops it the same way + // rather than handing an empty name to the executor factory. + return response.ToolCalls! + .Where(x => !string.IsNullOrEmpty(x.FunctionName)) + .ToList(); + } + + if (response.Role == AgentRole.Function && !string.IsNullOrEmpty(response.FunctionName)) + { + return [new LlmToolCall(response.ToolCallId, response.FunctionName, response.FunctionArgs)]; + } + + return []; + } + + /// + /// Runs every call the reply asked for, in order, and then hands the whole set of results back + /// to the model in one round. + /// + /// + /// The recursion into belongs here rather than inside + /// : a per-call recursion spends the turn on the first call, and + /// the calls after it never run at all. Deciding once for the whole batch is what lets a model + /// ask for three independent lookups and get three answers in a single round trip. + /// + private async Task InvokeToolCalls( + List toolCalls, + RoleDialogModel response, + RoleDialogModel source, + Agent agent, + List dialogs, + InvokeAgentOptions? options) + { + if (toolCalls.Count > 1) + { + // Worth a line of its own. Until now a reply like this ran one call and dropped the + // rest, so an agent whose prompt was tuned against that behaviour changes the moment + // this ships. This is where to look when one does. + _logger.LogInformation($"Agent {agent.Name} asked for {toolCalls.Count} tool calls in one reply: " + + $"{string.Join(", ", toolCalls.Select(x => x.FunctionName))}"); + } + + var completed = true; + + for (var i = 0; i < toolCalls.Count; i++) + { + var toolCall = toolCalls[i]; + + // Each call in the batch answers the same message. They are siblings of one reply, not + // a chain, so every one is built from the dialog that reply responded to rather than + // from the result the previous call just appended. + var message = RoleDialogModel.From(source, role: AgentRole.Function); + message.ToolCallId = toolCall.Id; + message.FunctionName = toolCall.FunctionName.NormalizeFunctionName(); + message.FunctionArgs = toolCall.FunctionArgs; + message.Thought = response.Thought != null ? new(response.Thought) : null; + message.MetaData = response.MetaData != null ? new(response.MetaData) : null; + message.Indication = response.Indication; + message.CurrentAgentId = agent.Id; + message.IsStreaming = response.IsStreaming; + message.MessageLabel = response.MessageLabel; + + completed = await InvokeFunction(message, dialogs, options); + if (!completed) + { + var skipped = toolCalls.Count - i - 1; + if (skipped > 0) + { + // The model asked for these without knowing an earlier call would end the + // turn. Running them into a finished turn produces results nothing reads, so + // they are dropped -- but not silently, which is how everything beyond the + // first call used to disappear. + _logger.LogInformation($"{message.FunctionName} ended the turn, skipping the remaining {skipped} tool call(s) of this reply: " + + $"{string.Join(", ", toolCalls.Skip(i + 1).Select(x => x.FunctionName))}"); + } + break; + } + } + + if (completed) + { + // One round trip for the whole batch: the model sees every result at once. The agent + // is read after the last call because a routing tool may have changed it. + var routing = _services.GetRequiredService(); + var curAgentId = routing.Context.GetCurrentAgentId(); + await InvokeAgent(curAgentId, dialogs, options); + } + } + + /// + /// Executes one tool call and appends its result to the dialogs. + /// + /// + /// Whether the turn should continue. False when this call ended it -- either the function set + /// and answered the user itself, or a response + /// template answered in its place. Both have always ended the turn; what is new is that the + /// caller is the one told about it, instead of this method deciding by recursing or not. + /// private async Task InvokeFunction( RoleDialogModel message, List dialogs, @@ -101,37 +202,7 @@ private async Task InvokeFunction( var funcOptions = options != null ? new InvokeFunctionOptions() { From = options.From } : null; await routing.InvokeFunction(message.FunctionName, message, options: funcOptions); - // Pass execution result to LLM to get response - if (!message.StopCompletion) - { - // Find response template - var templateService = _services.GetRequiredService(); - var responseTemplate = await templateService.RenderFunctionResponse(message.CurrentAgentId, message); - if (!string.IsNullOrEmpty(responseTemplate)) - { - var msg = RoleDialogModel.From(message, - role: AgentRole.Assistant, - content: responseTemplate); - dialogs.Add(msg); - Context.AddDialogs([msg]); - } - else - { - // Save to memory dialogs and to storage - var msg = RoleDialogModel.From(message, - role: AgentRole.Function, - content: message.Content); - - dialogs.Add(msg); - Context.AddDialogs([msg]); - await Persist(msg); - - // Send to Next LLM - var curAgentId = routing.Context.GetCurrentAgentId(); - await InvokeAgent(curAgentId, dialogs, options); - } - } - else + if (message.StopCompletion) { // 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. @@ -144,8 +215,33 @@ private async Task InvokeFunction( content: message.Content); dialogs.Add(msg); Context.AddDialogs([msg]); + + return false; } + // Find response template + var templateService = _services.GetRequiredService(); + var responseTemplate = await templateService.RenderFunctionResponse(message.CurrentAgentId, message); + if (!string.IsNullOrEmpty(responseTemplate)) + { + var msg = RoleDialogModel.From(message, + role: AgentRole.Assistant, + content: responseTemplate); + dialogs.Add(msg); + Context.AddDialogs([msg]); + + return false; + } + + // Save to memory dialogs and to storage + var functionMsg = RoleDialogModel.From(message, + role: AgentRole.Function, + content: message.Content); + + dialogs.Add(functionMsg); + Context.AddDialogs([functionMsg]); + await Persist(functionMsg); + return true; } diff --git a/src/Plugins/BotSharp.Plugin.OpenAI/BotSharp.Plugin.OpenAI.csproj b/src/Plugins/BotSharp.Plugin.OpenAI/BotSharp.Plugin.OpenAI.csproj index 7fc22b57a..2515ae550 100644 --- a/src/Plugins/BotSharp.Plugin.OpenAI/BotSharp.Plugin.OpenAI.csproj +++ b/src/Plugins/BotSharp.Plugin.OpenAI/BotSharp.Plugin.OpenAI.csproj @@ -23,4 +23,10 @@ + + + + + \ No newline at end of file diff --git a/src/Plugins/BotSharp.Plugin.OpenAI/Providers/Chat/ChatCompletionProvider.Chat.cs b/src/Plugins/BotSharp.Plugin.OpenAI/Providers/Chat/ChatCompletionProvider.Chat.cs index a0060e85e..f731a9842 100644 --- a/src/Plugins/BotSharp.Plugin.OpenAI/Providers/Chat/ChatCompletionProvider.Chat.cs +++ b/src/Plugins/BotSharp.Plugin.OpenAI/Providers/Chat/ChatCompletionProvider.Chat.cs @@ -769,7 +769,7 @@ private static List ToLlmToolCalls(IEnumerable? toolC /// Concatenating every fragment into a single string, as this did before, produced one /// malformed argument blob as soon as the model asked for more than one tool at a time. /// - private static List ReconstructToolCalls(List updates) + internal static List ReconstructToolCalls(List updates) { var calls = new List(); var args = new List(); diff --git a/tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj b/tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj index a56498e7e..46f2f5e5b 100644 --- a/tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj +++ b/tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj @@ -34,6 +34,8 @@ + + diff --git a/tests/BotSharp.Core.UnitTests/Routing/InvokeAgentToolCallTests.cs b/tests/BotSharp.Core.UnitTests/Routing/InvokeAgentToolCallTests.cs new file mode 100644 index 000000000..627f8883f --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/Routing/InvokeAgentToolCallTests.cs @@ -0,0 +1,361 @@ +using BotSharp.Abstraction.Agents; +using BotSharp.Abstraction.Agents.Enums; +using BotSharp.Abstraction.Agents.Models; +using BotSharp.Abstraction.Agents.Settings; +using BotSharp.Abstraction.Conversations; +using BotSharp.Abstraction.Conversations.Models; +using BotSharp.Abstraction.Functions.Models; +using BotSharp.Abstraction.MLTasks; +using BotSharp.Abstraction.Routing; +using BotSharp.Abstraction.Routing.Models; +using BotSharp.Abstraction.Routing.Settings; +using BotSharp.Abstraction.Templating; +using BotSharp.Core.Routing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace BotSharp.Core.UnitTests.Routing; + +/// +/// A model reply can ask for several tools at once, and every provider here reports the whole set +/// on . The routing engine read only the singular +/// FunctionName beside it, so calls two and three were dropped with no error, no log and nothing +/// told to the model -- it simply asked for them again on the next turn, or answered without them. +/// +/// These tests pin the fixed behaviour: every call runs, results are appended in order, and the +/// model is asked again once for the whole batch rather than once per call. +/// +public class InvokeAgentToolCallTests +{ + private const string AgentId = "agent-1"; + + /// + /// Returns the replies it was given, one per round, and a plain answer once they run out -- + /// which is what stops the recursion at the end of a test. + /// + private sealed class ScriptedChatCompletion(params RoleDialogModel[] replies) : IChatCompletion + { + private readonly Queue _replies = new(replies); + + public string Provider => "test-provider"; + public string Model => "test-model"; + + /// How many times the model was asked. One per LLM round trip. + public int Rounds { get; private set; } + + public void SetModelName(string model) { } + + public Task GetChatCompletions(Agent agent, List conversations) + { + Rounds++; + var reply = _replies.Count > 0 + ? _replies.Dequeue() + : new RoleDialogModel(AgentRole.Assistant, "final answer"); + return Task.FromResult(reply); + } + } + + private sealed record ExecutedCall(string Name, string? ToolCallId, string? Args); + + /// + /// Everything InvokeAgent reaches for, wired to stubs, plus the two things a test looks at: + /// the calls that actually reached the executor and how many LLM rounds it took. + /// + private sealed class Harness + { + public required RoutingService Routing { get; init; } + public required ScriptedChatCompletion Chat { get; init; } + public required List Executed { get; init; } + public required Mock Context { get; init; } + public required Mock Template { get; init; } + } + + private static Harness BuildHarness( + RoleDialogModel[] replies, + Action? onFunctionInvoked = null) + { + var agent = new Agent + { + Id = AgentId, + Name = "tester", + LlmConfig = new AgentLlmConfig + { + Provider = "test-provider", + Model = "test-model", + MaxRecursionDepth = 10 + } + }; + + var agentService = new Mock(); + agentService.Setup(x => x.LoadAgent(It.IsAny(), It.IsAny())).ReturnsAsync(agent); + + var context = new Mock(); + context.Setup(x => x.GetCurrentAgentId()).Returns(AgentId); + + var executed = new List(); + var routingStub = new Mock(); + routingStub.SetupGet(x => x.Context).Returns(context.Object); + routingStub + .Setup(x => x.InvokeFunction(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((string name, RoleDialogModel message, InvokeFunctionOptions? _) => + { + executed.Add(new ExecutedCall(name, message.ToolCallId, message.FunctionArgs)); + message.Content = $"{name} result"; + onFunctionInvoked?.Invoke(message); + return Task.FromResult(true); + }); + + // No template by default: the branch that renders one ends the turn, and only the test + // that is about that branch wants it. + var template = new Mock(); + template + .Setup(x => x.RenderFunctionResponse(It.IsAny(), It.IsAny())) + .ReturnsAsync(string.Empty); + + // Not in conversation mode, so Persist is a no-op and no storage is needed. + var conversation = new Mock(); + conversation.Setup(x => x.IsConversationMode()).Returns(false); + + var chat = new ScriptedChatCompletion(replies); + + var services = new ServiceCollection(); + services.AddLogging(); + services.AddSingleton(agentService.Object); + services.AddSingleton(new AgentSettings()); + services.AddSingleton(new Mock().Object); + services.AddSingleton(chat); + services.AddSingleton(routingStub.Object); + services.AddSingleton(template.Object); + services.AddSingleton(conversation.Object); + + var routing = new RoutingService( + services.BuildServiceProvider(), + new RoutingSettings(), + context.Object, + NullLogger.Instance); + + return new Harness + { + Routing = routing, + Chat = chat, + Executed = executed, + Context = context, + Template = template + }; + } + + /// A reply asking for the given calls, reported the way every provider reports them. + private static RoleDialogModel ToolReply(params (string Name, string Args, string Id)[] calls) + { + var toolCalls = calls.Select(x => new LlmToolCall(x.Id, x.Name, x.Args)).ToList(); + var first = toolCalls.First(); + + return new RoleDialogModel(AgentRole.Function, string.Empty) + { + CurrentAgentId = AgentId, + ToolCallId = first.Id, + FunctionName = first.FunctionName, + FunctionArgs = first.FunctionArgs, + ToolCalls = toolCalls + }; + } + + private static List NewDialogs() + => [new RoleDialogModel(AgentRole.User, "what is the weather and the time?") { MessageId = "msg-1" }]; + + [Fact] + public async Task InvokeAgent_RunsEveryCallOfTheReply() + { + var harness = BuildHarness( + [ + ToolReply( + ("get_weather", "{\"city\":\"Chicago\"}", "call_1"), + ("get_time", "{\"zone\":\"CST\"}", "call_2"), + ("get_rate", "{\"pair\":\"USDCNY\"}", "call_3")) + ]); + + var dialogs = NewDialogs(); + await harness.Routing.InvokeAgent(AgentId, dialogs); + + Assert.Equal( + ["get_weather", "get_time", "get_rate"], + harness.Executed.Select(x => x.Name)); + + // Each result has to go back under the id of the call it answers, or the provider cannot + // match them up. + Assert.Equal(["call_1", "call_2", "call_3"], harness.Executed.Select(x => x.ToolCallId)); + Assert.Equal( + ["{\"city\":\"Chicago\"}", "{\"zone\":\"CST\"}", "{\"pair\":\"USDCNY\"}"], + harness.Executed.Select(x => x.Args)); + } + + [Fact] + public async Task InvokeAgent_AppendsEveryResultThenAsksTheModelOnce() + { + var harness = BuildHarness( + [ + ToolReply( + ("get_weather", "{}", "call_1"), + ("get_time", "{}", "call_2")) + ]); + + var dialogs = NewDialogs(); + await harness.Routing.InvokeAgent(AgentId, dialogs); + + var functionResults = dialogs.Where(x => x.Role == AgentRole.Function).ToList(); + Assert.Equal(["get_weather result", "get_time result"], functionResults.Select(x => x.Content)); + Assert.Equal(["call_1", "call_2"], functionResults.Select(x => x.ToolCallId)); + + Assert.Equal(AgentRole.Assistant, dialogs.Last().Role); + Assert.Equal("final answer", dialogs.Last().Content); + + // Two rounds, not three: the batch is answered once, not once per call. + Assert.Equal(2, harness.Chat.Rounds); + } + + /// + /// Recursion depth is a budget on how many times the model gets to speak. Running a batch + /// inside one turn must not spend it per call, or a reply asking for four tools would exhaust + /// the default depth of three before the model ever saw a result. + /// + [Fact] + public async Task InvokeAgent_SpendsOneRecursionPerRoundNotPerCall() + { + var harness = BuildHarness( + [ + ToolReply( + ("a", "{}", "call_1"), + ("b", "{}", "call_2"), + ("c", "{}", "call_3")) + ]); + + await harness.Routing.InvokeAgent(AgentId, NewDialogs()); + + harness.Context.Verify(x => x.IncreaseRecursiveCounter(), Times.Exactly(2)); + } + + /// + /// A provider that fills only the singular fields -- every provider outside OpenAI and + /// Anthropic today -- has to keep working untouched. + /// + [Fact] + public async Task InvokeAgent_ReadsTheSingularFieldsWhenToolCallsIsAbsent() + { + var reply = new RoleDialogModel(AgentRole.Function, string.Empty) + { + CurrentAgentId = AgentId, + ToolCallId = "call_1", + FunctionName = "get_weather", + FunctionArgs = "{\"city\":\"Chicago\"}" + }; + + var harness = BuildHarness([reply]); + await harness.Routing.InvokeAgent(AgentId, NewDialogs()); + + var call = Assert.Single(harness.Executed); + Assert.Equal("get_weather", call.Name); + Assert.Equal("call_1", call.ToolCallId); + } + + /// + /// Names arrive on exactly as the model produced them, + /// so the repair that the singular field has always had has to be applied to each of them. + /// + [Fact] + public async Task InvokeAgent_NormalizesEveryCallsName() + { + var harness = BuildHarness( + [ + ToolReply( + ("weather_agent.get_weather", "{}", "call_1"), + ("clock/get_time", "{}", "call_2")) + ]); + + await harness.Routing.InvokeAgent(AgentId, NewDialogs()); + + Assert.Equal(["get_weather", "get_time"], harness.Executed.Select(x => x.Name)); + } + + /// + /// A function that answers the user itself ends the turn. The calls behind it were asked for + /// without knowing that, and running them into a finished turn would produce results nothing + /// reads -- so the batch stops there, and no further round is asked of the model. + /// + [Fact] + public async Task InvokeAgent_StopsAtTheCallThatEndsTheTurn() + { + var harness = BuildHarness( + [ + ToolReply( + ("get_weather", "{}", "call_1"), + ("hand_off_to_human", "{}", "call_2"), + ("get_rate", "{}", "call_3")) + ], + onFunctionInvoked: message => + { + if (message.FunctionName == "hand_off_to_human") + { + message.StopCompletion = true; + message.Content = "A colleague will take it from here."; + } + }); + + var dialogs = NewDialogs(); + await harness.Routing.InvokeAgent(AgentId, dialogs); + + Assert.Equal(["get_weather", "hand_off_to_human"], harness.Executed.Select(x => x.Name)); + Assert.Equal(1, harness.Chat.Rounds); + + Assert.Equal(AgentRole.Assistant, dialogs.Last().Role); + Assert.Equal("A colleague will take it from here.", dialogs.Last().Content); + } + + /// + /// A rendered response template answers in the function's place and has always ended the turn + /// too. It stops the batch for the same reason + /// does. + /// + [Fact] + public async Task InvokeAgent_StopsWhenAResponseTemplateAnswers() + { + var harness = BuildHarness( + [ + ToolReply( + ("get_weather", "{}", "call_1"), + ("get_time", "{}", "call_2")) + ]); + + harness.Template + .Setup(x => x.RenderFunctionResponse(It.IsAny(), It.IsAny())) + .ReturnsAsync("It is sunny in Chicago."); + + var dialogs = NewDialogs(); + await harness.Routing.InvokeAgent(AgentId, dialogs); + + Assert.Equal(["get_weather"], harness.Executed.Select(x => x.Name)); + Assert.Equal(1, harness.Chat.Rounds); + Assert.Equal("It is sunny in Chicago.", dialogs.Last().Content); + } + + /// + /// A reply with no calls at all still ends up as a plain assistant message. + /// + [Fact] + public async Task InvokeAgent_KeepsThePlainAnswerPathUnchanged() + { + var harness = BuildHarness( + [ + new RoleDialogModel(AgentRole.Assistant, "It is sunny.") { CurrentAgentId = AgentId } + ]); + + var dialogs = NewDialogs(); + await harness.Routing.InvokeAgent(AgentId, dialogs); + + Assert.Empty(harness.Executed); + Assert.Equal(1, harness.Chat.Rounds); + Assert.Equal(AgentRole.Assistant, dialogs.Last().Role); + Assert.Equal("It is sunny.", dialogs.Last().Content); + } +} diff --git a/tests/BotSharp.Core.UnitTests/Routing/StreamingToolCallReconstructionTests.cs b/tests/BotSharp.Core.UnitTests/Routing/StreamingToolCallReconstructionTests.cs new file mode 100644 index 000000000..461d76d67 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/Routing/StreamingToolCallReconstructionTests.cs @@ -0,0 +1,215 @@ +using BotSharp.Abstraction.Functions.Models; +using BotSharp.Plugin.OpenAI.Providers.Chat; +using OpenAI.Chat; +using Xunit; + +namespace BotSharp.Core.UnitTests.Routing; + +/// +/// The streamed counterpart of reading a reply's tool calls. A non-streaming reply hands over a +/// finished list; a streamed one arrives as fragments that have to be reassembled, and that +/// reassembly is the only part of the path with no production evidence behind it -- the streaming +/// branch logs its calls under #if DEBUG, so a Release deployment that uses it shows +/// nothing at all. +/// +/// +/// Why reassembly is guesswork in the first place: in +/// the OpenAI SDK exposes only FunctionArgumentsUpdate, FunctionName, Kind and ToolCallId. There +/// is no public index, so which call a fragment belongs to has to be inferred -- an update +/// carrying a tool call id different from the one in progress starts a new call, and everything +/// after it belongs to that one. +/// +/// The case these exist for: two calls in one reply. Appending every fragment to a single string, +/// as this did originally, is invisible while a reply asks for one tool and produces a single +/// malformed argument blob the moment it asks for two. +/// +/// +public class StreamingToolCallReconstructionTests +{ + /// + /// One streamed fragment. Null means the update continues the call in + /// progress, which is how the SDK reports every fragment after the first of a call. + /// + private static StreamingChatToolCallUpdate Update(string? id = null, string? name = null, string? args = null) + => OpenAIChatModelFactory.StreamingChatToolCallUpdate( + index: 0, + toolCallId: id!, + kind: ChatToolCallKind.Function, + functionName: name!, + functionArgumentsUpdate: args != null ? BinaryData.FromString(args) : null!); + + [Fact] + public void Reconstruct_JoinsTheFragmentsOfOneCall() + { + var calls = ChatCompletionProvider.ReconstructToolCalls( + [ + Update(id: "call_1", name: "get_weather", args: "{\"ci"), + Update(args: "ty\":\"Chi"), + Update(args: "cago\"}") + ]); + + var call = Assert.Single(calls); + Assert.Equal("call_1", call.Id); + Assert.Equal("get_weather", call.FunctionName); + Assert.Equal("{\"city\":\"Chicago\"}", call.FunctionArgs); + } + + /// + /// The regression this reassembly exists for. Both calls must come out whole and separate; + /// the failure it replaced produced one call whose arguments were the two JSON documents + /// concatenated. + /// + [Fact] + public void Reconstruct_KeepsTwoCallsArgumentsApart() + { + var calls = ChatCompletionProvider.ReconstructToolCalls( + [ + Update(id: "call_1", name: "get_weather", args: "{\"city\":"), + Update(args: "\"Chicago\"}"), + Update(id: "call_2", name: "get_time", args: "{\"zone\":"), + Update(args: "\"CST\"}") + ]); + + Assert.Equal(2, calls.Count); + Assert.Equal(["call_1", "call_2"], calls.Select(x => x.Id)); + Assert.Equal(["get_weather", "get_time"], calls.Select(x => x.FunctionName)); + Assert.Equal("{\"city\":\"Chicago\"}", calls[0].FunctionArgs); + Assert.Equal("{\"zone\":\"CST\"}", calls[1].FunctionArgs); + } + + /// + /// A reply really does ask for the same tool more than once -- production shows models doing + /// it with up to four copies of one name in a single reply. The id is the only thing telling + /// those apart, so grouping must not fall back to the name. + /// + [Fact] + public void Reconstruct_SeparatesTwoCallsOfTheSameTool() + { + var calls = ChatCompletionProvider.ReconstructToolCalls( + [ + Update(id: "call_1", name: "process_wo_avoidance", args: "{\"key_words\":\"toilet leak\"}"), + Update(id: "call_2", name: "process_wo_avoidance", args: "{\"key_words\":\"roof repair\"}") + ]); + + Assert.Equal(2, calls.Count); + Assert.Equal(["call_1", "call_2"], calls.Select(x => x.Id)); + Assert.Equal("{\"key_words\":\"toilet leak\"}", calls[0].FunctionArgs); + Assert.Equal("{\"key_words\":\"roof repair\"}", calls[1].FunctionArgs); + } + + /// + /// A provider that stamps the id on every fragment rather than only on the first must not be + /// read as opening a new call per fragment. + /// + [Fact] + public void Reconstruct_DoesNotReopenACallWhenTheIdRepeats() + { + var calls = ChatCompletionProvider.ReconstructToolCalls( + [ + Update(id: "call_1", name: "get_weather", args: "{\"city\":"), + Update(id: "call_1", args: "\"Chicago\"}") + ]); + + var call = Assert.Single(calls); + Assert.Equal("{\"city\":\"Chicago\"}", call.FunctionArgs); + } + + /// + /// The name does not have to arrive with the id. When it comes in a later fragment it still + /// has to land on the call in progress, or the call is dispatched under an empty name. + /// + [Fact] + public void Reconstruct_TakesTheNameFromALaterFragment() + { + var calls = ChatCompletionProvider.ReconstructToolCalls( + [ + Update(id: "call_1"), + Update(name: "get_weather"), + Update(args: "{}") + ]); + + var call = Assert.Single(calls); + Assert.Equal("call_1", call.Id); + Assert.Equal("get_weather", call.FunctionName); + Assert.Equal("{}", call.FunctionArgs); + } + + /// + /// A first fragment with no id still opens a call rather than indexing into an empty list. + /// + [Fact] + public void Reconstruct_OpensACallEvenWhenTheFirstFragmentHasNoId() + { + var calls = ChatCompletionProvider.ReconstructToolCalls( + [ + Update(name: "get_weather", args: "{\"city\":"), + Update(args: "\"Chicago\"}") + ]); + + var call = Assert.Single(calls); + Assert.Equal("get_weather", call.FunctionName); + Assert.Equal("{\"city\":\"Chicago\"}", call.FunctionArgs); + } + + /// + /// Arguments are never null on the way out: a call the model sent no arguments for has to + /// serialize as something the provider can put in the request. + /// + [Fact] + public void Reconstruct_GivesACallWithoutArgumentsAnEmptyString() + { + var calls = ChatCompletionProvider.ReconstructToolCalls( + [ + Update(id: "call_1", name: "list_work_orders") + ]); + + var call = Assert.Single(calls); + Assert.Equal(string.Empty, call.FunctionArgs); + } + + [Fact] + public void Reconstruct_ReturnsNothingForAnEmptyStream() + { + Assert.Empty(ChatCompletionProvider.ReconstructToolCalls([])); + } + + /// + /// Order is the contract the rest of the engine reads: results are appended in the order the + /// model asked, and a reply that stops the turn part way through keeps the calls before it. + /// + [Fact] + public void Reconstruct_KeepsTheOrderTheModelSentThemIn() + { + var calls = ChatCompletionProvider.ReconstructToolCalls( + [ + Update(id: "call_1", name: "a", args: "{}"), + Update(id: "call_2", name: "b", args: "{}"), + Update(id: "call_3", name: "c", args: "{}") + ]); + + Assert.Equal(["a", "b", "c"], calls.Select(x => x.FunctionName)); + } + + /// + /// What the engine reads afterwards. is the shape the routing side + /// dispatches on, and a call is only dispatchable with both an id to answer under and a name + /// to resolve. + /// + [Fact] + public void Reconstruct_ProducesCallsTheEngineCanDispatch() + { + var calls = ChatCompletionProvider.ReconstructToolCalls( + [ + Update(id: "call_1", name: "get_weather", args: "{\"city\":\"Chicago\"}"), + Update(id: "call_2", name: "get_time", args: "{\"zone\":\"CST\"}") + ]); + + Assert.Equal(2, calls.Count); + Assert.All(calls, call => + { + Assert.False(string.IsNullOrEmpty(call.Id)); + Assert.False(string.IsNullOrEmpty(call.FunctionName)); + Assert.NotNull(call.FunctionArgs); + }); + } +}