add vector payload by iceljc · Pull Request #1133 · SciSharp/BotSharp · GitHub
Skip to content

add vector payload - #1133

Merged
iceljc merged 2 commits into
SciSharp:masterfrom
iceljc:master
Aug 21, 2025
Merged

add vector payload#1133
iceljc merged 2 commits into
SciSharp:masterfrom
iceljc:master

Conversation

@iceljc

@iceljc iceljc commented Aug 21, 2025 •
edited by qodo-code-review Bot
Loading

Copy link
Copy Markdown
Collaborator

PR Type

Enhancement

Description

  • Add Payload property to vector storage models

  • Expose simplified payload data in API responses

  • Remove unused Newtonsoft.Json dependency

Diagram Walkthrough

flowchart LR
  VectorCollectionData["VectorCollectionData"] -- "adds" --> Payload["Payload property"]
  Payload -- "exposes" --> SimplifiedData["Simplified data values"]
  VectorKnowledgeViewModel["VectorKnowledgeViewModel"] -- "includes" --> Payload
  SimplifiedData -- "serialized to" --> APIResponse["API Response"]
Loading

File Walkthrough
Relevant files
Enhancement
VectorCollectionData.cs
Add Payload property for simplified data access                   

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

  • Add Payload property that converts Data dictionary to simplified
    key-value pairs
  • Property extracts DataValue from each VectorPayloadValue for easier
    access
 +1/-0     
VectorKnowledgeViewModel.cs
Add Payload property to view model                                             

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

  • Add Payload property with JSON serialization attribute
  • Update From methods to populate Payload from source objects
  • Expose simplified payload data in API responses
 +5/-0     
Formatting
VectorPayloadValue.cs
Remove unused import statement                                                     

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

  • Remove unused Newtonsoft.Json.Linq import statement
 +0/-1     

@iceljc
iceljc merged commit 12077f0 into SciSharp:master Aug 21, 2025
@qodo-code-review

qodo-code-review Bot commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

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

Null Handling

The new Payload getter accesses x.Value.DataValue without guarding against x.Value being null; confirm upstream guarantees or add a safe null-propagation to avoid potential NREs during serialization.

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

Exposing IDictionary<string, object> in Payload may lead to inconsistent JSON (boxing, type metadata). Validate that all possible DataValue types serialize as expected and are compatible/stable for API clients.

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

Adding payload alongside data changes API response shape; ensure clients are aware and that duplication between data and payload is intentional or consider marking deprecation to avoid confusion.

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

@qodo-code-review

qodo-code-review Bot commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Add null guards and defensive copies

Guard against null Data/Payload to avoid serialization-time nulls or downstream
mutations. Clone dictionaries to prevent accidental external modification of the
view model.

src/Infrastructure/BotSharp.OpenAPI/ViewModels/Knowledges/View/VectorKnowledgeViewModel.cs [26-47]

 public static VectorKnowledgeViewModel From(VectorSearchResult result)
 {
     return new VectorKnowledgeViewModel
     {
         Id = result.Id,
-        Data = result.Data,
-        Payload = result.Payload,
+        Data = result.Data != null ? new Dictionary<string, VectorPayloadValue>(result.Data) : new Dictionary<string, VectorPayloadValue>(),
+        Payload = result.Payload != null ? new Dictionary<string, object>(result.Payload) : null,
         Score = result.Score,
         VectorDimension = result.Vector?.Length
     };
 }
 
 public static VectorKnowledgeViewModel From(VectorCollectionData data)
 {
     return new VectorKnowledgeViewModel
     {
         Id = data.Id,
-        Data = data.Data,
-        Payload = data.Payload,
+        Data = data.Data != null ? new Dictionary<string, VectorPayloadValue>(data.Data) : new Dictionary<string, VectorPayloadValue>(),
+        Payload = data.Payload != null ? new Dictionary<string, object>(data.Payload) : null,
         VectorDimension = data.Vector?.Length
     };
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly recommends creating defensive copies of the dictionaries, which prevents unintended side effects and makes the view model more robust against external modifications.

Medium
Avoid repeated payload recomputation

Materialize the computed property to avoid repeated full-dictionary allocations
and potential performance hits on frequent access. Cache the transformed payload
and invalidate it when Data changes to ensure consistency.

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

-public Dictionary<string, object> Payload => Data?.ToDictionary(x => x.Key, x => x.Value.DataValue) ?? [];
+private Dictionary<string, object>? _payloadCache;
+public Dictionary<string, object> Payload
+    => _payloadCache ??= (Data?.ToDictionary(x => x.Key, x => x.Value.DataValue) ?? new Dictionary<string, object>());
 
+// Ensure cache invalidation when Data is reassigned
+private Dictionary<string, VectorPayloadValue> _data = new();
+public Dictionary<string, VectorPayloadValue> Data
+{
+    get => _data;
+    set
+    {
+        _data = value ?? new Dictionary<string, VectorPayloadValue>();
+        _payloadCache = null;
+    }
+}
+
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a performance issue where the Payload dictionary is recomputed on every access and proposes a robust caching mechanism with invalidation, improving efficiency.

Low
Null-safe payload serialization

Make Payload nullable and ignore it when null to prevent serialization of an
empty object and avoid null reference risks when source lacks payload. This
aligns with Score/VectorDimension behavior.

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

 [JsonPropertyName("payload")]
-public IDictionary<string, object> Payload { get; set; }
+[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
+public IDictionary<string, object>? Payload { get; set; }
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: This suggestion improves API response quality by making the Payload property's serialization behavior consistent with other optional properties like Score in the same view model.

Low
  • More

@dependabot dependabot Bot mentioned this pull request Jul 9, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@iceljc