feat: add LiteLLM as AI gateway provider - #1395
Open
prodmanpd wants to merge 1 commit into
Open
Conversation
Contributor
PR Summary by QodoAdd LiteLLM provider plugin (OpenAI-compatible gateway integration)
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
Contributor
Code Review by Qodo
1. Null settings cause NRE
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+10
to
+14
| var settingsService = services.GetRequiredService<ILlmProviderService>(); | ||
| var settings = settingsService.GetSetting(provider, model); | ||
| var options = !string.IsNullOrEmpty(settings.Endpoint) ? | ||
| new OpenAIClientOptions { Endpoint = new Uri(settings.Endpoint) } : null; | ||
| return new OpenAIClient(new ApiKeyCredential(settings.ApiKey), options); |
Contributor
There was a problem hiding this comment.
1. Null settings cause nre 🐞 Bug ☼ Reliability
ProviderHelper.GetClient dereferences settings.Endpoint/settings.ApiKey without null checks even though ILlmProviderService.GetSetting can return null, causing a NullReferenceException when provider/model config is missing or mismatched. This yields an opaque failure instead of a clear configuration error for LiteLLM requests.
Agent Prompt
### Issue description
`ProviderHelper.GetClient` assumes `GetSetting(provider, model)` always returns a non-null model setting and immediately dereferences it. However, the core `LlmProviderService.GetSetting` returns `null` when either the provider or model is not found, which will lead to a `NullReferenceException` in LiteLLM calls.
### Issue Context
This is a configuration edge case (wrong model name, missing provider config, etc.) that should fail fast with a descriptive exception rather than crashing with NRE.
### Fix Focus Areas
- src/Plugins/BotSharp.Plugin.LiteLLM/Providers/ProviderHelper.cs[8-15]
- src/Infrastructure/BotSharp.Core/Infrastructures/LlmProviderService.cs[77-94]
### Suggested fix
- If `settings == null`, throw an `InvalidOperationException` (or a domain exception) with a message like: `Missing LLM configuration for provider 'litellm' and model '{model}'`.
- Optionally validate `settings.Endpoint` and `settings.ApiKey` (when required) and throw similarly descriptive errors.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+316
to
+319
| var allText = textStream.GetText(); | ||
| _logger.LogCritical($"Text Content: {allText}"); | ||
|
|
||
| responseMessage = new RoleDialogModel(AgentRole.Assistant, allText) |
Contributor
There was a problem hiding this comment.
2. Critical logs leak responses 🐞 Bug ⛨ Security
GetChatCompletionsStreamingAsync logs the full assembled assistant response using LogCritical when streaming finishes, which can leak sensitive conversation content into production logs and create noisy high-severity alerts. This logging is unconditional (not DEBUG-gated) in the streaming completion path.
Agent Prompt
### Issue description
The LiteLLM streaming path logs the full generated assistant message content at `LogCritical` when the stream reaches a finish reason. This can expose sensitive user/LLM content and pollute critical-level telemetry.
### Issue Context
The log is unconditional in the streaming completion path (not wrapped in `#if DEBUG`). If content logging is needed for diagnostics, it should be opt-in and/or redacted and use an appropriate log level.
### Fix Focus Areas
- src/Plugins/BotSharp.Plugin.LiteLLM/Providers/Chat/ChatCompletionProvider.cs[314-320]
### Suggested fix
- Remove the full-content log line entirely, OR
- Change it to `LogDebug`/`LogTrace` behind an explicit config flag, and log only safe metadata by default (finish reason, token usage, conversationId/messageId).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+91
to
+94
| var state = _services.GetRequiredService<IConversationStateService>(); | ||
| var temperature = float.Parse(state.GetState("temperature", "0.0")); | ||
| var maxTokens = int.Parse(state.GetState("max_tokens", "1024")); | ||
|
|
Contributor
There was a problem hiding this comment.
3. Unsafe state parsing 🐞 Bug ☼ Reliability
TextCompletionProvider parses conversation state values with float.Parse/int.Parse and can throw FormatException when stored state values are empty or invalid, failing the completion request. ConversationStateService.GetState can return the stored Data string (including empty) rather than the provided default, so the parsing is not protected by defaults.
Agent Prompt
### Issue description
`TextCompletionProvider.PrepareOptions` uses `float.Parse` and `int.Parse` on unvalidated conversation-state strings. If state values are empty/malformed, this throws and the LiteLLM completion fails.
### Issue Context
`ConversationStateService.GetState` returns the stored `Data` for active states without checking for empty strings, meaning defaults passed to `GetState` won’t apply when the key exists but contains an empty value.
### Fix Focus Areas
- src/Plugins/BotSharp.Plugin.LiteLLM/Providers/Text/TextCompletionProvider.cs[89-99]
- src/Infrastructure/BotSharp.Core/Conversations/Services/ConversationStateService.cs[359-367]
### Suggested fix
- Replace `float.Parse`/`int.Parse` with `TryParse` (ideally `InvariantCulture`) and fall back to reasonable defaults.
- Optionally clamp ranges (e.g., temperature >= 0, max tokens > 0) to prevent invalid requests.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Adds LiteLLM as a first-class provider plugin (
BotSharp.Plugin.LiteLLM), mirroring the existingBotSharp.Plugin.DeepSeekAIpattern. LiteLLM is an AI gateway that exposes an OpenAI-compatible endpoint in front of 100+ LLM providers (OpenAI, Azure, Anthropic, Bedrock, Vertex, Gemini, Mistral, Ollama, local models, etc.), plus routing/load-balancing, virtual keys, and spend tracking.Because LiteLLM speaks the OpenAI wire protocol, the plugin reuses the same
OpenAIClient(with anEndpointoverride) thatDeepSeekAI,MiniMaxAI, and theopenaiprovider already use, so this is additive and low-risk, and gives BotSharp users a single provider entry to reach any model behind their gateway.Prior art
No existing LiteLLM integration in the repo (no
litellmreference in source, no prior or open PR). This plugin is modeled on the existingBotSharp.Plugin.DeepSeekAI(an OpenAI-compatible provider that overrides the clientEndpoint), so it stays consistent with current conventions.Changes
src/Plugins/BotSharp.Plugin.LiteLLM/(new plugin)LiteLLMPlugin.cs:IBotSharpPluginthat registersIChatCompletionandITextCompletion, provider keylitellmProviders/Chat/ChatCompletionProvider.cs: chat (sync), streaming-callback (GetChatCompletionsAsync), and streaming (GetChatCompletionsStreamingAsync), including tool/function calls and multimodal image partsProviders/Text/TextCompletionProvider.cs,Providers/ProviderHelper.cs,Using.cs,.csprojsrc/WebStarter/WebStarter.csproj: project referencesrc/WebStarter/appsettings.json: addedBotSharp.Plugin.LiteLLMto the plugin list and alitellmentry underLlmProviders(default endpointhttp://localhost:4000/v1/)BotSharp.sln: project added under the LLMs solution foldertests/BotSharp.LLM.Tests/: env-gatedCreateLiteLLMprovider factory wired into the existingChatCompletionTeststheory, plus aNullConversationServicetest stubTests
Built and tested with .NET 8 against a real LiteLLM proxy routed to Azure AI Foundry (
gpt-5.6-sol).Build: plugin and solution build clean (0 errors):
Live E2E: all 3 chat tests pass through the LiteLLM proxy to Azure
gpt-5.6-sol:These exercise the full chain:
ChatCompletionProvidercallsOpenAIClient(endpoint override), which calls the LiteLLM proxy, which routes to Azure. The proxy-side log confirms the round-trips:Wire-level smoke test (non-streaming and streaming) through the same proxy:
Note:
gpt-5.6-solis a reasoning model that rejects a non-defaulttemperature; LiteLLM'sdrop_paramsnormalizes this at the gateway, a concrete example of the compatibility benefit this integration provides.Risk / Compatibility
OpenAIpackage; no new runtime dependencies.litellmprovider is opt-in via config; base install and other providers are unaffected.Example usage
Run a LiteLLM proxy (
litellm --config config.yaml), then configure BotSharp:Switching any existing BotSharp code to the gateway is just changing the provider key to
litellmand every model behind LiteLLM is then reachable through this one provider.