Skip to content

Conversation

iceljc
Copy link
Collaborator

@iceljc iceljc commented Aug 12, 2025

PR Type

Enhancement


Description

  • Add reasoning effort level configuration to agent LLM settings

  • Implement parsing logic for low/medium/high effort levels

  • Update MongoDB storage to persist reasoning effort level

  • Downgrade OpenAI package version for compatibility


Diagram Walkthrough

flowchart LR
  A["LlmConstant"] --> B["AgentLlmConfig"]
  B --> C["MongoDB Storage"]
  B --> D["ChatCompletionProvider"]
  D --> E["ReasoningEffortLevel Parsing"]
Loading

File Walkthrough

Relevant files
Enhancement
LlmConstant.cs
Add default reasoning effort level constant                           

src/Infrastructure/BotSharp.Abstraction/Agents/Constants/LlmConstant.cs

  • Add default reasoning effort level constant set to "low"
+1/-0     
AgentLlmConfig.cs
Add reasoning effort level property to agent config           

src/Infrastructure/BotSharp.Abstraction/Agents/Models/AgentLlmConfig.cs

  • Add ReasoningEffortLevel property with JSON serialization attributes
  • Include proper documentation for the new property
+7/-0     
AgentLlmConfigMongoElement.cs
Update MongoDB model for reasoning effort level                   

src/Plugins/BotSharp.Plugin.MongoStorage/Models/AgentLlmConfigMongoElement.cs

  • Add ReasoningEffortLevel property to MongoDB model
  • Update conversion methods to include reasoning effort level mapping
+5/-2     
ChatCompletionProvider.cs
Implement reasoning effort level parsing and configuration

src/Plugins/BotSharp.Plugin.OpenAI/Providers/Chat/ChatCompletionProvider.cs

  • Add reasoning effort level parsing from state and agent config
  • Implement ParseReasoningEffortLevel method with low/medium/high
    mapping
  • Set reasoning effort level in chat completion options
+30/-1   
Miscellaneous
QdrantDb.cs
Refactor string comparison in sort order logic                     

src/Plugins/BotSharp.Plugin.Qdrant/QdrantDb.cs

  • Replace string comparison with IsEqualTo method for sort order
+1/-1     
Dependencies
Directory.Packages.props
Downgrade OpenAI package version                                                 

Directory.Packages.props

  • Downgrade OpenAI package from version 2.3.0 to 2.2.0-beta.4
+1/-1     

Copy link

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

Possible Issue

Parsing of reasoning effort level returns null when model not in default temperature map, which may unintentionally disable the feature for valid levels on other models.

private ChatReasoningEffortLevel? ParseReasoningEffortLevel(string? level)
{
    if (string.IsNullOrWhiteSpace(level) || !_defaultTemperature.ContainsKey(_model))
    {
        return null;
    }

    var effortLevel = ChatReasoningEffortLevel.Low;
    switch (level.ToLower())
    {
        case "medium":
            effortLevel = ChatReasoningEffortLevel.Medium;
            break;
        case "high":
            effortLevel = ChatReasoningEffortLevel.High;
            break;
        default:
            break;
    }

    return effortLevel;
}
Robustness

Temperature parsing uses float.Parse without invariant culture, which can throw or misparse in locales with comma decimal separators.

var temperature = float.Parse(state.GetState("temperature", "0.0"));
if (_defaultTemperature.ContainsKey(_model))
{
    temperature = _defaultTemperature[_model];
}
Null Safety

Using IsEqualTo on sort.Order without null check may throw if Order is null; consider safe navigation or default.

return new OrderBy
{
    Key = sort.Field,
    Direction = sort.Order.IsEqualTo("asc") ? Direction.Asc : Direction.Desc
};

@iceljc iceljc merged commit 4d13cd2 into SciSharp:master Aug 12, 2025
4 checks passed
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Validate reasoning level usage

