From ba855bc7cc2706457777c2b9d12eae96e6f073fa Mon Sep 17 00:00:00 2001 From: Kerry Jiang Date: Sat, 7 Dec 2024 13:47:17 -0800 Subject: [PATCH 1/7] introduce ConversationHookProvider to avoid ordering conversartion hooks in runtime --- .../Conversations/ConversationHookProvider.cs | 20 +++++++++++++++++++ .../ConversationService.TruncateMessage.cs | 2 +- .../ConversationService.UpdateBreakpoint.cs | 6 +++--- .../Services/ConversationService.cs | 3 ++- .../Functions/HumanInterventionNeededFn.cs | 6 +++--- .../Routing/RoutingService.InvokeFunction.cs | 6 +++--- .../BotSharp.Plugin.ChatHub/ChatHubPlugin.cs | 1 + 7 files changed, 33 insertions(+), 11 deletions(-) create mode 100644 src/Infrastructure/BotSharp.Abstraction/Conversations/ConversationHookProvider.cs diff --git a/src/Infrastructure/BotSharp.Abstraction/Conversations/ConversationHookProvider.cs b/src/Infrastructure/BotSharp.Abstraction/Conversations/ConversationHookProvider.cs new file mode 100644 index 00000000..00393da4 --- /dev/null +++ b/src/Infrastructure/BotSharp.Abstraction/Conversations/ConversationHookProvider.cs @@ -0,0 +1,20 @@ +namespace BotSharp.Abstraction.Conversations; + +public class ConversationHookProvider +{ + public IEnumerable Hooks { get; } + + private readonly Lazy> _hooksOrderByPriority; + + public IEnumerable HooksOrderByPriority + => _hooksOrderByPriority.Value; + + public ConversationHookProvider(IEnumerable conversationHooks) + { + Hooks = conversationHooks; + _hooksOrderByPriority = new Lazy>(() => + { + return conversationHooks.OrderBy(hook => hook.Priority).ToArray(); + }); + } +} \ No newline at end of file diff --git a/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.TruncateMessage.cs b/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.TruncateMessage.cs index ccc074e8..1a75d717 100644 --- a/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.TruncateMessage.cs +++ b/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.TruncateMessage.cs @@ -9,7 +9,7 @@ public partial class ConversationService : IConversationService var deleteMessageIds = db.TruncateConversation(conversationId, messageId, cleanLog: true); fileStorage.DeleteMessageFiles(conversationId, deleteMessageIds, messageId, newMessageId); - var hooks = _services.GetServices().ToList(); + var hooks = _services.GetServices(); foreach (var hook in hooks) { await hook.OnMessageDeleted(conversationId, messageId); diff --git a/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.UpdateBreakpoint.cs b/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.UpdateBreakpoint.cs index 8f88f44f..75a0bbfb 100644 --- a/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.UpdateBreakpoint.cs +++ b/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.UpdateBreakpoint.cs @@ -31,9 +31,9 @@ public partial class ConversationService : IConversationService states.CleanStates(excludedStates); } - var hooks = _services.GetServices() - .OrderBy(x => x.Priority) - .ToList(); + var hooks = _services + .GetRequiredService() + .HooksOrderByPriority; // Before executing functions foreach (var hook in hooks) diff --git a/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.cs b/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.cs index 55d80c18..03ff8aa4 100644 --- a/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.cs +++ b/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.cs @@ -103,7 +103,8 @@ public partial class ConversationService : IConversationService db.CreateNewConversation(record); - var hooks = _services.GetServices().ToList(); + var hooks = _services.GetServices(); + foreach (var hook in hooks) { // If user connect agent first time diff --git a/src/Infrastructure/BotSharp.Core/Routing/Functions/HumanInterventionNeededFn.cs b/src/Infrastructure/BotSharp.Core/Routing/Functions/HumanInterventionNeededFn.cs index d0c26be0..719dd805 100644 --- a/src/Infrastructure/BotSharp.Core/Routing/Functions/HumanInterventionNeededFn.cs +++ b/src/Infrastructure/BotSharp.Core/Routing/Functions/HumanInterventionNeededFn.cs @@ -15,9 +15,9 @@ public class HumanInterventionNeededFn : IFunctionCallback public async Task Execute(RoleDialogModel message) { - var hooks = _services.GetServices() - .OrderBy(x => x.Priority) - .ToList(); + var hooks = _services + .GetRequiredService() + .HooksOrderByPriority; foreach (var hook in hooks) { diff --git a/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeFunction.cs b/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeFunction.cs index 64b96136..fb074dfc 100644 --- a/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeFunction.cs +++ b/src/Infrastructure/BotSharp.Core/Routing/RoutingService.InvokeFunction.cs @@ -18,9 +18,9 @@ public partial class RoutingService var clonedMessage = RoleDialogModel.From(message); clonedMessage.FunctionName = name; - var hooks = _services.GetServices() - .OrderBy(x => x.Priority) - .ToList(); + var hooks = _services + .GetRequiredService() + .HooksOrderByPriority; var progressService = _services.GetService(); diff --git a/src/Plugins/BotSharp.Plugin.ChatHub/ChatHubPlugin.cs b/src/Plugins/BotSharp.Plugin.ChatHub/ChatHubPlugin.cs index 1eaad66d..e7cf082f 100644 --- a/src/Plugins/BotSharp.Plugin.ChatHub/ChatHubPlugin.cs +++ b/src/Plugins/BotSharp.Plugin.ChatHub/ChatHubPlugin.cs @@ -22,6 +22,7 @@ public class ChatHubPlugin : IBotSharpPlugin services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); From 1e974441ff47ec43658492e15cb64549900b0dab Mon Sep 17 00:00:00 2001 From: Kerry Jiang Date: Wed, 11 Dec 2024 17:16:59 -0800 Subject: [PATCH 2/7] ChatHubConversationHook priority --- .../Conversations/ConversationHookBase.cs | 18 +++++++----------- .../ConversationService.SendMessage.cs | 6 ++---- .../Hooks/ChatHubConversationHook.cs | 1 + 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/Infrastructure/BotSharp.Abstraction/Conversations/ConversationHookBase.cs b/src/Infrastructure/BotSharp.Abstraction/Conversations/ConversationHookBase.cs index 9239b865..da87646e 100644 --- a/src/Infrastructure/BotSharp.Abstraction/Conversations/ConversationHookBase.cs +++ b/src/Infrastructure/BotSharp.Abstraction/Conversations/ConversationHookBase.cs @@ -2,27 +2,23 @@ namespace BotSharp.Abstraction.Conversations; public abstract class ConversationHookBase : IConversationHook { - protected Agent _agent; - public Agent Agent => _agent; + public Agent Agent { get; private set; } - protected Conversation _conversation; - public Conversation Conversation => _conversation; + public Conversation Conversation { get; private set; } - protected List _dialogs; - public List Dialogs => _dialogs; + public List Dialogs { get; private set; } - protected int _priority = 0; - public int Priority => _priority; + public int Priority { get; protected set; } = 0; public IConversationHook SetAgent(Agent agent) { - _agent = agent; + Agent = agent; return this; } public IConversationHook SetConversation(Conversation conversation) { - _conversation = conversation; + Conversation = conversation; return this; } @@ -37,7 +33,7 @@ public abstract class ConversationHookBase : IConversationHook public virtual Task OnDialogsLoaded(List dialogs) { - _dialogs = dialogs; + Dialogs = dialogs; return Task.CompletedTask; } diff --git a/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs b/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs index 375cf3e6..bdf43c8a 100644 --- a/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs +++ b/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs @@ -28,7 +28,7 @@ public partial class ConversationService var dialogs = conv.GetDialogHistory(); var statistics = _services.GetRequiredService(); - var hooks = _services.GetServices().ToList(); + var hookProvider = _services.GetRequiredService(); RoleDialogModel response = message; bool stopCompletion = false; @@ -44,9 +44,7 @@ public partial class ConversationService message.Payload = replyMessage.Payload; } - // Before chat completion hook - hooks = ReOrderConversationHooks(hooks); - foreach (var hook in hooks) + foreach (var hook in hookProvider.HooksOrderByPriority) { hook.SetAgent(agent) .SetConversation(conversation); diff --git a/src/Plugins/BotSharp.Plugin.ChatHub/Hooks/ChatHubConversationHook.cs b/src/Plugins/BotSharp.Plugin.ChatHub/Hooks/ChatHubConversationHook.cs index b282fa53..6b425b4b 100644 --- a/src/Plugins/BotSharp.Plugin.ChatHub/Hooks/ChatHubConversationHook.cs +++ b/src/Plugins/BotSharp.Plugin.ChatHub/Hooks/ChatHubConversationHook.cs @@ -29,6 +29,7 @@ public class ChatHubConversationHook : ConversationHookBase _chatHub = chatHub; _user = user; _options = options; + Priority = -1; // Make sure this hook is the top one. } public override async Task OnConversationInitialized(Conversation conversation) From 4489683eebbc82720d2ab676dde9660fc7ba8998 Mon Sep 17 00:00:00 2001 From: Kerry Jiang Date: Wed, 11 Dec 2024 17:44:29 -0800 Subject: [PATCH 3/7] access public properties directly --- .../Evaluations/EvaluationConversationHook.cs | 22 +++++++++---------- .../Hooks/RateLimitConversationHook.cs | 2 +- .../RoutingConversationHook.cs | 6 ++--- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/Infrastructure/BotSharp.Core/Evaluations/EvaluationConversationHook.cs b/src/Infrastructure/BotSharp.Core/Evaluations/EvaluationConversationHook.cs index bfc0c1ec..b315b683 100644 --- a/src/Infrastructure/BotSharp.Core/Evaluations/EvaluationConversationHook.cs +++ b/src/Infrastructure/BotSharp.Core/Evaluations/EvaluationConversationHook.cs @@ -15,45 +15,45 @@ public class EvaluationConversationHook : ConversationHookBase public override Task OnMessageReceived(RoleDialogModel message) { - if (_conversation != null && _convSettings.EnableExecutionLog) + if (Conversation != null && _convSettings.EnableExecutionLog) { - _logger.Append(_conversation.Id, $"[{DateTime.Now}] {message.Role}: {message.Content}"); + _logger.Append(Conversation.Id, $"[{DateTime.Now}] {message.Role}: {message.Content}"); } return base.OnMessageReceived(message); } public override Task OnFunctionExecuted(RoleDialogModel message) { - if (_conversation != null && _convSettings.EnableExecutionLog) + if (Conversation != null && _convSettings.EnableExecutionLog) { - _logger.Append(_conversation.Id, $"[{DateTime.Now}] {message.Role}: {message.FunctionName}({message.FunctionArgs}) => {message.Content}"); + _logger.Append(Conversation.Id, $"[{DateTime.Now}] {message.Role}: {message.FunctionName}({message.FunctionArgs}) => {message.Content}"); } return base.OnFunctionExecuted(message); } public override Task OnResponseGenerated(RoleDialogModel message) { - if (_conversation != null && _convSettings.EnableExecutionLog) + if (Conversation != null && _convSettings.EnableExecutionLog) { - _logger.Append(_conversation.Id, $"[{DateTime.Now}] {message.Role}: {message.Content}"); - } + _logger.Append(Conversation.Id, $"[{DateTime.Now}] {message.Role}: {message.Content}"); + } return base.OnResponseGenerated(message); } public override Task OnHumanInterventionNeeded(RoleDialogModel message) { - if (_conversation != null && _convSettings.EnableExecutionLog) + if (Conversation != null && _convSettings.EnableExecutionLog) { - _logger.Append(_conversation.Id, $"[{DateTime.Now}] {AgentRole.Function}: trigger_event({{\"event\": \"{message.FunctionName}\"}})"); + _logger.Append(Conversation.Id, $"[{DateTime.Now}] {AgentRole.Function}: trigger_event({{\"event\": \"{message.FunctionName}\"}})"); } return base.OnHumanInterventionNeeded(message); } public override Task OnConversationEnding(RoleDialogModel message) { - if (_conversation != null && _convSettings.EnableExecutionLog) + if (Conversation != null && _convSettings.EnableExecutionLog) { - _logger.Append(_conversation.Id, $"[{DateTime.Now}] {AgentRole.Function}: trigger_event({{\"event\": \"{message.FunctionName}\"}})"); + _logger.Append(Conversation.Id, $"[{DateTime.Now}] {AgentRole.Function}: trigger_event({{\"event\": \"{message.FunctionName}\"}})"); } return base.OnConversationEnding(message); } diff --git a/src/Infrastructure/BotSharp.Logger/Hooks/RateLimitConversationHook.cs b/src/Infrastructure/BotSharp.Logger/Hooks/RateLimitConversationHook.cs index 1cb09f3e..a9a9478f 100644 --- a/src/Infrastructure/BotSharp.Logger/Hooks/RateLimitConversationHook.cs +++ b/src/Infrastructure/BotSharp.Logger/Hooks/RateLimitConversationHook.cs @@ -34,7 +34,7 @@ public class RateLimitConversationHook : ConversationHookBase } // Check message sending frequency - var userSents = _dialogs.Where(x => x.Role == AgentRole.User) + var userSents = Dialogs.Where(x => x.Role == AgentRole.User) .TakeLast(2).ToList(); if (userSents.Count > 1) diff --git a/src/Plugins/BotSharp.Plugin.RoutingSpeeder/RoutingConversationHook.cs b/src/Plugins/BotSharp.Plugin.RoutingSpeeder/RoutingConversationHook.cs index b7fab29e..b65e45c5 100644 --- a/src/Plugins/BotSharp.Plugin.RoutingSpeeder/RoutingConversationHook.cs +++ b/src/Plugins/BotSharp.Plugin.RoutingSpeeder/RoutingConversationHook.cs @@ -42,7 +42,7 @@ public class RoutingConversationHook: ConversationHookBase // Render by template var templateService = _services.GetRequiredService(); - var response = await templateService.RenderIntentResponse(_agent.Id, message); + var response = await templateService.RenderIntentResponse(Agent.Id, message); if (!string.IsNullOrEmpty(response)) { @@ -54,7 +54,7 @@ public class RoutingConversationHook: ConversationHookBase public override async Task OnResponseGenerated(RoleDialogModel message) { var routerSettings = _services.GetRequiredService(); - bool saveFlag = _agent.Type != AgentType.Routing; + bool saveFlag = Agent.Type != AgentType.Routing; if (saveFlag) { @@ -63,7 +63,7 @@ public class RoutingConversationHook: ConversationHookBase var rootDataPath = agentService.GetDataDir(); string rawDataDir = Path.Combine(rootDataPath, "raw_data", $"agent.{message.CurrentAgentId}.txt"); - var lastThreeDialogs = _dialogs.Where(x => x.Role == AgentRole.User || x.Role == AgentRole.Assistant) + var lastThreeDialogs = Dialogs.Where(x => x.Role == AgentRole.User || x.Role == AgentRole.Assistant) .Select(x => x.Content.Replace('\r', ' ').Replace('\n', ' ')) .TakeLast(3) .ToArray(); From f78425cffab83f0ccd86a034fbfa740bdf6542b5 Mon Sep 17 00:00:00 2001 From: Kerry Jiang Date: Thu, 12 Dec 2024 20:44:26 -0800 Subject: [PATCH 4/7] added unit test --- tests/UnitTest/MainTest.cs | 30 ++++++++++++++++++++++++++++++ tests/UnitTest/UnitTest.csproj | 8 +++++++- tests/UnitTest/UnitTest1.cs | 11 ----------- 3 files changed, 37 insertions(+), 12 deletions(-) create mode 100644 tests/UnitTest/MainTest.cs delete mode 100644 tests/UnitTest/UnitTest1.cs diff --git a/tests/UnitTest/MainTest.cs b/tests/UnitTest/MainTest.cs new file mode 100644 index 00000000..2e4c29bf --- /dev/null +++ b/tests/UnitTest/MainTest.cs @@ -0,0 +1,30 @@ +using Microsoft.Extensions.DependencyInjection; +using BotSharp.Abstraction.Conversations; +using BotSharp.Core.Evaluations; +using BotSharp.Plugin.ChatHub.Hooks; + +namespace UnitTest +{ + [TestClass] + public class MainTest + { + [TestMethod] + public void TestConversationHookProvider() + { + var services = new ServiceCollection(); + + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + + var serviceProvider = services.BuildServiceProvider(); + var conversationHookProvider = serviceProvider.GetService(); + + Assert.AreEqual(3, conversationHookProvider.Hooks.Count()); + + // ChatHubConversationHook has the top priority + Assert.IsInstanceOfType(conversationHookProvider.HooksOrderByPriority.FirstOrDefault()); + } + } +} \ No newline at end of file diff --git a/tests/UnitTest/UnitTest.csproj b/tests/UnitTest/UnitTest.csproj index be924573..3e1831ae 100644 --- a/tests/UnitTest/UnitTest.csproj +++ b/tests/UnitTest/UnitTest.csproj @@ -13,10 +13,16 @@ + + all runtime; build; native; contentfiles; analyzers; buildtransitive - + + + + + diff --git a/tests/UnitTest/UnitTest1.cs b/tests/UnitTest/UnitTest1.cs deleted file mode 100644 index ab3cd866..00000000 --- a/tests/UnitTest/UnitTest1.cs +++ /dev/null @@ -1,11 +0,0 @@ -namespace UnitTest -{ - [TestClass] - public class UnitTest1 - { - [TestMethod] - public void TestMethod1() - { - } - } -} \ No newline at end of file From b2334decff0c714bfdb9c1a97a69e72a8a529a1c Mon Sep 17 00:00:00 2001 From: Kerry Jiang Date: Sat, 14 Dec 2024 08:35:37 -0800 Subject: [PATCH 5/7] tried to fix the unit tests --- tests/UnitTest/MainTest.cs | 49 ++++++++++++++++++++++++++++------ tests/UnitTest/UnitTest.csproj | 2 -- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/tests/UnitTest/MainTest.cs b/tests/UnitTest/MainTest.cs index 2e4c29bf..4d2698d5 100644 --- a/tests/UnitTest/MainTest.cs +++ b/tests/UnitTest/MainTest.cs @@ -1,7 +1,5 @@ using Microsoft.Extensions.DependencyInjection; using BotSharp.Abstraction.Conversations; -using BotSharp.Core.Evaluations; -using BotSharp.Plugin.ChatHub.Hooks; namespace UnitTest { @@ -13,18 +11,53 @@ namespace UnitTest { var services = new ServiceCollection(); - services.AddSingleton(); - services.AddSingleton(); - services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); var serviceProvider = services.BuildServiceProvider(); var conversationHookProvider = serviceProvider.GetService(); - + Assert.AreEqual(3, conversationHookProvider.Hooks.Count()); - // ChatHubConversationHook has the top priority - Assert.IsInstanceOfType(conversationHookProvider.HooksOrderByPriority.FirstOrDefault()); + var prevHook = default(IConversationHook); + + // Assert priority + foreach (var hook in conversationHookProvider.HooksOrderByPriority) + { + if (prevHook != null) + { + Assert.IsTrue(prevHook.Priority < hook.Priority); + } + + prevHook = hook; + } + } + + class TestHookA : ConversationHookBase + { + public TestHookA() + { + Priority = 1; + } + } + + class TestHookB : ConversationHookBase + { + public TestHookA() + { + Priority = 2; + } + } + + class TestHookC : ConversationHookBase + { + public TestHookA() + { + Priority = 3; + } } } } \ No newline at end of file diff --git a/tests/UnitTest/UnitTest.csproj b/tests/UnitTest/UnitTest.csproj index 3e1831ae..67e7d504 100644 --- a/tests/UnitTest/UnitTest.csproj +++ b/tests/UnitTest/UnitTest.csproj @@ -22,7 +22,5 @@ - - From 4fde52456a18a9c12b503f3ff0073ac9577eaba2 Mon Sep 17 00:00:00 2001 From: Kerry Jiang Date: Sat, 14 Dec 2024 08:35:45 -0800 Subject: [PATCH 6/7] removed unused code --- .../Services/ConversationService.SendMessage.cs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs b/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs index bdf43c8a..f60d32cf 100644 --- a/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs +++ b/src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.SendMessage.cs @@ -171,18 +171,4 @@ public partial class ConversationService // Add to dialog history _storage.Append(_conversationId, response); } - - private List ReOrderConversationHooks(List hooks) - { - var target = "ChatHubConversationHook"; - var chathub = hooks.FirstOrDefault(x => x.GetType().Name == target); - var otherHooks = hooks.Where(x => x.GetType().Name != target).ToList(); - - if (chathub != null) - { - var newHooks = new List { chathub }.Concat(otherHooks); - return newHooks.ToList(); - } - return hooks; - } } From 7089f8f39fcb0c7dcdcf5e6aa803c69b66157906 Mon Sep 17 00:00:00 2001 From: Kerry Jiang Date: Sat, 14 Dec 2024 09:25:19 -0800 Subject: [PATCH 7/7] tried to fix the unit tests --- tests/UnitTest/MainTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/UnitTest/MainTest.cs b/tests/UnitTest/MainTest.cs index 4d2698d5..307855e4 100644 --- a/tests/UnitTest/MainTest.cs +++ b/tests/UnitTest/MainTest.cs @@ -46,7 +46,7 @@ namespace UnitTest class TestHookB : ConversationHookBase { - public TestHookA() + public TestHookB() { Priority = 2; } @@ -54,7 +54,7 @@ namespace UnitTest class TestHookC : ConversationHookBase { - public TestHookA() + public TestHookC() { Priority = 3; }