You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add agent function visibility mode with auto/manual filtering
Refactor vector storage to use typed payload values
Enhance vector filtering with complex query operators
Update all LLM providers to use new instruction/function preparation
Diagram Walkthrough
flowchart LR
A["Agent Model"] --> B["Function Visibility Mode"]
B --> C["Auto Function Filtering"]
B --> D["Manual Function Selection"]
E["Vector Storage"] --> F["Typed Payload Values"]
F --> G["Enhanced Query Filters"]
H["LLM Providers"] --> I["Unified Instruction Preparation"]
C --> I
D --> I
The auto function filtering uses a simple regex to collect words from the instruction and matches them directly to function names. This can over-filter (missing valid functions due to naming conventions, hyphens, casing, plurals) or include unintended tokens. Verify the matching logic and consider safer heuristics or opt-in keywords.
The JSON field name for function visibility mode is snake_case ('function_visibility_mode') in API models, while the domain model uses camelCase ('functionVisibilityMode'). Confirm this intentional divergence and ensure documentation/clients are updated.
/// <summary>/// Agent function visibility mode/// </summary>[JsonPropertyName("function_visibility_mode")]publicstring?FuncVisMode{get;set;}
Conditional compilation runs cron handlers directly in DEBUG but publishes events otherwise. Ensure side-effects are acceptable and that behavior differences won’t mask issues in production paths.
The new Auto function visibility filters tools by matching bare words in the rendered instruction, which is fragile and can hide needed functions across all providers. Consider centralizing a robust, opt-in filtering strategy (e.g., explicit tags/allowlists or LLM-guided selection with fallback) and ensure providers always include secondary/critical functions to avoid unintended capability loss.
publicIEnumerable<FunctionDef>FilterFunctions(stringinstruction,Agentagent){varfunctions=agent.Functions;if(agent.FuncVisMode=="Auto"&&!string.IsNullOrEmpty(instruction)){// A more robust strategy, e.g., using explicit markers in the instruction// or an LLM call to determine relevant functions.functions=SelectFunctionsBasedOnInstructionSemantics(instruction,functions);}returnfunctions.Concat(agent.SecondaryFunctions??[]);}
Suggestion importance[1-10]: 9
__
Why: This suggestion correctly identifies a significant design flaw in the new FilterFunctions logic, where the fragile word-matching approach can unintentionally disable agent capabilities across all providers.
High
Possible issue
✅ Fix comparer misuse and null checksSuggestion Impact:The commit refactored function filtering but still misused HashSet.Contains with a comparer in the new helper; however, it adopted null-safe handling elsewhere and partially addressed comparer defaulting. Overall, it shows partial uptake but not a full correct fix.
code diff:
+ 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));
Avoid passing a comparer to HashSet.Contains — it only accepts the element. Also, null-check function names to prevent NREs. Build the set with the comparer and call Contains with a single argument, and ensure x.Name is not null before accessing.
public IEnumerable<FunctionDef> FilterFunctions(string instruction, Agent agent, StringComparer? comparer = null)
{
var functions = agent.Functions.AsEnumerable();
if (agent.FuncVisMode.IsEqualTo(AgentFuncVisMode.Auto) && !string.IsNullOrWhiteSpace(instruction))
{
- comparer = comparer ?? StringComparer.OrdinalIgnoreCase;+ 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);
- functions = functions.Where(x => words.Contains(x.Name, comparer));+ functions = functions.Where(x => !string.IsNullOrEmpty(x.Name) && words.Contains(x.Name));
}
functions = functions.Concat(agent.SecondaryFunctions ?? []);
return functions;
}
[Suggestion processed]
Suggestion importance[1-10]: 9
__
Why: The suggestion correctly identifies that calling HashSet.Contains with a comparer is a compilation error and provides a valid fix, which is critical for the code's correctness.
High
General
Skip functions with invalid names
Guard against null or empty function names before adding tools to avoid runtime exceptions in tool creation. Skip functions without a valid Name.
var functions = agentService.FilterFunctions(instruction, agent);
foreach (var function in functions)
{
+ if (string.IsNullOrWhiteSpace(function.Name)) continue;
if (!agentService.RenderFunction(agent, function)) continue;
var property = agentService.RenderFunctionProperty(agent, function);
options.Tools.Add(ChatTool.CreateFunctionTool(
functionName: function.Name,
functionDescription: function.Description,
functionParameters: BinaryData.FromObjectAsJson(property)));
}
Apply / Chat
Suggestion importance[1-10]: 6
__
Why: The suggestion adds a sensible guard clause to check for null or whitespace function.Name, improving the code's robustness against potential runtime errors.
In BuildMatchCondition, "neq" handling routes to MustNot but BuildMatchFieldCondition only supports boolean/integer/text; other types (double, datetime) are not supported. Additionally, BuildRangeFieldCondition for datetime computes nanos via ticks % second * 100 which may be incorrect for protobuf Timestamp nanoseconds. Validate correctness and type coverage to avoid incorrect filters.
FilterFunctions uses regex word extraction and exact name matching to include functions. This may erroneously hide needed functions when names aren’t present verbatim in instructions, or include false positives from common words matching function names. Consider safer heuristics or fallbacks when Auto mode yields empty results.
MapPayload and places referencing x.Payload assume presence; ensure MapField and Value conversion covers arrays, nested objects, and nulls. Unknown kinds map to Unknown with empty string; verify consumers tolerate that and that previous supported kinds (e.g., lists) aren’t needed.
privateDictionary<string,VectorPayloadValue>MapPayload(MapField<string,Value>?payload){returnpayload?.ToDictionary(p =>p.Key, p =>p.Value.KindCaseswitch{Value.KindOneofCase.StringValue=>VectorPayloadValue.BuildStringValue(p.Value.StringValue),Value.KindOneofCase.BoolValue=>VectorPayloadValue.BuildBooleanValue(p.Value.BoolValue),Value.KindOneofCase.IntegerValue=>VectorPayloadValue.BuildIntegerValue(p.Value.IntegerValue),Value.KindOneofCase.DoubleValue=>VectorPayloadValue.BuildDoubleValue(p.Value.DoubleValue),
_ =>VectorPayloadValue.BuildUnkownValue(string.Empty)})??[];}
#endregion
The Auto function filtering relies on naive regex word matching against instruction text, which can easily miss valid functions (e.g., camelCase, spaces, synonyms) or include false positives, breaking tool availability across all providers. Consider a more robust selection strategy (e.g., explicit tags/keywords per function, semantic similarity to instruction/context, or LLM-assisted tool selection hints) and provide a safe fallback (e.g., minimum whitelist) when no matches are found.
// In AgentService.Rendering.cspublicIEnumerable<FunctionDef>FilterFunctions(stringinstruction,IEnumerable<FunctionDef>functions, ...){// Extracts words like "get", "weather" from "get weather"varmatches=Regex.Matches(instruction,@"\b[A-Za-z0-9_-]+\b");varwords=newHashSet<string>(matches.Select(m =>m.Value));// Only keeps functions if their name is an exact word in the instruction.// e.g., A function named "getWeather" will NOT be found.// A function named "get_weather" will be found.returnfunctions.Where(x =>words.Contains(x.Name));}
After:
// In AgentService.Rendering.cs - A more robust approachpublicIEnumerable<FunctionDef>FilterFunctions(stringinstruction,IEnumerable<FunctionDef>functions, ...){// Option 1: Use explicit keywords/tags defined on the functionreturnfunctions.Where(func =>func.Keywords.Any(kw =>instruction.Contains(kw,StringComparison.OrdinalIgnoreCase)));// Option 2 (more advanced): Use semantic similarityvarinstructionEmbedding=_embeddingService.GetVector(instruction);returnfunctions.OrderByDescending(func =>CosineSimilarity(instructionEmbedding,func.Embedding)).Take(5);// Take top N relevant functions}
Suggestion importance[1-10]: 9
__
Why: The suggestion correctly identifies a critical design flaw in the new auto function filtering logic, which uses a naive regex that will likely fail to select correct functions, impacting all LLM providers.
High
Possible issue
Use correct Qdrant range field types
Ensure BuildRangeFieldCondition sets the correct numeric range type for integer-only comparisons. When range.DataType is Integer, populate IntegerRange instead of generic Range to avoid Qdrant rejecting or misinterpreting filters.
private Condition? BuildRangeCondition(VectorFilterRange range)
{
var fieldCondition = BuildRangeFieldCondition(range);
if (fieldCondition == null)
{
return null;
}
return new Condition
{
Field = fieldCondition
};
}
+private FieldCondition? BuildRangeFieldCondition(VectorFilterRange range)+{+ if (string.IsNullOrEmpty(range.Key) || range.Conditions.IsNullOrEmpty())+ {+ return null;+ }++ FieldCondition? fieldCondition = null;++ if (range.DataType == VectorPayloadDataType.Datetime)+ {+ fieldCondition = new FieldCondition { Key = range.Key, DatetimeRange = new() };+ foreach (var condition in range.Conditions)+ {+ if (!DateTime.TryParse(condition.Value, out var dt)) continue;+ var utc = dt.ToUniversalTime();+ var seconds = new DateTimeOffset(utc).ToUnixTimeSeconds();+ var nanos = (int)((utc.Ticks % TimeSpan.TicksPerSecond) * 100);+ var timestamp = new Google.Protobuf.WellKnownTypes.Timestamp { Seconds = seconds, Nanos = nanos };+ switch (condition.Operator.ToLower())+ {+ case "lt": fieldCondition.DatetimeRange.Lt = timestamp; break;+ case "lte": fieldCondition.DatetimeRange.Lte = timestamp; break;+ case "gt": fieldCondition.DatetimeRange.Gt = timestamp; break;+ case "gte": fieldCondition.DatetimeRange.Gte = timestamp; break;+ }+ }+ }+ else if (range.DataType == VectorPayloadDataType.Integer)+ {+ fieldCondition = new FieldCondition { Key = range.Key, IntegerRange = new() };+ foreach (var condition in range.Conditions)+ {+ if (!long.TryParse(condition.Value, out var longVal)) continue;+ switch (condition.Operator.ToLower())+ {+ case "lt": fieldCondition.IntegerRange.Lt = longVal; break;+ case "lte": fieldCondition.IntegerRange.Lte = longVal; break;+ case "gt": fieldCondition.IntegerRange.Gt = longVal; break;+ case "gte": fieldCondition.IntegerRange.Gte = longVal; break;+ }+ }+ }+ else // Double/Float fallback+ {+ fieldCondition = new FieldCondition { Key = range.Key, Range = new() };+ foreach (var condition in range.Conditions)+ {+ if (!double.TryParse(condition.Value, out var doubleVal)) continue;+ switch (condition.Operator.ToLower())+ {+ case "lt": fieldCondition.Range.Lt = doubleVal; break;+ case "lte": fieldCondition.Range.Lte = doubleVal; break;+ case "gt": fieldCondition.Range.Gt = doubleVal; break;+ case "gte": fieldCondition.Range.Gte = doubleVal; break;+ }+ }+ }++ return fieldCondition;+}+
Apply / Chat
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly points out that the implementation for range filtering on Integer types was missing, which would cause incorrect filter behavior for integer fields.
Medium
Handle empty instruction in filtering
Guard against empty or null instruction to avoid returning an empty function set unexpectedly. When no instruction is provided, default to returning the original functions to preserve functionality. This prevents inadvertent tool loss and downstream null/empty iterations.
public IEnumerable<FunctionDef> FilterFunctions(string instruction, IEnumerable<FunctionDef> functions, StringComparer? comparer = null)
{
- comparer = comparer ?? StringComparer.OrdinalIgnoreCase;+ if (string.IsNullOrWhiteSpace(instruction))+ {+ return functions ?? Enumerable.Empty<FunctionDef>();+ }+ 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));+ return (functions ?? Enumerable.Empty<FunctionDef>()).Where(x => !string.IsNullOrEmpty(x.Name) && words.Contains(x.Name, comparer));
}
Apply / Chat
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies that an empty instruction would incorrectly filter out all functions and provides a robust fix to prevent this unintended behavior.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
iceljc
changed the title
PR Type
Enhancement
Description
Add agent function visibility mode with auto/manual filtering
Refactor vector storage to use typed payload values
Enhance vector filtering with complex query operators
Update all LLM providers to use new instruction/function preparation
Diagram Walkthrough
File Walkthrough
19 files
Add FuncVisMode field to agent enumDefine function visibility mode constantsAdd function filtering and instruction preparation methodsAdd FuncVisMode property and setter methodDefine vector payload data type enumerationCreate typed vector payload value modelEnhance vector filtering with complex operatorsCreate vector collection creation options modelUpdate interface to use typed payloadsImplement function filtering and instruction preparationUse new instruction and function preparationUse new instruction and function preparationUse new instruction and function preparationUse new instruction and function preparationUse new instruction and function preparationImplement typed payloads and enhanced filteringUpdate to use typed vector payload valuesAdd FuncVisMode field update supportAdd FuncVisMode field update support38 files