Merge pull request #785 from kerryjiang/master

Introduce ConversationHookProvider to avoid ordering conversation hooks in runtime

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<RoleDialogModel> _dialogs;
-    public List<RoleDialogModel> Dialogs => _dialogs;
+    public List<RoleDialogModel> 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<RoleDialogModel> dialogs)
     {
-        _dialogs = dialogs;
+        Dialogs = dialogs;
         return Task.CompletedTask;
     }
 

src/Infrastructure/BotSharp.Abstraction/Conversations/ConversationHookProvider.cs

@@ -0,0 +1,20 @@
+namespace BotSharp.Abstraction.Conversations;
+
+public class ConversationHookProvider
+{
+    public IEnumerable<IConversationHook> Hooks { get; }
+
+    private readonly Lazy<IEnumerable<IConversationHook>> _hooksOrderByPriority;
+
+    public IEnumerable<IConversationHook> HooksOrderByPriority
+        => _hooksOrderByPriority.Value;
+
+    public ConversationHookProvider(IEnumerable<IConversationHook> conversationHooks)
+    {
+        Hooks = conversationHooks;
+        _hooksOrderByPriority = new Lazy<IEnumerable<IConversationHook>>(() =>
+        {
+            return conversationHooks.OrderBy(hook => hook.Priority).ToArray();
+        });
+    }
+}

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<ITokenStatistics>();
-        var hooks = _services.GetServices<IConversationHook>().ToList();
+        var hookProvider = _services.GetRequiredService<ConversationHookProvider>();
 
         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);
@@ -173,18 +171,4 @@ public partial class ConversationService
         // Add to dialog history
         _storage.Append(_conversationId, response);
     }
-
-    private List<IConversationHook> ReOrderConversationHooks(List<IConversationHook> 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<IConversationHook> { chathub }.Concat(otherHooks);
-            return newHooks.ToList();
-        }
-        return hooks;
-    }
 }

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<IConversationHook>().ToList();
+        var hooks = _services.GetServices<IConversationHook>();
         foreach (var hook in hooks)
         {
             await hook.OnMessageDeleted(conversationId, messageId);

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<IConversationHook>()
-            .OrderBy(x => x.Priority)
-            .ToList();
+        var hooks = _services
+            .GetRequiredService<ConversationHookProvider>()
+            .HooksOrderByPriority;
 
         // Before executing functions
         foreach (var hook in hooks)

src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationService.cs

@@ -103,7 +103,8 @@ public partial class ConversationService : IConversationService
 
         db.CreateNewConversation(record);
 
-        var hooks = _services.GetServices<IConversationHook>().ToList();
+        var hooks = _services.GetServices<IConversationHook>();
+
         foreach (var hook in hooks)
         {
             // If user connect agent first time

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);
     }

src/Infrastructure/BotSharp.Core/Routing/Functions/HumanInterventionNeededFn.cs

@@ -15,9 +15,9 @@ public class HumanInterventionNeededFn : IFunctionCallback
 
     public async Task<bool> Execute(RoleDialogModel message)
     {
-        var hooks = _services.GetServices<IConversationHook>()
-            .OrderBy(x => x.Priority)
-            .ToList();
+        var hooks = _services
+            .GetRequiredService<ConversationHookProvider>()
+            .HooksOrderByPriority;
 
         foreach (var hook in hooks)
         {

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<IConversationHook>()
-            .OrderBy(x => x.Priority)
-            .ToList();
+        var hooks = _services
+            .GetRequiredService<ConversationHookProvider>()
+            .HooksOrderByPriority;
 
         var progressService = _services.GetService<IConversationProgressService>();
 

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)

src/Plugins/BotSharp.Plugin.ChatHub/ChatHubPlugin.cs

@@ -22,6 +22,7 @@ public class ChatHubPlugin : IBotSharpPlugin
         services.AddScoped<IConversationHook, ChatHubConversationHook>();
         services.AddScoped<IConversationHook, StreamingLogHook>();
         services.AddScoped<IConversationHook, WelcomeHook>();
+        services.AddScoped<ConversationHookProvider>();
         services.AddScoped<IRoutingHook, StreamingLogHook>();
         services.AddScoped<IContentGeneratingHook, StreamingLogHook>();
         services.AddScoped<ICrontabHook, ChatHubCrontabHook>();

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)

src/Plugins/BotSharp.Plugin.RoutingSpeeder/RoutingConversationHook.cs

@@ -42,7 +42,7 @@ public class RoutingConversationHook: ConversationHookBase
 
         // Render by template
         var templateService = _services.GetRequiredService<IResponseTemplateService>();
-        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<RoutingSettings>();
-        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();

tests/UnitTest/MainTest.cs

@@ -0,0 +1,63 @@
+using Microsoft.Extensions.DependencyInjection;
+using BotSharp.Abstraction.Conversations;
+
+namespace UnitTest
+{    
+    [TestClass]
+    public class MainTest
+    {
+        [TestMethod]
+        public void TestConversationHookProvider()
+        {
+            var services = new ServiceCollection();
+
+            services.AddSingleton<IConversationHook, TestHookC>();
+            services.AddSingleton<IConversationHook, TestHookA>();
+            services.AddSingleton<IConversationHook, TestHookB>();
+            
+            services.AddSingleton<ConversationHookProvider>();
+
+            var serviceProvider = services.BuildServiceProvider();
+            var conversationHookProvider = serviceProvider.GetService<ConversationHookProvider>();
+
+            Assert.AreEqual(3, conversationHookProvider.Hooks.Count());
+
+            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 TestHookB()
+            {
+                Priority = 2;
+            }
+        }
+
+        class TestHookC : ConversationHookBase
+        {
+            public TestHookC()
+            {
+                Priority = 3;
+            }
+        }
+    }
+}

tests/UnitTest/UnitTest.csproj

@@ -13,10 +13,14 @@
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
     <PackageReference Include="MSTest.TestAdapter" Version="3.1.1" />
     <PackageReference Include="MSTest.TestFramework" Version="3.1.1" />
+    <PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="8.0.2" />
+    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.1" />
     <PackageReference Include="coverlet.collector" Version="6.0.0">
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
   </ItemGroup>
-
+  <ItemGroup>
+    <ProjectReference Include="..\..\src\Infrastructure\BotSharp.Abstraction\BotSharp.Abstraction.csproj" />
+  </ItemGroup>
 </Project>

tests/UnitTest/UnitTest1.cs

@@ -1,11 +0,0 @@
-namespace UnitTest
-{
-    [TestClass]
-    public class UnitTest1
-    {
-        [TestMethod]
-        public void TestMethod1()
-        {
-        }
-    }
-}