Infrastructure for GPT 4.x model upgrading

[adenchen123]

No description provided.

[qodo-code-review]

Review Summary by Qodo

Add GPT 4.x model upgrade mapping infrastructure

✨ Enhancement

Walkthroughs

Description

• Add GPT 4.x model constants for latest OpenAI models
• Implement model upgrade mapping infrastructure with configuration support
• Add GetUpgradeModel method to enable dynamic model version upgrades
• Register ModelUpgradeMapSettings in dependency injection container

Diagram

flowchart LR
  A["Gpt4xModelConstants"] -- "defines" --> B["GPT model versions"]
  C["ModelUpgradeMapSettings"] -- "configures" --> D["Model mappings"]
  E["ISettingService"] -- "exposes" --> F["GetUpgradeModel method"]
  D -- "used by" --> F
  G["AgentPlugin"] -- "registers" --> D
  F -- "returns" --> B

Loading

File Changes

1. src/Infrastructure/BotSharp.Abstraction/Models/Gpt4xModelConstants.cs ✨ Enhancement +13/-0

Add GPT 4.x model constants

• New static class defining constants for GPT 4.x model identifiers
• Includes gpt-4o, gpt-4o-mini, gpt-4.1 variants and realtime preview models
• Centralizes model name definitions for consistency across codebase

src/Infrastructure/BotSharp.Abstraction/Models/Gpt4xModelConstants.cs

---

2. src/Infrastructure/BotSharp.Abstraction/Settings/ISettingService.cs ✨ Enhancement +2/-0

Add GetUpgradeModel interface method

• Add GetUpgradeModel method to interface for model version mapping
• Enables abstraction of model upgrade logic across implementations

src/Infrastructure/BotSharp.Abstraction/Settings/ISettingService.cs

---

3. src/Infrastructure/BotSharp.Abstraction/Settings/ModelUpgradeMapSettings.cs ✨ Enhancement +14/-0

Add model upgrade mapping settings classes

• New settings class for managing model upgrade mappings
• ModelUpgradeMapItem defines old/new model pairs with enable flag
• Supports configuration-driven model version upgrades

src/Infrastructure/BotSharp.Abstraction/Settings/ModelUpgradeMapSettings.cs

---

View more (2)

4. src/Infrastructure/BotSharp.Core/Agents/AgentPlugin.cs ⚙️ Configuration changes +10/-0

Register model upgrade settings in DI

• Register ModelUpgradeMapSettings in dependency injection container
• Bind configuration section to ModelUpgradeMap list
• Enable model upgrade settings to be injected across application

src/Infrastructure/BotSharp.Core/Agents/AgentPlugin.cs

---

5. src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs ✨ Enhancement +13/-0

Implement model upgrade resolution logic

• Implement GetUpgradeModel method to resolve model upgrades
• Look up old model name in upgrade map with case-insensitive comparison
• Return new model name if mapping exists and enabled, otherwise return original

src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (3) 📎 Requirement gaps (0)

1. Config bind bypasses settings 📘 Rule violation ✓ Correctness

Description

ModelUpgradeMapSettings is populated via direct IConfiguration.Bind into
settings.ModelUpgradeMap, which can misalign with the settings object shape and create
configuration drift. This can cause the upgrade map to remain empty depending on config structure,
leading to inconsistent model-upgrade behavior.

Code

src/Infrastructure/BotSharp.Core/Agents/AgentPlugin.cs[R52-58]

