Skip to content

Revert "Fix membase json" #1394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iceljc merged 1 commit into from
Aug 4, 2026
+2 −10
Merged

Revert "Fix membase json" #1394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Plugins/BotSharp.Plugin.Membase/Interfaces/IMembaseApi.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public interface IMembaseApi
Task<PgtValidationResponse> ValidatePgtDefinitionAsync(string graphId, string definitionId, [Body] PgtValidationRequest request);

[Post("/graph/{graphId}/pgt-external/{correlationId}/complete")]
Task<PgtExternalCompleteResponse> CompletePgtExternalAsync(string graphId, string correlationId, [FromBody] object body);
Task<PgtExternalCompleteResponse> CompletePgtExternalAsync(string graphId, string correlationId, [FromBody] object emptyBody);
#endregion

#region Procedure
Expand Down
6 changes: 1 addition & 5 deletions src/Plugins/BotSharp.Plugin.Membase/MembasePlugin.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,7 @@ public void RegisterDI(IServiceCollection services, IConfiguration config)

services.AddHttpContextAccessor();
services.AddTransient<MembaseAuthHandler>();
// Use plain Web options WITHOUT Refit's default ObjectToInferredTypesConverter:
// consumers (ObjectExtensions.TryGetValue<JsonElement>, GraphBuilder, GetNodePropertiesOrDefault)
// rely on Dictionary<string, object?> values deserializing as JsonElement.
services.AddRefitClient<IMembaseApi>(new RefitSettings(
new SystemTextJsonContentSerializer(new JsonSerializerOptions(JsonSerializerDefaults.Web)))
services.AddRefitClient<IMembaseApi>(new RefitSettings
{
CollectionFormat = CollectionFormat.Multi
})
Comment on lines +29 to 32

@qodo-code-review qodo-code-review Bot Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Unpinned membase json deserialization 🐞 Bug ≡ Correctness

MembasePlugin no longer configures a specific Refit JSON content serializer, but graph
construction relies on Dictionary<string, object?> values being JsonElement for node/edge
objects. If Refit deserializes those object values into inferred CLR types (e.g., nested
dictionaries) instead of JsonElement, GraphBuilder.Build will skip rows and can produce
incomplete/empty graphs.
Agent Prompt 
### Issue description
The Membase Refit client no longer pins JSON deserialization behavior. Downstream code expects `GraphQueryResult.Values` dictionaries to contain `JsonElement` objects for keys like `sourceNode`, `targetNode`, and `edge`.

### Issue Context
`MembaseGraphDb` passes `CypherQueryResponse.Data` straight through to `GraphQueryResult.Values`. `GraphBuilder.Build` uses `ObjectExtensions.TryGetValue<JsonElement>` and will `continue` (drop the row) when the value isn't a `JsonElement` (or already a `JsonElement`-deserializable `T`).

### Fix Focus Areas
- src/Plugins/BotSharp.Plugin.Membase/MembasePlugin.cs[21-39]
- src/Plugins/BotSharp.Plugin.Membase/GraphDb/MembaseGraphDb.cs[41-55]
- src/Infrastructure/BotSharp.Abstraction/Graph/Utils/GraphBuilder.cs[18-26]
- src/Infrastructure/BotSharp.Abstraction/Utilities/ObjectExtensions.cs[76-105]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Expand Down
4 changes: 0 additions & 4 deletions src/Plugins/BotSharp.Plugin.OpenAI/Providers/ProviderHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ public static OpenAIClient GetClient(string provider, string model, string? apiK
{
var settingsService = services.GetRequiredService<ILlmProviderService>();
var settings = settingsService.GetSetting(provider, model);
if (settings == null && string.IsNullOrEmpty(apiKey))
{
throw new InvalidOperationException($"No LLM model settings found for '{provider}.{model}'. Register the model under LlmProviders (appsettings/user secrets) or pass an api key.");
}
Comment on lines -12 to -15

@qodo-code-review qodo-code-review Bot Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Null settings dereference 🐞 Bug ☼ Reliability

ProviderHelper.GetClient can now throw NullReferenceException when
ILlmProviderService.GetSetting returns null and apiKey is null, because it still evaluates
settings!.ApiKey. This regresses from a clear InvalidOperationException explaining how to
configure the model/key.
Agent Prompt 
### Issue description
`ProviderHelper.GetClient` no longer validates the case where both the configured model setting is missing and the caller did not provide an API key, leading to a `NullReferenceException` from `settings!.ApiKey`.

### Issue Context
`ILlmProviderService.GetSetting` explicitly returns null when provider/model settings are not found, and several OpenAI providers call `ProviderHelper.GetClient(..., apiKey: null, ...)`, making the null-settings path reachable.

### Fix Focus Areas
- src/Plugins/BotSharp.Plugin.OpenAI/Providers/ProviderHelper.cs[8-15]
- src/Infrastructure/BotSharp.Core/Infrastructures/LlmProviderService.cs[77-95]
- src/Plugins/BotSharp.Plugin.OpenAI/Providers/Image/ImageCompletionProvider.Compose.cs[24-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

var options = !string.IsNullOrEmpty(settings?.Endpoint) ?
new OpenAIClientOptions { Endpoint = new Uri(settings.Endpoint) } : null;
return new OpenAIClient(new ApiKeyCredential(apiKey ?? settings!.ApiKey), options);
Expand Down
Loading