You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
Below is a summary of compliance checks for this PR:
Security Compliance
⚪
Sensitive info in logs
Description: New DEBUG logging prints potentially sensitive LLM content and tool-call arguments (which may include secrets/PII) via _logger.LogDebug(...) for both tool calls and full streamed text, risking sensitive information exposure in logs when debug logging is enabled. ChatCompletionProvider.cs [217-233]
Generic: Robust Error Handling and Edge Case Management
Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation
Status: Null handling missing: GetAnthropicClient dereferences settings.ApiKey without checking for null settings (or missing/empty API key), which can cause runtime failures without graceful handling.
Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging.
Status: Internal details exposed: The streaming path can emit delta.StopReason as assistant content, potentially exposing internal stop reasons to end users instead of returning a generic user-facing message.
Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data.
Status: Sensitive data in logs: Debug logging prints tool call arguments (toolCall?.Arguments) which may include sensitive user-provided data and should be redacted or omitted.
Generic: Security-First Input Validation and Data Handling
Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities
Status: External input validation: Provider/model values derived from options are used to resolve settings and capabilities without visible validation/allowlisting, requiring verification that upstream code constrains these inputs safely.
The Anthropic plugin's new dependency on BotSharp.Core for streaming violates the plugin architecture. To fix this, abstract streaming and message hub features into interfaces within BotSharp.Abstraction and make the plugin depend on those interfaces.
varhub=_services.GetRequiredService<MessageHub<HubObserveData<RoleDialogModel>>>();varconv=_services.GetRequiredService<IConversationService>();varmessageId=conversations.LastOrDefault()?.MessageId??string.Empty;varcontentHooks=_services.GetHooks<IContentGeneratingHook>(agent.Id);// Before chat completion hookforeach(varhookincontentHooks){awaithook.BeforeGenerating(agent,conversations);}
...(clipped 13lines)
Solution Walkthrough:
Before:
// In BotSharp.Plugin.AnthropicAI.csproj<ProjectReferenceInclude="..\..\Infrastructure\BotSharp.Core\BotSharp.Core.csproj"/>// In ChatCompletionProvider.csusing BotSharp.Core.Infrastructures.Streams;usingBotSharp.Core.MessageHub;publicasyncTask<RoleDialogModel>GetChatCompletionsStreamingAsync(...){// Direct dependency on concrete class from BotSharp.Corevarhub=_services.GetRequiredService<MessageHub<...>>();// Direct instantiation of concrete class from BotSharp.CoreusingvartextStream=newRealtimeTextStream();// ...}
After:
// In BotSharp.Plugin.AnthropicAI.csproj<ProjectReferenceInclude="..\..\Infrastructure\BotSharp.Abstraction\BotSharp.Abstraction.csproj"/>// In ChatCompletionProvider.cs (assuming new abstractions are created)using BotSharp.Abstraction.Messaging;// e.g., IMessageHubusingBotSharp.Abstraction.Streaming;// e.g., IRealtimeTextStreampublicasyncTask<RoleDialogModel>GetChatCompletionsStreamingAsync(...){// Dependency on abstraction from BotSharp.Abstractionvarhub=_services.GetRequiredService<IMessageHub<...>>();// Dependency on abstraction (e.g., via factory)usingvartextStream=_services.GetRequiredService<IRealtimeTextStream>();// ...}
Suggestion importance[1-10]: 9
__
Why: The suggestion correctly identifies a critical architectural flaw where the AnthropicAI plugin now depends on concrete implementations in BotSharp.Core, breaking the intended loose coupling and modularity of the plugin system.
High
Possible issue
Restore handling of file URLs
Restore the functionality to handle file URLs in CollectMessageContentParts by adding logic to process file.FileUrl in addition to file.FileData, ensuring support for images referenced by URL.
private void CollectMessageContentParts(List<ContentBase> contentParts, List<BotSharpFile> files)
{
foreach (var file in files)
{
if (!string.IsNullOrEmpty(file.FileData))
{
var (contentType, binary) = FileUtility.GetFileInfoFromData(file.FileData);
var contentPart = new ImageContent
{
Source = new ImageSource
{
MediaType = contentType,
Data = binary.ToArray()
}
};
contentParts.Add(contentPart);
}
+ else if (!string.IsNullOrEmpty(file.FileUrl))+ {+ var contentType = FileUtility.GetFileContentType(file.FileUrl);+ var contentPart = new ImageContent+ {+ Source = new ImageSource+ {+ MediaType = contentType,+ Url = file.FileUrl+ }+ };+ contentParts.Add(contentPart);+ }
}
}
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 8
__
Why: This suggestion correctly identifies a functional regression where handling for file.FileUrl was removed. Re-adding this logic is critical for maintaining feature parity and supporting multimodal messages that reference images by URL.
Medium
Check settings before client creation
Add validation to GetAnthropicClient to ensure that the retrieved settings and settings.ApiKey are not null or empty, throwing an InvalidOperationException if they are missing.
public static AnthropicClient GetAnthropicClient(string provider, string model, IServiceProvider services)
{
var settingsService = services.GetRequiredService<ILlmProviderService>();
- var settings = settingsService.GetSetting(provider, model);- var client = new AnthropicClient(new APIAuthentication(settings.ApiKey));- return client;+ var settings = settingsService.GetSetting(provider, model)+ ?? throw new InvalidOperationException($"No LLM settings found for provider '{provider}', model '{model}'.");+ if (string.IsNullOrWhiteSpace(settings.ApiKey))+ {+ throw new InvalidOperationException($"API key is not configured for provider '{provider}', model '{model}'.");+ }+ return new AnthropicClient(new APIAuthentication(settings.ApiKey));
}
Apply / Chat
Suggestion importance[1-10]: 8
__
Why: This suggestion correctly points out missing validation for settings and settings.ApiKey, which could lead to a NullReferenceException. Adding explicit checks and throwing informative exceptions significantly improves the robustness and debuggability of the client creation logic.
Medium
Avoid adding empty text content part
Avoid adding an empty TextContent part to assistant messages by checking if message.LlmContent is not null or empty before adding it to the content parts.
var contentParts = new List<ContentBase>();
if (allowMultiModal && !message.Files.IsNullOrEmpty())
{
CollectMessageContentParts(contentParts, message.Files);
}
-contentParts.Add(new TextContent() { Text = message.LlmContent });++if (!string.IsNullOrEmpty(message.LlmContent))+{+ contentParts.Add(new TextContent() { Text = message.LlmContent });+}+
messages.Add(new Message
{
Role = RoleType.Assistant,
Content = contentParts
});
Apply / Chat
Suggestion importance[1-10]: 6
__
Why: This is a valid improvement that makes the API payload cleaner and more robust by preventing the addition of an empty TextContent part, which could be unnecessary or cause issues.
Low
Learned best practice
Prevent null converter usage
Avoid calling ConvertPdfToImages with a null converter; if the model supports PDF reading, skip conversion, otherwise fail fast with a clear error when no converter is configured.
var converter = GetImageConverter(options?.ImageConverter);
if (converter == null && modelSettings?.AllowPdfReading != true)
{
var fileCoreSettings = _services.GetRequiredService<FileCoreSettings>();
converter = GetImageConverter(fileCoreSettings?.ImageConverter?.Provider);
}
-targetFiles = await ConvertPdfToImages(converter, pdfFiles);+if (converter == null)+{+ if (modelSettings?.AllowPdfReading == true)+ {+ targetFiles = pdfFiles;+ }+ else+ {+ throw new InvalidOperationException("No PDF-to-image converter configured for models that don't support direct PDF reading.");+ }+}+else+{+ targetFiles = await ConvertPdfToImages(converter, pdfFiles);+}
Apply / Chat
Suggestion importance[1-10]: 6
__
Why:
Relevant best practice - Add precise null/state checks to avoid passing null dependencies into downstream operations (defensive coding).
Low
General
Simplify and align function arguments serialization
Simplify and align function argument serialization in the streaming implementation by using ToJsonString() instead of a complex chain of ToString(), IfNullOrEmptyAs(), and null-coalescing.
Why: This suggestion improves code quality by simplifying the serialization of FunctionArgs and aligning the streaming implementation with the non-streaming one, which enhances readability and maintainability.
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.
PR Type
Enhancement
Description
Implement Claude streaming support with async chat completions
Add PDF reading capability detection via model settings
Refactor Anthropic provider with improved tool handling and thinking parameters
Enhance image converter logic with null safety checks
Diagram Walkthrough
File Walkthrough
LlmModelSetting.cs
Add PDF reading capability propertysrc/Infrastructure/BotSharp.Abstraction/MLTasks/Settings/LlmModelSetting.cs
AllowPdfReadingproperty that checks if model supports PDFreading capability
CapabilitiescontainsLlmModelCapability.PdfReadingFileInstructService.Pdf.cs
Refactor PDF reading with model capability detectionsrc/Infrastructure/BotSharp.Core/Files/Services/Instruct/FileInstructService.Pdf.cs
ILlmProviderServiceto check PDF readingcapability
AllowPdfReadingStopReason.cs
Add Claude stop reason constantssrc/Plugins/BotSharp.Plugin.AnthropicAI/Constants/StopReason.cs
EndTurn,MaxTokens,ToolUse,StopSequence,ContentFilter, andGuardRailChatCompletionProvider.cs
Implement Claude streaming and enhance chat completionsrc/Plugins/BotSharp.Plugin.AnthropicAI/Providers/ChatCompletionProvider.cs
GetChatCompletionsAsyncmethod with streaming callbacksupport
GetChatCompletionsStreamingAsyncmethod with real-time textstreaming and message hub integration
IConversationStateServicedependency for state managementresponse.ToolCallsinstead of parsingcontent blocks
GetThinkingParamsmethodProviderHelper.GetAnthropicClientmessages
PrepareOptionsmethodProviderHelper.cs
Add Anthropic client factory helpersrc/Plugins/BotSharp.Plugin.AnthropicAI/Providers/ProviderHelper.cs
client
authentication
Using.cs
Reorganize and expand global using statementssrc/Plugins/BotSharp.Plugin.AnthropicAI/Using.cs
BotSharp.Core.Infrastructures.StreamsandBotSharp.Core.MessageHubFileInstructService.cs
Improve image converter null safetysrc/Infrastructure/BotSharp.Core/Files/Services/Instruct/FileInstructService.cs
GetImageConvertermethod
default
BotSharp.Plugin.AnthropicAI.csproj
Update project dependency to Coresrc/Plugins/BotSharp.Plugin.AnthropicAI/BotSharp.Plugin.AnthropicAI.csproj
BotSharp.AbstractiontoBotSharp.Core