Skip to content

Conversation

iceljc
Copy link
Collaborator

@iceljc iceljc commented Sep 8, 2025

PR Type

Enhancement


Description

  • Add MessageLabel property to conversation models

  • Update data transfer objects with label support

  • Modify storage and routing services for label handling

  • Enhance API responses with message labeling


Diagram Walkthrough

flowchart LR
  A["Conversation Models"] --> B["Storage Layer"]
  B --> C["API Controllers"]
  C --> D["Chat Responses"]
  A --> E["Routing Services"]
  E --> F["Function Execution"]
Loading

File Walkthrough

Relevant files
Enhancement
10 files
ChatResponseDto.cs
Add MessageLabel property to chat response DTO                     
+4/-0     
Conversation.cs
Add MessageLabel to DialogMetaData model                                 
+4/-0     
RoleDialogModel.cs
Add MessageLabel property and copy logic                                 
+6/-0     
ConversationStorage.cs
Update storage service with MessageLabel handling               
+11/-13 
RoutingService.InvokeAgent.cs
Add MessageLabel assignment in agent invocation                   
+2/-0     
RoutingService.InvokeFunction.cs
Include MessageLabel in function invocation cloning           
+1/-0     
ConversationController.cs
Add MessageLabel to API dialog responses                                 
+2/-0     
PlotChartFn.cs
Set chart_report_summary label for chart messages               
+2/-1     
ChatHubConversationHook.cs
Include MessageLabel in chat hub responses                             
+2/-0     
DialogMongoElement.cs
Add MessageLabel to MongoDB storage models                             
+3/-0     

Copy link

qodo-merge-pro bot commented Sep 8, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Null Handling

New assignments rely on nullable meta fields but also set defaults; verify that using default values (e.g., empty strings or default DateTime) does not mask data issues and that consumers tolerate these defaults without misbehavior.

var record = new RoleDialogModel(role, content)
{
    CurrentAgentId = meta?.AgentId ?? string.Empty,
    MessageId = meta?.MessageId ?? string.Empty,
    MessageType = meta?.MessageType ?? string.Empty,
    MessageLabel = meta?.MessageLabel,
    CreatedAt = meta?.CreatedTime ?? default,
    SenderId = senderId,
    FunctionName = meta?.FunctionName,
    FunctionArgs = meta?.FunctionArgs,
    ToolCallId = meta?.ToolCallId,
    RichContent = richContent,
    SecondaryContent = secondaryContent,
    SecondaryRichContent = secondaryRichContent,
    Payload = payload
Label Propagation

MessageLabel is propagated from response to messages; confirm all code paths (function and assistant responses) consistently set or clear labels to avoid stale labels persisting across messages.

{
    // Forgot about what situation needs to handle in this way
    response.Content = "Apologies, I'm not quite sure I understand. Could you please provide additional clarification or context?";
}

message = RoleDialogModel.From(message, role: AgentRole.Assistant, content: response.Content);
message.CurrentAgentId = agent.Id;
message.IsStreaming = response.IsStreaming;
message.MessageLabel = response.MessageLabel;
message.AdditionalMessageWrapper = null;
Hardcoded Label

Using a hardcoded MessageLabel value may impact analytics or filtering; ensure naming is stable and documented or consider centralizing label constants.

new(AgentRole.Assistant, obj.ReportSummary)
{
    MessageId = message.MessageId,
    MessageLabel = "chart_report_summary",
    Indication = "Summarizing",
    CurrentAgentId = message.CurrentAgentId,
    FunctionName = message.FunctionName,
    FunctionArgs = message.FunctionArgs,
    CreatedAt = DateTime.UtcNow

@iceljc iceljc merged commit ae2b5e2 into SciSharp:master Sep 8, 2025
3 of 4 checks passed
Copy link

qodo-merge-pro bot commented Sep 8, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Persist label across all storages

MessageLabel is wired through models and APIs, but only the Mongo storage was
updated to persist/hydrate it; other repository backends will silently drop the
label on save/load. This leads to inconsistent behavior depending on the storage
provider and breaks the feature end-to-end. Update all storage implementations
(schemas/mappers/migrations) and projections to consistently persist and read
message_label.

Examples:

src/Plugins/BotSharp.Plugin.MongoStorage/Models/DialogMongoElement.cs [49]
    public string? MessageLabel { get; set; }
src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationStorage.cs [80]
                MessageLabel = meta?.MessageLabel,

Solution Walkthrough:

Before:

// In ConversationStorage.cs
public List<RoleDialogModel> GetDialogs(string conversationId)
{
    var db = _services.GetRequiredService<IBotSharpRepository>();
    // For non-Mongo providers, `GetConversationDialogs` won't load the label.
    var dialogs = db.GetConversationDialogs(conversationId);

    foreach (var dialog in dialogs)
    {
        var record = new RoleDialogModel(...)
        {
            // ...
            MessageLabel = dialog.MetaData?.MessageLabel, // This will be null.
            // ...
        };
    }
}

After:

// In a hypothetical other storage provider (e.g., EntityFramework)
// The DB entity would be updated.
public class DialogEntity
{
    // ...
    public string? MessageLabel { get; set; }
}

// The repository implementation would map the new field.
public List<DialogElement> GetConversationDialogs(string conversationId)
{
    // ... query DB ...
    return dbDialogs.Select(d => new DialogElement
    {
        MetaData = new DialogMetaData {
            // ...
            MessageLabel = d.MessageLabel // The label is now correctly loaded.
        },
        // ...
    }).ToList();
}
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical omission where the new MessageLabel feature is only implemented for one storage provider (MongoStorage), leading to data loss and inconsistent behavior for other backends.

High
Possible issue
Prevent null sender and invalid dates

Ensure senderId is never null and avoid using DateTime.MinValue when metadata is
missing. Coalesce senderId to an empty string and default CreatedAt to
DateTime.UtcNow to keep message timelines and non-nullable fields consistent.

src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationStorage.cs [69-89]

-var senderId = role == AgentRole.Function ? meta?.AgentId : meta?.SenderId;
+var senderId = role == AgentRole.Function ? (meta?.AgentId ?? string.Empty) : (meta?.SenderId ?? string.Empty);
 ...
 var record = new RoleDialogModel(role, content)
 {
     CurrentAgentId = meta?.AgentId ?? string.Empty,
     MessageId = meta?.MessageId ?? string.Empty,
     MessageType = meta?.MessageType ?? string.Empty,
     MessageLabel = meta?.MessageLabel,
-    CreatedAt = meta?.CreatedTime ?? default,
+    CreatedAt = meta?.CreatedTime ?? DateTime.UtcNow,
     SenderId = senderId,
     FunctionName = meta?.FunctionName,
     FunctionArgs = meta?.FunctionArgs,
     ToolCallId = meta?.ToolCallId,
     RichContent = richContent,
     SecondaryContent = secondaryContent,
     SecondaryRichContent = secondaryRichContent,
     Payload = payload

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that CreatedAt could be initialized to default(DateTime), which is DateTime.MinValue. Using DateTime.UtcNow is a more sensible default. It also makes senderId non-null, which is good practice, although its immediate impact is less critical.

Medium
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant