fix template rendering by states

[iceljc]

PR Type

Enhancement

---

Description

• Refactor template rendering methods to accept optional render data

• Add centralized render data collection with conversation states

• Update all LLM providers to use consistent rendering approach

• Move function filtering methods to private scope

---

Diagram Walkthrough

flowchart LR
  A["Agent Service Interface"] --> B["Render Methods"]
  B --> C["CollectRenderData()"]
  C --> D["Conversation States"]
  C --> E["Agent TemplateDict"]
  B --> F["LLM Providers"]
  F --> G["Consistent Rendering"]

Loading

File Walkthrough

	Relevant files
Enhancement	14 files IAgentService.cs Add optional renderData parameters to rendering methods    +6/-7      BasicAgentHook.cs Use centralized render data for visibility checks                +3/-2      AgentService.Rendering.cs Implement centralized render data collection and refactor methods +48/-38  ChatCompletionProvider.cs Update to use centralized render data collection                  +2/-1      ChatCompletionProvider.cs Apply consistent rendering with render data parameter        +7/-15    ChatCompletionProvider.cs Update rendering calls with render data parameter                +7/-3      GeminiChatCompletionProvider.cs Implement render data parameter in function rendering        +7/-3      PalmChatCompletionProvider.cs Add render data collection for instruction preparation      +2/-1      RealTimeCompletionProvider.cs Update realtime provider with render data support                +7/-3      ChatCompletionProvider.cs Apply render data collection to instruction preparation    +2/-1      MicrosoftExtensionsAIChatCompletionProvider.cs Update function rendering with render data parameter          +4/-3      ChatCompletionProvider.cs Implement consistent rendering with render data support    +7/-3      RealTimeCompletionProvider.cs Add render data parameter to realtime completion                  +7/-3      ChatCompletionProvider.cs Update instruction preparation with render data collection +2/-1
Tests	2 files TestAgentService.cs Update test implementation with new method signatures        +10/-15  SemanticKernelChatCompletionProviderTests.cs Fix test mock to match updated method signature                    +1/-1

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Null Handling In RenderInstruction and RenderTemplate, when a non-null renderData is passed, a new dictionary is created from it without adding conversation states via CollectRenderData, which differs from the fallback path. Confirm this asymmetry is intentional and that callers always include needed state keys in renderData, otherwise templates/functions might miss state-derived values. // update states var renderDict = renderData != null ? new Dictionary<string, object>(renderData ?? []) : CollectRenderData(agent); renderDict[TemplateRenderConstant.RENDER_AGENT] = agent; var res = render.Render(string.Join("\r\n", instructions), renderDict); return res; } public bool RenderFunction(Agent agent, FunctionDef def, Dictionary<string, object>? renderData = null) { var renderDict = renderData ?? agent.TemplateDict; var isRender = true; var channels = def.Channels; Visibility Context RenderFunction and RenderFunctionProperty use renderData ?? agent.TemplateDict. If renderData is provided but does not include the same keys previously present in TemplateDict, visibility evaluation may change unexpectedly. Validate that all provider call sites pass consistent renderData gathered from CollectRenderData to avoid regressions. public bool RenderFunction(Agent agent, FunctionDef def, Dictionary<string, object>? renderData = null) { var renderDict = renderData ?? agent.TemplateDict; var isRender = true; var channels = def.Channels; if (channels != null) { var state = _services.GetRequiredService<IConversationStateService>(); var channel = state.GetState("channel"); if (!string.IsNullOrWhiteSpace(channel)) { isRender = isRender && channels.Contains(channel); } } if (!isRender) return false; if (!string.IsNullOrWhiteSpace(def.VisibilityExpression)) { isRender = RenderVisibility(def.VisibilityExpression, renderDict); } return isRender; } public FunctionParametersDef? RenderFunctionProperty(Agent agent, FunctionDef def, Dictionary<string, object>? renderData = null) { var parameterDef = def?.Parameters; var propertyDef = parameterDef?.Properties; if (propertyDef == null) return null; var renderDict = renderData ?? agent.TemplateDict; var visibleExpress = "visibility_expression"; var root = propertyDef.RootElement; var iterator = root.EnumerateObject(); var visibleProps = new List<string>(); while (iterator.MoveNext()) { var prop = iterator.Current; var name = prop.Name; var node = prop.Value; var matched = true; if (node.TryGetProperty(visibleExpress, out var element)) { var expression = element.GetString(); matched = RenderVisibility(expression, renderDict); } API Consistency FilterFunctions moved to private scope; ensure no external components relied on public overloads removed from IAgentService. This is a breaking change for consumers outside this assembly; confirm intended surface area change. #region Private methods private IEnumerable<FunctionDef> FilterFunctions(string instruction, Agent agent, StringComparer? comparer = null) { var functions = agent.Functions.Concat(agent.SecondaryFunctions ?? []); if (agent.FuncVisMode.IsEqualTo(AgentFuncVisMode.Auto) && !string.IsNullOrWhiteSpace(instruction)) { functions = FilterFunctions(instruction, functions, comparer); } return functions; } private IEnumerable<FunctionDef> FilterFunctions(string instruction, IEnumerable<FunctionDef> functions, StringComparer? comparer = null) { comparer = comparer ?? StringComparer.OrdinalIgnoreCase; var matches = Regex.Matches(instruction, @"\b[A-Za-z0-9_-]+\b"); var words = new HashSet<string>(matches.Select(m => m.Value), comparer); return functions.Where(x => words.Contains(x.Name, comparer)); } #endregion

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix invalid empty dictionary syntax Fix a compilation error by replacing the invalid empty collection expression [] with new Dictionary<string, object>() when initializing renderDict. src/Infrastructure/BotSharp.Core/Agents/Services/AgentService.Rendering.cs [156-167] public Dictionary<string, object> CollectRenderData(Agent agent) { var state = _services.GetRequiredService<IConversationStateService>(); - var renderDict = new Dictionary<string, object>(agent?.TemplateDict ?? []); + var renderDict = new Dictionary<string, object>(agent?.TemplateDict ?? new Dictionary<string, object>()); foreach (var t in state.GetStates()) { renderDict[t.Key] = t.Value; } return renderDict; } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a compilation error in the new CollectRenderData method due to invalid syntax ([]) for creating an empty dictionary and provides the correct fix.	High
High-level	Unify fallback logic in rendering methods The rendering methods in AgentService.Rendering.cs exhibit inconsistent fallback logic. To prevent bugs and improve API usability, all rendering methods should call CollectRenderData when a data dictionary is not provided, ensuring a complete and consistent rendering context. Examples: src/Infrastructure/BotSharp.Core/Agents/Services/AgentService.Rendering.cs [28-30] public bool RenderFunction(Agent agent, FunctionDef def, Dictionary<string, object>? renderData = null) { var renderDict = renderData ?? agent.TemplateDict; src/Infrastructure/BotSharp.Core/Agents/Services/AgentService.Rendering.cs [54-60] public FunctionParametersDef? RenderFunctionProperty(Agent agent, FunctionDef def, Dictionary<string, object>? renderData = null) { var parameterDef = def?.Parameters; var propertyDef = parameterDef?.Properties; if (propertyDef == null) return null; var renderDict = renderData ?? agent.TemplateDict; Solution Walkthrough: Before: // In AgentService.Rendering.cs public string RenderInstruction(Agent agent, Dictionary<string, object>? renderData = null) { var renderDict = renderData != null ? ... : CollectRenderData(agent); // ... } public bool RenderFunction(Agent agent, FunctionDef def, Dictionary<string, object>? renderData = null) { var renderDict = renderData ?? agent.TemplateDict; // Inconsistent: omits conversation states // ... } public FunctionParametersDef? RenderFunctionProperty(Agent agent, FunctionDef def, Dictionary<string, object>? renderData = null) { var renderDict = renderData ?? agent.TemplateDict; // Inconsistent: omits conversation states // ... } After: // In AgentService.Rendering.cs public string RenderInstruction(Agent agent, Dictionary<string, object>? renderData = null) { var renderDict = renderData ?? CollectRenderData(agent); // ... } public bool RenderFunction(Agent agent, FunctionDef def, Dictionary<string, object>? renderData = null) { var renderDict = renderData ?? CollectRenderData(agent); // Consistent: includes conversation states // ... } public FunctionParametersDef? RenderFunctionProperty(Agent agent, FunctionDef def, Dictionary<string, object>? renderData = null) { var renderDict = renderData ?? CollectRenderData(agent); // Consistent: includes conversation states // ... } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a significant design inconsistency in the fallback logic for the new rendering methods, which makes the API fragile and could lead to subtle bugs in the future.	Medium
More