Skip to content

Conversation

iceljc
Copy link
Collaborator

@iceljc iceljc commented Aug 22, 2025

PR Type

Enhancement


Description

  • Add dynamic web search model selection logic

  • Introduce IsDefault flag for web search settings

  • Replace hardcoded model with configurable selection


Diagram Walkthrough

flowchart LR
  A["LlmModelSetting"] -- "adds IsDefault flag" --> B["WebSearchSetting"]
  C["WebIntelligentSearchFn"] -- "queries models" --> D["LlmProviderService"]
  D -- "returns models" --> E["Model Selection Logic"]
  E -- "selects default or fallback" --> F["ChatCompletion"]
Loading

File Walkthrough

Relevant files
Configuration changes
LlmModelSetting.cs
Add IsDefault flag to WebSearchSetting                                     

src/Infrastructure/BotSharp.Abstraction/MLTasks/Settings/LlmModelSetting.cs

  • Add IsDefault boolean property to WebSearchSetting class
+1/-0     
Enhancement
WebIntelligentSearchFn.cs
Implement dynamic web search model selection                         

src/Infrastructure/BotSharp.Core/WebSearch/Functions/WebIntelligentSearchFn.cs

  • Replace hardcoded model with dynamic selection logic
  • Query provider models and select based on IsDefault flag
  • Add fallback mechanism for model selection
  • Set default provider and model as constants
+9/-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

Null Handling

Ensure GetProviderModels cannot return null and that model entries have non-null Name before selection; otherwise the fallback may still propagate a null Name into CompletionProvider.

var models = llmProviderService.GetProviderModels(provider);
var webSearchModel = models.FirstOrDefault(x => x.WebSearch?.IsDefault == true)?.Name
                    ?? models.FirstOrDefault(x => x.WebSearch != null)?.Name
                    ?? defaultModel;

var completion = CompletionProvider.GetChatCompletion(_services, provider: provider, model: webSearchModel);
var response = await completion.GetChatCompletions(agent, dialogs);
Hardcoded Defaults

The provider and default model are hardcoded; consider sourcing from configuration to avoid coupling and ease future changes or multi-provider scenarios.

var provider = "openai";
var defaultModel = "gpt-4o-mini-search-preview";

var llmProviderService = _services.GetRequiredService<ILlmProviderService>();
var models = llmProviderService.GetProviderModels(provider);
var webSearchModel = models.FirstOrDefault(x => x.WebSearch?.IsDefault == true)?.Name
                    ?? models.FirstOrDefault(x => x.WebSearch != null)?.Name
                    ?? defaultModel;

var completion = CompletionProvider.GetChatCompletion(_services, provider: provider, model: webSearchModel);
var response = await completion.GetChatCompletions(agent, dialogs);
Multiple Defaults

Clarify behavior if multiple models have WebSearch.IsDefault=true; current FirstOrDefault selection may be nondeterministic without ordering.

    public bool IsDefault { get; set; }
    public string? SearchContextSize { get; set; }
}

Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Add null and empty safeguards

Guard against null or empty results from GetProviderModels to avoid
NullReferenceException. Also ensure webSearchModel is non-empty before passing
to the completion provider, otherwise use the defaultModel.

src/Infrastructure/BotSharp.Core/WebSearch/Functions/WebIntelligentSearchFn.cs [56-65]

 var provider = "openai";
 var defaultModel = "gpt-4o-mini-search-preview";
 
 var llmProviderService = _services.GetRequiredService<ILlmProviderService>();
-var models = llmProviderService.GetProviderModels(provider);
+var models = llmProviderService.GetProviderModels(provider) ?? Enumerable.Empty<LlmModelSetting>();
+
 var webSearchModel = models.FirstOrDefault(x => x.WebSearch?.IsDefault == true)?.Name
                     ?? models.FirstOrDefault(x => x.WebSearch != null)?.Name
                     ?? defaultModel;
 
+if (string.IsNullOrWhiteSpace(webSearchModel))
+{
+    webSearchModel = defaultModel;
+}
+
 var completion = CompletionProvider.GetChatCompletion(_services, provider: provider, model: webSearchModel);
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that GetProviderModels could return null, causing a NullReferenceException, and also handles the edge case of a model having an empty name, improving the code's robustness.

Medium
General
Filter by compatible model type

Prefer stricter selection by ensuring the model Type supports chat completions
before choosing it for web search. This avoids selecting an embedding or
incompatible model accidentally marked with WebSearch.

src/Infrastructure/BotSharp.Core/WebSearch/Functions/WebIntelligentSearchFn.cs [61-63]

-var webSearchModel = models.FirstOrDefault(x => x.WebSearch?.IsDefault == true)?.Name
-                    ?? models.FirstOrDefault(x => x.WebSearch != null)?.Name
-                    ?? defaultModel;
+var webSearchModel = models
+    .Where(x => x.Type == LlmModelType.Chat)
+    .FirstOrDefault(x => x.WebSearch?.IsDefault == true)?.Name
+    ?? models.Where(x => x.Type == LlmModelType.Chat)
+             .FirstOrDefault(x => x.WebSearch != null)?.Name
+    ?? defaultModel;
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion improves the model selection logic by adding a filter for LlmModelType.Chat, making the selection more robust by preventing the use of incompatible model types.

Low
  • More

@iceljc iceljc merged commit 944869e into SciSharp:master Aug 22, 2025
4 checks passed
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