+        services.AddScoped(provider =>
+        {
+            var settingService = provider.GetRequiredService<ISettingService>();
+            var config = provider.GetRequiredService<IConfiguration>();
+            var settings = new ModelUpgradeMapSettings();
+            config.Bind(ModelUpgradeMapSettings.Key, settings.ModelUpgradeMap);
+            return settings;

Evidence

Compliance requires consistent configuration behavior and avoiding redundant/incorrect sources of
truth; the DI registration binds the section ModelUpgradeMap directly into the list property
rather than binding the settings object, risking contract mismatch with expected configuration
structure.

src/Infrastructure/BotSharp.Core/Agents/AgentPlugin.cs[52-58]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ModelUpgradeMapSettings` is registered by binding `ModelUpgradeMap` config directly into `settings.ModelUpgradeMap`, which can mismatch the intended settings object contract and lead to empty mappings.
## Issue Context
The compliance requirement calls for consistent configuration/contract behavior and avoiding redundant sources of truth. Current DI wiring bypasses the settings object and may not match the expected configuration schema.
## Fix Focus Areas
- src/Infrastructure/BotSharp.Core/Agents/AgentPlugin.cs[52-58]

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

---

2. Culture-based model name compare ☑ 📘 Rule violation ✓ Correctness

Description

Model identifier matching uses StringComparison.InvariantCultureIgnoreCase, which is
culture-sensitive and can cause inconsistent matches for identifier-like strings. This violates the
requirement to use robust comparers for identifier matching.

Code

src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs[57]

+        var mapping = modelUpgradeMapSettings.ModelUpgradeMap.FirstOrDefault(x => x.OldModel.Equals(oldModelName, StringComparison.InvariantCultureIgnoreCase));

Evidence

Compliance calls out robust matching comparers (e.g., OrdinalIgnoreCase) for identifiers; the new
lookup compares model names using InvariantCultureIgnoreCase, which is not the recommended
comparer for identifiers and can behave unexpectedly under certain cultures.

src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs[57-57]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Model-name matching uses a culture-based comparison (`InvariantCultureIgnoreCase`) which is not robust for identifier matching and may yield inconsistent results.
## Issue Context
Model names (like `gpt-4o`) are identifiers and should typically be compared using ordinal semantics to avoid culture-specific casing behavior.
## Fix Focus Areas
- src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs[57-57]

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

---

3. Missing DI causes crash 🐞 Bug ⛯ Reliability

Description

SettingService.GetUpgradeModel resolves ModelUpgradeMapSettings via GetRequiredService, which will
throw at runtime if the AgentPlugin (the only registrar) is not loaded in the current host/test
setup. This makes the new ISettingService API unsafe to call outside the Agent plugin composition
path.

Code

src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs[R54-57]

+    public string GetUpgradeModel(string oldModelName)
+    {
+        var modelUpgradeMapSettings = _services.GetRequiredService<ModelUpgradeMapSettings>();
+        var mapping = modelUpgradeMapSettings.ModelUpgradeMap.FirstOrDefault(x => x.OldModel.Equals(oldModelName, StringComparison.InvariantCultureIgnoreCase));

Evidence

GetUpgradeModel hard-requires ModelUpgradeMapSettings from DI, but ModelUpgradeMapSettings is only
registered in AgentPlugin; meanwhile ISettingService is registered broadly in AddBotSharpCore, so
the method can be called in contexts that don't include AgentPlugin registrations.

src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs[54-57]
src/Infrastructure/BotSharp.Core/Agents/AgentPlugin.cs[52-59]
src/Infrastructure/BotSharp.Core/BotSharpCoreExtensions.cs[24-33]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SettingService.GetUpgradeModel` calls `GetRequiredService<ModelUpgradeMapSettings>()`, which throws if `ModelUpgradeMapSettings` is not registered. Since registration is currently only done in `AgentPlugin`, any host/test that wires up `ISettingService` but not the Agent plugin will crash when `GetUpgradeModel` is called.
### Issue Context
`ISettingService` is registered as part of `AddBotSharpCore`, so it is broadly available. `ModelUpgradeMapSettings` should either be registered in the same core composition root, or `GetUpgradeModel` should degrade gracefully if the setting is absent.
### Fix Focus Areas
- src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs[54-65]
- src/Infrastructure/BotSharp.Core/Agents/AgentPlugin.cs[52-59]
- src/Infrastructure/BotSharp.Core/BotSharpCoreExtensions.cs[24-43]

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

---

4. oldModelName lacks null guard 📘 Rule violation ⛯ Reliability

Description

GetUpgradeModel does not guard against null/empty oldModelName and can return a null value
despite the return type being string. This can propagate unpredictable behavior at the boundary
where this value is used for model selection.

Code

src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs[R54-62]

+    public string GetUpgradeModel(string oldModelName)
+    {
+        var modelUpgradeMapSettings = _services.GetRequiredService<ModelUpgradeMapSettings>();
+        var mapping = modelUpgradeMapSettings.ModelUpgradeMap.FirstOrDefault(x => x.OldModel.Equals(oldModelName, StringComparison.InvariantCultureIgnoreCase));
+        
+        if(mapping == null || !mapping.Enable)
+        {
+            return oldModelName;
+        }

Evidence

The compliance checklist requires null/empty guards and safe fallbacks at boundaries; the new method
accepts a string oldModelName and returns it directly when no mapping exists, without checking for
null/empty or providing a safe fallback.

src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs[54-62]
Best Practice: Learned patterns

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`GetUpgradeModel` lacks null/empty guarding for `oldModelName` and returns it directly, which can propagate null/invalid values to model selection.
## Issue Context
This method likely sits at an external boundary (configuration/user/provider-derived model names). Compliance requires explicit guards and safe fallbacks to avoid runtime issues.
## Fix Focus Areas
- src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs[54-62]

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

---

5. Unvalidated new model 🐞 Bug ✓ Correctness

Description

GetUpgradeModel returns mapping.NewModel without validating it; a misconfigured mapping
(empty/whitespace NewModel) can propagate an invalid model name into providers that use the model
string directly in API requests. This can cause hard-to-diagnose downstream request failures.

Code

src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs[R57-65]

+        var mapping = modelUpgradeMapSettings.ModelUpgradeMap.FirstOrDefault(x => x.OldModel.Equals(oldModelName, StringComparison.InvariantCultureIgnoreCase));
+        
+        if(mapping == null || !mapping.Enable)
+        {
+            return oldModelName;
+        }
+
+        return mapping.NewModel;
+    }

Evidence

ModelUpgradeMapItem.NewModel defaults to empty string and GetUpgradeModel returns it as-is when
mapping is enabled. Providers (e.g., OpenAI text completion) place _model directly into the
request payload without guarding against empty values, so an empty upgraded model can break
requests.

src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs[56-65]
src/Infrastructure/BotSharp.Abstraction/Settings/ModelUpgradeMapSettings.cs[9-13]
src/Plugins/BotSharp.Plugin.OpenAI/Providers/Text/TextCompletionProvider.cs[102-108]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`GetUpgradeModel` can return an empty/whitespace `NewModel`, which can propagate into provider requests that require a valid non-empty model name.
### Issue Context
`ModelUpgradeMapItem.NewModel` defaults to `string.Empty`, and providers often put the model name directly into request payloads.
### Fix Focus Areas
- src/Infrastructure/BotSharp.Core/Infrastructures/SettingService.cs[54-65]
- src/Infrastructure/BotSharp.Abstraction/Settings/ModelUpgradeMapSettings.cs[9-13]

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

---

ⓘ The new review experience is currently in Beta. Learn more

[JackJiang1234]

reviewed

[yileicn]

reviewed