refine chart handler instruction & allow sending multiple messages

[iceljc]

User description

1. refine chart handler instruction
2. add mechanism to allow sending multiple messages in a single round

---

PR Type

Enhancement

---

Description

• Add mechanism for sending multiple sequential messages

• Refine chart handler instruction and configuration

• Improve conversation storage with additional message support

• Remove unnecessary dependencies and clean up code

---

Diagram Walkthrough

flowchart LR
  A["RoleDialogModel"] --> B["AdditionalMessageWrapper"]
  B --> C["Sequential Messages"]
  C --> D["ConversationStorage"]
  D --> E["Database"]
  F["ChatHub"] --> G["Delayed Message Sending"]
  H["ChartHandler"] --> I["Refined Instructions"]

Loading

File Walkthrough

	Relevant files
Enhancement	7 files ChatResponseDto.cs Add IsAppend property for message appending                            +3/-0      RoleDialogModel.cs Add AdditionalMessageWrapper for sequential messages          +16/-1    ConversationStorage.cs Support additional messages in conversation storage            +74/-52  RoutingService.InvokeFunction.cs Copy AdditionalMessageWrapper in function invocation          +1/-0      PlotChartFn.cs Replace delayed message sending with wrapper mechanism      +25/-54  ChatHubConversationHook.cs Handle additional messages in chat hub                                      +36/-2    util-chart-plot_instruction.liquid Refine chart plotting instruction template                              +21/-11
Formatting	2 files ConversationService.SendMessage.cs Remove unused import statement                                                      +0/-1      EventEmitter.cs Change class visibility to internal                                            +2/-2
Configuration changes	1 files ChartHandlerSettings.cs Add MaxOutputTokens configuration option                                  +3/-2
Dependencies	1 files BotSharp.Plugin.ChartHandler.csproj Remove ChatHub project reference dependency                            +0/-1

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Null Handling When expanding dialogs with AdditionalMessageWrapper, inner messages inherit minimal fields; ensure BuildDialogElement can tolerate nulls like MessageId, RichContent, or SecondaryRichContent and that CreatedAt/SenderId are set appropriately to avoid incomplete DB records. foreach ( var dialog in dialogs) { var innerList = new List<RoleDialogModel> { dialog }; if (dialog.AdditionalMessageWrapper != null && dialog.AdditionalMessageWrapper.SaveToDb && dialog.AdditionalMessageWrapper.Messages?.Count > 0) { innerList.AddRange(dialog.AdditionalMessageWrapper.Messages); } foreach (var item in innerList) { var element = BuildDialogElement(item); if (element != null) { dialogElements.Add(element); } } } Message Identity Additional appended messages reuse or may omit MessageId; verify each appended message has a unique MessageId to prevent client-side duplication, ordering issues, or DB primary key conflicts. if (message.AdditionalMessageWrapper?.Messages?.Count > 0) { action.SenderAction = SenderActionEnum.TypingOn; await SendEvent(ChatEvent.OnSenderActionGenerated, conv.ConversationId, action); foreach (var item in message.AdditionalMessageWrapper.Messages) { await Task.Delay(message.AdditionalMessageWrapper.IntervalMilliSeconds); data = new ChatResponseDto { ConversationId = conv.ConversationId, MessageId = item.MessageId, Text = !string.IsNullOrEmpty(item.SecondaryContent) ? item.SecondaryContent : item.Content, Function = item.FunctionName, RichContent = item.SecondaryRichContent ?? item.RichContent, Data = item.Data, States = state.GetStates(), IsAppend = true, Sender = new() { FirstName = "AI", LastName = "Assistant", Role = AgentRole.Assistant } }; await SendEvent(ChatEvent.OnMessageReceivedFromAssistant, conv.ConversationId, data); } action.SenderAction = SenderActionEnum.TypingOff; await SendEvent(ChatEvent.OnSenderActionGenerated, conv.ConversationId, action); } Instruction Consistency The template enforces ECharts import and a custom toolbox fullscreen feature; validate that the specified toolbox.feature 'myFullscreen' usage aligns with ECharts API and that only one script import occurs even when multiple <script> blocks are generated. Please take a look at "Plotting Requirement" and generate a javascript code that can be used to render the charts on an html element. You must strictly follow the "Hard Requirements", "Render Requirements", "Code Requirements" and "Response Format" below. === Plotting Requirement === {{ plotting_requirement }} ***** Hard Requirements ***** ** Your output javascript code must be wrapped in one or multiple <script>...</script> blocks with everything needed inside. ** You need to import ECharts.js exactly once to plot the charts. The script source is "https://cdnjs.cloudflare.com/ajax/libs/echarts/5.5.1/echarts.min.js". ** You must add an ECharts Toolbox bar at the top left corner of the chart. ** ALWAYS add a "Full screen" button right next to the Toolbox bar. ** The "Full screen" button can toggle the chart container in and out of fullscreen using the Fullscreen API. Requirements for this button: * Always call the Fullscreen API on the chart container div itself (document.getElementById("{{ chart_element_id }}")), not on the document. * Use el.requestFullscreen() with fallbacks to el.webkitRequestFullscreen || el.msRequestFullscreen. * Exit fullscreen with document.exitFullscreen() and vendor fallbacks. * Listen for fullscreenchange, webkitfullscreenchange, and msfullscreenchange to keep the button working across repeated clicks and ESC exits. * Ensure the chart fully expands and scales to the entire screen when fullscreen is active. * fullscreenBtn must be a fully-formed object {show: true, name, title, icon: 'path://M3 3 H9 V5 H5 V9 H3 Z M15 3 H21 V9 H19 V5 H15 Z M3 15 H5 V19 H9 V21 H3 Z M19 15 H21 V21 H15 V19 H19 Z', onclick}. * When using "chart.setOption" to define the fullscreen button, DO NOT use "graphic". Include the fullscreenBtn object in toolbox.feature with name 'myFullscreen'. ***** Render Requirements ***** ** You must render the charts under the div html element with id {{ chart_element_id }}. ** You must not create any new html element. ** You must ensure the generated charts have visible height (at least 500px) and width (at least 800px). DO NOT generate charts with zero height or zero width. ** You must not apply any styles on any html element. ***** Code Requirements ***** ** You must strictly follow the valid javascript and ECharts.js syntax. ** You must ensure no syntax/runtime error before returning the response. ** Please ensure the code can be executed and the charts are rendered correctly. ***** Response Format ***** You must output the response in the following JSON format: { "greeting_message": "A short polite message that informs user that the charts have been generated.", "js_code": "The javascript code that can generate the charts as requested.",

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Avoid blocking delays in hooks The sequential message feature uses Task.Delay inside ChatHubConversationHook.OnResponseGenerated, which blocks the hook pipeline (and likely the request thread) for each delayed message, harming throughput and risking timeouts under load. Offload these delayed sends to a background dispatcher (e.g., queue/hosted service) or a fire-and-forget workflow with proper error handling and cancellation, so request processing and other hooks are not blocked. Persisting additional messages is already handled by ConversationStorage, so dispatching can be decoupled from the response lifecycle. Examples: src/Plugins/BotSharp.Plugin.ChatHub/Hooks/ChatHubConversationHook.cs [125-158] if (message.AdditionalMessageWrapper?.Messages?.Count > 0) { action.SenderAction = SenderActionEnum.TypingOn; await SendEvent(ChatEvent.OnSenderActionGenerated, conv.ConversationId, action); foreach (var item in message.AdditionalMessageWrapper.Messages) { await Task.Delay(message.AdditionalMessageWrapper.IntervalMilliSeconds); data = new ChatResponseDto ... (clipped 24 lines) Solution Walkthrough: Before: // In ChatHubConversationHook.cs public override async Task OnResponseGenerated(RoleDialogModel message) { // ... send initial message ... await SendEvent(...); if (message.AdditionalMessageWrapper?.Messages?.Count > 0) { // ... foreach (var item in message.AdditionalMessageWrapper.Messages) { // This delays the entire hook execution await Task.Delay(message.AdditionalMessageWrapper.IntervalMilliSeconds); await SendEvent(...); } // ... } // This call is delayed until all additional messages are sent await base.OnResponseGenerated(message); } After: // In ChatHubConversationHook.cs public override async Task OnResponseGenerated(RoleDialogModel message) { // ... send initial message ... await SendEvent(...); if (message.AdditionalMessageWrapper?.Messages?.Count > 0) { // Offload to a background task and do not wait for it _ = SendAdditionalMessagesInBackground(message, ...); } // This runs immediately, completing the hook quickly await base.OnResponseGenerated(message); } private async Task SendAdditionalMessagesInBackground(RoleDialogModel message, ...) { // This method runs in the background (fire-and-forget) // It should handle its own dependencies and error logging foreach (var item in message.AdditionalMessageWrapper.Messages) { await Task.Delay(message.AdditionalMessageWrapper.IntervalMilliSeconds); await SendEvent(...); } } Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical design flaw where await Task.Delay in a hook prolongs the request lifecycle, impacting scalability and robustness, and proposes a standard, effective architectural solution.	High
Possible issue	Generate unique MessageId for additional messages The additional message uses the same MessageId as the original message, which could cause conflicts in the database or message tracking systems. Generate a unique MessageId for the additional message to ensure proper identification and storage. src/Plugins/BotSharp.Plugin.ChartHandler/Functions/PlotChartFn.cs [76-94] message.AdditionalMessageWrapper = new() { IntervalMilliSeconds = 1500, SaveToDb = true, Messages = new List<RoleDialogModel> { new() { Role = AgentRole.Assistant, - MessageId = message.MessageId, + MessageId = Guid.NewGuid().ToString(), CurrentAgentId = message.CurrentAgentId, Content = obj.ReportSummary, FunctionName = message.FunctionName, FunctionArgs = message.FunctionArgs, CreatedAt = DateTime.UtcNow } } }; [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that reusing message.MessageId for an additional message could lead to data conflicts, and proposes generating a new unique ID, which is critical for message tracking and database integrity.	Medium
	Add null validation for messages The loop processes additional messages from AdditionalMessageWrapper but doesn't validate if the messages are properly initialized. Add null checks for critical properties like MessageId and Role before processing to prevent potential runtime errors. src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationStorage.cs [43-50] foreach (var item in innerList) { + if (item == null || string.IsNullOrEmpty(item.MessageId) || string.IsNullOrEmpty(item.Role)) + { + continue; + } var element = BuildDialogElement(item); if (element != null) { dialogElements.Add(element); } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a potential NullReferenceException if an item in innerList is null and proposes adding checks, which improves the code's robustness.	Low
More