rename graph query

[iceljc]

PR Type

Enhancement

---

Description

• Rename graph query method from SearchAsync to ExecuteQueryAsync

• Rename result class from GraphSearchResult to GraphQueryResult

• Refactor options classes into hierarchy with GraphQueryExecuteOptions base

• Update all implementations and callers across multiple plugins

---

Diagram Walkthrough

flowchart LR
  A["SearchAsync method"] -- "renamed to" --> B["ExecuteQueryAsync"]
  C["GraphSearchResult class"] -- "renamed to" --> D["GraphQueryResult"]
  E["GraphSearchOptions"] -- "refactored to" --> F["GraphQueryOptions + GraphQueryExecuteOptions"]
  B --> G["Updated in IGraphDb"]
  B --> H["Updated in IGraphKnowledgeService"]
  D --> I["Updated across all implementations"]

Loading

File Walkthrough

Relevant files

Enhancement

11 files IGraphDb.cs Rename SearchAsync to ExecuteQueryAsync                                    +1/-1      IGraphKnowledgeService.cs Rename SearchAsync to ExecuteQueryAsync                                    +1/-1      GraphQueryResult.cs Rename GraphSearchResult to GraphQueryResult                          +1/-1      GraphQueryOptions.cs Refactor options into inheritance hierarchy                            +7/-3      KnowledgeBaseController.cs Update to use new ExecuteQueryAsync and GraphQueryOptions +2/-2      GraphDb.cs Update SearchAsync to ExecuteQueryAsync implementation      +5/-5      GraphKnowledgeService.cs Update SearchAsync to ExecuteQueryAsync implementation      +3/-3      KnowledgeHook.cs Update method calls and add Provider property                        +6/-4      MembaseController.cs Update SearchAsync to ExecuteQueryAsync call                          +1/-1      MembaseGraphDb.cs Update SearchAsync to ExecuteQueryAsync implementation      +2/-2      MembaseService.cs Update GraphSearchResult to GraphQueryResult                          +2/-2

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Null deserialization risk: The code assigns result = JsonSerializer.Deserialize<GraphQueryResult>(...) without a null check, so a failed/empty deserialization can return null and violate the non-null Task<GraphQueryResult> contract. Referred Code Content = new StringContent(data, Encoding.UTF8, MediaTypeNames.Application.Json) }; AddHeaders(client); var rawResponse = await client.SendAsync(message); rawResponse.EnsureSuccessStatusCode(); var responseStr = await rawResponse.Content.ReadAsStringAsync(); result = JsonSerializer.Deserialize<GraphQueryResult>(responseStr, _jsonOptions); return result; Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Query logged in errors: The error log interpolates the raw query into the log message, which can leak sensitive/user-provided content into logs. Referred Code public async Task<GraphQueryResult> ExecuteQueryAsync(string query, GraphQueryOptions? options = null) { try { var db = GetGraphDb(options?.Provider); var result = await db.ExecuteQueryAsync(query, options); return result; } catch (Exception ex) { _logger.LogError(ex, $"Error when searching graph knowledge (Query: {query})."); return new GraphQueryResult(); } Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Missing graph query validation: The /knowledge/graph/search handler forwards request.Query and related fields to ExecuteQueryAsync without visible validation/sanitization, so it may allow unsafe or malformed query execution depending on downstream implementations. Referred Code [HttpPost("/knowledge/graph/search")] public async Task<GraphKnowledgeViewModel> SearchGraphKnowledge([FromBody] SearchGraphKnowledgeRequest request) { var options = new GraphQueryOptions { Provider = request.Provider, GraphId = request.GraphId, Arguments = request.Arguments, Method = request.Method }; var result = await _graphKnowledgeService.ExecuteQueryAsync(request.Query, options); return new GraphKnowledgeViewModel Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix default dictionary arguments In MembaseService.cs, replace the null-coalescing default [] with new Dictionary<string, object>() to match the expected Parameters type. src/Plugins/BotSharp.Plugin.Membase/Services/MembaseService.cs [22] -Parameters = args ?? [] +Parameters = args ?? new Dictionary<string, object>() Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a type mismatch that will cause a compilation error. The fix is necessary for the code to be valid.	Medium
	Make graph provider property nullable Change the Provider property in GraphQueryOptions to be nullable (string?) to avoid forcing callers to specify a provider and allow using the system's default. src/Infrastructure/BotSharp.Abstraction/Graph/Options/GraphQueryOptions.cs [3-6] public class GraphQueryOptions : GraphQueryExecuteOptions { - public string Provider { get; set; } + public string? Provider { get; set; } } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that making Provider non-nullable is a regression, as it forces hardcoding the provider and prevents using the configured default. Making it nullable restores the previous, more flexible behavior.	Medium
General	Add null-coalescing after deserialize In GraphDb.cs, add a null-coalescing fallback to new GraphQueryResult() after JsonSerializer.Deserialize to prevent potential null reference exceptions. src/Plugins/BotSharp.Plugin.Graph/GraphDb.cs [82-83] -result = JsonSerializer.Deserialize<GraphQueryResult>(responseStr, _jsonOptions); +result = JsonSerializer.Deserialize<GraphQueryResult>(responseStr, _jsonOptions) + ?? new GraphQueryResult(); return result; Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a potential NullReferenceException if JsonSerializer.Deserialize returns null. Adding a null-coalescing operator makes the code more robust against malformed API responses.	Low
	Populate result JSON field In MembaseService.cs, populate the Result property of GraphQueryResult by serializing response.Data to JSON for consistency with other implementations. src/Plugins/BotSharp.Plugin.Membase/Services/MembaseService.cs [25-29] return new GraphQueryResult { Keys = response.Columns, - Values = response.Data + Values = response.Data, + Result = JsonSerializer.Serialize(response.Data) }; Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out an inconsistency with other implementations like MembaseGraphDb.cs where the Result property is populated. This improves consistency and ensures the full data is available to consumers.	Low
Learned best practice	Copy returned mutable collections Copy response.Columns and response.Data (including per-row dictionaries) so the returned GraphQueryResult can’t be affected by shared references from the API/client layer. src/Plugins/BotSharp.Plugin.Membase/GraphDb/MembaseGraphDb.cs [42-47] return new GraphQueryResult { - Keys = response.Columns, - Values = response.Data, + Keys = response.Columns?.ToArray() ?? [], + Values = response.Data?.Select(row => new Dictionary<string, object?>(row)).ToArray() ?? [], Result = JsonSerializer.Serialize(response.Data) }; Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - When transferring mutable collections between objects, copy them to avoid shared-state side effects.	Low
More