The new reasoning_effort_level is applied unconditionally when a model has a
defaultTemperature entry, but there’s no check that the selected OpenAI model or
SDK version actually supports ChatReasoningEffortLevel, which risks runtime
failures or silent no-ops across providers. Gate the option by model capability
(e.g., only for o3/o4 or known-supported models) and the installed SDK version,
and consider logging a warning or ignoring the setting when unsupported.

Examples:

src/Plugins/BotSharp.Plugin.OpenAI/Providers/Chat/ChatCompletionProvider.cs [520-525]
private ChatReasoningEffortLevel? ParseReasoningEffortLevel(string? level)
{
    if (string.IsNullOrWhiteSpace(level) || !_defaultTemperature.ContainsKey(_model))
    {
        return null;
    }

Solution Walkthrough:

Before:

private ChatReasoningEffortLevel? ParseReasoningEffortLevel(string? level)
{
    if (string.IsNullOrWhiteSpace(level) || !_defaultTemperature.ContainsKey(_model))
    {
        return null;
    }
    // ... logic to parse level and return effortLevel
}

After:

private bool IsReasoningEffortSupported(string model)
{
    // Explicitly check for models that support this feature
    return model.StartsWith("gpt-4o") || model.StartsWith("gpt-4-turbo");
}

private ChatReasoningEffortLevel? ParseReasoningEffortLevel(string? level)
{
    if (string.IsNullOrWhiteSpace(level) || !IsReasoningEffortSupported(_model))
    {
        return null;
    }
    // ... logic to parse level and return effortLevel
}
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical flaw where ReasoningEffortLevel is applied based on an indirect condition (_defaultTemperature.ContainsKey), which could cause runtime failures if the selected model does not support this feature.

High
Possible issue
Use invariant float parsing

Avoid culture-sensitive parsing for temperature to prevent exceptions or
incorrect values in locales using comma decimals. Use invariant culture and safe
parsing with a fallback.

src/Plugins/BotSharp.Plugin.OpenAI/Providers/Chat/ChatCompletionProvider.cs [497-501]

-var temperature = float.Parse(state.GetState("temperature", "0.0"));
+var temperatureStr = state.GetState("temperature", "0.0");
+if (!float.TryParse(temperatureStr, NumberStyles.Float, CultureInfo.InvariantCulture, out var temperature))
+{
+    temperature = 0.0f;
+}
 if (_defaultTemperature.ContainsKey(_model))
 {
     temperature = _defaultTemperature[_model];
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: This suggestion correctly identifies a potential culture-specific bug in float.Parse and proposes a robust solution using float.TryParse with CultureInfo.InvariantCulture, improving code reliability.

Medium
General
Improve parsing robustness

Normalize input safely and handle unexpected values explicitly. Use
ToLowerInvariant() and return null on unrecognized values to avoid silently
forcing "low" when an invalid string is supplied.

src/Plugins/BotSharp.Plugin.OpenAI/Providers/Chat/ChatCompletionProvider.cs [520-541]

 private ChatReasoningEffortLevel? ParseReasoningEffortLevel(string? level)
 {
-    if (string.IsNullOrWhiteSpace(level) || !_defaultTemperature.ContainsKey(_model))
+    if (string.IsNullOrWhiteSpace(level))
     {
         return null;
     }
 
-    var effortLevel = ChatReasoningEffortLevel.Low;
-    switch (level.ToLower())
+    var normalized = level.Trim().ToLowerInvariant();
+    return normalized switch
     {
-        case "medium":
-            effortLevel = ChatReasoningEffortLevel.Medium;
-            break;
-        case "high":
-            effortLevel = ChatReasoningEffortLevel.High;
-            break;
-        default:
-            break;
-    }
-
-    return effortLevel;
+        "low" => ChatReasoningEffortLevel.Low,
+        "medium" => ChatReasoningEffortLevel.Medium,
+        "high" => ChatReasoningEffortLevel.High,
+        _ => (ChatReasoningEffortLevel?)null
+    };
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that the current implementation defaults to "low" for any invalid input; returning null for unrecognized values is more robust and explicit.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant