From 9bd520b5aa2c9812e79120b40fcc89d8823ee286 Mon Sep 17 00:00:00 2001 From: "nick.yi" Date: Thu, 29 Oct 2026 13:58:24 +0800 Subject: [PATCH] feat(agent-testing): force the test model in CompletionProvider and record used models - Apply a run's model override in CompletionProvider.GetChatCompletion instead of an agent-load hook, so it also covers callers that build their own agent or pick a model themselves (template llm_config, web search replacement model, conversation state) - Remove AgentTestModelOverrideHook; it also carried the agent's reasoning effort over to the forced model - Record the provider/model of every completion in a case as UsedModels, so an "agent default" result still shows which model produced it --- .../ISyntheticConversationProbe.cs | 8 +++ .../Infrastructures/CompletionProvider.cs | 26 ++++++- .../AgentTestingPlugin.cs | 8 +-- .../Models/AgentTestCaseResult.cs | 15 +++- .../Models/AgentTestDtos.cs | 5 +- .../Runtime/AgentTestModelOverrideHook.cs | 71 ------------------- .../Runtime/AgentTestModelUsageHook.cs | 36 ++++++++++ .../Runtime/AgentTestRunRegistry.cs | 30 ++++++-- .../AgentTestSyntheticConversationProbe.cs | 10 +++ .../Services/AgentTestCaseRunner.cs | 1 + 10 files changed, 122 insertions(+), 88 deletions(-) delete mode 100644 src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestModelOverrideHook.cs create mode 100644 src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestModelUsageHook.cs diff --git a/src/Infrastructure/BotSharp.Abstraction/Conversations/ISyntheticConversationProbe.cs b/src/Infrastructure/BotSharp.Abstraction/Conversations/ISyntheticConversationProbe.cs index d15ff6427..aef6aae64 100644 --- a/src/Infrastructure/BotSharp.Abstraction/Conversations/ISyntheticConversationProbe.cs +++ b/src/Infrastructure/BotSharp.Abstraction/Conversations/ISyntheticConversationProbe.cs @@ -1,3 +1,5 @@ +using BotSharp.Abstraction.Agents.Models; + namespace BotSharp.Abstraction.Conversations; /// @@ -28,4 +30,10 @@ public interface ISyntheticConversationProbe /// traffic from the very limits it is meant to be held to. /// bool IsSynthetic(string conversationId); + + /// + /// The provider/model the harness forces every chat completion in this conversation onto, or null + /// to leave it alone. Applied by CompletionProvider.GetChatCompletion. + /// + AgentLlmConfig? GetModelOverride(string conversationId) => null; } diff --git a/src/Infrastructure/BotSharp.Core/Infrastructures/CompletionProvider.cs b/src/Infrastructure/BotSharp.Core/Infrastructures/CompletionProvider.cs index 58018ca4a..12e33c1e8 100644 --- a/src/Infrastructure/BotSharp.Core/Infrastructures/CompletionProvider.cs +++ b/src/Infrastructure/BotSharp.Core/Infrastructures/CompletionProvider.cs @@ -60,7 +60,13 @@ public static IChatCompletion GetChatCompletion( AgentLlmConfig? agentConfig = null) { var completions = services.GetServices(); - (provider, model) = GetProviderAndModel(services, provider: provider, model: model, modelId: modelId, + var over = GetModelOverride(services); + if (over != null) + { + (provider, model) = (over.Provider, over.Model); + } + + (provider, model) = GetProviderAndModel(services, provider: provider, model: model, modelId: modelId, multiModal: multiModal, agentConfig: agentConfig); @@ -238,6 +244,24 @@ public static ILiveCompletion GetLiveCompletion( return completer; } + /// + /// A harness can force every chat completion in its conversation onto one model. Applied here, not + /// on the agent, so it holds no matter where the caller got its provider/model from -- an agent, + /// a template's llm_config, a hard-coded replacement model, or conversation state. + /// + private static AgentLlmConfig? GetModelOverride(IServiceProvider services) + { + var conversationId = services.GetService()?.ConversationId; + if (string.IsNullOrEmpty(conversationId)) + { + return null; + } + + return services.GetServices() + .Select(p => p.GetModelOverride(conversationId)) + .FirstOrDefault(x => x != null); + } + private static (string, string) GetProviderAndModel( IServiceProvider services, string? provider = null, diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/AgentTestingPlugin.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/AgentTestingPlugin.cs index 3695f85ba..53806976b 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/AgentTestingPlugin.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/AgentTestingPlugin.cs @@ -1,4 +1,5 @@ using BotSharp.Abstraction.Agents; +using BotSharp.Abstraction.Loggers; using BotSharp.Abstraction.Repositories.Settings; using BotSharp.Abstraction.Users.Enums; using BotSharp.Plugin.AgentTesting.Repositories; @@ -55,10 +56,9 @@ public void RegisterDI(IServiceCollection services, IConfiguration config) // working -- the runner's canary self-check is what catches that. services.AddScoped(); - // The model-override seam, which lets one run sweep the same agent across several models. - // Losing this line does not raise an error: every model would run on the agent's own - // LlmConfig and the comparison grid would show them all behaving identically. - services.AddScoped(); + // Records the provider/model each completion actually ran on, so an "agent default" result + // still says which model produced it. + services.AddScoped(); services.AddScoped(); diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCaseResult.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCaseResult.cs index d5ebdcce7..76a448ee4 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCaseResult.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCaseResult.cs @@ -24,13 +24,22 @@ public class AgentTestCaseResult : MongoBase /// /// Which model produced this result. Null = the agent's own LlmConfig was used (the run named - /// no models). Unlike the deleted AgentTestRunTriggerRequest.Provider/Model, these values were - /// genuinely applied to this execution by AgentTestModelOverrideHook -- they are not a - /// decorative record of something that never took effect. + /// no models); says what that resolved to. Unlike the deleted + /// AgentTestRunTriggerRequest.Provider/Model, these values were genuinely applied to this + /// execution by CompletionProvider.GetChatCompletion -- they are not a decorative record of something + /// that never took effect. /// public string? Provider { get; set; } public string? Model { get; set; } + /// + /// "provider/model" of every completion the agents under test actually ran on, in first-use + /// order. Filled whether or not the run named a model, so an "agent default" result still says + /// what it ran on. Can hold more than one entry: a router and the agent it hands off to may use + /// different models. + /// + public List UsedModels { get; set; } = []; + /// /// Wall clock for the whole case, including the canary, the mock lookups and the conversation /// reads. Comparable between models because every model pays the same overhead, but it is not a diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestDtos.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestDtos.cs index bede63e89..972dbce5c 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestDtos.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestDtos.cs @@ -213,9 +213,8 @@ public class AgentTestRunTriggerRequest /// /// This is not a revival of the two deleted Provider/Model fields above: those were stored and /// never read back, so they lied. This one genuinely takes effect -- - /// AgentTestModelOverrideHook rewrites agent.LlmConfig to the named provider/model as the agent - /// loads, so the main conversation path (RoutingService.InvokeAgent) reads the rewritten value, - /// and every AgentTestCaseResult records which model produced it. + /// CompletionProvider.GetChatCompletion forces every chat completion in the case's conversation + /// onto the named provider/model, and every AgentTestCaseResult records which model produced it. /// public List? Models { get; set; } diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestModelOverrideHook.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestModelOverrideHook.cs deleted file mode 100644 index 2891ea8a1..000000000 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestModelOverrideHook.cs +++ /dev/null @@ -1,71 +0,0 @@ -using BotSharp.Abstraction.Agents; -using BotSharp.Abstraction.Agents.Settings; - -namespace BotSharp.Plugin.AgentTesting.Runtime; - -/// -/// Lets one test run force a specific model, which is what makes "sweep the same agent across -/// several models and compare them" possible. -/// -/// Why an agent-load hook and not somewhere else: the main conversation path, -/// RoutingService.InvokeAgent, reads agent.LlmConfig.Provider/Model and passes them -/// EXPLICITLY to CompletionProvider.GetChatCompletion; and -/// CompletionProvider.GetProviderAndModel only consults the provider/model -/// conversation-state override when what it was passed is empty. In other words, writing those two -/// keys into conversation state has no effect on the main path -- the only moment left where an -/// override still takes is after LoadAgent has produced the agent and before InvokeAgent runs. -/// -/// Structurally identical to TestMockExecutorProvider: look the conversation up in the registry, act -/// only on a hit, leave every other conversation untouched. No AsyncLocal, for the same reason as -/// MockFunctionExecutor (it is silently lost across a background queue or SideCar boundary). -/// -/// SelfId is overridden to the empty string: AgentHookBase's default implementation throws, and this -/// hook has to apply to ANY agent under test -- including downstream agents a test conversation -/// reaches via route_to_agent. Those must run on the requested model too, otherwise a model -/// comparison only swaps the entry agent and the results mean nothing. -/// -public class AgentTestModelOverrideHook : AgentHookBase -{ - private readonly IAgentTestRunRegistry _registry; - private readonly IConversationService _conversations; - private readonly ILogger _logger; - - public override string SelfId => string.Empty; - - public AgentTestModelOverrideHook( - IServiceProvider services, - AgentSettings settings, - IAgentTestRunRegistry registry, - IConversationService conversations, - ILogger logger) - : base(services, settings) - { - _registry = registry; - _conversations = conversations; - _logger = logger; - } - - public override Task OnAgentLoaded(Agent agent) - { - var active = _registry.TryGet(_conversations.ConversationId); - var over = active?.ModelOverride; - if (over == null || agent == null) - { - // Not a conversation under test, or this run named no model: touch nothing. - return Task.CompletedTask; - } - - // LlmConfig can be null (an agent.json with no llmConfig block) and the override still has - // to apply there -- otherwise the agents that most need to be told which model to use are - // exactly the ones it silently skips. - agent.LlmConfig ??= new AgentLlmConfig(); - agent.LlmConfig.Provider = over.Provider; - agent.LlmConfig.Model = over.Model; - - _logger.LogDebug( - "Agent test run overrode agent {AgentId} to {Provider}/{Model} for conversation {ConversationId}.", - agent.Id, over.Provider, over.Model, active!.ConversationId); - - return Task.CompletedTask; - } -} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestModelUsageHook.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestModelUsageHook.cs new file mode 100644 index 000000000..b41c8e561 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestModelUsageHook.cs @@ -0,0 +1,36 @@ +using BotSharp.Abstraction.Loggers; + +namespace BotSharp.Plugin.AgentTesting.Runtime; + +/// +/// Records which provider/model each completion in a test conversation actually ran on. Without a +/// model override the result's Provider/Model stay null ("agent default"), and the agent's LlmConfig +/// can change after the run -- so this is the only record of what that run really used. +/// +/// Read after the call rather than off agent.LlmConfig at load time: LlmConfig may be empty, in +/// which case CompletionProvider falls back to a default the hook would otherwise never see. +/// +public class AgentTestModelUsageHook : IContentGeneratingHook +{ + private readonly IAgentTestRunRegistry _registry; + private readonly IConversationService _conversations; + + public AgentTestModelUsageHook(IAgentTestRunRegistry registry, IConversationService conversations) + { + _registry = registry; + _conversations = conversations; + } + + public Task AfterGenerated(RoleDialogModel message, TokenStatsModel tokenStats) + { + // The llmJudge call shares the case's conversation id but runs under a Guid.Empty prompt + // agent; it is the suite's judge model, not a model of the agent under test. + if (message?.CurrentAgentId == Guid.Empty.ToString() || string.IsNullOrEmpty(tokenStats?.Model)) + { + return Task.CompletedTask; + } + + _registry.TryGet(_conversations.ConversationId)?.RecordModel($"{tokenStats.Provider}/{tokenStats.Model}"); + return Task.CompletedTask; + } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestRunRegistry.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestRunRegistry.cs index 53e288a05..1a5496975 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestRunRegistry.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestRunRegistry.cs @@ -15,12 +15,11 @@ public class ActiveTestRun /// /// The model this execution is forced onto; null = no override, use the agent's own LlmConfig. /// - /// Applied by AgentTestModelOverrideHook, writing into agent.LlmConfig at the moment the agent - /// finishes loading. That moment is the only one that works: the main conversation path, - /// RoutingService.InvokeAgent, passes agent.LlmConfig.Provider/Model to CompletionProvider - /// EXPLICITLY, and CompletionProvider.GetProviderAndModel only consults the conversation-state - /// override when what it was passed is empty. So writing provider/model into conversation state - /// has no effect on the main path, and overriding any later than agent load is already too late. + /// Applied by CompletionProvider.GetChatCompletion (read through + /// AgentTestSyntheticConversationProbe), so it holds whatever provider/model the caller passed -- + /// an agent's, a template's llm_config, or a hard-coded one. Writing provider/model into + /// conversation state would not do: GetProviderAndModel only consults state when what it was + /// passed is empty. The agent's own LlmConfig is left untouched. /// public TestModel? ModelOverride { get; set; } @@ -57,6 +56,25 @@ public void Record(ObservedToolCall call) /// public int NextCallOrdinal(string functionName) => _ordinals.AddOrUpdate(functionName, 0, (_, prev) => prev + 1); + + private readonly List _usedModels = []; + + /// + /// "provider/model" of every completion this case ran, in first-use order. Recorded by + /// AgentTestModelUsageHook. + /// + public IReadOnlyList UsedModels + { + get { lock (_usedModels) return _usedModels.ToList(); } + } + + public void RecordModel(string model) + { + lock (_usedModels) + { + if (!_usedModels.Contains(model)) _usedModels.Add(model); + } + } } public interface IAgentTestRunRegistry diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestSyntheticConversationProbe.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestSyntheticConversationProbe.cs index 3280f4576..2ccee1a0e 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestSyntheticConversationProbe.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestSyntheticConversationProbe.cs @@ -28,4 +28,14 @@ public AgentTestSyntheticConversationProbe(IAgentTestRunRegistry registry) public bool IsSynthetic(string conversationId) => !string.IsNullOrEmpty(conversationId) && _registry.TryGet(conversationId) != null; + + /// + /// The model this run forces, which CompletionProvider.GetChatCompletion applies to every chat + /// completion in the conversation. + /// + public AgentLlmConfig? GetModelOverride(string conversationId) + { + var over = _registry.TryGet(conversationId)?.ModelOverride; + return over == null ? null : new AgentLlmConfig { Provider = over.Provider, Model = over.Model }; + } } diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestCaseRunner.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestCaseRunner.cs index 7390f7343..c7c413619 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestCaseRunner.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestCaseRunner.cs @@ -341,6 +341,7 @@ async Task AwaitOrHandOffAsync(Task driverTask) stopwatch.Stop(); result.DurationMs = stopwatch.ElapsedMilliseconds; result.ModelDurationMs = modelDurationMs; + result.UsedModels = active.UsedModels.ToList(); // In the finally block so a timed-out or crashed case still reports what it spent. A run // that fell over having burned the budget is exactly the run whose cost matters.