Skip to content

Conversation

iceljc
Copy link
Collaborator

@iceljc iceljc commented Aug 22, 2025

PR Type

Bug fix


Description

  • Swap Data and Payload properties in vector collection models

  • Fix property mapping inconsistencies across vector storage implementations

  • Update JSON serialization property names to match corrected structure


Diagram Walkthrough

flowchart LR
  A["VectorCollectionData"] --> B["Swap Data/Payload properties"]
  B --> C["Update VectorSearchResult mapping"]
  B --> D["Fix ViewModel JSON properties"]
  B --> E["Update MemoryVectorDb implementation"]
  B --> F["Update QdrantDb implementation"]
  B --> G["Update SemanticKernel implementation"]
Loading

File Walkthrough

Relevant files
Bug fix
VectorCollectionData.cs
Swap Data and Payload property definitions                             

src/Infrastructure/BotSharp.Abstraction/VectorStorage/Models/VectorCollectionData.cs

  • Swap Data and Payload properties definitions
  • Payload now holds raw VectorPayloadValue dictionary
  • Data becomes computed property returning object dictionary
+2/-2     
VectorSearchResult.cs
Update property mapping in CopyFrom method                             

src/Infrastructure/BotSharp.Abstraction/VectorStorage/Models/VectorSearchResult.cs

  • Update property assignment from Data to Payload
  • Fix mapping consistency with base model changes
+1/-1     
VectorKnowledgeViewModel.cs
Fix JSON property names and types                                               

src/Infrastructure/BotSharp.OpenAPI/ViewModels/Knowledges/View/VectorKnowledgeViewModel.cs

  • Swap JSON property names for data and payload
  • Update property types to match corrected structure
+4/-4     
MemoryVectorDb.cs
Update property assignment in search results                         

src/Plugins/BotSharp.Plugin.KnowledgeBase/MemVecDb/MemoryVectorDb.cs

  • Change property assignment from Data to Payload
  • Fix vector search result construction
+1/-1     
QdrantDb.cs
Fix property mapping across Qdrant methods                             

src/Plugins/BotSharp.Plugin.Qdrant/QdrantDb.cs

  • Update three methods to use Payload instead of Data
  • Fix property mapping in GetPagedCollectionData, GetCollectionData, and
    Search methods
+3/-3     
SemanticKernelMemoryStoreProvider.cs
Update property assignment in search method                           

src/Plugins/BotSharp.Plugin.SemanticKernel/SemanticKernelMemoryStoreProvider.cs

  • Change property assignment from Data to Payload
  • Fix search result construction consistency
+1/-1     

Copy link

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

Serialization

With Data now a computed property and Payload the backing store, ensure JSON serialization attributes are correct so external APIs receive data and payload as intended, and that Data is not inadvertently serialized twice or omitted where required.

public Dictionary<string, VectorPayloadValue> Payload { get; set; } = new();
public Dictionary<string, object> Data => Payload?.ToDictionary(x => x.Key, x => x.Value.DataValue) ?? [];
public double? Score { get; set; }

