Skip to content

Conversation

iceljc
Copy link
Collaborator

@iceljc iceljc commented Jul 22, 2025

User description

Allow using template across agents


PR Type

Enhancement


Description

  • Enhanced template linking to support cross-agent template usage

  • Added case-insensitive agent name filtering

  • Implemented from_agent filter for template rendering

  • Refactored link tag from identifier to expression-based


Diagram Walkthrough

flowchart LR
  A["Agent A"] -- "link template from_agent" --> B["Agent B Template"]
  C["Template Render"] -- "case-insensitive filter" --> D["Agent Service"]
  E["Expression Tag"] -- "evaluates" --> F["Cross-Agent Template"]
Loading

File Walkthrough

Relevant files
Enhancement
FileRepository.Agent.cs
Case-insensitive agent name filtering                                       

src/Infrastructure/BotSharp.Core/Repository/FileRepository/FileRepository.Agent.cs

  • Added case-insensitive string comparison for agent name filtering
+1/-1     
TemplateRender.cs
Cross-agent template linking implementation                           

src/Infrastructure/BotSharp.Core/Templating/TemplateRender.cs

  • Added from_agent filter for cross-agent template access
  • Refactored link tag from identifier to expression-based rendering
  • Enhanced template resolution to support agent-specific template lookup
  • Added regex parsing for template name and agent name extraction
+48/-8   

Copy link

qodo-merge-pro bot commented Jul 22, 2025

PR Reviewer Guide 🔍

(Review updated until commit b1eb754)

Here are some key observations to aid the review process:

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

Possible Issue

The regex split logic may not handle edge cases properly. The code splits on 'from_agent' keyword but doesn't validate the resulting array structure, potentially causing issues with malformed template expressions.

var splited = Regex.Split(expStr, @"\s*from_agent\s*", RegexOptions.IgnoreCase | RegexOptions.CultureInvariant)
                   .Where(x => !string.IsNullOrWhiteSpace(x))
                   .Select(x => x.Trim())
                   .ToArray();

var templateName = splited.ElementAtOrDefault(0);
var agentName = splited.ElementAtOrDefault(1);
Performance Concern

Creating a new service scope and making an async database call inside the template rendering process could impact performance, especially for templates with multiple cross-agent references.

    using var scope = services.CreateScope();
    var agentService = scope.ServiceProvider.GetRequiredService<IAgentService>();
    var result = await agentService.GetAgents(new() { SimilarName = agentName });
    agent = result?.Items?.FirstOrDefault();
}
Logic Issue

The agent lookup uses SimilarName filter which may return multiple agents, but only the first one is selected without validation. This could lead to incorrect agent selection when multiple agents have similar names.

    var result = await agentService.GetAgents(new() { SimilarName = agentName });
    agent = result?.Items?.FirstOrDefault();
}

Copy link

qodo-merge-pro bot commented Jul 22, 2025

PR Code Suggestions ✨

Latest suggestions up to b1eb754
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Use exact name matching

The agent lookup uses SimilarName filter which may not find exact matches and
could return unexpected results. Consider using AgentNames filter for more
precise agent matching when looking up by name.

src/Infrastructure/BotSharp.Core/Templating/TemplateRender.cs [113-119]

 if (splited.Length > 1 && !agentName.IsEqualTo(agent?.Name))
 {
     using var scope = services.CreateScope();
     var agentService = scope.ServiceProvider.GetRequiredService<IAgentService>();
-    var result = await agentService.GetAgents(new() { SimilarName = agentName });
+    var result = await agentService.GetAgents(new() { AgentNames = new[] { agentName } });
     agent = result?.Items?.FirstOrDefault();
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that using SimilarName could lead to incorrect agent selection, and appropriately recommends using AgentNames for a more precise, case-insensitive match, which aligns with other changes in the PR.

Medium
  • More

Previous suggestions

Suggestions up to commit 1cabd91
CategorySuggestion                                                                                                                                    Impact
General
Compile regex for performance optimization

The regex pattern should be compiled for better performance since it's used in
template rendering which could be called frequently. Consider using a static
compiled regex to avoid recompilation overhead.

src/Infrastructure/BotSharp.Core/Templating/TemplateRender.cs [102-105]

-var splited = Regex.Split(expStr, @"\s*from\s*", RegexOptions.IgnoreCase | RegexOptions.CultureInvariant)
-                       .Where(x => !string.IsNullOrWhiteSpace(x))
-                       .Select(x => x.Trim())
-                       .ToArray();
+private static readonly Regex FromSyntaxRegex = new Regex(@"\s*from\s*", RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.Compiled);
 
+var splited = FromSyntaxRegex.Split(expStr)
+                            .Where(x => !string.IsNullOrWhiteSpace(x))
+                            .Select(x => x.Trim())
+                            .ToArray();
+
Suggestion importance[1-10]: 6

__

Why: This is a valid performance optimization, as using a static compiled regex for a frequently used pattern avoids recompilation overhead.

Low

@iceljc iceljc marked this pull request as draft July 22, 2025 14:46
@iceljc iceljc marked this pull request as ready for review July 22, 2025 19:05
@iceljc iceljc merged commit 7bec0fa into SciSharp:master Jul 23, 2025
4 checks passed
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