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
Refine vector storage operations with enhanced filtering and indexing
Add time range filtering to logs and conversation queries
Improve repository methods with async operations and better performance
Add payload index management for vector collections
Diagram Walkthrough
flowchart LR
A["Vector Storage"] --> B["Enhanced Filtering"]
A --> C["Payload Indexing"]
A --> D["Query Options"]
E["Repository Layer"] --> F["Async Operations"]
E --> G["Time Range Filters"]
B --> H["Filter Groups"]
B --> I["Order By Support"]
C --> J["Create/Delete Indexes"]
Loading
File Walkthrough
Relevant files
Enhancement
47 files
IKnowledgeService.cs
Add vector collection data query and index management methods
In GetPagedCollectionData, StartId is passed directly as a UUID string without validating Qdrant’s expected format; if StartId is not a valid UUID, request may fail. Consider guarding with Guid.TryParse before constructing PointId.
GetCollectionData now has a new signature but still throws NotImplementedException; this will break new API endpoints relying on point retrieval from the in-memory provider.
The new filter-group feature passes raw strings straight into Qdrant Match without centralized validation or sanitization; numeric/bool coercion is attempted but lacks schema awareness and safe typing across providers. Add a validation layer (schema-driven or per-collection payload index metadata) to enforce allowed fields, types, and operators before building filters to prevent mismatches, silent failures, or inefficient scans when indexes/types don't align.
// In QdrantDb.BuildQueryFilterforeach(varfilterinfilters){varfieldCondition=newFieldCondition{Key=filter.Key};// Blindly attempt type coercion, defaulting to text searchif(bool.TryParse(filter.Value,outvarb)){fieldCondition.Match=newMatch{Boolean=b};}elseif(long.TryParse(filter.Value,outvarl)){fieldCondition.Match=newMatch{Integer=l};}else{fieldCondition.Match=newMatch{Text=filter.Value};}// ... add condition}
After:
// In QdrantDb.BuildQueryFilter, now with schema accessvarschema=awaitGetPayloadIndexSchema(collectionName);foreach(varfilterinfilters){varfieldSchemaType=schema.Get(filter.Key);// e.g., "integer", "bool"varfieldCondition=newFieldCondition{Key=filter.Key};// Build match condition based on known schema typeswitch(fieldSchemaType){case"bool":fieldCondition.Match=newMatch{Boolean=bool.Parse(filter.Value)};break;case"integer":fieldCondition.Match=newMatch{Integer=long.Parse(filter.Value)};break;// ... other typesdefault:fieldCondition.Match=newMatch{Text=filter.Value};break;}// ... add condition}
Suggestion importance[1-10]: 9
__
Why: This suggestion correctly identifies a critical design flaw in the new filtering feature that could lead to incorrect query results, making it a high-impact issue.
High
General
Preserve payload value types
Preserve original numeric and boolean types instead of string-based parsing to avoid locale issues and loss of precision. Use type checks and handle arrays/objects appropriately so Qdrant payloads reflect true data types.
foreach (var item in payload)
{
- var value = item.Value?.ToString();- if (value == null || item.Key.IsEqualTo(KnowledgePayloadName.Text))+ if (item.Key.IsEqualTo(KnowledgePayloadName.Text)) continue;+ var v = item.Value;+ switch (v)
{
- continue;- }-- if (bool.TryParse(value, out var b))- {- point.Payload[item.Key] = b;- }- else if (long.TryParse(value, out var l))- {- point.Payload[item.Key] = l;- }- else if (double.TryParse(value, out var d))- {- point.Payload[item.Key] = d;- }- else- {- point.Payload[item.Key] = value;+ case null:+ continue;+ case bool b:+ point.Payload[item.Key] = b;+ break;+ case sbyte or byte or short or ushort or int or uint or long:+ point.Payload[item.Key] = Convert.ToInt64(v);+ break;+ case float or double or decimal:+ point.Payload[item.Key] = Convert.ToDouble(v, System.Globalization.CultureInfo.InvariantCulture);+ break;+ case IEnumerable enumerable when v is not string:+ point.Payload[item.Key] = enumerable.Cast<object>().Select(x => x?.ToString()).ToArray();+ break;+ default:+ point.Payload[item.Key] = v.ToString();+ break;
}
}
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies that converting all payload values to strings can cause loss of precision and type information, and it proposes a robust solution using type-switching to preserve original data types.
In GetPagedCollectionData, CountAsync returns CountResponse; ensure you're using the actual count value (e.g., .Result or .Count) and that the return type for Count aligns with the expected int. Also verify offset construction with string UUID matches Qdrant expectations.
vartotalCountTask=client.CountAsync(collectionName,filter:queryFilter);vardataResponseTask=client.ScrollAsync(collectionName,limit:(uint)filter.Size,offset:!string.IsNullOrWhiteSpace(filter.StartId)?newPointId{Uuid=filter.StartId}:null,filter:queryFilter,payloadSelector:payloadSelector,vectorsSelector:filter.WithVector);awaitTask.WhenAll([totalCountTask,dataResponseTask]);vartotalPointCount=totalCountTask.Result;varresponse=dataResponseTask.Result;varpoints=response?.Result?.Select(x =>newVectorCollectionData{Id=x.Id?.Uuid??string.Empty,Data=x.Payload.ToDictionary(p =>p.Key, p =>p.Value.KindCaseswitch{Value.KindOneofCase.StringValue=>p.Value.StringValue,Value.KindOneofCase.BoolValue=>p.Value.BoolValue,Value.KindOneofCase.IntegerValue=>p.Value.IntegerValue,Value.KindOneofCase.DoubleValue=>p.Value.DoubleValue,
_ =>newobject()}),Vector=filter.WithVector?x.Vectors?.Vector?.Data?.ToArray():null})?.ToList()??newList<VectorCollectionData>();returnnewStringIdPagedItems<VectorCollectionData>{Count=totalPointCount,NextId=response?.NextPageOffset?.Uuid,Items= points
Payload conversion adds support for DoubleValue, but integer parsing uses long in filters; confirm consistency with Qdrant schema and potential overflow when casting to int elsewhere. Validate that skipping KnowledgePayloadName.Text during upsert is intentional and won’t remove needed text in payload queries.
varpoints=response?.Result?.Select(x =>newVectorCollectionData{Id=x.Id?.Uuid??string.Empty,Data=x.Payload.ToDictionary(p =>p.Key, p =>p.Value.KindCaseswitch{Value.KindOneofCase.StringValue=>p.Value.StringValue,Value.KindOneofCase.BoolValue=>p.Value.BoolValue,Value.KindOneofCase.IntegerValue=>p.Value.IntegerValue,Value.KindOneofCase.DoubleValue=>p.Value.DoubleValue,
_ =>newobject()}),Vector=filter.WithVector?x.Vectors?.Vector?.Data?.ToArray():null})?.ToList()??newList<VectorCollectionData>();returnnewStringIdPagedItems<VectorCollectionData>{Count=totalPointCount,NextId=response?.NextPageOffset?.Uuid,Items=points};}publicasyncTask<IEnumerable<VectorCollectionData>>GetCollectionData(stringcollectionName,IEnumerable<Guid>ids,VectorQueryOptions?options=null){if(ids.IsNullOrEmpty()){returnEnumerable.Empty<VectorCollectionData>();}varexist=awaitDoesCollectionExist(collectionName);if(!exist){returnEnumerable.Empty<VectorCollectionData>();}varclient=GetClient();varpointIds=ids.Select(x =>newPointId{Uuid=x.ToString()}).Distinct().ToList();varpoints=awaitclient.RetrieveAsync(collectionName,pointIds,options?.WithPayload??false,options?.WithVector??false);returnpoints.Select(x =>newVectorCollectionData{Id=x.Id?.Uuid??string.Empty,Data=x.Payload?.ToDictionary(p =>p.Key, p =>p.Value.KindCaseswitch{Value.KindOneofCase.StringValue=>p.Value.StringValue,Value.KindOneofCase.BoolValue=>p.Value.BoolValue,Value.KindOneofCase.IntegerValue=>p.Value.IntegerValue,Value.KindOneofCase.DoubleValue=>p.Value.DoubleValue,
_ =>newobject()})??new(),Vector=x.Vectors?.Vector?.Data?.ToArray()});
ReasoningEffortLevel is applied only when _model is in _defaultTemperature map; this silently disables the feature for other models. Confirm intended gating and whether temperature override for these models should be decoupled from reasoning effort support.
The PR downgrades OpenAI to a beta and changes Qdrant.Client versions while introducing new API surfaces (reasoning effort, filter groups, payload indexes). Verify compatibility: o3/gpt-5 reasoning features require specific SDKs, and Qdrant 1.15 API changes must match your BuildQueryFilter/index calls. If these SDKs don't support the added options, vector search and chat may fail at runtime; pin stable versions or add capability checks/fallbacks.
// In Directory.Packages.props<PackageVersionInclude="OpenAI"Version="2.2.0-beta.4"/><PackageVersion Include="Qdrant.Client"Version="1.15.0"/>// In ChatCompletionProvider.csvarlevel= ...;var reasoningEffortLevel =ParseReasoningEffortLevel(level);returnnewChatCompletionOptions(){
...ReasoningEffortLevel= reasoningEffortLevel
};
After:
// In Directory.Packages.props<PackageVersionInclude="OpenAI"Version="2.2.0"/>// Pinned to stable<PackageVersion Include="Qdrant.Client" Version="1.15.0" /> // Keep upgrade after verification
// In ChatCompletionProvider.cs
var level= ...;// Add capability check before using beta featuresif(IsReasoningEffortSupported()){varreasoningEffortLevel=ParseReasoningEffortLevel(level);options.ReasoningEffortLevel=reasoningEffortLevel;}returnoptions;
Suggestion importance[1-10]: 9
__
Why: This suggestion correctly identifies a risky downgrade of the OpenAI package to a beta version and an upgrade of Qdrant.Client, and accurately links these to new features (ReasoningEffortLevel, FilterGroups) that could cause runtime failures if compatibility isn't ensured.
High
Possible issue
✅ Correct reasoning capability gatingSuggestion Impact:The commit removed the reasoning effort level handling entirely (deleted retrieval, parsing, and the ParseReasoningEffortLevel method), thereby eliminating the flawed gating tied to the temperature map. While it did not introduce a separate capability set as suggested, it addressed the core issue by removing the coupled logic.
code diff:
- var level = state.GetState("reasoning_effort_level")- .IfNullOrEmptyAs(agent?.LlmConfig?.ReasoningEffortLevel ?? string.Empty)- .IfNullOrEmptyAs(LlmConstant.DEFAULT_REASONING_EFFORT_LEVEL);- var reasoningEffortLevel = ParseReasoningEffortLevel(level);-
return new ChatCompletionOptions()
{
Temperature = temperature,
- MaxOutputTokenCount = maxTokens,- ReasoningEffortLevel = reasoningEffortLevel+ MaxOutputTokenCount = maxTokens
};
- }-- private ChatReasoningEffortLevel? ParseReasoningEffortLevel(string? level)- {- if (string.IsNullOrWhiteSpace(level) || !_defaultTemperature.ContainsKey(_model))- {- return null;- }-- var effortLevel = ChatReasoningEffortLevel.Low;- switch (level.ToLower())- {- case "medium":- effortLevel = ChatReasoningEffortLevel.Medium;- break;- case "high":- effortLevel = ChatReasoningEffortLevel.High;- break;- default:- break;- }-- return effortLevel;
}
Decouple reasoning-effort applicability from the temperature map. Using _defaultTemperature.ContainsKey(_model) to gate reasoning will disable it for supported models not listed. Introduce a separate check (e.g., a set of reasoning-capable models) or remove the gate to honor provided levels when the SDK accepts it.
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 6
__
Why: This suggestion correctly identifies a design flaw where reasoning capability is coupled with temperature settings, and proposes a good refactoring that decouples them, improving code maintainability and robustness.
Low
General
Safely parse GUID list
Avoid using an implicitly scoped guid from TryParse in an anonymous projection; some compilers may treat the out variable as shared across iterations, causing incorrect IDs. Parse in a preliminary loop and add validated GUIDs to a list to ensure correctness.
-var pointIds = ids.Select(x => new { Id = x, IsValid = Guid.TryParse(x, out var guid), ParseResult = guid })- .Where(x => x.IsValid)- .Select(x => x.ParseResult)- .ToList();+var pointIds = new List<Guid>();+foreach (var id in ids)+{+ if (Guid.TryParse(id, out var gid))+ {+ pointIds.Add(gid);+ }+}
Apply / Chat
Suggestion importance[1-10]: 3
__
Why: The existing LINQ code is correct on modern C# compilers. While the suggestion's concern about out variable capturing was valid for older language versions, it's not an issue here. The proposed change is a more verbose but equally correct alternative.
The updated code assigns user from a pre-fetched list but the user variable isn't declared in the shown scope; ensure a local variable exists and that users retrieval matches expected shape (list vs paged) to avoid null reference or compile errors.
Multiple catch blocks now return empty arrays [] or null; ensure these sentinel returns are acceptable by callers and do not break existing expectations (e.g., GetVectorCollections previously returned Enumerable.Empty<>).
}catch(Exceptionex){_logger.LogError(ex,$"Error when getting vector db collections.");return[];}}publicasyncTask<VectorCollectionDetails?>GetVectorCollectionDetails(stringcollectionName){try{if(string.IsNullOrWhiteSpace(collectionName))returnnull;vardb=_services.GetRequiredService<IBotSharpRepository>();varconfigs=db.GetKnowledgeCollectionConfigs(newVectorCollectionConfigFilter{CollectionNames=[collectionName]}).ToList();varvectorDb=GetVectorDb();vardetails=awaitvectorDb.GetCollectionDetails(collectionName);if(details!=null){details.BasicInfo=configs.FirstOrDefault();}returndetails;}catch(Exceptionex){_logger.LogError(ex,$"Error when getting vector db collection details.");returnnull;
BuildQueryFilter coerces values using bool/long parsing; check that this matches Qdrant payload schema types and that mixing Must of grouped Should/Must conditions yields intended results across AND/OR groups.
The PR converts many repository methods to async/ValueTask but leaves several service/controller call sites and interface default implementations inconsistent (e.g., IBotSharpRepository still throws NotImplementedException synchronously for many methods, while services now await them). Ensure all repository implementations (File, Mongo, DbContext) and consumers align on async signatures and semantics, otherwise deadlocks or runtime exceptions will surface under load. Additionally, the vector DB API was reshaped (search/filter/index/query options); all providers (Memory, SemanticKernel, Qdrant) should fully implement the new options consistently to avoid silent feature gaps in filtering, ordering, and payload selection.
// IVectorDb interface defines new search optionspublicinterfaceIVectorDb{Task<IEnumerable<Data>>Search(stringcollection,float[]vector,VectorSearchOptions?options);}// But some implementations ignore the new filtering capabilitiespublicclassMemoryVectorDb:IVectorDb{publicasyncTask<IEnumerable<Data>>Search(stringcollection,float[]vector,VectorSearchOptions?options){// options.FilterGroups is ignored, leading to incorrect results.varresults= ...// Logic only considers limit and vector.
return results;}}
After:
// IVectorDb interface defines new search optionspublicinterfaceIVectorDb{Task<IEnumerable<Data>>Search(stringcollection,float[]vector,VectorSearchOptions?options);}// All implementations should fully support the new optionspublicclassMemoryVectorDb:IVectorDb{publicasyncTask<IEnumerable<Data>>Search(stringcollection,float[]vector,VectorSearchOptions?options){varresults= ...;if(options?.FilterGroups!=null){// Apply filtering logic based on FilterGroupsresults=ApplyFilters(results,options.FilterGroups);}returnresults;}}
Suggestion importance[1-10]: 9
__
Why: This suggestion correctly identifies two critical, cross-cutting architectural inconsistencies that could lead to runtime exceptions and silent bugs, affecting both the data repository layer and the vector database abstraction.
High
Possible issue
Fix payload numeric parsing and culture issues
The current numeric parsing uses float.TryParse but assigns to DoubleValue, which may cause precision loss and parsing failures in locales. Parse with double.TryParse using invariant culture. Also handle arrays and nested objects by serializing them to string to avoid runtime errors.
foreach (var item in payload)
{
- var value = item.Value?.ToString();- if (value == null || item.Key.IsEqualTo(KnowledgePayloadName.Text))+ if (item.Key.IsEqualTo(KnowledgePayloadName.Text) || item.Value is null)+ continue;++ Value payloadValue;+ switch (item.Value)
{
- continue;+ case bool b:+ payloadValue = new Value { BoolValue = b };+ break;+ case int i:+ payloadValue = new Value { IntegerValue = i };+ break;+ case long l:+ payloadValue = new Value { IntegerValue = l };+ break;+ case float f32:+ payloadValue = new Value { DoubleValue = Convert.ToDouble(f32, CultureInfo.InvariantCulture) };+ break;+ case double f64:+ payloadValue = new Value { DoubleValue = f64 };+ break;+ case string s:+ if (bool.TryParse(s, out var b2))+ payloadValue = new Value { BoolValue = b2 };+ else if (long.TryParse(s, NumberStyles.Integer, CultureInfo.InvariantCulture, out var l2))+ payloadValue = new Value { IntegerValue = l2 };+ else if (double.TryParse(s, NumberStyles.Float | NumberStyles.AllowThousands, CultureInfo.InvariantCulture, out var d2))+ payloadValue = new Value { DoubleValue = d2 };+ else+ payloadValue = new Value { StringValue = s };+ break;+ default:+ // Fallback: serialize complex types to string+ payloadValue = new Value { StringValue = JsonSerializer.Serialize(item.Value) };+ break;
}
- if (bool.TryParse(value, out var b))- {- payloadValue = new Value { BoolValue = b };- }- else if (long.TryParse(value, out var l))- {- payloadValue = new Value { IntegerValue = l };- }- else if (float.TryParse(value, out var f))- {- payloadValue = new Value { DoubleValue = f };- }- else- {- payloadValue = new Value { StringValue = value };- }-- payloadPairs.Add(item.Key, payloadValue);+ payloadPairs[item.Key] = payloadValue;
}
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies and fixes multiple issues in payload processing, such as inefficient type conversion, potential data loss, and culture-specific parsing errors, by implementing a robust type-switching mechanism and using CultureInfo.InvariantCulture.
Medium
Use culture-invariant lowercasing
schemaType.ToLower() is culture-sensitive and can break in locales like Turkish. Use ToLowerInvariant() to avoid culture bugs when mapping schema types.
private PayloadSchemaType ConvertPayloadSchemaType(string schemaType)
{
- PayloadSchemaType res;- switch (schemaType.ToLower())+ switch ((schemaType ?? string.Empty).ToLowerInvariant())
{
- case "text":- res = PayloadSchemaType.Text;- break;- case "keyword":- res = PayloadSchemaType.Keyword;- break;- case "integer":- res = PayloadSchemaType.Integer;- break;- case "float":- res = PayloadSchemaType.Float;- break;- case "bool":- res = PayloadSchemaType.Bool;- break;- case "geo":- res = PayloadSchemaType.Geo;- break;- case "datetime":- res = PayloadSchemaType.Datetime;- break;- case "uuid":- res = PayloadSchemaType.Uuid;- break;- default:- res = PayloadSchemaType.UnknownType;- break;+ case "text": return PayloadSchemaType.Text;+ case "keyword": return PayloadSchemaType.Keyword;+ case "integer": return PayloadSchemaType.Integer;+ case "float": return PayloadSchemaType.Float;+ case "bool": return PayloadSchemaType.Bool;+ case "geo": return PayloadSchemaType.Geo;+ case "datetime": return PayloadSchemaType.Datetime;+ case "uuid": return PayloadSchemaType.Uuid;+ default: return PayloadSchemaType.UnknownType;
}
-- return res;
}
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly points out a potential internationalization bug by replacing ToLower() with ToLowerInvariant(), which is a best practice for culture-agnostic string comparisons, preventing issues in locales like Turkish.
Medium
General
Ensure text saved in payload
The code ensures data_source exists but does not ensure the primary text is persisted in payload. Downstream code expects payload[KnowledgePayloadName.Text] for comparisons. Add the text into payload to keep consistency with updates and searches.
var payload = create.Payload ?? new();
if (!payload.TryGetValue(KnowledgePayloadName.DataSource, out _))
{
- payload[KnowledgePayloadName.DataSource] = !string.IsNullOrWhiteSpace(create.DataSource) ?- create.DataSource : VectorDataSource.Api;+ payload[KnowledgePayloadName.DataSource] = !string.IsNullOrWhiteSpace(create.DataSource)+ ? create.DataSource+ : VectorDataSource.Api;
}
+// Ensure text is present for later comparisons/reads+payload[KnowledgePayloadName.Text] = create.Text;
return await db.Upsert(collectionName, guid, vector, create.Text, payload);
Apply / Chat
Suggestion importance[1-10]: 6
__
Why: The suggestion correctly identifies that the text should be part of the payload for consistency, as seen in the UpsertVectorCollectionData method. While the QdrantDb.Upsert method already handles this, adding it here improves code clarity and consistency within the KnowledgeService.
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.
PR Type
Enhancement
Description
Refine vector storage operations with enhanced filtering and indexing
Add time range filtering to logs and conversation queries
Improve repository methods with async operations and better performance
Add payload index management for vector collections
Diagram Walkthrough
File Walkthrough
47 files
Add vector collection data query and index management methodsAdd generic success/fail response modelAdd time range filtering propertiesAdd time range filtering propertiesAdd time range filtering propertiesConvert repository methods to async ValueTaskAdd batch user retrieval methodChange Count property from int to longRefactor vector database interface with options patternAdd vector collection index management modelsReplace search pairs with filter groups and order byAdd vector filter group model for complex filteringAdd vector query options modelEnhance search options with filter groups and defaultsAdd vector sorting modelUpdate to use async repository methodUpdate to use async repository methodOptimize instruction log retrieval with batch operationsConvert to async ValueTask methodAdd async operations and time range filteringConvert to async ValueTask methodConvert to async ValueTask methodAdd async operations and time range filteringConvert to async ValueTask methodUpdate to use async repository methodAdd batch user retrieval and async operationsOptimize user data retrieval in conversationsAdd vector collection data query and index endpointsAdd vector data query request modelAdd filter groups support to search requestAdd vector collection index request modelsRemove default data source valueReplace vector array with dimension countUpdate to use VectorSearchOptions patternUpdate error logging and async operationsUpdate error logging levelAdd vector collection payload index managementEnhance vector operations with better error handlingAdd async operations with parallel executionAdd async operations and time range filteringAdd async operations with parallel executionAdd async operations with parallel executionAdd async operations and time range filteringAdd async operations with parallel executionAdd temperature defaults for new OpenAI modelsMajor refactor with enhanced filtering and index supportUpdate to use VectorSearchOptions pattern1 files
Fix typo in log message1 files
Remove unused method implementations2 files
Minor code formatting cleanupRemove trailing comma formatting1 files
Update OpenAI and Qdrant.Client package versions