[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public float[]? Vector { get; set; }
Nullability

Payload and Data are non-nullable but not initialized; deserialization or mapping may hit null refs. Consider initializing to empty dictionaries or marking as nullable with appropriate guards.

[JsonPropertyName("payload")]
public IDictionary<string, VectorPayloadValue> Payload { get; set; }

[JsonPropertyName("data")]
public IDictionary<string, object> Data { get; set; }

[JsonPropertyName("score")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
public double? Score { get; set; }
Mapping Consistency

After swapping to Payload, verify MapPayload returns Dictionary<string, VectorPayloadValue> and all downstream consumers expecting Data are updated, especially for filtering and search flows.

    await Task.WhenAll([totalCountTask, dataResponseTask]);

    var totalPointCount = totalCountTask.Result;
    var response = dataResponseTask.Result;

    var points = response?.Result?.Select(x => new VectorCollectionData
    {
        Id = x.Id?.Uuid ?? string.Empty,
        Payload = MapPayload(x.Payload),
        Vector = filter.WithVector ? x.Vectors?.Vector?.Data?.ToArray() : null
    })?.ToList() ?? new List<VectorCollectionData>();

    return new StringIdPagedItems<VectorCollectionData>
    {
        Count = totalPointCount,
        NextId = response?.NextPageOffset?.Uuid,
        Items = points
    };
}


public async Task<IEnumerable<VectorCollectionData>> GetCollectionData(string collectionName, IEnumerable<Guid> ids, VectorQueryOptions? options = null)
{
    if (ids.IsNullOrEmpty())
    {
        return Enumerable.Empty<VectorCollectionData>();
    }

    var exist = await DoesCollectionExist(collectionName);
    if (!exist)
    {
        return Enumerable.Empty<VectorCollectionData>();
    }

    var client = GetClient();
    var pointIds = ids.Select(x => new PointId { Uuid = x.ToString() }).Distinct().ToList();
    var points = await client.RetrieveAsync(collectionName, pointIds, options?.WithPayload ?? false, options?.WithVector ?? false);
    return points.Select(x => new VectorCollectionData
    {
        Id = x.Id?.Uuid ?? string.Empty,
        Payload = MapPayload(x.Payload),
        Vector = x.Vectors?.Vector?.Data?.ToArray()
    });
}

public async Task<bool> Upsert(string collectionName, Guid id, float[] vector, string text, Dictionary<string, VectorPayloadValue>? payload = null)
{
    // Insert vectors
    var point = new PointStruct()
    {
        Id = new PointId()
        {
            Uuid = id.ToString()
        },
        Vectors = vector,
        Payload =
        {
            { KnowledgePayloadName.Text, text }
        }
    };

    if (payload != null)
    {
        foreach (var item in payload)
        {
            var value = item.Value.DataValue?.ToString();
            if (value == null || item.Key.IsEqualTo(KnowledgePayloadName.Text))
            {
                continue;
            }

            switch (item.Value.DataType)
            {
                case VectorPayloadDataType.Boolean when bool.TryParse(value, out var boolVal):
                    point.Payload[item.Key] = boolVal;
                    break;
                case VectorPayloadDataType.Integer when long.TryParse(value, out var longVal):
                    point.Payload[item.Key] = longVal;
                    break;
                case VectorPayloadDataType.Double when double.TryParse(value, out var doubleVal):
                    point.Payload[item.Key] = doubleVal;
                    break;
                case VectorPayloadDataType.Datetime when DateTime.TryParse(value, out var dt):
                    point.Payload[item.Key] = dt.ToUniversalTime().ToString("o");
                    break;
                case VectorPayloadDataType.String:
                    point.Payload[item.Key] = value;
                    break;
                default:
                    break;
            }
        }
    }

    var client = GetClient();
    var result = await client.UpsertAsync(collectionName, points: new List<PointStruct>
    {
        point
    });

    return result.Status == UpdateStatus.Completed;
}

public async Task<IEnumerable<VectorCollectionData>> Search(string collectionName, float[] vector, VectorSearchOptions? options = null)
{
    var results = new List<VectorCollectionData>();

    var exist = await DoesCollectionExist(collectionName);
    if (!exist)
    {
        return results;
    }

    options ??= VectorSearchOptions.Default();
    Filter? queryFilter = BuildQueryFilter(options.FilterGroups);
    WithPayloadSelector? payloadSelector = BuildPayloadSelector(options.Fields);

    var client = GetClient();
    var points = await client.SearchAsync(collectionName,
                                        vector,
                                        limit: (ulong)options.Limit.GetValueOrDefault(),
                                        scoreThreshold: options.Confidence,
                                        filter: queryFilter,
                                        payloadSelector: payloadSelector,
                                        vectorsSelector: options.WithVector);

    results = points.Select(x => new VectorCollectionData
    {
        Id = x.Id.Uuid,
        Payload = MapPayload(x.Payload),
        Score = x.Score,
        Vector = x.Vectors?.Vector?.Data?.ToArray()
    }).ToList();

Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Ensure backward compatibility

Swapping Data and Payload across models and view models is a breaking change
that risks deserializing old persisted data and external API contracts
incorrectly. Provide explicit backward-compat handling (migration for stored
payloads, dual-accept JSON input, and versioned API fields) to prevent runtime
errors and client breakage.

Examples:

src/Infrastructure/BotSharp.OpenAPI/ViewModels/Knowledges/View/VectorKnowledgeViewModel.cs [11-15]
    [JsonPropertyName("payload")]
    public IDictionary<string, VectorPayloadValue> Payload { get; set; }

    [JsonPropertyName("data")]
    public IDictionary<string, object> Data { get; set; }
src/Infrastructure/BotSharp.Abstraction/VectorStorage/Models/VectorCollectionData.cs [6-7]
    public Dictionary<string, VectorPayloadValue> Payload { get; set; } = new();
    public Dictionary<string, object> Data => Payload?.ToDictionary(x => x.Key, x => x.Value.DataValue) ?? [];

Solution Walkthrough:

Before:

// File: src/Infrastructure/BotSharp.OpenAPI/ViewModels/Knowledges/View/VectorKnowledgeViewModel.cs
// This implementation breaks existing API clients and persisted data.
public class VectorKnowledgeViewModel
{
    [JsonPropertyName("id")]
    public string Id { get; set; }

    [JsonPropertyName("payload")]
    public IDictionary<string, VectorPayloadValue> Payload { get; set; }

    [JsonPropertyName("data")]
    public IDictionary<string, object> Data { get; set; }

    // ...
}

After:

// File: src/Infrastructure/BotSharp.OpenAPI/ViewModels/Knowledges/View/VectorKnowledgeViewModel.cs
// This version adds backward compatibility using a custom converter.
public class VectorKnowledgeViewModel
{
    [JsonPropertyName("id")]
    public string Id { get; set; }

    // The converter handles reading from the old "data" field if "payload" is missing.
    // It ensures that on serialization, the "payload" field is always written.
    [JsonPropertyName("payload")]
    [JsonConverter(typeof(LegacyCompatiblePayloadConverter))]
    public IDictionary<string, VectorPayloadValue> Payload { get; set; }

    // The new "data" field is computed from the unified Payload property.
    [JsonPropertyName("data")]
    public IDictionary<string, object> Data => Payload?.ToDictionary(x => x.Key, x => x.Value.DataValue);

    // ...
}
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical breaking change in API contracts and data persistence that the PR overlooks, proposing valid architectural solutions to prevent production failures.

High
Possible issue
Harden computed property

Ensure Data never throws if Payload contains null values or is mutated
concurrently. Cache the computed dictionary and guard against null Value to
avoid NullReferenceException.

src/Infrastructure/BotSharp.Abstraction/VectorStorage/Models/VectorCollectionData.cs [6-7]

 public Dictionary<string, VectorPayloadValue> Payload { get; set; } = new();
-public Dictionary<string, object> Data => Payload?.ToDictionary(x => x.Key, x => x.Value.DataValue) ?? [];
+private Dictionary<string, object>? _dataCache;
+public Dictionary<string, object> Data
+    => _dataCache ??= (Payload == null
+        ? new Dictionary<string, object>()
+        : Payload.ToDictionary(x => x.Key, x => (object?)x.Value?.DataValue ?? DBNull.Value));
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential NullReferenceException if a value in the Payload dictionary is null and provides a robust fix, while also adding a performance improvement through caching.

Medium
Prevent null reference issues

Mark reference properties as nullable and initialize to empty to prevent
serialization nulls and deserialization NREs. This avoids runtime crashes when
API consumers omit fields.

src/Infrastructure/BotSharp.OpenAPI/ViewModels/Knowledges/View/VectorKnowledgeViewModel.cs [11-15]

 [JsonPropertyName("payload")]
-public IDictionary<string, VectorPayloadValue> Payload { get; set; }
+public IDictionary<string, VectorPayloadValue>? Payload { get; set; } = new Dictionary<string, VectorPayloadValue>();
 
 [JsonPropertyName("data")]
-public IDictionary<string, object> Data { get; set; }
+public IDictionary<string, object>? Data { get; set; } = new Dictionary<string, object>();
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly proposes initializing the Payload and Data properties to prevent potential null reference exceptions, which is a good practice for data transfer objects.

Low
  • More

@iceljc iceljc merged commit aca1145 into SciSharp:master Aug 22, 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