diff --git a/src/Infrastructure/BotSharp.Abstraction/Conversations/ISyntheticConversationProbe.cs b/src/Infrastructure/BotSharp.Abstraction/Conversations/ISyntheticConversationProbe.cs new file mode 100644 index 000000000..d15ff6427 --- /dev/null +++ b/src/Infrastructure/BotSharp.Abstraction/Conversations/ISyntheticConversationProbe.cs @@ -0,0 +1,31 @@ +namespace BotSharp.Abstraction.Conversations; + +/// +/// Reports whether a conversation is synthetic -- driven by an automated harness rather than by a +/// person -- so that guards aimed at human overuse can stand aside for it. +/// +/// This exists because per-user volume limits count the wrong thing for a test harness. A regression +/// suite legitimately opens one conversation per case per model, at machine speed, and does it under +/// whatever identity the background worker happens to have. Measured as human behaviour that looks +/// like abuse, and the limit then fails every test with a message about conversation quotas -- which +/// says nothing about the agent under test and is indistinguishable, in a report, from the agent +/// having regressed. +/// +/// Deliberately a query rather than a flag on the message: a flag would have to be threaded through +/// every call that creates or continues a conversation, and anything that forgot would silently be +/// treated as human traffic. Asking by conversation id keeps the answer in one place. +/// +/// Implementations must be cheap and side-effect free -- this is called on the message path -- and +/// must answer false for anything they do not recognise. Several may be registered; a conversation is +/// synthetic if any of them claims it. None being registered is the normal case, and then nothing is +/// exempt. +/// +public interface ISyntheticConversationProbe +{ + /// + /// True when this conversation is being driven by a harness. Must not throw, and must not treat + /// an unknown or blank id as synthetic: getting this wrong in that direction exempts real user + /// traffic from the very limits it is meant to be held to. + /// + bool IsSynthetic(string conversationId); +} diff --git a/src/Infrastructure/BotSharp.Logger/Hooks/RateLimitConversationHook.cs b/src/Infrastructure/BotSharp.Logger/Hooks/RateLimitConversationHook.cs index e9d5cdaea..53cb91b42 100644 --- a/src/Infrastructure/BotSharp.Logger/Hooks/RateLimitConversationHook.cs +++ b/src/Infrastructure/BotSharp.Logger/Hooks/RateLimitConversationHook.cs @@ -48,6 +48,19 @@ public override async Task OnMessageReceived(RoleDialogModel message) return; } + // Everything below this point is a volume guard against human overuse, and neither guard + // measures anything meaningful for an automated harness: a regression suite legitimately + // opens one conversation per case per model and drives its turns as fast as the model + // answers. Left in force they fail every test with a message about quotas, which says nothing + // about the agent under test and reads, in a report, exactly like the agent having regressed. + // + // The input length check above still applies. That one is about a single message being too + // large, which is a real condition a test should surface rather than be excused from. + if (IsSyntheticConversation(convId)) + { + return; + } + // Check message sending frequency var userSents = Dialogs.Where(x => x.Role == AgentRole.User) .TakeLast(2).ToList(); @@ -86,4 +99,44 @@ public override async Task OnMessageReceived(RoleDialogModel message) } } } + + /// + /// Whether this conversation is driven by a harness rather than a person. False when nothing is + /// registered to answer, which is the normal case -- an absent probe must never exempt anyone. + /// + private bool IsSyntheticConversation(string conversationId) + { + if (string.IsNullOrEmpty(conversationId)) + { + return false; + } + + var probes = _services.GetServices().ToList(); + if (probes.Count == 0) + { + return false; + } + + foreach (var probe in probes) + { + try + { + if (probe.IsSynthetic(conversationId)) + { + return true; + } + } + catch (Exception ex) + { + // A probe that throws must not take the message down with it, and must not be read as + // a yes: failing closed here means the worst case is a harness message being rate + // limited, rather than real traffic escaping the limit. + _logger.LogWarning(ex, + "A synthetic conversation probe failed for conversation {ConversationId}; " + + "treating it as real traffic.", conversationId); + } + } + + return false; + } } diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/AgentTestingPlugin.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/AgentTestingPlugin.cs index 865423073..3695f85ba 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/AgentTestingPlugin.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/AgentTestingPlugin.cs @@ -44,6 +44,13 @@ public void RegisterDI(IServiceCollection services, IConfiguration config) // Singleton: the test context has to be visible across requests and across threads. services.AddSingleton(); + // Lets BotSharp's own rate limiting recognise a harness conversation and stand aside. Backed + // by the registry rather than by the conversation's "test-set" tag on purpose: the tag is + // only written once the conversation row exists, which is after the first message has already + // been through the rate limit hook, so a tag-based check would still block the first turn of + // every case. The registry entry exists before the conversation is opened at all. + services.AddSingleton(); + // The seam that takes over function execution. Lose this line and mocking silently stops // working -- the runner's canary self-check is what catches that. services.AddScoped(); @@ -76,6 +83,14 @@ public void RegisterDI(IServiceCollection services, IConfiguration config) // AgentTestController, so it needs an explicit registration the same way every other // service in this method does (there is no auto-discovery mechanism in this plugin). services.AddScoped(); + + // Scoped, like the segmenter: it resolves IChatCompletion implementations out of the same + // scope and holds no state between calls. + services.AddScoped(); + + // Scoped for the same reason, and reads the case store to ground its prompt in the suite's + // existing cases and the edited case's last run. + services.AddScoped(); services.AddScoped(); // AgentTestRunQueue is both a singleton and a BackgroundService: all three lines point at diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Controllers/AgentTestController.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Controllers/AgentTestController.cs index 1d6315c17..f507f545d 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Controllers/AgentTestController.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Controllers/AgentTestController.cs @@ -1,4 +1,6 @@ using System.Security.Claims; +using MongoDB.Bson; +using MongoDB.Bson.Serialization; using BotSharp.Abstraction.Agents; using BotSharp.Abstraction.MLTasks; using BotSharp.Abstraction.Infrastructures.Attributes; @@ -25,6 +27,7 @@ public class AgentTestController : ControllerBase private readonly IAgentTestRunQueue _queue; private readonly IAgentService _agents; private readonly AgentTestRecorder _recorder; + private readonly ICaseAuthor _author; private readonly ILlmProviderService _llmProviders; public AgentTestController( @@ -32,12 +35,14 @@ public AgentTestController( IAgentTestRunQueue queue, IAgentService agents, AgentTestRecorder recorder, + ICaseAuthor author, ILlmProviderService llmProviders) { _repo = repo; _queue = queue; _agents = agents; _recorder = recorder; + _author = author; _llmProviders = llmProviders; } @@ -127,7 +132,12 @@ public async Task> CreateCase([FromBody] AgentTestCa return BadRequest("suiteId is required"); } - if (ValidateCasePayload(request) is { } validationError) + if (await ValidateEntryAgentAsync(request) is { } entryAgentError) + { + return BadRequest(entryAgentError); + } + + if (CaseValidation.Validate(request) is { } validationError) { return BadRequest(validationError); } @@ -166,7 +176,12 @@ public async Task> UpdateCase(string id, [FromBody] return NotFound($"agent test case {id} not found"); } - if (ValidateCasePayload(request) is { } validationError) + if (await ValidateEntryAgentAsync(request) is { } entryAgentError) + { + return BadRequest(entryAgentError); + } + + if (CaseValidation.Validate(request) is { } validationError) { return BadRequest(validationError); } @@ -193,6 +208,220 @@ public async Task> UpdateCase(string id, [FromBody] return testCase; } + /// + /// Duplicates a case inside its own suite and returns the copy. + /// + /// Server-side rather than a client GET-then-POST because the copy has to carry EVERY field the + /// case has. A client that rebuilds the payload from its own form drops whatever it does not know + /// about, and a copy missing its mocks is indistinguishable in the list from a correct one -- + /// right up to the run where it blocks every tool the agent reaches for. + /// + /// Not cross-suite on purpose: moving a case between suites also changes which agent it runs + /// against, so the copy would need a different entry agent and different mock targets to mean + /// anything. That is an edit, not a copy. + /// + /// + /// Which cases a change needs to run, and -- just as importantly -- which it does not, with the + /// reason for each. + /// + /// Read-only and side-effect free: it plans a run, it does not start one. Triggering stays + /// per-suite (POST suites/{id}/run), so a caller takes the included case ids from here and + /// triggers each suite that appears among them. + /// + [HttpPost("scope")] + public async Task> SelectScope([FromBody] ScopeSelectionRequest request) + { + request ??= new ScopeSelectionRequest(); + + if (request.Batch is { } batch && !CaseBatches.All.Contains(batch)) + { + return BadRequest($"batch must be one of {string.Join(", ", CaseBatches.All)}, not {batch}"); + } + + var targets = (request.TargetAgentIds ?? []) + .Where(id => !string.IsNullOrWhiteSpace(id)) + .Select(id => id.Trim()) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToList(); + + // Naming no agents and not declaring a platform-wide change would narrow to nothing at all, + // and an empty scope reported as a successful plan is the single most dangerous answer this + // endpoint could give. + if (targets.Count == 0 && !request.FullPlatform) + { + return BadRequest( + "name at least one target agent, or set fullPlatform for a change with no single target"); + } + + var query = new ScopeQuery + { + TargetAgentIds = targets, + FullPlatform = request.FullPlatform, + Batch = request.Batch + }; + + var response = new ScopeSelectionResponse + { + TargetAgentIds = targets, + FullPlatform = request.FullPlatform, + Batch = request.Batch + }; + + foreach (var suite in await _repo.ListSuitesAsync(null)) + { + foreach (var testCase in await _repo.ListCasesAsync(suite.Id)) + { + var decision = CaseScope.Decide(testCase, suite.AgentId, query); + var dto = new ScopedCaseDto + { + CaseId = testCase.Id, + CaseName = testCase.Name, + SuiteId = suite.Id, + SuiteName = suite.Name, + CaseType = testCase.CaseType, + Priority = testCase.Priority, + Severity = testCase.Severity, + CrossCutting = testCase.CrossCutting, + Enabled = testCase.Enabled, + Batch = decision.Batch, + InvolvedAgentIds = decision.InvolvedAgentIds.ToList(), + Reason = decision.Reason + }; + + response.TotalCases++; + (decision.Included ? response.Included : response.Excluded).Add(dto); + } + } + + return response; + } + + [HttpPost("cases/{id}/copy")] + public async Task> CopyCase(string id) + { + var source = await _repo.GetCaseAsync(id); + if (source == null) + { + return NotFound($"agent test case {id} not found"); + } + + // Cloned by a BSON round trip rather than field by field. A hand-written clone has exactly + // the drift problem this endpoint exists to prevent: the next field added to AgentTestCase + // would be silently absent from every copy, and nothing would fail until a run. + var copy = BsonSerializer.Deserialize(source.ToBsonDocument()); + + // Blank so UpsertCaseAsync mints a new one -- the round trip copied the source's _id, and + // keeping it would make the "copy" a full overwrite of the original. + copy.Id = string.Empty; + + var siblings = await _repo.ListCasesAsync(source.SuiteId); + copy.Name = NextCopyName(source.Name, siblings.Select(c => c.Name)); + + // Disabled regardless of the source. An exact duplicate that joins the next run measures the + // same thing twice: it pads the pass-rate denominator, and for a routing case it + // double-weights one routing decision. A copy is made in order to be edited into a variant, + // so it waits for that edit -- the same reason a recorded draft lands disabled. + copy.Enabled = false; + + // The round trip also copied CreateDate, which would have the copy claim to be as old as the + // case it came from. UpdateDate is set by the repository on write. + copy.CreateDate = DateTime.UtcNow; + + await _repo.UpsertCaseAsync(copy); + return copy; + } + + /// + /// "x" becomes "x (copy)", then "x (copy 2)", "x (copy 3)". Names are not unique in this store, + /// so this is presentation rather than a constraint -- but two rows both called "x (copy)" are + /// impossible to tell apart in the list, which is the one place copies are managed. + /// + /// Capped at the length the case editor's own input accepts, so the copy stays editable: a name + /// the form cannot hold would have to be trimmed by hand before any other change could be saved. + /// + private static string NextCopyName(string name, IEnumerable existing) + { + const int maxNameLength = 200; + + var taken = new HashSet(existing, StringComparer.OrdinalIgnoreCase); + + for (var attempt = 1; ; attempt++) + { + var suffix = attempt == 1 ? " (copy)" : $" (copy {attempt})"; + var room = maxNameLength - suffix.Length; + var stem = name.Length > room ? name[..room] : name; + var candidate = stem + suffix; + + if (!taken.Contains(candidate)) + { + return candidate; + } + } + } + + /// + /// Clears run history: removes the named runs and the case results underneath them. + /// + /// Bulk-only, and the single-row button in the UI calls it with one id. A separate + /// DELETE runs/{id} would be a second path to the same destructive operation, and the guard + /// below is one of the things worth having in exactly one place. + /// + /// Behind the same admin gate as triggering a run. Runs are the audit trail for whether an agent + /// change was evaluated at all, so removing them is at least as consequential as creating them. + /// + [BotSharpAuth] + [HttpPost("runs/delete")] + public async Task> DeleteRuns( + [FromBody] AgentTestRunDeleteRequest request) + { + var runIds = (request?.RunIds ?? []) + .Where(id => !string.IsNullOrWhiteSpace(id)) + .Select(id => id.Trim()) + .Distinct(StringComparer.Ordinal) + .ToList(); + + if (runIds.Count == 0) + { + return BadRequest("name at least one run to delete"); + } + + var response = new AgentTestRunDeleteResponse(); + + foreach (var runId in runIds) + { + var run = await _repo.GetRunAsync(runId); + if (run == null) + { + // Already gone. Reported rather than treated as an error: two people clearing the + // same history is a normal race, not a failure worth refusing the whole batch over. + response.Skipped.Add(new SkippedRunDto + { + RunId = runId, + Reason = "already deleted" + }); + continue; + } + + if (!IsTerminalStatus(run.Status)) + { + // Deleting a run that is still executing does not stop it: the queue keeps driving + // cases, keeps spending tokens, and keeps writing results for a run id that no longer + // exists -- results nothing can ever list again. Cancel first. + response.Skipped.Add(new SkippedRunDto + { + RunId = runId, + Reason = $"the run is {run.Status.ToLowerInvariant()}; cancel it before deleting" + }); + continue; + } + + response.DeletedResultCount += await _repo.DeleteRunAsync(runId); + response.DeletedRunIds.Add(runId); + } + + return response; + } + [HttpDelete("cases/{id}")] public async Task DeleteCase(string id) { @@ -254,6 +483,73 @@ public async Task>> RecordCase([FromBody] Agent } } + /// + /// One turn of authoring a case by conversation: the caller sends what they want in their own + /// words plus the draft as it stands, and gets back the new draft, a field-level diff, and + /// anything wrong with it. + /// + /// Saves nothing. The draft returned here still has to go through POST/PUT /agent-test/cases, + /// which is the only path that runs the entry-agent lookup and the only one a human presses -- + /// see for why an authoring model is not given write access to the + /// case store. + /// + /// [BotSharpAuth]: the same two escalations RecordCase and TriggerRun carry. It spends token + /// quota on every call with no throttling, and it sends the agent's instruction, the draft, and + /// (when the case has run before) the agent's real replies and real tool arguments to the model + /// vendor -- a wider egress than the recorder's, which withholds tool arguments. + /// + [BotSharpAuth] + [HttpPost("author")] + public async Task> AuthorCase([FromBody] AgentTestAuthorRequest request) + { + if (string.IsNullOrWhiteSpace(request.SuiteId)) + { + return BadRequest("suiteId is required"); + } + + // Same guard as RecordCase: a model this host cannot run fails here with a readable message, + // not deep inside the completion call. + if (ValidateRequestedModels(request.Model == null ? null : [request.Model]) is { } modelError) + { + return BadRequest(modelError); + } + + var suite = await _repo.GetSuiteAsync(request.SuiteId); + if (suite == null) + { + return NotFound($"agent test suite {request.SuiteId} not found"); + } + + if (!string.IsNullOrWhiteSpace(request.CaseId)) + { + // Checked rather than ignored: a caseId belonging to another suite would silently produce + // an ungrounded draft (the run-result lookup is scoped to this suite), and the author + // would never learn that the "based on the last run" part did nothing. + var existing = await _repo.GetCaseAsync(request.CaseId); + if (existing == null) + { + return NotFound($"agent test case {request.CaseId} not found"); + } + + if (!string.Equals(existing.SuiteId, suite.Id, StringComparison.Ordinal)) + { + return BadRequest($"case {request.CaseId} does not belong to suite {suite.Id}"); + } + } + + try + { + return await _author.AuthorAsync(suite, request, HttpContext.RequestAborted); + } + catch (CaseAuthorUnavailableException ex) + { + // No draft was produced. Surfaced verbatim, the same way RecordCase surfaces a rejected + // segmentation: "the model did not return JSON" tells the caller to retry, and "set this + // suite's judgeProvider" tells them what to fix. A 500 would tell them neither. + return BadRequest($"AI authoring failed: {ex.Message}"); + } + } + /// /// [BotSharpAuth]: every trigger really calls the model and really spends token quota, with no /// usage throttling anywhere -- a cost-escalation surface just like RecordCase, so it is @@ -373,16 +669,10 @@ public async Task>> GetMockTargets([FromQuery] string? return NotFound($"agent {agentId} not found"); } - var names = new List(); - names.AddRange((agent.Functions ?? []).Select(f => f.Name)); - names.AddRange((agent.SecondaryFunctions ?? []).Select(f => f.Name)); - names.AddRange((agent.McpTools ?? []).SelectMany(t => t.Functions ?? []).Select(f => f.Name)); - - return names - .Where(n => !string.IsNullOrWhiteSpace(n)) - .Distinct(StringComparer.OrdinalIgnoreCase) - .OrderBy(n => n, StringComparer.OrdinalIgnoreCase) - .ToList(); + // Same derivation the authoring prompt uses, so the names the editor offers and the names + // a model is allowed to mock can never diverge. Still names only on the wire: the case + // editor's pickers consume a plain string list. + return MockTargetCatalogue.Names(agent); } /// @@ -443,50 +733,65 @@ private static void ApplyCase(AgentTestCase testCase, AgentTestCaseUpsertRequest testCase.SuiteId = request.SuiteId; testCase.Name = request.Name; testCase.Enabled = request.Enabled; + // Normalised, not taken verbatim: CaseValidation.Validate has already rejected anything that is + // neither blank nor a known type, so this only fixes casing ("routing" -> "Routing") and + // maps blank onto the default. Storing the caller's casing would break every later + // Ordinal comparison against CaseTypes.Routing. + testCase.CaseType = CaseTypes.Normalize(request.CaseType) ?? CaseTypes.Agent; + testCase.EntryAgentId = string.IsNullOrWhiteSpace(request.EntryAgentId) ? null : request.EntryAgentId.Trim(); testCase.Turns = request.Turns ?? []; testCase.Assertions = request.Assertions ?? []; testCase.InitialStates = request.InitialStates ?? []; + testCase.History = (request.History ?? []) + .Select(m => new TestHistoryMessage + { + // Normalised for the same reason CaseType is: the driver and every later comparison + // use the canonical lowercase constants. + Role = HistoryRoles.Normalize(m.Role) ?? HistoryRoles.User, + Content = m.Content ?? string.Empty + }) + .ToList(); testCase.Mocks = request.Mocks ?? []; testCase.UnmockedToolPolicy = request.UnmockedToolPolicy; testCase.SourceConversationId = request.SourceConversationId; + + // Normalised, not verbatim: validation has already rejected anything unrecognised, so this + // only fixes casing. Storing "p0" would leave a case that never matches an Ordinal comparison + // against CasePriorities.P0 -- it would run, and then be filed in the wrong batch. + testCase.Priority = CasePriorities.Normalize(request.Priority) ?? CasePriorities.P1; + testCase.Severity = CaseSeverities.Normalize(request.Severity) ?? CaseSeverities.S1; + testCase.Batch = request.Batch; + testCase.CrossCutting = request.CrossCutting; + testCase.InvolvedAgents = (request.InvolvedAgents ?? []) + .Where(id => !string.IsNullOrWhiteSpace(id)) + .Select(id => id.Trim()) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToList(); + testCase.BusinessDomain = string.IsNullOrWhiteSpace(request.BusinessDomain) + ? null + : request.BusinessDomain.Trim(); + testCase.ExpectedOutcome = string.IsNullOrWhiteSpace(request.ExpectedOutcome) + ? null + : request.ExpectedOutcome.Trim(); + testCase.LastReviewedDate = request.LastReviewedDate; } /// - /// Shared create/update validation for a case payload. Null means the payload is acceptable; - /// otherwise the string is a caller-facing 400 message. + /// Null when the request's entry agent is usable. Separate from + /// because it needs IAgentService, so it cannot be static, and + /// a lookup is worth doing only once the cheap checks have passed. /// - private static string? ValidateCasePayload(AgentTestCaseUpsertRequest request) + private async Task ValidateEntryAgentAsync(AgentTestCaseUpsertRequest request) { - if (IsUnsupportedUnmockedToolPolicy(request.UnmockedToolPolicy)) + if (string.IsNullOrWhiteSpace(request.EntryAgentId)) { - return "Passthrough is not supported in P1"; - } - - var allAssertions = (request.Turns ?? []) - .SelectMany(t => t.Assertions ?? []) - .Concat(request.Assertions ?? []); - - foreach (var assertion in allAssertions) - { - var error = AssertionValidation.Validate(assertion); - if (error != null) - { - return error; - } + return null; } - return null; + var agent = await _agents.GetAgent(request.EntryAgentId.Trim()); + return agent == null ? $"entry agent {request.EntryAgentId} not found" : null; } - /// - /// Passthrough was specified in the design/plan and even had a (dead) code path, but nothing - /// ever back-fills an ObservedToolCall for a tool the provider let run for real -- under it, - /// toolNotCalled always vacuously passed against a tool that genuinely executed with real side - /// effects. Rejected here rather than implementing the back-fill (project owner decision). - /// - private static bool IsUnsupportedUnmockedToolPolicy(string? policy) - => string.Equals(policy, "Passthrough", StringComparison.OrdinalIgnoreCase); - private static bool IsTerminalStatus(string status) => status is AgentTestStatus.Passed or AgentTestStatus.Failed or AgentTestStatus.Error or AgentTestStatus.Cancelled; diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestAuthorDtos.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestAuthorDtos.cs new file mode 100644 index 000000000..8f636fbb2 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestAuthorDtos.cs @@ -0,0 +1,159 @@ +namespace BotSharp.Plugin.AgentTesting.Models; + +/// +/// Body of POST /agent-test/author -- one turn of authoring a case by conversation. +/// +/// Deliberately carries the whole conversation and the whole current draft, because the server keeps +/// no authoring session: there is no fifth collection, no draft lifecycle, no lock, and nothing to +/// clean up when someone closes the tab mid-sentence. The editor page already holds the draft as its +/// form state, so the draft it sends is exactly what the user is looking at, and what comes back +/// replaces it. +/// +public class AgentTestAuthorRequest +{ + /// Required: supplies the agent under test, and the judge model this falls back to. + public string SuiteId { get; set; } = string.Empty; + + /// + /// The case being edited, when there is one. Null while creating. + /// + /// Used only to ground the model: with a case id its most recent run result can be read, which + /// is what turns "add an assertion" from invention into a proposal based on what the agent + /// actually said and actually passed to its tools. + /// + public string? CaseId { get; set; } + + /// The authoring conversation so far, oldest first. The last entry is the new instruction. + public List Messages { get; set; } = []; + + /// + /// The draft as it stands. Null means start from nothing, which is the first turn of creating a + /// case. + /// + public AgentTestCaseUpsertRequest? Draft { get; set; } + + /// + /// Optional model override. Null falls back to the suite's judge model, and if that is unset the + /// request is refused rather than defaulting to a model nobody chose -- the same stance + /// LlmAgentTestJudge takes. + /// + public TestModel? Model { get; set; } +} + +/// One message in the authoring conversation. Only user and assistant: see . +public class AuthorChatMessage +{ + public string Role { get; set; } = AuthorChatRoles.User; + public string Content { get; set; } = string.Empty; +} + +public static class AuthorChatRoles +{ + public const string User = "user"; + public const string Assistant = "assistant"; + + public static readonly string[] All = [User, Assistant]; + + public static string? Normalize(string? value) + => All.FirstOrDefault(r => string.Equals(r, value?.Trim(), StringComparison.OrdinalIgnoreCase)); +} + +/// +/// Result of one authoring turn: what to say to the user, and what the draft now is. +/// +/// The draft is never saved by this endpoint. Persistence stays on POST/PUT /agent-test/cases, which +/// is the only path that runs the full validation including the entry-agent lookup -- and which the +/// human presses deliberately. An authoring model that could write to the case store would be one +/// misread instruction away from editing a case nobody asked it to touch. +/// +public class AgentTestAuthorResponse +{ + /// What the model says about what it did, shown in the chat panel. + public string Reply { get; set; } = string.Empty; + + /// + /// The draft after the merge. Always populated, even when nothing changed, so the client can + /// assign it unconditionally. + /// + public AgentTestCaseUpsertRequest Draft { get; set; } = new(); + + /// True when differs from the one that was sent in. + public bool DraftChanged { get; set; } + + /// + /// Field-level diff, computed by comparing the incoming draft with the merged one -- never taken + /// from the model's own account of what it did. A model that says it added one assertion while + /// actually rewriting every turn has to be caught by something that does not ask it. + /// + public List Changes { get; set; } = []; + + /// + /// What says about the merged + /// draft. Non-empty means the draft cannot be saved as it stands, and the client shows it as + /// such rather than letting the user discover it on the save button. + /// + public List ValidationErrors { get; set; } = []; + + /// + /// Things that were silently wrong and got corrected, or are suspect and were left alone: a mock + /// dropped for naming a function this agent cannot call, a state key nothing else in the suite + /// has ever used, an llmJudge assertion on a suite with no judge model. + /// + public List Warnings { get; set; } = []; +} + +/// One changed field of a draft. +public class AuthorChange +{ + /// The draft field name, as the client knows it (camelCase, e.g. "turns"). + public string Field { get; set; } = string.Empty; + + /// Short human summary of the change, e.g. "3 turns -> 4 turns". + public string Detail { get; set; } = string.Empty; +} + +/// +/// The draft fields an authoring model is allowed to change. +/// +/// A whitelist, and the merge copies everything else straight off the incoming draft, so a model +/// that omits a field from its answer cannot delete it -- the failure mode of returning a whole +/// document is silent data loss, and this is what removes it. The visible cost is that a model which +/// forgets to declare a field it meant to change makes no change at all, which the user sees and can +/// simply ask for again. +/// +/// Four writable case fields are deliberately absent: +/// suiteId -- comes from the request, not from a model; +/// unmockedToolPolicy -- only Block is supported, and a model proposing Passthrough is exactly the +/// thing validation exists to stop; +/// sourceConversationId -- a provenance record, not authoring; +/// lastReviewedDate -- a human attestation that the case still reflects reality, which is worth +/// nothing if a model can stamp it. +/// +public static class AuthorFields +{ + public const string Name = "name"; + public const string Enabled = "enabled"; + public const string CaseType = "caseType"; + public const string EntryAgentId = "entryAgentId"; + public const string Turns = "turns"; + public const string Assertions = "assertions"; + public const string InitialStates = "initialStates"; + public const string History = "history"; + public const string Mocks = "mocks"; + public const string Priority = "priority"; + public const string Severity = "severity"; + public const string Batch = "batch"; + public const string CrossCutting = "crossCutting"; + public const string InvolvedAgents = "involvedAgents"; + public const string BusinessDomain = "businessDomain"; + public const string ExpectedOutcome = "expectedOutcome"; + + public static readonly string[] All = + [ + Name, Enabled, CaseType, EntryAgentId, Turns, Assertions, InitialStates, History, Mocks, + Priority, Severity, Batch, CrossCutting, InvolvedAgents, BusinessDomain, ExpectedOutcome + ]; + + public static string? Normalize(string? value) + => All.FirstOrDefault(f => string.Equals(f, value?.Trim(), StringComparison.OrdinalIgnoreCase)); +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCase.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCase.cs index 1bc7db5cb..7cfabd6da 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCase.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCase.cs @@ -8,6 +8,30 @@ public class AgentTestCase : MongoBase public string Name { get; set; } = default!; public bool Enabled { get; set; } = true; + /// + /// See . Defaults to Agent, which is also what every case stored before + /// this field existed deserialises to -- a missing BSON element leaves the property at this + /// initialiser, so old documents read back as Agent cases rather than as an invalid blank. + /// + /// Not cosmetic: a Routing case is validated differently (single turn, must actually assert a + /// routing outcome, no llmJudge -- see CaseValidation.Validate) and is the only + /// type counted towards a run's routing accuracy. + /// + public string CaseType { get; set; } = CaseTypes.Agent; + + /// + /// The agent the conversation is opened on, overriding the suite's own AgentId. Null uses the + /// suite's. + /// + /// This is what makes Routing and Agent cases separable without one suite per entry point. + /// BotSharp dispatches on the agent's own type: ConversationService.SendMessage sends a Routing + /// agent through RoutingService.InstructLoop (the router runs, and can hand off) and everything + /// else through InstructDirect (straight into that agent, router never consulted). So pointing a + /// case at the Copilot entry agent tests routing, and pointing it at a leaf agent tests that + /// agent in isolation -- the two modes the evaluation framework calls Routing and Agent cases. + /// + public string? EntryAgentId { get; set; } + /// A length of 1 is a single-turn case. public List Turns { get; set; } = []; @@ -17,6 +41,22 @@ public class AgentTestCase : MongoBase /// Injected before the conversation starts; maps to BotSharp's MessageState. public List InitialStates { get; set; } = []; + /// + /// Prior conversation turns written into the conversation before the case's own turns run, so a + /// real question-and-answer exchange can be replayed as the fixed starting context for a test. + /// + /// Different from : states are the machine-readable context an agent + /// reads, this is the dialogue the model sees. It is what makes "given the resident already told + /// us the fridge is leaking, does asking for an ETA still route correctly" expressible without + /// re-driving those turns through the model on every run -- which would be slow, would cost + /// tokens, and would make the preamble itself a source of flakiness. + /// + /// Never counted in a result's AgentChain: these messages are authored, not something the agent + /// under test did, and letting them in would fail an exact chain assertion for a reason the + /// author did not cause. + /// + public List History { get; set; } = []; + public List Mocks { get; set; } = []; /// @@ -28,6 +68,68 @@ public class AgentTestCase : MongoBase /// The conversation this was recorded from, for traceability; null when hand-written. public string? SourceConversationId { get; set; } + /// + /// See . Decides which batch the case runs in, and therefore whether + /// a failure stops the evaluation (batch 1) or is merely reported (batch 3). + /// + /// Defaults to P1, which is what every case stored before this field existed reads back as. P0 + /// would put all of them in the stop-loss batch, where one failure halts everything; P2 would + /// drop them out of the mandatory batches altogether. P1 is mandatory but not stop-loss, which is + /// the honest position for a case nobody has triaged yet. + /// + public string Priority { get; set; } = CasePriorities.P1; + + /// + /// See . What a failure of this case means, as opposed to how urgent + /// it is to run. + /// + /// Defaults to S1 for the same reason as Priority: S0 would make every untriaged legacy failure + /// an immediate no-go, and S2 would let a genuine safety failure hide as an experience nit. + /// + public string Severity { get; set; } = CaseSeverities.S1; + + /// + /// Overrides the batch derived from and ; null + /// uses the derivation. See . + /// + public int? Batch { get; set; } + + /// + /// A cross-cutting case runs in EVERY evaluation scope, whatever the change was. That is what + /// safety cases are: the scope-narrowing rules exist to save time on cases a change cannot + /// affect, and a claim that a change cannot affect safety is exactly the claim not to take on + /// trust. + /// + public bool CrossCutting { get; set; } + + /// + /// Agents this case actually exercises, as ids. Empty is the normal state and does NOT mean "no + /// agents": the effective set then falls back to the case's entry agent, which is already known. + /// See . + /// + /// Worth filling in for a routing case, where the entry agent is the router and the agents that + /// matter are the ones downstream of it -- those cannot be inferred from the case definition, + /// only observed by running it. + /// + public List InvolvedAgents { get; set; } = []; + + /// Business domain, for pulling a subset by business line rather than by agent. + public string? BusinessDomain { get; set; } + + /// + /// What this case is supposed to achieve, in business terms, for whoever reviews the result. + /// Deliberately free text and never evaluated: an expected outcome a machine could check is an + /// assertion, and belongs in where it will actually be enforced. + /// + public string? ExpectedOutcome { get; set; } + + /// + /// When a human last confirmed this case still reflects reality. Null means never. Not touched by + /// editing or running the case -- a case can be edited many times and still be built on an + /// assumption nobody has questioned in a year, and conflating the two would hide exactly that. + /// + public DateTime? LastReviewedDate { get; set; } + public DateTime CreateDate { get; set; } = DateTime.UtcNow; public DateTime UpdateDate { get; set; } = DateTime.UtcNow; } @@ -100,11 +202,141 @@ public class TestAssertion public bool Fatal { get; set; } } +/// +/// One authored message in a case's . Deliberately just a role +/// and text: a mocked tool call belongs in , and a fabricated +/// function-call dialog would let a case claim a tool ran when nothing did. +/// +[BsonIgnoreExtraElements(Inherited = true)] +public class TestHistoryMessage +{ + /// See . + public string Role { get; set; } = HistoryRoles.User; + + public string Content { get; set; } = string.Empty; +} + +/// +/// Roles an authored history message may take. Only these two: a system message would compete with +/// the agent's own instruction, and a function message would fake a tool call. +/// +public static class HistoryRoles +{ + public const string User = "user"; + public const string Assistant = "assistant"; + + public static readonly string[] All = [User, Assistant]; + + /// Canonical role for any casing; null for anything unsupported, so it is rejected. + public static string? Normalize(string? value) + => All.FirstOrDefault(r => string.Equals(r, value?.Trim(), StringComparison.OrdinalIgnoreCase)); +} + public static class UnmockedToolPolicies { public const string Block = "Block"; } +/// +/// How urgent it is to run a case, which is what decides its batch. Distinct from +/// : priority is about scheduling, severity is about consequence. +/// +public static class CasePriorities +{ + public const string P0 = "P0"; + public const string P1 = "P1"; + public const string P2 = "P2"; + + public static readonly string[] All = [P0, P1, P2]; + + public static string? Normalize(string? value) + => All.FirstOrDefault(p => string.Equals(p, value?.Trim(), StringComparison.OrdinalIgnoreCase)); +} + +/// +/// What a failure of this case means. +/// +/// S0 -- zero tolerance. Data leakage, an unauthorised action taken without confirmation, a missed +/// critical escalation. One of these is a stop, not a statistic. +/// S1 -- non-inferiority. Wrong routing, wrong tool, a fabricated fact, a wrong business state: +/// allowed to move within a threshold, not allowed to get worse than it. +/// S2 -- experience quality. Phrasing, repetition, awkward hand-offs. Must never be able to mask an +/// S0 or S1 result. +/// +public static class CaseSeverities +{ + public const string S0 = "S0"; + public const string S1 = "S1"; + public const string S2 = "S2"; + + public static readonly string[] All = [S0, S1, S2]; + + public static string? Normalize(string? value) + => All.FirstOrDefault(s => string.Equals(s, value?.Trim(), StringComparison.OrdinalIgnoreCase)); +} + +/// +/// Which batch a case belongs to. Batches run in order and exist to stop early: batch 1 is the +/// stop-loss batch, batch 3 does not block a release decision. +/// +public static class CaseBatches +{ + public const int StopLoss = 1; + public const int Mandatory = 2; + public const int Optional = 3; + + public static readonly int[] All = [StopLoss, Mandatory, Optional]; + + /// + /// An explicit wins. Otherwise a cross-cutting case is batch 1 + /// whatever its priority -- a safety case that only runs after everything else has passed cannot + /// stop anything -- and priority maps P0/P1/P2 onto 1/2/3. + /// + public static int Effective(AgentTestCase testCase) + { + if (testCase.Batch is { } explicitBatch && All.Contains(explicitBatch)) + { + return explicitBatch; + } + + if (testCase.CrossCutting) + { + return StopLoss; + } + + return testCase.Priority switch + { + CasePriorities.P0 => StopLoss, + CasePriorities.P2 => Optional, + _ => Mandatory + }; + } +} + +/// +/// What a case is verifying, which decides how it is validated and how it is aggregated. +/// +/// Routing -- single turn from the entry agent, asserting only which agent took the conversation. +/// Carries no quality judgement: the whole verdict is "expected agent == actual agent". +/// Agent -- one agent's own behaviour, normally entered directly so the router is not part of what +/// is being measured. Multi-agent journeys are Agent cases too; the agentChain assertion +/// is what describes their hand-offs. +/// +public static class CaseTypes +{ + public const string Routing = "Routing"; + public const string Agent = "Agent"; + + public static readonly string[] All = [Routing, Agent]; + + /// + /// Maps any casing of a known type onto its canonical constant; null for anything unknown, so + /// the caller can reject it rather than storing a value nothing else will ever match. + /// + public static string? Normalize(string? value) + => All.FirstOrDefault(t => string.Equals(t, value?.Trim(), StringComparison.OrdinalIgnoreCase)); +} + public static class AgentTestStatus { public const string Pending = "Pending"; diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCaseResult.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCaseResult.cs index bea9793ed..2b04b3684 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCaseResult.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestCaseResult.cs @@ -11,6 +11,14 @@ public class AgentTestCaseResult : MongoBase /// Passed | Failed | Error | Cancelled -- see . public string Status { get; set; } = AgentTestStatus.Pending; + /// + /// Copied off the case so a result is self-describing -- see . Aggregating + /// by type (routing accuracy separately from agent pass rate, as the evaluation framework gates + /// them separately) has to work from the result rows alone, without re-reading cases that may + /// have been edited or deleted since the run. + /// + public string CaseType { get; set; } = CaseTypes.Agent; + /// The conversation this execution created; live conversations are never reused. public string? ConversationId { get; set; } @@ -23,8 +31,37 @@ public class AgentTestCaseResult : MongoBase public string? Provider { get; set; } public string? Model { 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 + /// model-latency measurement -- see for that. + /// public long DurationMs { get; set; } + /// + /// Time spent inside the agent calls alone, summed over the turns. This is what a latency gate + /// should be read against: also contains harness work, and on a fast + /// case that overhead is a large enough share to move a percentile. + /// + public long ModelDurationMs { get; set; } + + /// + /// Tokens this case consumed, measured as the delta across its own execution rather than an + /// absolute reading, so it stays correct even when the statistics service outlives one case. + /// + /// Total only: the input/output split lives in TokenStatistics' private fields and is not + /// reachable through ITokenStatistics, which exposes Total, Cost and AccumulatedCost. Recording a + /// guessed split would be worse than recording none. + /// + public long TotalTokens { get; set; } + + /// + /// What those tokens cost, priced by the model's own configured unit costs. Comparable only + /// within a run, or across runs whose pricing snapshot matches -- see + /// . + /// + public double Cost { get; set; } + /// /// Infrastructure-level reason for failure (a timeout, a dead canary), kept distinct from an /// assertion failure. @@ -38,6 +75,19 @@ public class AgentTestCaseResult : MongoBase public List ObservedToolCalls { get; set; } = []; + /// + /// Every agent that produced an assistant message over the whole case, in order, with + /// consecutive repeats collapsed -- so ["Copilot", "WorkOrder"] means the entry agent answered + /// and then handed off once, however many messages each of them emitted. + /// + /// This is the only record of the hand-offs. route_to_agent is on the allow list + /// (AgentTestRunRegistry) and therefore never reaches MockFunctionExecutor, which is the only + /// caller of ActiveTestRun.Record -- so routing decisions produce no ObservedToolCall and would + /// otherwise be invisible. Reconstructed instead from the conversation's own assistant dialogs, + /// each of which carries the agent that wrote it. + /// + public List AgentChain { get; set; } = []; + public DateTime CreateDate { get; set; } = DateTime.UtcNow; } @@ -48,6 +98,21 @@ public class TurnResult public string UserMessage { get; set; } = default!; public string? Output { get; set; } public List Assertions { get; set; } = []; + + /// + /// Time the agent call for this turn took. Excludes the assertion evaluation and the conversation + /// reads that follow it, so summing this over the turns gives a latency figure that does not + /// drift as the harness itself gains work. + /// + public long ModelDurationMs { get; set; } + + /// + /// The agents that answered during THIS turn only, in order, consecutive repeats collapsed. The + /// case-level is the whole conversation; this is + /// the slice added by this turn, which is what makes "the second turn should have stayed with + /// the same agent" expressible. + /// + public List AgentChain { get; set; } = []; } [BsonIgnoreExtraElements(Inherited = true)] @@ -59,6 +124,13 @@ public class AssertionResult public string? Actual { get; set; } public bool Passed { get; set; } public string? Message { get; set; } + + /// + /// The numeric score, on llmJudge results only; null everywhere else. Actual already carries it + /// as text for display, but a quality gate has to average scores across a run, and re-parsing a + /// display string to do arithmetic is how a formatting change quietly breaks a gate. + /// + public double? Score { get; set; } } [BsonIgnoreExtraElements(Inherited = true)] diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestDtos.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestDtos.cs index e8b6c2b9a..ddac1975f 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestDtos.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestDtos.cs @@ -37,12 +37,122 @@ public class AgentTestCaseUpsertRequest public string SuiteId { get; set; } = string.Empty; public string Name { get; set; } = string.Empty; public bool Enabled { get; set; } = true; + + /// + /// See . Blank or omitted means Agent, so an existing client that never + /// sends this field keeps creating exactly the cases it created before. Any other unrecognised + /// value is a 400 rather than a silent fallback -- storing "Rounting" as Agent would leave the + /// author with a case that looks routing-shaped and is never counted as one. + /// + public string? CaseType { get; set; } + + /// + /// Authored prior turns, replayed as the case's opening context. See + /// . + /// + public List History { get; set; } = []; + + /// + /// Optional entry agent, overriding the suite's. See + /// for why this is the switch between testing routing + /// and testing one agent in isolation. Validated to exist at save time: a typo here would + /// otherwise turn every run of the case into an opaque infrastructure Error. + /// + public string? EntryAgentId { get; set; } + public List Turns { get; set; } = []; public List Assertions { get; set; } = []; public List InitialStates { get; set; } = []; public List Mocks { get; set; } = []; public string UnmockedToolPolicy { get; set; } = UnmockedToolPolicies.Block; public string? SourceConversationId { get; set; } + + /// See . Blank or omitted keeps P1. + public string? Priority { get; set; } + + /// See . Blank or omitted keeps S1. + public string? Severity { get; set; } + + /// Explicit batch override; null derives it from priority and the cross-cutting flag. + public int? Batch { get; set; } + + public bool CrossCutting { get; set; } + + /// Agent ids; empty falls back to the case's entry agent. See . + public List InvolvedAgents { get; set; } = []; + + public string? BusinessDomain { get; set; } + public string? ExpectedOutcome { get; set; } + + /// + /// Sent explicitly by whoever reviewed the case. Never set by the server on save: a case can be + /// edited many times and still rest on an assumption nobody has questioned, and stamping this on + /// every write would hide precisely that. + /// + public DateTime? LastReviewedDate { get; set; } +} + +/// +/// Body of POST /agent-test/scope -- work out which cases a change needs to run, before running +/// anything. See . +/// +public class ScopeSelectionRequest +{ + /// Agent ids the change touches. Ignored when is set. + public List TargetAgentIds { get; set; } = []; + + /// A platform-wide change: every enabled case is in scope. + public bool FullPlatform { get; set; } + + /// Narrow to one batch (1, 2 or 3); null covers all of them. + public int? Batch { get; set; } +} + +/// +/// One case's place in a scope. Carries the metadata the decision was made from, not just the verdict, +/// so the scope can be reviewed rather than taken on trust. +/// +public class ScopedCaseDto +{ + public string CaseId { get; set; } = string.Empty; + public string CaseName { get; set; } = string.Empty; + public string SuiteId { get; set; } = string.Empty; + public string SuiteName { get; set; } = string.Empty; + public string CaseType { get; set; } = string.Empty; + public string Priority { get; set; } = string.Empty; + public string Severity { get; set; } = string.Empty; + public bool CrossCutting { get; set; } + public bool Enabled { get; set; } + + /// The effective batch, after the priority and cross-cutting derivation. + public int Batch { get; set; } + + /// The set the decision was made against, authored or derived from the entry agent. + public List InvolvedAgentIds { get; set; } = []; + + /// See . + public string Reason { get; set; } = string.Empty; +} + +/// +/// The answer to "what will this change actually test". +/// +/// Both lists are returned on purpose. A scope report that only showed what it included would let a +/// change be signed off against a set of cases that quietly left out the interesting one, and an +/// excluded case produces no result to notice -- which is why the exclusions, with their reasons, are +/// the half worth reading. +/// +public class ScopeSelectionResponse +{ + public List TargetAgentIds { get; set; } = []; + public bool FullPlatform { get; set; } + public int? Batch { get; set; } + + /// Every registered case, whatever its state -- the denominator for coverage. + public int TotalCases { get; set; } + + public List Included { get; set; } = []; + public List Excluded { get; set; } = []; } /// @@ -103,6 +213,39 @@ public class AgentTestRunTriggerRequest public List? Models { get; set; } } +/// +/// Body of POST /agent-test/runs/delete -- clear run history. +/// +public class AgentTestRunDeleteRequest +{ + public List RunIds { get; set; } = []; +} + +/// +/// What a delete actually did. +/// +/// Reports the skipped runs rather than failing the whole call: a bulk delete over a list that +/// happens to include a still-running row should remove the rest, and the caller needs to be able to +/// say WHY one survived instead of leaving the user to notice a row that quietly stayed. +/// +public class AgentTestRunDeleteResponse +{ + public List DeletedRunIds { get; set; } = []; + + /// Case results removed along with those runs. + public long DeletedResultCount { get; set; } + + public List Skipped { get; set; } = []; +} + +public class SkippedRunDto +{ + public string RunId { get; set; } = string.Empty; + + /// Caller-facing text, already explaining what to do about it. + public string Reason { get; set; } = string.Empty; +} + /// /// Body of GET /agent-test/runs/{id}: one run plus every AgentTestCaseResult belonging to it. /// diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestRun.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestRun.cs index 31b3f6af5..2111649d4 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestRun.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Models/AgentTestRun.cs @@ -46,6 +46,37 @@ public class AgentTestRun : MongoBase public int FailedCount { get; set; } public int ErrorCount { get; set; } + /// + /// Routing accuracy for this run, one row per model swept, counting only cases whose CaseType is + /// Routing. + /// + /// Kept per model rather than as one run-wide number because that is the whole point of a + /// comparison run: a single figure covering every model averages the candidate together with the + /// baseline and hides exactly the difference the run exists to measure. + /// + /// A summary of the case results, not a second source of truth -- the AgentTestCaseResult rows + /// remain authoritative and carry CaseType themselves. It lives here so the run LIST can show + /// the figure without loading every result of every run. + /// + public List RoutingAccuracies { get; set; } = []; + + /// + /// Latency, token and cost figures for this run, one row per model swept. Computed once when the + /// run finishes rather than accumulated per case, because a percentile cannot be updated + /// incrementally -- it needs every value at once. + /// + public List PerformanceSummaries { get; set; } = []; + + /// + /// The unit costs actually in force for each model when this run executed. + /// + /// A cost figure is meaningless without them: a provider price change makes this run's cost + /// incomparable with an earlier one's, and a run that only recorded a version STRING would leave + /// nobody able to check whether two versions differ. Snapshotting the numbers makes that + /// checkable instead of a matter of trust. + /// + public List ModelPricing { get; set; } = []; + /// /// Why a run ended as -- an infrastructure stop that /// happened before or instead of executing cases (suite gone, suite disabled, the CaseIds @@ -63,3 +94,69 @@ public class AgentTestRun : MongoBase public DateTime? CompletedAt { get; set; } public DateTime CreateDate { get; set; } = DateTime.UtcNow; } + +/// +/// Latency, tokens and cost for one model within a run. Sums and percentiles are stored; averages are +/// not, because an average is Total/CaseCount and a stored copy is one more thing that can disagree +/// with the rows it came from. +/// +[BsonIgnoreExtraElements(Inherited = true)] +public class PerformanceSummary +{ + /// Null for both when the run swept no models and used each agent's own LlmConfig. + public string? Provider { get; set; } + public string? Model { get; set; } + + /// + /// Results this row covers. Only cases that actually executed: an Error case that never reached + /// the model would drag a latency percentile towards zero and make a broken run look fast. + /// + public int CaseCount { get; set; } + + /// Median and 95th percentile of AgentTestCaseResult.ModelDurationMs. + public long LatencyP50Ms { get; set; } + public long LatencyP95Ms { get; set; } + + public long TotalTokens { get; set; } + public double TotalCost { get; set; } +} + +/// +/// One model's configured unit costs at the moment a run executed. Text tokens only -- the harness +/// drives text conversations, and carrying audio and image tiers that are always zero here would +/// suggest they had been checked. +/// +[BsonIgnoreExtraElements(Inherited = true)] +public class ModelPricingSnapshot +{ + public string? Provider { get; set; } + public string? Model { get; set; } + + /// Null when the model's settings could not be read, which is itself worth recording. + public float? TextInputCost { get; set; } + public float? TextOutputCost { get; set; } +} + +/// +/// How many Routing cases one model got right in a run. Stored as counts, never as a percentage: +/// a stored ratio would go stale the moment another case result arrives, and "3/4" says something +/// "75%" does not -- how much the figure is worth trusting. +/// +[BsonIgnoreExtraElements(Inherited = true)] +public class RoutingAccuracy +{ + /// Null for both when the run swept no models and used each agent's own LlmConfig. + public string? Provider { get; set; } + public string? Model { get; set; } + + /// Routing cases executed under this model, whatever their outcome. + public int CaseCount { get; set; } + + /// + /// Of those, how many passed. A routing case passes only when its routing assertions held, so + /// Passed here means the conversation reached the expected agent. Error rows (a timeout, a dead + /// canary) count towards CaseCount but never towards PassedCount -- treating "could not tell" + /// as correct is how a broken harness starts reporting perfect accuracy. + /// + public int PassedCount { get; set; } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Repositories/AgentTestRepository.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Repositories/AgentTestRepository.cs index d2dcbeb88..4773ebde9 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Repositories/AgentTestRepository.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Repositories/AgentTestRepository.cs @@ -34,6 +34,16 @@ public interface IAgentTestRepository Task UpdateRunAsync(AgentTestRun run); + /// + /// Removes a run and every case result belonging to it, and reports how many results went with + /// it. + /// + /// The cascade is the point. Case results are keyed by run id and are reachable no other way, so + /// deleting the run alone would leave rows nothing can ever list, read or clean up again -- and + /// every later aggregate that scans results would keep counting them. + /// + Task DeleteRunAsync(string id); + Task AddCaseResultAsync(AgentTestCaseResult result); Task> ListCaseResultsAsync(string runId); } @@ -149,6 +159,16 @@ public async Task> ListRunsByStatusAsync(string status) public async Task UpdateRunAsync(AgentTestRun run) => await _mongoDbContext.AgentTestRuns.ReplaceOneAsync(x => x.Id == run.Id, run); + public async Task DeleteRunAsync(string id) + { + // Results first. If the process dies between the two deletes, an orphaned RUN is visible and + // deletable again; orphaned RESULTS are not reachable at all once their run is gone. Losing + // the recoverable half is the better failure. + var results = await _mongoDbContext.AgentTestCaseResults.DeleteManyAsync(x => x.RunId == id); + await _mongoDbContext.AgentTestRuns.DeleteOneAsync(x => x.Id == id); + return results.DeletedCount; + } + public async Task AddCaseResultAsync(AgentTestCaseResult result) { if (string.IsNullOrEmpty(result.Id)) diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestSyntheticConversationProbe.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestSyntheticConversationProbe.cs new file mode 100644 index 000000000..3280f4576 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Runtime/AgentTestSyntheticConversationProbe.cs @@ -0,0 +1,31 @@ +using BotSharp.Abstraction.Conversations; + +namespace BotSharp.Plugin.AgentTesting.Runtime; + +/// +/// Tells BotSharp which conversations belong to a test run, so its per-user volume limits can stand +/// aside for them. +/// +/// Answers from -- the same registry that decides whether a +/// function call is intercepted -- rather than from the conversation's "test-set" tag. The tag is +/// written only once the conversation row exists, which is after the first message has already passed +/// through the rate limit hook, so a tag-based answer would still block the first turn of every case. +/// The registry entry is created before the conversation is opened at all. +/// +/// It is also the same source of truth as the mock seam, which matters: a conversation the harness +/// mocks tools for and a conversation the harness is exempt from rate limiting are, by construction, +/// the same set. Two independent notions of "is this a test" could disagree, and the disagreement +/// would show up as either real traffic escaping the limits or tests being blocked by them. +/// +public class AgentTestSyntheticConversationProbe : ISyntheticConversationProbe +{ + private readonly IAgentTestRunRegistry _registry; + + public AgentTestSyntheticConversationProbe(IAgentTestRunRegistry registry) + { + _registry = registry; + } + + public bool IsSynthetic(string conversationId) + => !string.IsNullOrEmpty(conversationId) && _registry.TryGet(conversationId) != null; +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestCaseRunner.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestCaseRunner.cs index bbe3d7070..ad36f2641 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestCaseRunner.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestCaseRunner.cs @@ -7,16 +7,30 @@ public class AgentTestCaseRunner : ICaseRunner { private readonly IAgentTestRunRegistry _registry; private readonly IAgentConversationDriver _driver; + private readonly IAgentTestJudge? _judge; + private readonly ITokenStatistics? _tokens; private readonly ILogger _logger; public AgentTestCaseRunner( IAgentTestRunRegistry registry, IAgentConversationDriver driver, - ILogger logger) + ILogger logger, + IAgentTestJudge? judge = null, + ITokenStatistics? tokens = null) { _registry = registry; _driver = driver; _logger = logger; + // Optional so the orchestration tests can build a runner without one. ITokenStatistics is + // registered scoped, and AgentTestRunQueue.ScopedCaseRunner opens a fresh scope per case, so + // in production this is the same instance the conversation's completion provider reports + // into -- which is what makes the delta below attributable to this case alone. + _tokens = tokens; + // Optional so the orchestration tests can construct a runner without a vendor. A null judge + // is not a silent skip: EvaluateAsync turns an llmJudge assertion into the same Error as an + // unreachable vendor, because a case whose quality assertion was never scored has an unknown + // verdict, not a passing one. + _judge = judge; } public async Task RunAsync( @@ -37,7 +51,11 @@ public async Task RunAsync( // timeout) still say which model they were meant to run under -- a result that cannot // be attributed to a model is useless in a comparison run. Provider = model?.Provider, - Model = model?.Model + Model = model?.Model, + // Stamped up front like Provider/Model: aggregating routing accuracy separately from + // agent pass rate has to work off the result rows alone, including the rows produced by + // the early returns below. + CaseType = testCase.CaseType }; // Turns.SelectMany(...).Concat(caseAssertions).All(a => a.Passed) is vacuously true on an @@ -51,6 +69,15 @@ public async Task RunAsync( return result; } + // Which agent the conversation opens on. BotSharp dispatches on that agent's own type + // (ConversationService.SendMessage: a Routing agent goes through RoutingService.InstructLoop + // and can hand off, everything else through InstructDirect and cannot), so this one value is + // what decides whether the router is part of what the case measures. The suite's agent stays + // the default, so every case authored before EntryAgentId existed behaves exactly as before. + var entryAgentId = string.IsNullOrWhiteSpace(testCase.EntryAgentId) + ? suite.AgentId + : testCase.EntryAgentId!; + var active = new ActiveTestRun { ConversationId = conversationId, @@ -63,6 +90,14 @@ public async Task RunAsync( }; var stopwatch = Stopwatch.StartNew(); + + // Read as a delta, not an absolute: a scope that outlived an earlier case would otherwise + // have this case billed for that one's tokens too. On a fresh scope the baseline is zero and + // the delta is simply the final reading. + var tokensBefore = _tokens?.Total ?? 0; + var costBefore = _tokens?.AccumulatedCost ?? 0f; + long modelDurationMs = 0; + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(ct); timeout.CancelAfter(TimeSpan.FromSeconds(Math.Max(1, suite.CaseTimeoutSeconds))); @@ -118,7 +153,31 @@ async Task AwaitOrHandOffAsync(Task driverTask) _registry.Register(active); try { - await _driver.PrepareAsync(conversationId, suite.AgentId, testCase.InitialStates); + await _driver.PrepareAsync(conversationId, entryAgentId, testCase.InitialStates); + + // Authored history goes in before the canary and before any turn, so the model sees it + // as the conversation's opening context. A short count means the write silently did + // nothing (see IAgentConversationDriver.InjectHistoryAsync) -- that has to be an Error, + // because a case running without the context it was written around would otherwise + // report an ordinary pass or fail about a scenario that never existed. + var injectedHistory = await AwaitOrHandOffAsync( + _driver.InjectHistoryAsync(conversationId, entryAgentId, testCase.History)); + + if (injectedHistory != testCase.History.Count) + { + result.Status = AgentTestStatus.Error; + result.Error = $"only {injectedHistory} of {testCase.History.Count} history messages " + + "could be written to the conversation, so this case would have run " + + "without the context it was written around"; + return result; + } + + // Authored history is not something the agent under test did, so it must not appear in + // the chain. Read once here and used as the offset for both the per-turn slices and the + // case-level chain, so the exclusion cannot drift between them. + var historyAssistantMessages = testCase.History.Count == 0 + ? 0 + : (await _driver.ReadAssistantAgentSequenceAsync(conversationId)).Count; // Prove the seam is live first. A dead seam means mocking silently does nothing and // real tools execute, so this has to happen before a single user message is sent. @@ -129,7 +188,7 @@ async Task AwaitOrHandOffAsync(Task driverTask) // active.CanaryIntercepted would look stricter but checks the same fact twice, and it // would stop a fake driver from unit-testing the orchestration at all -- a fake driver // has no ActiveTestRun and can never set that flag. - if (!await AwaitOrHandOffAsync(_driver.RunCanaryAsync(conversationId, suite.AgentId, timeout.Token))) + if (!await AwaitOrHandOffAsync(_driver.RunCanaryAsync(conversationId, entryAgentId, timeout.Token))) { result.Status = AgentTestStatus.Error; result.Error = "the mock seam is not live: IFunctionExecutorProvider was not consulted. " @@ -138,33 +197,57 @@ async Task AwaitOrHandOffAsync(Task driverTask) return result; } + // How many assistant messages the chain has already accounted for. The driver hands + // back the whole conversation on every read -- that is what the dialog store holds -- + // and only the runner knows where each turn started, so the per-turn slice is taken + // here. + var consumedAssistantMessages = historyAssistantMessages; + var fatalStop = false; foreach (var turn in testCase.Turns.OrderBy(t => t.Index)) { if (fatalStop) break; active.CurrentTurnIndex = turn.Index; + // Timed around the agent call ONLY. The case's own DurationMs also covers the + // canary, the mock lookups and the conversation reads, and on a fast case that + // overhead is a big enough share to move a latency percentile. + var turnTimer = Stopwatch.StartNew(); var output = await AwaitOrHandOffAsync( - _driver.SendAsync(conversationId, suite.AgentId, turn.UserMessage, timeout.Token)); + _driver.SendAsync(conversationId, entryAgentId, turn.UserMessage, timeout.Token)); + turnTimer.Stop(); + modelDurationMs += turnTimer.ElapsedMilliseconds; + + var agentSequence = await _driver.ReadAssistantAgentSequenceAsync(conversationId); + var turnChain = CollapseConsecutiveRepeats(agentSequence.Skip(consumedAssistantMessages)); + consumedAssistantMessages = agentSequence.Count; var turnResult = new TurnResult { Index = turn.Index, UserMessage = turn.UserMessage, - Output = output + Output = output, + ModelDurationMs = turnTimer.ElapsedMilliseconds, + // Names, not hops: a result is read by a person, and the ids are only needed + // while an assertion is being evaluated. + AgentChain = turnChain.Select(hop => hop.Name).ToList() }; + // This turn's slice, not the whole conversation: routedToAgent reads the chain's + // last entry, and a turn-level context carrying the conversation's chain would let a + // turn that produced no answer at all inherit the previous turn's agent and pass an + // assertion about routing that never happened. var turnContext = new AssertionContext { Output = output, ToolCalls = active.ObservedCalls.Where(c => c.TurnIndex == turn.Index).ToList(), States = await _driver.ReadStatesAsync(conversationId), - RoutedToAgent = await _driver.ReadRoutedAgentNameAsync(conversationId) + AgentChain = turnChain }; foreach (var assertion in turn.Assertions) { - var evaluated = AssertionEvaluator.Evaluate(assertion, turnContext); + var evaluated = await EvaluateAsync(assertion, turnContext, suite, timeout.Token); turnResult.Assertions.Add(evaluated); if (!evaluated.Passed && assertion.Fatal) { @@ -175,17 +258,21 @@ async Task AwaitOrHandOffAsync(Task driverTask) result.Turns.Add(turnResult); } + var caseChain = CollapseConsecutiveRepeats( + (await _driver.ReadAssistantAgentSequenceAsync(conversationId)).Skip(historyAssistantMessages)); + result.AgentChain = caseChain.Select(hop => hop.Name).ToList(); + var finalContext = new AssertionContext { Output = result.Turns.LastOrDefault()?.Output, ToolCalls = active.ObservedCalls, States = await _driver.ReadStatesAsync(conversationId), - RoutedToAgent = await _driver.ReadRoutedAgentNameAsync(conversationId) + AgentChain = caseChain }; foreach (var assertion in testCase.Assertions) { - result.Assertions.Add(AssertionEvaluator.Evaluate(assertion, finalContext)); + result.Assertions.Add(await EvaluateAsync(assertion, finalContext, suite, timeout.Token)); } result.ObservedToolCalls = active.ObservedCalls.ToList(); @@ -215,6 +302,18 @@ async Task AwaitOrHandOffAsync(Task driverTask) result.Status = AgentTestStatus.Cancelled; result.ObservedToolCalls = active.ObservedCalls.ToList(); } + catch (AgentTestJudgeUnavailableException ex) + { + // The judge never reached a verdict. That is Error, not Failed: a vendor timeout or an + // unconfigured judge model says nothing about the agent under test, and reporting it as + // a failing assertion would make provider noise indistinguishable from an agent + // regression. Logged at warning, not error -- this is a configuration or vendor + // condition, not a crash in the harness. + _logger.LogWarning(ex, "Agent test case {CaseId} could not be judged.", testCase.Id); + result.Status = AgentTestStatus.Error; + result.Error = ex.Message; + result.ObservedToolCalls = active.ObservedCalls.ToList(); + } catch (Exception ex) { // Reaches here for any OperationCanceledException that was neither our own timeout nor @@ -238,11 +337,82 @@ async Task AwaitOrHandOffAsync(Task driverTask) } stopwatch.Stop(); result.DurationMs = stopwatch.ElapsedMilliseconds; + result.ModelDurationMs = modelDurationMs; + + // 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. + result.TotalTokens = Math.Max(0, (_tokens?.Total ?? 0) - tokensBefore); + result.Cost = Math.Max(0, (_tokens?.AccumulatedCost ?? 0f) - costBefore); + + if (result.TotalTokens == 0 && result.Turns.Count > 0 && _tokens != null) + { + // Every turn calls the model, so zero tokens across completed turns means this + // runner's ITokenStatistics is not the instance the completion provider reported + // into. Logged rather than failed: usage accounting being wrong says nothing about + // whether the agent behaved, and failing the case would report a metering problem as + // an agent regression. + _logger.LogWarning( + "Agent test case {CaseId} completed {TurnCount} turn(s) but measured zero tokens; " + + "token and cost figures for this run are not trustworthy.", + testCase.Id, result.Turns.Count); + } } return result; } + /// + /// Consecutive duplicates removed, so ["A", "A", "B", "A"] becomes ["A", "B", "A"]. Only + /// consecutive ones: a conversation that really went A -> B -> A did visit A twice, and + /// flattening that to ["A", "B"] would hide the return hop from an ordered agentChain assertion. + /// One agent emitting several messages in a row is not a hand-off and does collapse. + /// + private static List CollapseConsecutiveRepeats(IEnumerable hops) + { + var chain = new List(); + foreach (var hop in hops) + { + // Compared by id, not name: two agents can share a display name, and collapsing those + // together would hide a real hand-off. + if (chain.Count == 0 || !string.Equals(chain[^1].Id, hop.Id, StringComparison.OrdinalIgnoreCase)) + { + chain.Add(hop); + } + } + + return chain; + } + + /// + /// Evaluates one assertion. Everything except llmJudge goes to the pure, synchronous + /// ; llmJudge needs a model call, so it goes to + /// instead. Keeping the split here rather than inside the evaluator + /// is what lets every other assertion type stay reproducible and I/O-free. + /// + /// Any propagates deliberately: the caller turns + /// it into a case-level Error. Swallowing it into a failing assertion would report a vendor + /// problem as an agent regression. + /// + private async Task EvaluateAsync( + TestAssertion assertion, + AssertionContext context, + AgentTestSuite suite, + CancellationToken ct) + { + if (!string.Equals(assertion.Type, AssertionTypes.LlmJudge, StringComparison.Ordinal)) + { + return AssertionEvaluator.Evaluate(assertion, context); + } + + if (_judge == null) + { + throw new AgentTestJudgeUnavailableException( + "no IAgentTestJudge is registered, so llmJudge assertions cannot be scored"); + } + + return await _judge.JudgeAsync(assertion, context, suite, ct); + } + /// /// A blocked tool call fails the case, as a synthetic case-level assertion. /// diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRunExecutor.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRunExecutor.cs index 9faef9f05..6a293cee1 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRunExecutor.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AgentTestRunExecutor.cs @@ -1,3 +1,5 @@ +using BotSharp.Abstraction.MLTasks; +using BotSharp.Abstraction.MLTasks.Settings; using BotSharp.Plugin.AgentTesting.Repositories; namespace BotSharp.Plugin.AgentTesting.Services; @@ -30,14 +32,20 @@ public class AgentTestRunExecutor { private readonly IAgentTestRepository _repo; private readonly ICaseRunner _caseRunner; + private readonly ILlmProviderService? _llmProviders; private readonly ILogger _logger; public AgentTestRunExecutor( IAgentTestRepository repo, ICaseRunner caseRunner, - ILogger logger) + ILogger logger, + ILlmProviderService? llmProviders = null) { _repo = repo; + // Optional so the run-orchestration tests can construct an executor without a provider + // registry. Without it the pricing snapshot is simply absent, which reads as "unknown" rather + // than as "free". + _llmProviders = llmProviders; _caseRunner = caseRunner; _logger = logger; } @@ -186,6 +194,7 @@ public async Task ExecuteAsync(string runId, CancellationToken ct) // weight in a grid keyed by model. Provider = model?.Provider, Model = model?.Model, + CaseType = testCase.CaseType, Status = AgentTestStatus.Error, Error = ex.Message }; @@ -204,6 +213,7 @@ public async Task ExecuteAsync(string runId, CancellationToken ct) run = await _repo.GetRunAsync(runId) ?? run; run.TotalCount++; + TallyRoutingAccuracy(run, result); switch (result.Status) { case AgentTestStatus.Passed: @@ -234,6 +244,159 @@ public async Task ExecuteAsync(string runId, CancellationToken ct) ? AgentTestStatus.Passed : AgentTestStatus.Failed; run.CompletedAt = DateTime.UtcNow; + + await SummarisePerformanceAsync(run); + await _repo.UpdateRunAsync(run); } + + /// + /// Fills in the run's per-model latency, token and cost figures, plus the pricing that produced + /// the cost. + /// + /// Once, at the end, reading the results back -- not accumulated per case like the counts are. + /// A percentile is not incrementally computable: it needs every value at once, and keeping the + /// whole list on the run document to update it in place would store the same numbers twice. + /// + private async Task SummarisePerformanceAsync(AgentTestRun run) + { + List results; + try + { + results = await _repo.ListCaseResultsAsync(run.Id); + } + catch (Exception ex) + { + // Reporting figures must never cost the run its terminal status: the case results are + // already stored and are the source of truth, so a failure here loses a summary, not + // data. + _logger.LogWarning(ex, "Could not summarise performance for agent test run {RunId}.", run.Id); + return; + } + + run.PerformanceSummaries = results + .GroupBy(r => (r.Provider, r.Model)) + .Select(group => BuildSummary(group.Key.Provider, group.Key.Model, group.ToList())) + .ToList(); + + run.ModelPricing = SnapshotPricing(run); + } + + private static PerformanceSummary BuildSummary( + string? provider, string? model, List results) + { + // Only cases that reached the model. An Error case that died before its first turn has a + // ModelDurationMs of zero, and letting those into the percentile makes a run that mostly + // crashed look like the fastest one on record. + var latencies = results + .Where(r => r.ModelDurationMs > 0) + .Select(r => r.ModelDurationMs) + .OrderBy(ms => ms) + .ToList(); + + return new PerformanceSummary + { + Provider = provider, + Model = model, + CaseCount = latencies.Count, + LatencyP50Ms = Percentile(latencies, 0.50), + LatencyP95Ms = Percentile(latencies, 0.95), + // Tokens and cost come from EVERY result, unlike latency: a case that errored still spent + // whatever it spent, and hiding that would understate the run's real cost. + TotalTokens = results.Sum(r => r.TotalTokens), + TotalCost = results.Sum(r => r.Cost) + }; + } + + /// + /// Nearest-rank percentile over an already-sorted list: the value at ceil(p * n) - 1, so P95 of + /// twenty samples is the 19th and P50 of an even count is the lower of the two middle values. + /// + /// Deliberately not interpolated. An interpolated P95 returns a duration no case actually took, + /// which is indefensible when someone asks which case was the slow one -- and with the handful of + /// cases a real suite starts with, interpolation invents most of the answer. + /// + private static long Percentile(List sorted, double percentile) + { + if (sorted.Count == 0) + { + return 0; + } + + var rank = (int)Math.Ceiling(percentile * sorted.Count) - 1; + return sorted[Math.Clamp(rank, 0, sorted.Count - 1)]; + } + + /// + /// The unit costs in force for each model this run swept. Without them a cost figure cannot be + /// compared with any other run's: a provider price change would show up as a cost regression with + /// nothing to point at. + /// + private List SnapshotPricing(AgentTestRun run) + { + if (_llmProviders == null || run.Models is not { Count: > 0 }) + { + // No models named means each agent ran on its own LlmConfig, which this method cannot + // resolve without reading every agent involved. Left empty rather than guessed. + return []; + } + + return run.Models + .Select(m => + { + LlmModelSetting? setting = null; + try + { + setting = _llmProviders.GetSetting(m.Provider, m.Model); + } + catch (Exception ex) + { + _logger.LogWarning(ex, + "Could not read pricing for {Provider}/{Model} while summarising run {RunId}.", + m.Provider, m.Model, run.Id); + } + + return new ModelPricingSnapshot + { + Provider = m.Provider, + Model = m.Model, + TextInputCost = setting?.Cost?.TextInputCost, + TextOutputCost = setting?.Cost?.TextOutputCost + }; + }) + .ToList(); + } + + /// + /// Folds one case result into the run's per-model routing accuracy. Only Routing cases count: + /// the evaluation framework gates routing accuracy separately from the agent pass rate, so + /// mixing an agent case into this figure would make the gate unreadable. + /// + /// Rows are keyed by (provider, model) and created on first sight, which keeps this correct for + /// a run that sweeps no models at all -- that produces the single (null, null) row. + /// + private static void TallyRoutingAccuracy(AgentTestRun run, AgentTestCaseResult result) + { + if (!string.Equals(result.CaseType, CaseTypes.Routing, StringComparison.OrdinalIgnoreCase)) + { + return; + } + + var row = run.RoutingAccuracies.FirstOrDefault(a => + string.Equals(a.Provider, result.Provider, StringComparison.OrdinalIgnoreCase) + && string.Equals(a.Model, result.Model, StringComparison.OrdinalIgnoreCase)); + + if (row == null) + { + row = new RoutingAccuracy { Provider = result.Provider, Model = result.Model }; + run.RoutingAccuracies.Add(row); + } + + row.CaseCount++; + if (string.Equals(result.Status, AgentTestStatus.Passed, StringComparison.Ordinal)) + { + row.PassedCount++; + } + } + } diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionContext.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionContext.cs index 6234163e7..0f34f6541 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionContext.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionContext.cs @@ -9,5 +9,41 @@ public class AssertionContext public string? Output { get; set; } public IReadOnlyList ToolCalls { get; set; } = []; public IReadOnlyDictionary States { get; set; } = new Dictionary(); - public string? RoutedToAgent { get; set; } + + /// + /// Which agents answered, in order, consecutive repeats collapsed. Turn-level contexts carry only + /// that turn's slice; the case-level context carries the whole conversation. + /// + /// There is deliberately no separate "routed to agent" field: routedToAgent is this chain's last + /// entry, and two fields fed from one read are two things that can drift apart. + /// + public IReadOnlyList AgentChain { get; set; } = []; +} + +/// +/// One agent in a chain, carrying BOTH identifiers on purpose. +/// +/// An assertion has to accept either. The name is what a human reads and what the UI shows, but it +/// is also mutable -- renaming an agent would silently break every routing case asserting on it. The +/// id is stable but is a guid nobody recognises, and it is exactly what an author copies out of the +/// agent list, which is how the first real routing case came to assert an id against a name and could +/// never pass. +/// +/// Not persisted: stores names, because that is what a +/// person reads off a result. +/// +public class AgentChainHop +{ + public string Id { get; set; } = string.Empty; + + /// Falls back to the id when the agent cannot be loaded, so this is never blank. + public string Name { get; set; } = string.Empty; + + /// + /// Whether a token an author typed refers to this hop. Case-insensitive, and matches the id just + /// as readily as the name. + /// + public bool Matches(string token) + => string.Equals(Name, token, StringComparison.OrdinalIgnoreCase) + || string.Equals(Id, token, StringComparison.OrdinalIgnoreCase); } diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionEvaluator.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionEvaluator.cs index 6041beb7c..ac783dc23 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionEvaluator.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/AssertionEvaluator.cs @@ -12,13 +12,14 @@ public static class AssertionTypes public const string ToolNotCalled = "toolNotCalled"; public const string StateEquals = "stateEquals"; public const string RoutedToAgent = "routedToAgent"; + public const string AgentChain = "agentChain"; public const string LlmJudge = "llmJudge"; /// /// Result-only. Never authored on a case and never evaluated -- AgentTestCaseRunner synthesises /// it when the mock seam blocked a tool, so the block surfaces in the ordinary assertion table /// instead of only in Observed Tool Calls. Deliberately absent from AssertionValidation's - /// Requirements map, which covers the eight authorable types. + /// Requirements map, which covers the nine authorable types. /// public const string NoBlockedTools = "noBlockedTools"; } @@ -177,28 +178,35 @@ public static AssertionResult Evaluate(TestAssertion assertion, AssertionContext break; case AssertionTypes.RoutedToAgent: - result.Actual = context.RoutedToAgent; + var lastHop = context.AgentChain.Count > 0 ? context.AgentChain[^1] : null; + result.Actual = lastHop?.Name; if (string.IsNullOrWhiteSpace(assertion.Expected)) { - // A null Expected compares equal to a null RoutedToAgent (e.g. the canary/no - // routing information case) -- a blank/omitted Expected must not read as - // "expect no routing," which verifies nothing and would always pass. + // A blank Expected would compare equal to "no agent answered", which verifies + // nothing and would always pass. result.Passed = false; - result.Message = "routedToAgent requires a non-empty 'expected' agent name"; + result.Message = "routedToAgent requires a non-empty 'expected' agent name or id"; break; } - result.Passed = string.Equals(context.RoutedToAgent, assertion.Expected, - StringComparison.OrdinalIgnoreCase); + // Either identifier is accepted -- see AgentChainHop.Matches. + result.Passed = lastHop?.Matches(assertion.Expected.Trim()) == true; if (!result.Passed) result.Message = "the conversation was handled by a different agent"; break; + case AssertionTypes.AgentChain: + EvaluateAgentChain(assertion, context, result); + break; + case AssertionTypes.LlmJudge: - // P2 will wire this to an IInstructService judge. P1 fails explicitly and never - // passes silently -- passing silently would show a case that verified nothing as - // green. + // Unreachable on the normal path: the runner routes llmJudge to IAgentTestJudge + // instead of here, because scoring it needs a model call and this method is a pure, + // synchronous, I/O-free function. Kept as a loud failure rather than removed, so + // that a caller who evaluates assertions without going through the runner gets a + // verdict it cannot mistake for a pass. Silently passing would show a case that + // verified nothing as green. result.Passed = false; - result.Message = "llmJudge is not available in P1"; + result.Message = "llmJudge must be evaluated through IAgentTestJudge, not AssertionEvaluator"; break; default: @@ -209,6 +217,125 @@ public static AssertionResult Evaluate(TestAssertion assertion, AssertionContext return result; } + + /// + /// Compares the agents that answered against an expected list. Complements routedToAgent rather + /// than replacing it: routedToAgent asks "who answered last", which cannot express a hand-off at + /// all -- in Entry -> A -> B only B is visible, and if control returns to the entry agent and it + /// emits the closing message, a correctly routed case reads as routed to the entry agent. + /// + /// is a comma-separated list of agent names. + /// selects the mode -- see -- + /// and defaults to Contains when omitted. + /// + private static void EvaluateAgentChain( + TestAssertion assertion, AssertionContext context, AssertionResult result) + { + result.Actual = string.Join(" -> ", context.AgentChain.Select(hop => hop.Name)); + + var expected = SplitAgentNames(assertion.Expected); + if (expected.Count == 0) + { + // An empty expected list is a subset of, and an ordered subsequence of, any chain, so + // both Contains and Ordered would pass vacuously; Exact would silently assert "no agent + // ever answered", which no author means to write. + result.Passed = false; + result.Message = "agentChain requires a non-empty comma-separated 'expected' agent list"; + return; + } + + // Deliberately not defaulted on a typo: falling back to Contains for an unrecognised mode + // would turn "orderd" into the loosest available check and quietly verify much less than the + // author asked for. + var mode = AgentChainModes.Normalize(assertion.Target); + if (mode == null) + { + result.Passed = false; + result.Message = "agentChain 'target' must be one of " + + string.Join(", ", AgentChainModes.All) + + " (or empty for " + AgentChainModes.Contains + "), not '" + + assertion.Target + "'"; + return; + } + + switch (mode) + { + case AgentChainModes.Exact: + result.Passed = context.AgentChain.Count == expected.Count + && context.AgentChain.Zip(expected).All(pair => pair.First.Matches(pair.Second)); + if (!result.Passed) result.Message = "the agent chain differs from the expected chain"; + break; + + case AgentChainModes.Ordered: + result.Passed = IsOrderedSubsequence(expected, context.AgentChain); + if (!result.Passed) result.Message = "the expected agents did not all appear, in that order"; + break; + + default: + var missing = expected + .Where(e => !context.AgentChain.Any(hop => hop.Matches(e))) + .ToList(); + result.Passed = missing.Count == 0; + if (!result.Passed) + { + result.Message = "the agent chain does not include " + string.Join(", ", missing); + } + break; + } + } + + /// + /// Whether every expected name appears in the chain in the given relative order, with other + /// agents allowed in between -- so ["Copilot", "WorkOrder"] matches + /// Copilot -> Diagnosis -> WorkOrder. Asserting the hand-offs an author cares about must not + /// require enumerating every agent the conversation happened to pass through; Exact is for that. + /// + private static bool IsOrderedSubsequence(List expected, IReadOnlyList chain) + { + var next = 0; + foreach (var hop in chain) + { + if (next < expected.Count && hop.Matches(expected[next])) + { + next++; + } + } + + return next == expected.Count; + } + + private static List SplitAgentNames(string? value) + => (value ?? string.Empty) + .Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries) + .ToList(); +} + +/// +/// How an agentChain assertion compares its expected list against the actual chain. +/// +/// Contains -- every expected agent appears somewhere, order ignored. The default, and the right +/// choice for "this agent must have been involved at all". +/// Ordered -- every expected agent appears, in that relative order, other agents allowed in +/// between. This is the hand-off assertion. +/// Exact -- the chain is precisely the expected list and nothing else. Strictest, and also how to +/// assert isolation: an Exact chain of one agent means nothing routed away. +/// +public static class AgentChainModes +{ + public const string Contains = "contains"; + public const string Ordered = "ordered"; + public const string Exact = "exact"; + + public static readonly string[] All = [Contains, Ordered, Exact]; + + /// + /// Canonical mode for any casing, with blank meaning ; null for an + /// unrecognised value, so the caller rejects it instead of guessing. + /// + public static string? Normalize(string? value) + => string.IsNullOrWhiteSpace(value) + ? Contains + : All.FirstOrDefault(m => string.Equals(m, value.Trim(), StringComparison.OrdinalIgnoreCase)); } /// @@ -224,7 +351,7 @@ public static class AssertionValidation { private enum RequiredField { Expected, Target } - // One row per AssertionTypes constant -- eight total. + // One row per authorable AssertionTypes constant -- nine total. private static readonly Dictionary Requirements = new(StringComparer.Ordinal) { [AssertionTypes.OutputContains] = RequiredField.Expected, @@ -234,9 +361,29 @@ private enum RequiredField { Expected, Target } [AssertionTypes.ToolNotCalled] = RequiredField.Target, [AssertionTypes.StateEquals] = RequiredField.Target, [AssertionTypes.RoutedToAgent] = RequiredField.Expected, + [AssertionTypes.AgentChain] = RequiredField.Expected, [AssertionTypes.LlmJudge] = RequiredField.Expected, }; + /// + /// The nine authorable types, in the order the Requirements map above declares them. + /// + /// Exposed so that ICaseAuthor can generate the assertion vocabulary it puts in front of a model + /// from this map rather than from a hand-written copy in a prompt string. A prompt that lists a + /// type this map does not know, or omits one it does, produces drafts that fail validation for + /// reasons the author cannot see. + /// + public static IReadOnlyList Authorable { get; } = Requirements.Keys.ToArray(); + + /// + /// Which field this assertion type must carry: "expected", "target", or null for a type with no + /// such requirement. + /// + public static string? RequiredFieldName(string type) + => Requirements.TryGetValue(type, out var required) + ? required == RequiredField.Expected ? "expected" : "target" + : null; + /// Null when the assertion is well-formed; otherwise a caller-facing error message. public static string? Validate(TestAssertion assertion) { diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/BotSharpAgentConversationDriver.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/BotSharpAgentConversationDriver.cs index 75ff79711..ee3198c97 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/BotSharpAgentConversationDriver.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/BotSharpAgentConversationDriver.cs @@ -1,5 +1,6 @@ using BotSharp.Abstraction.Agents; using BotSharp.Abstraction.Agents.Enums; +using BotSharp.Abstraction.Conversations.Enums; using BotSharp.Abstraction.Models; using BotSharp.Abstraction.Repositories; using BotSharp.Abstraction.Routing; @@ -37,6 +38,9 @@ public class BotSharpAgentConversationDriver : IAgentConversationDriver // see AgentTestRunQueue.ScopedCaseRunner), so exactly one conversation ever passes through it. private bool _conversationTagged; + /// See . Scoped to one case, like the driver itself. + private readonly Dictionary _agentNameCache = new(StringComparer.Ordinal); + public BotSharpAgentConversationDriver( IConversationService conversations, IRoutingService routing, @@ -78,6 +82,50 @@ public async Task PrepareAsync(string conversationId, string agentId, IReadOnlyL await _conversations.SetConversationId(conversationId, states); } + public async Task InjectHistoryAsync( + string conversationId, string agentId, IReadOnlyList history) + { + if (history.Count == 0) + { + return 0; + } + + // The conversation's dialog document has to exist before AppendConversationDialogs can push + // into it -- see the interface note. Created through the same call SendMessage itself uses, + // so the row a history-bearing case runs against is identical to the row it would have got + // anyway, rather than one this method invented. + await _conversations.GetConversationRecordOrCreateNew(agentId); + + var elements = history + .Select(message => new DialogElement + { + MetaData = new DialogMetaData + { + // Already normalised and rejected at save time; falling back to user here just + // avoids writing a role BotSharp would not recognise if that ever slipped through. + Role = HistoryRoles.Normalize(message.Role) ?? AgentRole.User, + AgentId = agentId, + MessageId = Guid.NewGuid().ToString(), + MessageType = MessageTypeName.Plain, + CreatedTime = DateTime.UtcNow + }, + Content = message.Content + }) + .ToList(); + + await _repository.AppendConversationDialogs(conversationId, elements); + + // Read back and count only the messages this call generated, matched by their own message + // ids. Counting every dialog in the conversation would also count anything else that got + // there, which is exactly the confusion this verification exists to avoid. + var written = elements + .Select(e => e.MetaData!.MessageId) + .ToHashSet(StringComparer.Ordinal); + + var stored = await _repository.GetConversationDialogs(conversationId); + return stored.Count(d => d.MetaData?.MessageId != null && written.Contains(d.MetaData.MessageId)); + } + public async Task SendAsync(string conversationId, string agentId, string userMessage, CancellationToken ct) { // Fail fast only if the case had already timed out before we ever got here (e.g. a prior @@ -181,18 +229,70 @@ public async Task RunCanaryAsync(string conversationId, string agentId, Ca return Task.FromResult>(result); } - public async Task ReadRoutedAgentNameAsync(string conversationId) + public async Task> ReadAssistantAgentSequenceAsync(string conversationId) { var dialogs = await _repository.GetConversationDialogs(conversationId); - var lastAssistantDialog = dialogs.LastOrDefault(d => d.MetaData?.Role == AgentRole.Assistant); - var agentId = lastAssistantDialog?.MetaData?.AgentId; - if (string.IsNullOrEmpty(agentId)) + var sequence = new List(); + foreach (var dialog in dialogs) { - return null; + if (dialog.MetaData?.Role != AgentRole.Assistant) + { + continue; + } + + var agentId = dialog.MetaData?.AgentId; + if (string.IsNullOrEmpty(agentId)) + { + // An assistant message with no agent attribution cannot be placed in the chain. + // Skipped rather than recorded as a blank hop, which would make an agentChain + // assertion fail for a reason the author has no way to act on. + continue; + } + + sequence.Add(new AgentChainHop + { + Id = agentId, + Name = await ResolveAgentNameAsync(agentId) + }); + } + + return sequence; + } + + /// + /// Agent id to display name, cached for the lifetime of this driver -- which is one case, since + /// the driver is resolved from the per-case DI scope AgentTestRunQueue.ScopedCaseRunner opens. + /// The chain is re-read after every turn and a long conversation revisits the same few agents + /// many times, so without the cache one case would issue GetAgent once per assistant message per + /// turn. + /// + /// Falls back to the id when the agent cannot be loaded (deleted since the conversation ran, or + /// a permission-scoped lookup): a chain entry that names something is far more actionable than a + /// silently dropped hop, and an assertion against it fails with a message the author can read. + /// + private async Task ResolveAgentNameAsync(string agentId) + { + if (_agentNameCache.TryGetValue(agentId, out var cached)) + { + return cached; + } + + string name; + try + { + var agent = await _agents.GetAgent(agentId); + name = string.IsNullOrWhiteSpace(agent?.Name) ? agentId : agent!.Name; + } + catch (Exception ex) + { + _logger.LogWarning(ex, + "Could not resolve agent {AgentId} while building an agent test chain; using the id.", + agentId); + name = agentId; } - var agent = await _agents.GetAgent(agentId); - return agent?.Name; + _agentNameCache[agentId] = name; + return name; } } diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/CaseScope.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/CaseScope.cs new file mode 100644 index 000000000..faee58382 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/CaseScope.cs @@ -0,0 +1,172 @@ +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// Works out which cases a given change actually needs to run. +/// +/// A model or prompt change does not justify running everything: the cost is real and mostly buys +/// nothing, since a change to one agent cannot affect a case that never touches it. But the reverse +/// error is far worse -- a case wrongly left out reports nothing at all, and "not run" is +/// indistinguishable from "passed" once the numbers are in a report. So every rule here resolves +/// towards including, and the one thing the caller must never be able to do is quietly shrink the +/// scope without it showing up in the excluded list. +/// +/// A pure function, like : the same (case, suite agent, query) always +/// yields the same decision with no I/O, which is what makes the rules exhaustively testable rather +/// than something to be argued about. +/// +public static class CaseScope +{ + /// + /// The agents a case exercises, as ids. + /// + /// An authored wins. Otherwise it falls back to the + /// case's entry agent, which is already known and is definitionally involved -- so an Agent case + /// is correctly picked up by a change to the agent it runs against without anyone maintaining a + /// list. + /// + /// That fallback is exactly right for an Agent case and only a starting point for a Routing case, + /// where the entry agent is the router and the agents that matter are downstream of it. Those + /// cannot be derived from the case definition -- they are only visible once it has run -- which is + /// what authoring InvolvedAgents is for. + /// + public static IReadOnlyList InvolvedAgentIds(AgentTestCase testCase, string? suiteAgentId) + { + if (testCase.InvolvedAgents.Count > 0) + { + return testCase.InvolvedAgents + .Where(id => !string.IsNullOrWhiteSpace(id)) + .Select(id => id.Trim()) + .ToList(); + } + + var entry = string.IsNullOrWhiteSpace(testCase.EntryAgentId) ? suiteAgentId : testCase.EntryAgentId; + return string.IsNullOrWhiteSpace(entry) ? [] : [entry.Trim()]; + } + + /// + /// Whether this case belongs in the scope, and which rule decided it. Rules are applied in the + /// order below and the first match wins. + /// + public static ScopeDecision Decide(AgentTestCase testCase, string? suiteAgentId, ScopeQuery query) + { + var involved = InvolvedAgentIds(testCase, suiteAgentId); + var batch = CaseBatches.Effective(testCase); + + // Before the rules: a disabled case is skipped by the executor, so calling it "in scope" + // would overstate coverage by exactly the cases nobody is running. Reported as excluded with + // its own reason so a disabled cross-cutting safety case stands out rather than blending into + // the cases a change genuinely cannot affect. + if (!testCase.Enabled) + { + return new ScopeDecision(false, ScopeReasons.Disabled, involved, batch); + } + + // A separate axis from the rules below: batches exist to run in order and stop early, so + // narrowing to one batch is scheduling rather than scoping. Null means every batch. + if (query.Batch is { } wanted && batch != wanted) + { + return new ScopeDecision(false, ScopeReasons.OtherBatch, involved, batch); + } + + // Rule 1. Cross-cutting cases run in every scope, whatever changed. The whole point of + // narrowing is to skip cases a change cannot affect, and "this change cannot affect safety" + // is precisely the claim not to accept without checking. + if (testCase.CrossCutting) + { + return new ScopeDecision(true, ScopeReasons.CrossCutting, involved, batch); + } + + // Rule 2. A platform-wide change -- foundation model, provider swap, infrastructure -- turns + // narrowing off entirely, because there is no agent it demonstrably does not touch. + if (query.FullPlatform) + { + return new ScopeDecision(true, ScopeReasons.FullPlatform, involved, batch); + } + + // Rule 3. The case touches at least one of the changed agents. + if (involved.Any(id => query.TargetAgentIds.Any( + target => string.Equals(id, target, StringComparison.OrdinalIgnoreCase)))) + { + return new ScopeDecision(true, ScopeReasons.TargetAgent, involved, batch); + } + + // Rule 4. Nothing matched. Note this is only reachable when the case HAS a known involved + // set: with neither an authored list nor an entry agent the set is empty, which is handled + // below rather than falling through to an exclusion. + if (involved.Count == 0) + { + // Fail open. An unknown involved set means the harness cannot show the change does not + // affect this case, and a wrongly excluded case is silent -- it produces no result to + // notice. Running one case more than necessary costs tokens; skipping one hides a + // regression. + return new ScopeDecision(true, ScopeReasons.UnknownAgents, involved, batch); + } + + return new ScopeDecision(false, ScopeReasons.NotInvolved, involved, batch); + } +} + +/// What the caller says changed. +public class ScopeQuery +{ + /// Agent ids the change touches. Ignored when is set. + public IReadOnlyList TargetAgentIds { get; set; } = []; + + /// A platform-wide change: narrowing is switched off and every enabled case is in. + public bool FullPlatform { get; set; } + + /// Narrow to one batch; null covers all of them. + public int? Batch { get; set; } +} + +/// Why one case is in or out. +public class ScopeDecision +{ + public ScopeDecision(bool included, string reason, IReadOnlyList involvedAgentIds, int batch) + { + Included = included; + Reason = reason; + InvolvedAgentIds = involvedAgentIds; + Batch = batch; + } + + public bool Included { get; } + + /// See . Always populated, for excluded cases too. + public string Reason { get; } + + /// The set the decision was actually made against, authored or derived. + public IReadOnlyList InvolvedAgentIds { get; } + + /// The effective batch, after the priority and cross-cutting derivation. + public int Batch { get; } +} + +/// +/// Which rule decided a case. Reported per case rather than only as a count, because a scope nobody +/// can explain is a scope nobody can review -- and reviewing it is the only defence against a change +/// being signed off against a set of cases that quietly excluded the interesting one. +/// +public static class ScopeReasons +{ + /// Included: cross-cutting, so it runs in every scope. + public const string CrossCutting = "crossCutting"; + + /// Included: the change is platform-wide, so narrowing is off. + public const string FullPlatform = "fullPlatform"; + + /// Included: the case exercises one of the changed agents. + public const string TargetAgent = "targetAgent"; + + /// Included: no involved agents are known, so it cannot be shown to be unaffected. + public const string UnknownAgents = "unknownAgents"; + + /// Excluded: the case does not touch any changed agent. + public const string NotInvolved = "notInvolved"; + + /// Excluded: the case is disabled, so no run would execute it. + public const string Disabled = "disabled"; + + /// Excluded: the case belongs to a different batch than the one asked for. + public const string OtherBatch = "otherBatch"; +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/CaseValidation.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/CaseValidation.cs new file mode 100644 index 000000000..6a413e44b --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/CaseValidation.cs @@ -0,0 +1,146 @@ +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// Shared create/update validation for a case payload, lifted out of AgentTestController so that +/// every path which produces a case runs the same rules. +/// +/// That sharing is the point, not tidiness: hands a model-authored draft +/// back to a human to save, and a draft that only fails when they press Save wastes the round trip +/// and teaches them to distrust the feature. The author service runs these rules itself and gives +/// the model one chance to repair its own output against the real error text. +/// +/// Everything here is static and I/O-free. The one case rule that needs a service -- +/// does the entry agent exist -- stays on the controller as ValidateEntryAgentAsync. +/// +public static class CaseValidation +{ + /// + /// Shared create/update validation for a case payload. Null means the payload is acceptable; + /// otherwise the string is a caller-facing 400 message. + /// + public static string? Validate(AgentTestCaseUpsertRequest request) + { + if (IsUnsupportedUnmockedToolPolicy(request.UnmockedToolPolicy)) + { + return "Passthrough is not supported in P1"; + } + + var caseType = CaseTypes.Normalize(request.CaseType); + if (caseType == null && !string.IsNullOrWhiteSpace(request.CaseType)) + { + return $"caseType must be one of {string.Join(", ", CaseTypes.All)}, not '{request.CaseType}'"; + } + + if (CasePriorities.Normalize(request.Priority) == null && !string.IsNullOrWhiteSpace(request.Priority)) + { + return $"priority must be one of {string.Join(", ", CasePriorities.All)}, not '{request.Priority}'"; + } + + if (CaseSeverities.Normalize(request.Severity) == null && !string.IsNullOrWhiteSpace(request.Severity)) + { + return $"severity must be one of {string.Join(", ", CaseSeverities.All)}, not '{request.Severity}'"; + } + + // Rejected rather than clamped: a batch of 4 is a mistake, and silently filing the case in + // batch 3 would leave the author believing it runs somewhere it does not. + if (request.Batch is { } batch && !CaseBatches.All.Contains(batch)) + { + return $"batch must be one of {string.Join(", ", CaseBatches.All)}, not {batch}"; + } + + foreach (var (message, index) in (request.History ?? []).Select((m, i) => (m, i))) + { + if (HistoryRoles.Normalize(message?.Role) == null) + { + return $"history message {index + 1} has role '{message?.Role}'; only " + + $"{string.Join(" and ", HistoryRoles.All)} are supported"; + } + + // An empty message is dropped by BotSharp's own dialog storage (ConversationStorage + // skips elements with blank content), so it would silently not be there at run time -- + // and the runner's count check would then fail the whole case with a confusing message + // about the write having vanished. + if (string.IsNullOrWhiteSpace(message!.Content)) + { + return $"history message {index + 1} has no content"; + } + } + + var allAssertions = (request.Turns ?? []) + .SelectMany(t => t.Assertions ?? []) + .Concat(request.Assertions ?? []) + .ToList(); + + foreach (var assertion in allAssertions) + { + var error = AssertionValidation.Validate(assertion); + if (error != null) + { + return error; + } + } + + return ValidateRoutingCase(caseType ?? CaseTypes.Agent, request, allAssertions); + } + + /// + /// The extra rules a Routing case has to satisfy. A Routing case is not a label -- it is the only + /// type counted towards a run's routing accuracy, so one that cannot actually establish a routing + /// outcome would quietly move that figure without measuring anything. + /// + /// Enforced at save time for the same reason AssertionValidation is: the alternative is a case + /// that saves cleanly and then reports a meaningless green every run. + /// + private static string? ValidateRoutingCase( + string caseType, AgentTestCaseUpsertRequest request, List allAssertions) + { + if (!string.Equals(caseType, CaseTypes.Routing, StringComparison.Ordinal)) + { + return null; + } + + // Routing is a single-turn question: which agent picks this message up. A second turn is + // either a different question (making it an Agent case) or an accident, and either way its + // result would be counted as routing accuracy. + // + // Authored History is not a turn and is deliberately not counted here: replaying a prior + // exchange and then asking one question is still a single routing decision, and it is the + // most realistic way to test routing that depends on context. + if ((request.Turns ?? []).Count != 1) + { + return "a Routing case must have exactly one turn; use an Agent case for a multi-turn case"; + } + + // Without one of these the case asserts nothing about routing, yet still counts towards + // routing accuracy -- it would report Passed for having successfully said anything at all. + var assertsRouting = allAssertions.Any(a => + string.Equals(a.Type, AssertionTypes.RoutedToAgent, StringComparison.Ordinal) + || string.Equals(a.Type, AssertionTypes.AgentChain, StringComparison.Ordinal)); + if (!assertsRouting) + { + return $"a Routing case needs at least one '{AssertionTypes.RoutedToAgent}' or " + + $"'{AssertionTypes.AgentChain}' assertion, otherwise it verifies no routing outcome"; + } + + // The framework this implements scores routing purely as expected-agent == actual-agent and + // deliberately applies no quality judgement to it. An llmJudge here would also make the + // routing figure depend on a vendor call, so a vendor outage would read as a routing + // regression. + if (allAssertions.Any(a => string.Equals(a.Type, AssertionTypes.LlmJudge, StringComparison.Ordinal))) + { + return $"a Routing case cannot use '{AssertionTypes.LlmJudge}': routing is judged only by " + + "which agent handled the conversation"; + } + + return null; + } + + /// + /// Passthrough was specified in the design/plan and even had a (dead) code path, but nothing + /// ever back-fills an ObservedToolCall for a tool the provider let run for real -- under it, + /// toolNotCalled always vacuously passed against a tool that genuinely executed with real side + /// effects. Rejected here rather than implementing the back-fill (project owner decision). + /// + private static bool IsUnsupportedUnmockedToolPolicy(string? policy) + => string.Equals(policy, "Passthrough", StringComparison.OrdinalIgnoreCase); +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/IAgentConversationDriver.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/IAgentConversationDriver.cs index 4226daebb..de6fce7cb 100644 --- a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/IAgentConversationDriver.cs +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/IAgentConversationDriver.cs @@ -9,6 +9,19 @@ public interface IAgentConversationDriver { Task PrepareAsync(string conversationId, string agentId, IReadOnlyList initialStates); + /// + /// Writes authored history into the conversation before any turn runs, and returns how many of + /// those messages are actually readable back out of the store. + /// + /// The return value is the point. IBotSharpRepository.AppendConversationDialogs is an UpdateOne + /// with no upsert, so it silently does nothing when the conversation's dialog document does not + /// exist yet -- and PrepareAsync deliberately does not create it (the row is created by the + /// first SendMessage). A case whose history vanished would run against no context at all and + /// still report Passed, so the caller compares this count against what it asked for. + /// + Task InjectHistoryAsync( + string conversationId, string agentId, IReadOnlyList history); + /// Drives one turn and returns that turn's output text. Task SendAsync(string conversationId, string agentId, string userMessage, CancellationToken ct); @@ -17,5 +30,16 @@ public interface IAgentConversationDriver Task> ReadStatesAsync(string conversationId); - Task ReadRoutedAgentNameAsync(string conversationId); + /// + /// The agent behind every assistant message so far, in dialog order, NOT de-duplicated and NOT + /// sliced per turn -- the caller does both, because only it knows where each turn began. + /// + /// Replaces an earlier ReadRoutedAgentNameAsync that returned just the last assistant message's + /// agent. That was the only routing signal available and it could not describe a hand-off: for + /// Entry -> A -> B it reported B alone, and when control returned to the entry agent and that + /// agent emitted the closing message it reported the entry agent, failing a correctly routed + /// case. Returning the sequence lets the runner derive both the last agent and the chain from + /// one read, so they cannot disagree, and costs no extra round trip. + /// + Task> ReadAssistantAgentSequenceAsync(string conversationId); } diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/IAgentTestJudge.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/IAgentTestJudge.cs new file mode 100644 index 000000000..903dfc772 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/IAgentTestJudge.cs @@ -0,0 +1,55 @@ +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// Raised when the judge could not reach a verdict at all: no judge model is configured on the +/// suite, its provider is not registered, the vendor call failed, or the reply could not be read as +/// a score. +/// +/// Deliberately an exception rather than a failing . "The judge could +/// not decide" is not "the agent regressed" -- folding the two together would let a vendor timeout, +/// a rate limit or a malformed reply read as an agent defect, which is exactly the Failed/Error +/// confusion the runner keeps apart everywhere else. turns this +/// into a case-level Error carrying this message. +/// +public class AgentTestJudgeUnavailableException : Exception +{ + public AgentTestJudgeUnavailableException(string message) : base(message) + { + } + + public AgentTestJudgeUnavailableException(string message, Exception inner) : base(message, inner) + { + } +} + +/// +/// Scores one llmJudge assertion with a model. +/// +/// This is the one assertion type that is NOT a pure function, which is why it lives here instead of +/// in : that class is a pure, synchronous, I/O-free function, and the +/// runner depends on that purity for reproducible verdicts. A model call is neither pure nor +/// synchronous, so llmJudge is evaluated in a separate pass and the reproducible assertions stay +/// reproducible. +/// +/// What reaches the vendor is deliberately narrow: the fixed rubric, the criterion the case author +/// wrote () and the agent's reply text. Tool arguments, tool +/// results, conversation state and the user's own messages are NOT sent. That is the same boundary +/// draws, with one unavoidable difference: judging the quality of a +/// reply requires sending that reply, and an agent's reply can contain PII (phone numbers, +/// addresses, tenant names). Callers need to know that. It also means a criterion has to be +/// self-contained -- "the reply asks for a work order number" works, "the reply answers the user's +/// question" does not, because the judge never sees the question. +/// +public interface IAgentTestJudge +{ + /// + /// Scores against . Throws + /// when no verdict could be reached; a returned + /// result is always a real verdict, passing or failing. + /// + Task JudgeAsync( + TestAssertion assertion, + AssertionContext context, + AgentTestSuite suite, + CancellationToken ct); +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/ICaseAuthor.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/ICaseAuthor.cs new file mode 100644 index 000000000..d65105ac8 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/ICaseAuthor.cs @@ -0,0 +1,52 @@ +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// Authors and edits a test case by conversation: the user picks an agent's suite, says what they +/// want in their own words, and gets back a draft they can keep talking to. +/// +/// Contract: +/// - Produces a draft, never a stored case. Nothing here writes to the case store; the response goes +/// back to the editor, and the human presses save. See . +/// - Never deletes by omission. The model declares which fields it is changing and only those are +/// taken from its answer; everything else is copied off the draft that came in. See +/// . +/// - Never invents a callable function. A mock or a toolCalled assertion naming a function the agent +/// cannot call is dropped and reported, because it could otherwise never match at run time. +/// - Runs on its own output and gives the model one chance to repair a +/// rejection against the real error text. A draft that still fails is returned as the unchanged +/// original plus the errors -- never as a broken draft presented as progress. +/// +/// Boundary -- what leaves this system. The agent's own instruction, its function names and +/// descriptions, the suite's existing case names, the draft, and the user's authoring messages are +/// all sent to the configured model vendor. When the case has run before, so are the agent's actual +/// reply texts and its actual tool arguments from that run, which for a case recorded from a real +/// conversation can include the customer data that conversation contained. That is a wider egress +/// than the recorder's (which withholds tool arguments and results), and it is why the endpoint is +/// admin-only. +/// +public interface ICaseAuthor +{ + /// + /// One authoring turn. + /// + /// + /// No usable model, the vendor call failed, or the model's answer could not be read as an + /// authoring result. All of these mean "no draft was produced", which is a different thing from + /// "the draft is invalid" -- the latter comes back in the response. + /// + Task AuthorAsync( + AgentTestSuite suite, + AgentTestAuthorRequest request, + CancellationToken ct); +} + +/// +/// No draft could be produced. Separate from a validation failure on purpose: a validation failure +/// still returns a draft and a next step, while this means the authoring turn did not happen and the +/// user should retry or fix configuration. +/// +public class CaseAuthorUnavailableException : Exception +{ + public CaseAuthorUnavailableException(string message) : base(message) { } + public CaseAuthorUnavailableException(string message, Exception inner) : base(message, inner) { } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/LlmAgentTestJudge.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/LlmAgentTestJudge.cs new file mode 100644 index 000000000..5525df4c1 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/LlmAgentTestJudge.cs @@ -0,0 +1,236 @@ +using System.Text.Json; +using BotSharp.Abstraction.Agents.Enums; +using BotSharp.Abstraction.MLTasks; + +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// The model-backed . Contract, boundary and the reason this is not part +/// of all live on the interface. +/// +/// Extends the model's answer no trust: a blank reply, a non-JSON reply, malformed JSON or a score +/// outside the 1-5 scale are all rejected as "no verdict" rather than coerced into a pass or a fail. +/// A judge that ignored the rubric has not graded anything, and reading pass/fail out of that is +/// reading meaning into noise. +/// +public class LlmAgentTestJudge : IAgentTestJudge +{ + /// + /// Pass mark when a case does not set . 4/5 is the bar the + /// evaluation framework uses for its own human quality score, and the rubric below is worded to + /// match that framework's 1-5 definitions -- so a model score and a human score mean the same + /// thing on the same scale, and the two can be compared or swapped later. + /// + public const double DefaultMinScore = 4; + + private const double MinValidScore = 1; + private const double MaxValidScore = 5; + + private readonly IServiceProvider _services; + private readonly ILogger _logger; + + public LlmAgentTestJudge(IServiceProvider services, ILogger logger) + { + _services = services; + _logger = logger; + } + + public async Task JudgeAsync( + TestAssertion assertion, + AssertionContext context, + AgentTestSuite suite, + CancellationToken ct) + { + var criterion = assertion.Expected; + if (string.IsNullOrWhiteSpace(criterion)) + { + // AssertionValidation already rejects this at case create/update. Repeated here because + // a case saved before that rule existed would otherwise reach the vendor with an empty + // criterion and come back with a meaningless score. + throw new AgentTestJudgeUnavailableException( + "this llmJudge assertion has no 'expected' criterion to judge against"); + } + + // No silent default. BotSharp's own InstructService falls back to openai/gpt-4o when a + // provider and model are not given; inheriting that here would score cases with a model + // nobody chose, and the run would look conclusive. A missing judge model is an Error with a + // message naming the fix. + if (string.IsNullOrWhiteSpace(suite.JudgeProvider) || string.IsNullOrWhiteSpace(suite.JudgeModel)) + { + throw new AgentTestJudgeUnavailableException( + "this suite has no judge model configured, so llmJudge assertions cannot be scored. " + + "Set the suite's judgeProvider and judgeModel, or remove the llmJudge assertion."); + } + + if (string.IsNullOrWhiteSpace(context.Output)) + { + // Not a failing verdict: the agent produced no text at all, so there is nothing for the + // judge to grade. Whatever went wrong upstream is the real finding, and the other + // assertions on this case will say so far more usefully than a fabricated score would. + throw new AgentTestJudgeUnavailableException( + "the agent produced no reply text, so there is nothing for llmJudge to score"); + } + + ct.ThrowIfCancellationRequested(); + + // Resolved straight from DI rather than through BotSharp.Core's CompletionProvider helper, + // for the same two reasons LlmCaseSegmenter gives: this plugin references only + // BotSharp.Abstraction, and that helper also writes provider/model into the ambient + // conversation state -- which here would leak the JUDGE's model into the conversation under + // test and change what the agent itself runs on. + var completion = _services.GetServices() + .FirstOrDefault(x => string.Equals(x.Provider, suite.JudgeProvider, StringComparison.OrdinalIgnoreCase)); + + if (completion == null) + { + throw new AgentTestJudgeUnavailableException( + $"no chat completion provider is registered for judge provider '{suite.JudgeProvider}'"); + } + + completion.SetModelName(suite.JudgeModel); + + var promptAgent = new Agent + { + Id = Guid.Empty.ToString(), + Name = "AgentTestJudge", + Instruction = BuildInstruction() + }; + + string raw; + try + { + var response = await completion.GetChatCompletions( + promptAgent, + [new RoleDialogModel(AgentRole.User, BuildPrompt(criterion, context.Output))]); + + raw = response?.Content ?? string.Empty; + } + catch (Exception ex) + { + // A vendor timeout, a rate limit, a bad key. None of these say anything about the agent + // under test, so none of them may surface as a failing assertion. + throw new AgentTestJudgeUnavailableException( + $"the judge model call failed: {ex.Message}", ex); + } + + var verdict = ParseVerdict(raw); + var threshold = assertion.MinScore ?? DefaultMinScore; + + var result = new AssertionResult + { + Type = assertion.Type, + Target = assertion.Target, + Expected = criterion, + Actual = verdict.Score.ToString("0.#"), + Score = verdict.Score, + Passed = verdict.Score >= threshold + }; + + // The reason is recorded whether it passed or failed: a pass at exactly the threshold is + // worth being able to read afterwards, and a judge's stated reason is the only way to tell + // "graded correctly" from "graded plausibly but wrongly". + result.Message = string.IsNullOrWhiteSpace(verdict.Reason) + ? $"judged {verdict.Score:0.#}/5 against a threshold of {threshold:0.#}" + : $"judged {verdict.Score:0.#}/5 against a threshold of {threshold:0.#}: {verdict.Reason}"; + + _logger.LogInformation( + "llmJudge scored {Score}/5 (threshold {Threshold}) using {Provider}/{Model}.", + verdict.Score, threshold, suite.JudgeProvider, suite.JudgeModel); + + return result; + } + + /// + /// Parses and validates the judge's reply. Public and static so it can be unit-tested without a + /// vendor: everything that can realistically go wrong with a model's answer is decided here. + /// + public static JudgeVerdict ParseVerdict(string raw) + { + var json = ExtractJson(raw); + if (json == null) + { + throw new AgentTestJudgeUnavailableException( + $"the judge model did not return JSON. First 200 chars: {Truncate(raw, 200)}"); + } + + JudgeVerdict? verdict; + try + { + verdict = JsonSerializer.Deserialize(json, new JsonSerializerOptions + { + PropertyNameCaseInsensitive = true + }); + } + catch (JsonException ex) + { + throw new AgentTestJudgeUnavailableException( + $"the judge model returned malformed JSON: {ex.Message}"); + } + + if (verdict == null) + { + throw new AgentTestJudgeUnavailableException("the judge model returned an empty verdict"); + } + + if (verdict.Score < MinValidScore || verdict.Score > MaxValidScore || double.IsNaN(verdict.Score)) + { + // A score off the scale means the rubric was not followed. Clamping it would silently + // turn "did not grade" into a grade. + throw new AgentTestJudgeUnavailableException( + $"the judge model returned {verdict.Score:0.#}, outside the 1-5 scale it was given"); + } + + return verdict; + } + + private static string? ExtractJson(string raw) + { + var start = raw.IndexOf('{'); + var end = raw.LastIndexOf('}'); + return start >= 0 && end > start ? raw[start..(end + 1)] : null; + } + + private static string Truncate(string text, int max) + => text.Length <= max ? text : text[..max].TrimEnd() + "..."; + + /// + /// The 1-5 definitions are worded to match the evaluation framework's human scoring scale, so + /// that a model score and a human score are directly comparable. Changing them decouples the two. + /// + private static string BuildInstruction() => + """ + You grade one reply produced by a customer-service AI agent, as part of an automated + regression test. + + You are given exactly one CRITERION and the agent's REPLY. Judge only how well the reply + satisfies that criterion. Do not grade tone, length, formatting or style unless the criterion + asks about them. Do not judge whether the underlying business facts are true -- you have no + way to check them and other assertions already cover that. + + Score on this scale: + 5 - clearly satisfies the criterion, no substantive problem + 4 - satisfies the criterion, only minor problems + 3 - broadly usable, but with an obvious gap + 2 - noticeable problems that affect use + 1 - unacceptable + + Reply with JSON only. No prose, no code fence, no explanation outside the JSON: + {"score": , "reason": ""} + """; + + private static string BuildPrompt(string criterion, string output) => + $""" + CRITERION: + {criterion} + + REPLY: + {output} + """; +} + +/// One judge verdict, as returned by the model. +public class JudgeVerdict +{ + public double Score { get; set; } + public string? Reason { get; set; } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/LlmCaseAuthor.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/LlmCaseAuthor.cs new file mode 100644 index 000000000..e023ef878 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/LlmCaseAuthor.cs @@ -0,0 +1,1032 @@ +using System.Text; +using System.Text.Json; +using System.Text.Json.Nodes; +using System.Text.Json.Serialization; +using BotSharp.Abstraction.Agents; +using BotSharp.Abstraction.Agents.Enums; +using BotSharp.Abstraction.MLTasks; +using BotSharp.Plugin.AgentTesting.Repositories; + +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// The model-backed . Contract and boundary live on the interface. +/// +/// Extends the model's answer no trust, in four separate places, because each guards a different +/// failure: the whitelist merge stops a field being deleted by omission, the function-name check +/// stops a mock that could never match, the validation pass stops a draft that cannot be saved, and +/// the diff is computed rather than read off the model's own account of what it did. +/// +public class LlmCaseAuthor : ICaseAuthor +{ + /// + /// How much of the authoring conversation is replayed. The draft carries the state, so older + /// turns add tokens without adding information -- and every turn of this chat pays for the whole + /// context block again. + /// + private const int MaxChatMessages = 20; + + private const int MaxInstructionChars = 4000; + private const int MaxExistingCases = 25; + private const int MaxGroundedTurns = 10; + private const int MaxGroundedOutputChars = 600; + private const int MaxGroundedToolCalls = 20; + private const int MaxGroundedArgsChars = 300; + + /// + /// How many recent runs are searched for a result belonging to the case being edited. Bounded + /// because each one is a separate query, and a case that has not run in the last five runs of + /// its own suite is one whose old output would be misleading grounding anyway. + /// + private const int MaxRunsScanned = 5; + + /// + /// camelCase both ways: this is the shape the case editor already posts to /agent-test/cases, so + /// a draft can travel from the model straight into the editor's form and back out to the save + /// endpoint without a second naming convention in between. + /// + private static readonly JsonSerializerOptions DraftJson = new() + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + PropertyNameCaseInsensitive = true, + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull + }; + + private readonly IServiceProvider _services; + private readonly IAgentService _agents; + private readonly IAgentTestRepository _repo; + private readonly ILogger _logger; + + public LlmCaseAuthor( + IServiceProvider services, + IAgentService agents, + IAgentTestRepository repo, + ILogger logger) + { + _services = services; + _agents = agents; + _repo = repo; + _logger = logger; + } + + public async Task AuthorAsync( + AgentTestSuite suite, + AgentTestAuthorRequest request, + CancellationToken ct) + { + var messages = (request.Messages ?? []) + .Where(m => !string.IsNullOrWhiteSpace(m?.Content)) + .ToList(); + + if (messages.Count == 0 + || !string.Equals(AuthorChatRoles.Normalize(messages[^1].Role), AuthorChatRoles.User, StringComparison.Ordinal)) + { + throw new CaseAuthorUnavailableException( + "the last message must be a user message saying what to do"); + } + + var model = ResolveModel(suite, request.Model); + var completion = _services.GetServices() + .FirstOrDefault(x => string.Equals(x.Provider, model.Provider, StringComparison.OrdinalIgnoreCase)) + ?? throw new CaseAuthorUnavailableException( + $"no chat completion provider is registered for '{model.Provider}'"); + + completion.SetModelName(model.Model); + + // The draft's own entry agent wins over the suite's, for the same reason the runner honours + // it: a case pointed at a leaf agent is testing that agent, and authoring it against the + // router's instruction and the router's functions would describe the wrong thing. + var agentId = string.IsNullOrWhiteSpace(request.Draft?.EntryAgentId) + ? suite.AgentId + : request.Draft!.EntryAgentId!.Trim(); + + var agent = await _agents.GetAgent(agentId) + ?? throw new CaseAuthorUnavailableException( + $"agent {agentId} not found, so there is nothing to author a case against"); + + var targets = MockTargetCatalogue.Describe(agent); + var existingCases = await _repo.ListCasesAsync(suite.Id); + var grounding = await LoadGroundingAsync(suite.Id, request.CaseId, ct); + var agentNames = await AgentNamesAsync(); + + // The baseline is what every merge and every diff is measured against. Cloned so a merge + // cannot mutate the caller's object and leave the diff comparing a thing with itself. + var baseline = Clone(request.Draft ?? new AgentTestCaseUpsertRequest()); + baseline.SuiteId = suite.Id; + + var instruction = BuildInstruction(); + var dialogs = new List + { + new(AgentRole.User, BuildContext(agent, targets, existingCases, request.CaseId, grounding, agentNames)) + }; + + foreach (var message in messages.Take(messages.Count - 1).TakeLast(MaxChatMessages)) + { + var role = string.Equals(AuthorChatRoles.Normalize(message.Role), AuthorChatRoles.Assistant, StringComparison.Ordinal) + ? AgentRole.Assistant + : AgentRole.User; + + dialogs.Add(new RoleDialogModel(role, message.Content)); + } + + // The draft goes in the last message rather than the context block: it is the thing that + // changes every turn, and it is what the instruction tells the model to echo field names from. + dialogs.Add(new RoleDialogModel(AgentRole.User, BuildTask(baseline, messages[^1].Content))); + + var attempt = await AskWithParseRepairAsync(completion, instruction, dialogs, model, ct); + var response = Assemble(baseline, attempt, targets, existingCases, suite); + + if (response.ValidationErrors.Count == 0) + { + _logger.LogInformation( + "Case author changed {ChangeCount} field(s) with {Model}, {WarningCount} warning(s).", + response.Changes.Count, model, response.Warnings.Count); + + return response; + } + + // One repair round against the real error text -- the same single-retry stance the segmenter + // and the judge take. Merged from the baseline again, never from the rejected draft: + // repairing on top of something already invalid compounds the mistake. + _logger.LogInformation( + "Case author draft was rejected ({Error}); asking {Model} to repair it once.", + response.ValidationErrors[0], model); + + dialogs.Add(new RoleDialogModel(AgentRole.Assistant, attempt.Raw)); + dialogs.Add(new RoleDialogModel(AgentRole.User, + $""" + That draft was rejected by validation: + {string.Join("\n", response.ValidationErrors)} + + Fix exactly that and answer with the same JSON envelope again. + """)); + + var repaired = await AskWithParseRepairAsync(completion, instruction, dialogs, model, ct); + var second = Assemble(baseline, repaired, targets, existingCases, suite); + + if (second.ValidationErrors.Count == 0) + { + return second; + } + + // Still invalid. The baseline comes back untouched and the errors are stated: an invalid + // draft presented as progress would overwrite a working one in the editor. + _logger.LogWarning( + "Case author could not produce a valid draft after a repair round: {Error}", + second.ValidationErrors[0]); + + return new AgentTestAuthorResponse + { + Reply = second.Reply, + Draft = baseline, + DraftChanged = false, + Changes = [], + ValidationErrors = second.ValidationErrors, + Warnings = second.Warnings + }; + } + + /// + /// No silent default, for the reason LlmAgentTestJudge gives: BotSharp's own InstructService falls + /// back to openai/gpt-4o, and inheriting that here would author cases with a model nobody chose. + /// The suite's judge model is used as the fallback rather than a new setting, because a suite that + /// has one has already had a model chosen for it deliberately. + /// + private static TestModel ResolveModel(AgentTestSuite suite, TestModel? requested) + { + if (!string.IsNullOrWhiteSpace(requested?.Provider) && !string.IsNullOrWhiteSpace(requested?.Model)) + { + return requested!; + } + + if (!string.IsNullOrWhiteSpace(suite.JudgeProvider) && !string.IsNullOrWhiteSpace(suite.JudgeModel)) + { + return new TestModel { Provider = suite.JudgeProvider!, Model = suite.JudgeModel! }; + } + + throw new CaseAuthorUnavailableException( + "no model to author with: pass a provider and model, or set this suite's judgeProvider " + + "and judgeModel"); + } + + /// + /// Agent names, because routedToAgent and agentChain match by name or id -- and a model writing a + /// name it invented produces an assertion that can never pass, which reads as a routing + /// regression forever after. + /// + private async Task> AgentNamesAsync() + { + var options = await _agents.GetAgentOptions(); + + return (options ?? []) + .Select(o => o.Name) + .Where(n => !string.IsNullOrWhiteSpace(n)) + .Distinct(StringComparer.OrdinalIgnoreCase) + .OrderBy(n => n, StringComparer.OrdinalIgnoreCase) + .ToList(); + } + + /// + /// The most recent result for the case being edited, if it has ever run. This is what lets the + /// model propose an outputContains against text the agent really produced and an argsMatchJson + /// against arguments it really passed, instead of inventing both. + /// + private async Task LoadGroundingAsync(string suiteId, string? caseId, CancellationToken ct) + { + if (string.IsNullOrWhiteSpace(caseId)) + { + return null; + } + + var runs = await _repo.ListRunsAsync(suiteId); // newest first + foreach (var run in runs.Take(MaxRunsScanned)) + { + ct.ThrowIfCancellationRequested(); + + var results = await _repo.ListCaseResultsAsync(run.Id); + var hit = results.FirstOrDefault(r => string.Equals(r.CaseId, caseId, StringComparison.Ordinal)); + if (hit != null) + { + return hit; + } + } + + return null; + } + + /// + /// Calls the model and parses its reply, retrying once if the reply cannot be read at all -- + /// the same single-retry stance the validation-repair round in takes, + /// extended to cover the other way a model's answer can be unusable. already + /// repairs the commonest cause (a JSON-as-text field written as a nested object) deterministically + /// before this is ever reached, so landing here needs a genuinely different mistake -- truncated + /// output, a stray comma, prose with no JSON at all. + /// + private async Task AskWithParseRepairAsync( + IChatCompletion completion, + string instruction, + List dialogs, + TestModel model, + CancellationToken ct) + { + var (raw, attempt, error) = await AskOnceAsync(completion, instruction, dialogs, ct); + if (attempt != null) + { + return attempt; + } + + _logger.LogInformation( + "Case author reply could not be read ({Error}); asking {Model} to resend it.", error, model); + + dialogs.Add(new RoleDialogModel(AgentRole.Assistant, raw)); + dialogs.Add(new RoleDialogModel(AgentRole.User, + $""" + That reply could not be read: {error} + + Resend the full JSON envelope, valid this time. Remember: argsMatchJson, resultContent and + any state "value" must be a JSON string (escaped), never a nested object or array. + """)); + + var (_, repaired, repairError) = await AskOnceAsync(completion, instruction, dialogs, ct); + return repaired ?? throw new CaseAuthorUnavailableException( + $"the authoring model did not return a usable reply after a retry: {repairError}"); + } + + /// + /// One model call, parsed but never throwing on a parse failure -- the caller decides whether + /// that is retryable. A genuine vendor failure (timeout, rate limit, bad key) still throws + /// immediately: asking the same vendor again in the same turn cannot fix that, and treating it as + /// retryable would waste a call and hide the real error behind a generic "no usable reply". + /// + private async Task<(string Raw, AuthorAttempt? Attempt, string? Error)> AskOnceAsync( + IChatCompletion completion, + string instruction, + List dialogs, + CancellationToken ct) + { + ct.ThrowIfCancellationRequested(); + + var promptAgent = new Agent + { + Id = Guid.Empty.ToString(), + Name = "AgentTestCaseAuthor", + Instruction = instruction + }; + + string raw; + try + { + var response = await completion.GetChatCompletions(promptAgent, dialogs); + raw = response?.Content ?? string.Empty; + } + catch (Exception ex) + { + // A vendor timeout, a rate limit, a bad key. None of these produced a draft, and none of + // them should look like one. + throw new CaseAuthorUnavailableException($"the authoring model call failed: {ex.Message}", ex); + } + + try + { + return (raw, Parse(raw), null); + } + catch (CaseAuthorUnavailableException ex) + { + return (raw, null, ex.Message); + } + } + + /// + /// Reads the envelope. Public and static so the parsing rules can be tested without a vendor. + /// + public static AuthorAttempt Parse(string raw) + { + var json = ExtractJson(raw) + ?? throw new CaseAuthorUnavailableException( + $"the authoring model did not return JSON. First 200 chars: {Truncate(raw, 200)}"); + + json = CoerceStructuredJsonStringFields(json); + + AuthorEnvelope? envelope; + try + { + envelope = JsonSerializer.Deserialize(json, DraftJson); + } + catch (JsonException ex) + { + throw new CaseAuthorUnavailableException($"the authoring model returned malformed JSON: {ex.Message}"); + } + + if (envelope == null) + { + throw new CaseAuthorUnavailableException("the authoring model returned an empty result"); + } + + var declared = (envelope.ChangedFields ?? []) + .Select(AuthorFields.Normalize) + .Where(f => f != null) + .Select(f => f!) + .Distinct(StringComparer.Ordinal) + .ToList(); + + // An unwritable field name does not fail the turn -- the merge would ignore it anyway -- but + // it is reported, because "I renamed the suite for you" needs to be visibly untrue. + var rejected = (envelope.ChangedFields ?? []) + .Where(f => !string.IsNullOrWhiteSpace(f) && AuthorFields.Normalize(f) == null) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToList(); + + return new AuthorAttempt(envelope.Reply ?? string.Empty, declared, rejected, envelope.Draft, raw); + } + + /// + /// Merge, sanitise, validate, diff. Deterministic given the same attempt, catalogue and suite. + /// + /// Public and static for the reason and LlmAgentTestJudge.ParseVerdict are: + /// everything that decides what a model is allowed to do to a draft happens here, and it should + /// be testable without a vendor call. + /// + public static AgentTestAuthorResponse Assemble( + AgentTestCaseUpsertRequest baseline, + AuthorAttempt attempt, + List targets, + List existingCases, + AgentTestSuite suite) + { + var warnings = new List(); + + foreach (var field in attempt.RejectedFields) + { + warnings.Add($"the model asked to change '{field}', which is not a field it may change; ignored"); + } + + var merged = Clone(baseline); + + if (attempt.DeclaredFields.Count > 0) + { + if (attempt.Draft == null) + { + warnings.Add("the model said it changed the draft but returned no draft; nothing was changed"); + } + else + { + foreach (var field in attempt.DeclaredFields) + { + Apply(merged, attempt.Draft, field); + } + } + } + + // Never from the model: the suite id comes from the request, and only Block is supported. + merged.SuiteId = baseline.SuiteId; + merged.UnmockedToolPolicy = UnmockedToolPolicies.Block; + + Sanitise(merged, targets, existingCases, suite, warnings); + + var validationError = CaseValidation.Validate(merged); + var changes = Diff(baseline, merged); + + return new AgentTestAuthorResponse + { + Reply = attempt.Reply, + Draft = merged, + DraftChanged = changes.Count > 0, + Changes = changes, + ValidationErrors = validationError == null ? [] : [validationError], + Warnings = warnings + }; + } + + /// + /// Copies one declared field off the model's draft. An explicit switch rather than reflection: + /// which fields a model may write is a decision worth being able to read in one place, not one + /// that should emerge from property metadata and change whenever the DTO gains a field. + /// + private static void Apply(AgentTestCaseUpsertRequest target, AgentTestCaseUpsertRequest source, string field) + { + switch (field) + { + case AuthorFields.Name: target.Name = source.Name ?? string.Empty; break; + case AuthorFields.Enabled: target.Enabled = source.Enabled; break; + case AuthorFields.CaseType: target.CaseType = source.CaseType; break; + case AuthorFields.EntryAgentId: target.EntryAgentId = source.EntryAgentId; break; + case AuthorFields.Turns: target.Turns = source.Turns ?? []; break; + case AuthorFields.Assertions: target.Assertions = source.Assertions ?? []; break; + case AuthorFields.InitialStates: target.InitialStates = source.InitialStates ?? []; break; + case AuthorFields.History: target.History = source.History ?? []; break; + case AuthorFields.Mocks: target.Mocks = source.Mocks ?? []; break; + case AuthorFields.Priority: target.Priority = source.Priority; break; + case AuthorFields.Severity: target.Severity = source.Severity; break; + case AuthorFields.Batch: target.Batch = source.Batch; break; + case AuthorFields.CrossCutting: target.CrossCutting = source.CrossCutting; break; + case AuthorFields.InvolvedAgents: target.InvolvedAgents = source.InvolvedAgents ?? []; break; + case AuthorFields.BusinessDomain: target.BusinessDomain = source.BusinessDomain; break; + case AuthorFields.ExpectedOutcome: target.ExpectedOutcome = source.ExpectedOutcome; break; + } + } + + /// + /// Corrects what is provably wrong and flags what is merely suspect. + /// + /// The split matters. A function name is authoritative -- it comes from the agent definition, so a + /// mock naming something else can never match and is dropped. A state key is not: nothing in this + /// system enumerates the keys an agent writes (a state value records the message it was written + /// on and a coarse source, never a function name), so the only key list available is the one + /// other cases in this suite happen to use. Dropping a key for being absent from an admittedly + /// incomplete list would delete correct work, so an unknown key is a warning and stays. + /// + private static void Sanitise( + AgentTestCaseUpsertRequest draft, + List targets, + List existingCases, + AgentTestSuite suite, + List warnings) + { + var callable = new HashSet(targets.Select(t => t.Name), StringComparer.OrdinalIgnoreCase); + + var keptMocks = new List(); + foreach (var mock in draft.Mocks ?? []) + { + if (string.IsNullOrWhiteSpace(mock.FunctionName) || !callable.Contains(mock.FunctionName)) + { + warnings.Add($"dropped a mock for '{mock.FunctionName}': this agent has no such function, " + + "so the mock could never match at run time"); + continue; + } + + keptMocks.Add(mock); + } + draft.Mocks = keptMocks; + + var turns = new List(); + foreach (var (turn, index) in (draft.Turns ?? []).Select((t, i) => (t, i))) + { + // Re-indexed here as well as in the editor: the runner reads turns in order, and a model + // that renumbered them while inserting one would silently reorder the case. + turn.Index = index; + turn.Assertions = FilterAssertions(turn.Assertions, callable, $"turn {index + 1}", warnings); + turns.Add(turn); + } + draft.Turns = turns; + + draft.Assertions = FilterAssertions(draft.Assertions, callable, "the case", warnings); + + var knownStateKeys = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var key in StateKeysOf(existingCases)) + { + knownStateKeys.Add(key); + } + + var allAssertions = (draft.Assertions ?? []) + .Concat((draft.Turns ?? []).SelectMany(t => t.Assertions ?? [])) + .ToList(); + + var assertedStateKeys = allAssertions + .Where(a => string.Equals(a.Type, AssertionTypes.StateEquals, StringComparison.Ordinal)) + .Select(a => a.Target) + .Where(k => !string.IsNullOrWhiteSpace(k)) + .Select(k => k!) + .Distinct(StringComparer.OrdinalIgnoreCase); + + foreach (var key in assertedStateKeys) + { + if (!knownStateKeys.Contains(key)) + { + warnings.Add($"state key '{key}' is not used by any other case in this suite; " + + "check it is the key the agent really writes"); + } + } + + var usesJudge = allAssertions.Any(a => string.Equals(a.Type, AssertionTypes.LlmJudge, StringComparison.Ordinal)); + if (usesJudge + && (string.IsNullOrWhiteSpace(suite.JudgeProvider) || string.IsNullOrWhiteSpace(suite.JudgeModel))) + { + warnings.Add("this draft uses llmJudge but the suite has no judge model configured, so that " + + "assertion will fail rather than pass when the case runs"); + } + } + + /// + /// Every conversation state key any case in the suite touches -- injected, written by a mock, or + /// asserted on. The nearest thing to a state key catalogue this system has; see + /// for why it is treated as incomplete. + /// + private static IEnumerable StateKeysOf(List cases) + => cases + .SelectMany(c => c.InitialStates.Select(s => s.Key) + .Concat(c.Mocks.SelectMany(m => m.StateWrites ?? []).Select(s => s.Key)) + .Concat(c.Assertions.Concat(c.Turns.SelectMany(t => t.Assertions)) + .Where(a => string.Equals(a.Type, AssertionTypes.StateEquals, StringComparison.Ordinal)) + .Select(a => a.Target ?? string.Empty))) + .Where(k => !string.IsNullOrWhiteSpace(k)) + .Distinct(StringComparer.OrdinalIgnoreCase) + .OrderBy(k => k, StringComparer.OrdinalIgnoreCase); + + private static List FilterAssertions( + List? assertions, + HashSet callable, + string where, + List warnings) + { + var kept = new List(); + + foreach (var assertion in assertions ?? []) + { + var isToolAssertion = + string.Equals(assertion.Type, AssertionTypes.ToolCalled, StringComparison.Ordinal) + || string.Equals(assertion.Type, AssertionTypes.ToolNotCalled, StringComparison.Ordinal); + + if (isToolAssertion && !string.IsNullOrWhiteSpace(assertion.Target) && !callable.Contains(assertion.Target!)) + { + warnings.Add($"dropped a {assertion.Type} assertion on {where}: this agent has no function " + + $"called '{assertion.Target}'"); + continue; + } + + kept.Add(assertion); + } + + return kept; + } + + /// + /// What actually changed, by comparing the two drafts field by field. Only whitelisted fields are + /// compared, because the merge cannot have moved anything else. + /// + private static List Diff(AgentTestCaseUpsertRequest before, AgentTestCaseUpsertRequest after) + { + var changes = new List(); + + foreach (var field in AuthorFields.All) + { + var oldValue = Read(before, field); + var newValue = Read(after, field); + + if (string.Equals(Json(oldValue), Json(newValue), StringComparison.Ordinal)) + { + continue; + } + + changes.Add(new AuthorChange { Field = field, Detail = Describe(oldValue, newValue) }); + } + + return changes; + } + + private static object? Read(AgentTestCaseUpsertRequest draft, string field) => field switch + { + AuthorFields.Name => draft.Name, + AuthorFields.Enabled => draft.Enabled, + AuthorFields.CaseType => draft.CaseType, + AuthorFields.EntryAgentId => draft.EntryAgentId, + AuthorFields.Turns => draft.Turns, + AuthorFields.Assertions => draft.Assertions, + AuthorFields.InitialStates => draft.InitialStates, + AuthorFields.History => draft.History, + AuthorFields.Mocks => draft.Mocks, + AuthorFields.Priority => draft.Priority, + AuthorFields.Severity => draft.Severity, + AuthorFields.Batch => draft.Batch, + AuthorFields.CrossCutting => draft.CrossCutting, + AuthorFields.InvolvedAgents => draft.InvolvedAgents, + AuthorFields.BusinessDomain => draft.BusinessDomain, + AuthorFields.ExpectedOutcome => draft.ExpectedOutcome, + _ => null + }; + + private static string Describe(object? before, object? after) + { + if (before is System.Collections.ICollection oldList && after is System.Collections.ICollection newList) + { + return oldList.Count == newList.Count + ? $"{newList.Count} item(s), edited" + : $"{oldList.Count} -> {newList.Count} item(s)"; + } + + return $"{Truncate(Text(before), 60)} -> {Truncate(Text(after), 60)}"; + } + + private static string Text(object? value) + => value is null || (value is string s && string.IsNullOrWhiteSpace(s)) + ? "(empty)" + : value.ToString() ?? "(empty)"; + + private static string Json(object? value) => JsonSerializer.Serialize(value, DraftJson); + + private static AgentTestCaseUpsertRequest Clone(AgentTestCaseUpsertRequest source) + => JsonSerializer.Deserialize(Json(source), DraftJson) + ?? new AgentTestCaseUpsertRequest(); + + private static string? ExtractJson(string raw) + { + var start = raw.IndexOf('{'); + var end = raw.LastIndexOf('}'); + return start >= 0 && end > start ? raw[start..(end + 1)] : null; + } + + /// + /// Fixes the one JSON-shape mistake models reliably make against this schema: writing a + /// "JSON as text" field -- an assertion or mock's argsMatchJson, a mock's resultContent, or a + /// state's value -- as a nested object or array instead of a string holding escaped JSON. Both + /// forms are syntactically valid JSON (which is why cannot catch this), + /// they only disagree on the CLR type the strongly-typed field expects, and re-serialising the + /// nested value back to text is the one lossless, unambiguous reading of what the model meant -- + /// unlike LlmAgentTestJudge.ParseVerdict's out-of-range score, there is no second plausible + /// interpretation here to guard against by rejecting instead of coercing. + /// + /// Silent about anything it does not recognise: a genuine JSON syntax error is left for the real + /// deserializer in to report, with the exact path and reason a retry needs. + /// + private static string CoerceStructuredJsonStringFields(string json) + { + JsonNode? root; + try + { + root = JsonNode.Parse(json); + } + catch (JsonException) + { + return json; + } + + if (root is not JsonObject envelope || FindObject(envelope, "draft") is not { } draft) + { + return json; + } + + foreach (var assertion in FindArrayOfObjects(draft, "assertions")) + { + StringifyIfStructured(assertion, "argsMatchJson"); + } + + foreach (var turn in FindArrayOfObjects(draft, "turns")) + { + foreach (var assertion in FindArrayOfObjects(turn, "assertions")) + { + StringifyIfStructured(assertion, "argsMatchJson"); + } + } + + foreach (var mock in FindArrayOfObjects(draft, "mocks")) + { + StringifyIfStructured(mock, "argsMatchJson"); + StringifyIfStructured(mock, "resultContent"); + + foreach (var write in FindArrayOfObjects(mock, "stateWrites")) + { + StringifyIfStructured(write, "value"); + } + } + + foreach (var state in FindArrayOfObjects(draft, "initialStates")) + { + StringifyIfStructured(state, "value"); + } + + return envelope.ToJsonString(); + } + + private static JsonObject? FindObject(JsonObject obj, string name) + => FindProperty(obj, name).Value as JsonObject; + + private static IEnumerable FindArrayOfObjects(JsonObject? obj, string name) + { + if (obj != null && FindProperty(obj, name).Value is JsonArray array) + { + foreach (var item in array) + { + if (item is JsonObject o) + { + yield return o; + } + } + } + } + + /// + /// Case-insensitive lookup: this runs before 's + /// PropertyNameCaseInsensitive gets a say, and a model that capitalises a field unexpectedly + /// should not skip normalisation just because the casing assumed here did not match verbatim. + /// + private static KeyValuePair FindProperty(JsonObject obj, string name) + => obj.FirstOrDefault(kv => string.Equals(kv.Key, name, StringComparison.OrdinalIgnoreCase)); + + private static void StringifyIfStructured(JsonObject obj, string propertyName) + { + var found = FindProperty(obj, propertyName); + if (found.Key != null && found.Value is JsonObject or JsonArray) + { + obj[found.Key] = JsonValue.Create(found.Value!.ToJsonString()); + } + } + + private static string Truncate(string text, int max) + => text.Length <= max ? text : text[..max].TrimEnd() + "..."; + + /// + /// The rules. The assertion vocabulary is generated from + /// rather than written out here, so a new assertion + /// type cannot exist in validation and be invisible to the author. + /// + private static string BuildInstruction() + { + var builder = new StringBuilder(); + + builder.AppendLine( + """ + You help a QA engineer write and edit ONE regression test case for a customer-service AI + agent, by conversation. You do not run tests and you do not save anything: you propose a + draft, and the human saves it. + + A case is: some turns of user messages, mocked tool returns so that nothing real happens, + and assertions that decide pass or fail. + + HOW TO ANSWER + - Reply in the language the user is writing in. + - `reply` is what you say to them: what you changed and why, or a question when their + request is genuinely ambiguous. Asking is better than guessing at a business rule. + - `changedFields` lists ONLY the top-level fields you are changing this turn. Fields you + do not list are kept exactly as they are, so listing a field you did not mean to touch + is how someone's work gets lost. + - When you are only answering a question or asking one, return an empty `changedFields`. + - Return the FULL new value of every field you list, never a fragment. To add one turn, + return all the turns including the new one. + + WHAT MAKES A GOOD ASSERTION + - Prefer assertions about what the agent DID: toolCalled, stateEquals, routedToAgent, + agentChain. Those survive a reworded reply. + - Avoid outputContains unless the user names the exact text that matters (an id format, a + required disclosure). Never write one from wording you imagined the agent would use -- + it fails the first time the model phrases things differently. + - For "is the reply any good" requirements use llmJudge, with the criterion in `expected`. + - Only use function names from CALLABLE FUNCTIONS. Never invent one. + - Only use state keys from KNOWN STATE KEYS, or ones the user gave you. If you need a key + you do not have, ask for it. + - Every tool the case will trigger needs a mock, or the run blocks the call and the case + fails. When a mocked function passes data to later turns through conversation state, + put that in the mock's stateWrites. + + FIELDS THAT HOLD JSON AS TEXT + argsMatchJson (on an assertion or a mock), a mock's resultContent, and any state "value" + are JSON STRINGS, not nested objects: their value must be the escaped JSON text, exactly + like this -- + "argsMatchJson": "{\"work_order_id\":\"12345\"}" + NOT this -- + "argsMatchJson": {"work_order_id": "12345"} + The second form is invalid for that field even though it is valid JSON overall. + + ASSERTION TYPES + """); + + foreach (var type in AssertionValidation.Authorable) + { + var required = AssertionValidation.RequiredFieldName(type); + var purpose = type switch + { + AssertionTypes.OutputContains => "the reply contains this text", + AssertionTypes.OutputNotContains => "the reply does not contain this text", + AssertionTypes.OutputRegex => "the reply matches this regular expression", + AssertionTypes.ToolCalled => "this function was called; the optional argsMatchJson is an argument subset", + AssertionTypes.ToolNotCalled => "this function was not called", + AssertionTypes.StateEquals => "conversation state at this key equals expected", + AssertionTypes.RoutedToAgent => "the agent named in expected handled the conversation", + AssertionTypes.AgentChain => "expected is a comma-separated agent list, target is contains|ordered|exact", + AssertionTypes.LlmJudge => "a model scores the reply 1-5 against the criterion in expected; minScore is the bar, 4 by default", + _ => "see the case editor" + }; + + builder.Append("- ").Append(type); + if (required != null) + { + builder.Append(" (requires `").Append(required).Append("`)"); + } + builder.Append(": ").AppendLine(purpose); + } + + builder.AppendLine( + """ + + CASE TYPE RULES + - caseType "Routing" means "did the router pick the right agent": exactly one turn, at + least one routedToAgent or agentChain assertion, and no llmJudge. + - caseType "Agent" is everything else, including multi-agent journeys. + - priority P0/P1/P2 decides which batch runs first; severity S0/S1/S2 says what a failure + means. Leave them at P1/S1 unless the user says otherwise. + + OUTPUT + Reply with JSON only. No prose outside it and no code fence: + {"reply":"...","changedFields":["turns"],"draft":{ the full case draft }} + + The draft uses exactly the field names shown in CURRENT DRAFT. + """); + + return builder.ToString(); + } + + private static string BuildContext( + Agent agent, + List targets, + List existingCases, + string? caseId, + AgentTestCaseResult? grounding, + List agentNames) + { + var builder = new StringBuilder(); + + builder.AppendLine("AGENT UNDER TEST").AppendLine($"name: {agent.Name}"); + + if (!string.IsNullOrWhiteSpace(agent.Description)) + { + builder.AppendLine($"description: {agent.Description}"); + } + + if (!string.IsNullOrWhiteSpace(agent.Instruction)) + { + // The most useful thing here -- it is where the business rules and the required slots + // live -- and also the longest, so it is capped rather than left to crowd out the + // function catalogue and the existing cases. + builder.AppendLine("instruction:").AppendLine(Truncate(agent.Instruction, MaxInstructionChars)); + } + + builder.AppendLine().AppendLine("CALLABLE FUNCTIONS (mock and assert against these names only)"); + if (targets.Count == 0) + { + builder.AppendLine("(none -- this agent calls no tools, so the case needs no mocks)"); + } + + foreach (var target in targets) + { + builder.Append("- ").Append(target.Name); + if (!string.IsNullOrWhiteSpace(target.Parameters)) + { + builder.Append(" [").Append(target.Parameters).Append(']'); + } + if (!string.IsNullOrWhiteSpace(target.Description)) + { + builder.Append(": ").Append(Truncate(target.Description!, 200)); + } + builder.AppendLine(); + } + + if (agentNames.Count > 0) + { + builder.AppendLine().AppendLine("AGENTS THAT EXIST (for routedToAgent and agentChain)") + .AppendLine(string.Join(", ", agentNames)); + } + + var stateKeys = StateKeysOf(existingCases).ToList(); + builder.AppendLine().AppendLine("KNOWN STATE KEYS (from other cases in this suite -- not a complete list)") + .AppendLine(stateKeys.Count == 0 ? "(none yet)" : string.Join(", ", stateKeys)); + + var others = existingCases + .Where(c => !string.Equals(c.Id, caseId, StringComparison.Ordinal)) + .Take(MaxExistingCases) + .ToList(); + + if (others.Count > 0) + { + // Names and shapes only, never the bodies: this exists so the model does not write a + // duplicate, and a full dump of every case would cost more than the whole rest of the + // prompt. + builder.AppendLine().AppendLine("CASES ALREADY IN THIS SUITE (do not duplicate them)"); + foreach (var other in others) + { + var assertionTypes = string.Join("/", other.Assertions + .Concat(other.Turns.SelectMany(t => t.Assertions)) + .Select(a => a.Type) + .Distinct(StringComparer.Ordinal)); + + builder.Append("- ").Append(other.Name) + .Append(" [").Append(other.CaseType).Append(", ").Append(other.Turns.Count).Append(" turn(s)"); + + if (!string.IsNullOrWhiteSpace(assertionTypes)) + { + builder.Append(", ").Append(assertionTypes); + } + + builder.AppendLine("]"); + } + } + + if (grounding != null) + { + builder.AppendLine().AppendLine( + $"WHAT HAPPENED LAST TIME THIS CASE RAN (status {grounding.Status}) -- real replies and real " + + "tool arguments. Base any outputContains or argsMatchJson on THIS, not on invention."); + + foreach (var turn in grounding.Turns.Take(MaxGroundedTurns)) + { + builder.Append("- turn ").Append(turn.Index).Append(" user: ").AppendLine(Truncate(turn.UserMessage, 200)); + + if (!string.IsNullOrWhiteSpace(turn.Output)) + { + builder.Append(" agent said: ").AppendLine(Truncate(turn.Output!, MaxGroundedOutputChars)); + } + + var failed = string.Join("; ", turn.Assertions + .Where(a => !a.Passed) + .Select(a => $"{a.Type} ({a.Message})")); + + if (!string.IsNullOrWhiteSpace(failed)) + { + builder.Append(" failed: ").AppendLine(Truncate(failed, 300)); + } + } + + foreach (var call in grounding.ObservedToolCalls.Take(MaxGroundedToolCalls)) + { + builder.Append("- turn ").Append(call.TurnIndex).Append(" called ").Append(call.FunctionName) + .Append(" [").Append(call.Outcome).Append(']'); + + if (!string.IsNullOrWhiteSpace(call.ArgsJson)) + { + builder.Append(" args: ").Append(Truncate(call.ArgsJson!, MaxGroundedArgsChars)); + } + + builder.AppendLine(); + } + } + + return builder.ToString(); + } + + private static string BuildTask(AgentTestCaseUpsertRequest draft, string instruction) + { + var builder = new StringBuilder(); + + builder.AppendLine("CURRENT DRAFT").AppendLine(JsonSerializer.Serialize(draft, DraftJson)).AppendLine(); + builder.AppendLine("WHAT TO DO").AppendLine(instruction).AppendLine(); + + // The format is restated here, not only in the instruction: the replayed assistant turns of + // this chat are plain prose (only the `reply` field is kept client-side), and without a + // reminder next to the actual request a model will follow that example and answer in prose. + builder.AppendLine( + """ + Answer with the JSON envelope only: + {"reply":"...","changedFields":[...],"draft":{...}} + """); + + return builder.ToString(); + } +} + +/// One parsed model answer, before it is merged or trusted. +/// What to show the user. +/// Recognised field names from changedFields. +/// changedFields entries naming no writable field. +/// The model's draft; only declared fields are ever read from it. +/// The raw reply, replayed to the model when asking it to repair a rejection. +public record AuthorAttempt( + string Reply, + List DeclaredFields, + List RejectedFields, + AgentTestCaseUpsertRequest? Draft, + string Raw); + +/// Wire shape of the model's answer. +internal class AuthorEnvelope +{ + public string? Reply { get; set; } + public List? ChangedFields { get; set; } + public AgentTestCaseUpsertRequest? Draft { get; set; } +} diff --git a/src/Plugins/BotSharp.Plugin.AgentTesting/Services/MockTargetCatalogue.cs b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/MockTargetCatalogue.cs new file mode 100644 index 000000000..4a180e9d5 --- /dev/null +++ b/src/Plugins/BotSharp.Plugin.AgentTesting/Services/MockTargetCatalogue.cs @@ -0,0 +1,112 @@ +using BotSharp.Abstraction.Functions.Models; +using System.Text; +using System.Text.Json; + +namespace BotSharp.Plugin.AgentTesting.Services; + +/// +/// The functions an agent can actually call, derived live from the agent definition. +/// +/// Two shapes of the same list, deliberately kept together so they cannot drift: +/// is what GET /agent-test/mock-targets returns to the case editor, and +/// is the richer form puts in front of a model. +/// +/// Derived live rather than read from IFunctionCallback-full-detail-report.md on purpose: that +/// document is a point-in-time snapshot and drifts, and a mock authored against a function this +/// agent cannot call is a case that can never pass. +/// +public static class MockTargetCatalogue +{ + /// Function names only, sorted and de-duplicated. The wire shape the UI consumes. + public static List Names(Agent agent) + => Describe(agent).Select(t => t.Name).ToList(); + + /// + /// Every callable function with whatever description and parameter shape the agent definition + /// carries. + /// + /// MCP tools come back name-only: has nothing but a Name, so a model + /// authoring a mock for one is working from the name alone. That is a real gap, not an oversight + /// here -- it is why an MCP mock's argsMatchJson is more likely to need a human fix than a + /// plugin function's. + /// + public static List Describe(Agent agent) + { + var targets = new List(); + + foreach (var fn in (agent.Functions ?? []).Concat(agent.SecondaryFunctions ?? [])) + { + if (string.IsNullOrWhiteSpace(fn?.Name)) continue; + targets.Add(new MockTargetInfo(fn!.Name, fn.Description, ParameterSummary(fn.Parameters))); + } + + foreach (var fn in (agent.McpTools ?? []).SelectMany(t => t.Functions ?? [])) + { + if (string.IsNullOrWhiteSpace(fn?.Name)) continue; + targets.Add(new MockTargetInfo(fn!.Name, null, null)); + } + + // First entry wins on a duplicate name: a function declared both primary and secondary is + // one function, and the primary declaration is the one with the fuller definition. + return targets + .GroupBy(t => t.Name, StringComparer.OrdinalIgnoreCase) + .Select(g => g.First()) + .OrderBy(t => t.Name, StringComparer.OrdinalIgnoreCase) + .ToList(); + } + + /// + /// A one-line rendering of a function's parameters, e.g. work_order_id* (string), note, + /// with a star on the required ones. + /// + /// Flattened rather than passed as raw JSON schema: the whole catalogue goes into one prompt, and + /// full schemas for thirty functions crowd out the agent instruction and the existing cases -- + /// which are what actually make an authored case realistic. Names and types are enough for the + /// only thing the model writes with them, an argsMatchJson subset. + /// + private static string? ParameterSummary(FunctionParametersDef? parameters) + { + var properties = parameters?.Properties; + if (properties == null) return null; + + JsonElement root; + try + { + root = properties.RootElement; + } + catch (ObjectDisposedException) + { + // The agent definition is shared, and a JsonDocument someone else already disposed must + // not take down the authoring request with it. + return null; + } + + if (root.ValueKind != JsonValueKind.Object) return null; + + var required = parameters!.Required ?? []; + var parts = new List(); + + foreach (var property in root.EnumerateObject()) + { + var builder = new StringBuilder(property.Name); + if (required.Contains(property.Name, StringComparer.OrdinalIgnoreCase)) builder.Append('*'); + + if (property.Value.ValueKind == JsonValueKind.Object + && property.Value.TryGetProperty("type", out var type) + && type.ValueKind == JsonValueKind.String) + { + builder.Append(" (").Append(type.GetString()).Append(')'); + } + + parts.Add(builder.ToString()); + } + + return parts.Count == 0 ? null : string.Join(", ", parts); + } +} + +/// One callable function, as much as the agent definition knows about it. +/// The function name a mock or a toolCalled assertion has to match exactly. +/// Null for MCP tools -- see . +/// One-line parameter summary, or null when the function takes none. +public record MockTargetInfo(string Name, string? Description, string? Parameters); diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentChainAssertionTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentChainAssertionTests.cs new file mode 100644 index 000000000..4772510e9 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentChainAssertionTests.cs @@ -0,0 +1,282 @@ +using System.Collections.Generic; +using System.Linq; +using BotSharp.Plugin.AgentTesting.Services; +using BotSharp.Plugin.AgentTesting.Models; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// agentChain is the only assertion that can describe a hand-off, so its three modes are the whole +/// vocabulary available for asserting hand-offs. routedToAgent answers "who spoke last", which +/// cannot express a hand-off at all: for Entry -> A -> B it sees only B, and when control returns to +/// the entry agent and that agent closes the conversation it sees the entry agent, so a correctly +/// routed case reads as a routing failure. +/// +/// Two properties matter more than the happy paths and are pinned first: an empty expected list and +/// an unrecognised mode both have to FAIL rather than pass vacuously or fall back to the loosest +/// check, because either would show a case that verified nothing as green. +/// +public class AgentChainAssertionTests +{ + /// + /// Names only, ids left blank. The id-matching path has its own tests at the end of the file, + /// where the distinction between the two identifiers is the point. + /// + private static AssertionContext Context(params string[] chain) => new() + { + AgentChain = chain.Select(name => new AgentChainHop { Id = string.Empty, Name = name }).ToList() + }; + + private static AssertionResult Evaluate(string? expected, string? mode, params string[] chain) + => AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.AgentChain, Expected = expected, Target = mode }, + Context(chain)); + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData(",")] + [InlineData(" , , ")] + public void An_empty_expected_list_fails_instead_of_passing_vacuously(string? expected) + { + // An empty list is a subset of, and an ordered subsequence of, every chain, so Contains and + // Ordered would both pass while checking nothing. Exact would assert "no agent ever + // answered", which no author writes on purpose. Note that "," and " , , " parse to an empty + // list too, so the guard has to be on the parsed result rather than on the raw string. + var result = Evaluate(expected, mode: null, "Copilot"); + + Assert.False(result.Passed); + Assert.Contains("non-empty", result.Message); + } + + [Theory] + [InlineData("orderd")] + [InlineData("subsequence")] + [InlineData("any")] + public void An_unrecognised_mode_fails_rather_than_falling_back_to_the_loosest_check(string mode) + { + // Defaulting a typo to Contains would silently downgrade "these agents, in this order" to + // "these agents, in any order" -- the assertion still goes green, having verified much less + // than it says. The message names the accepted values so the author can fix it. + var result = Evaluate("Copilot, Work Order Creator", mode, "Work Order Creator", "Copilot"); + + Assert.False(result.Passed); + Assert.Contains(AgentChainModes.Ordered, result.Message); + Assert.Contains(mode, result.Message); + } + + [Fact] + public void A_blank_mode_means_contains() + { + // The common case is "this agent must have been involved", so the default is the loosest + // mode -- but only when the author left it blank, never as a recovery from a typo. + var result = Evaluate("Work Order Creator", mode: null, "Copilot", "Work Order Creator"); + + Assert.True(result.Passed); + } + + [Fact] + public void Contains_ignores_order() + { + var result = Evaluate( + "Work Order Creator, Copilot", AgentChainModes.Contains, "Copilot", "Work Order Creator"); + + Assert.True(result.Passed); + } + + [Fact] + public void Contains_names_the_agents_that_are_missing() + { + // Which agent was absent is the whole diagnostic. "the chain does not match" would leave the + // author diffing two lists by eye. + var result = Evaluate( + "Copilot, Diagnosis, Work Order Creator", AgentChainModes.Contains, "Copilot", "Work Order Creator"); + + Assert.False(result.Passed); + Assert.Contains("Diagnosis", result.Message); + Assert.DoesNotContain("Copilot", result.Message); + } + + [Fact] + public void Ordered_allows_other_agents_in_between() + { + // Asserting the hand-offs an author cares about must not require enumerating every agent the + // conversation happened to pass through -- that is what Exact is for. + var result = Evaluate( + "Copilot, Work Order Creator", AgentChainModes.Ordered, + "Copilot", "Diagnosis", "Work Order Creator"); + + Assert.True(result.Passed); + } + + [Fact] + public void Ordered_rejects_the_reverse_order() + { + // The one thing Contains cannot see. Both agents are present, so Contains would pass; the + // hand-off went the wrong way round. + var chain = new[] { "Work Order Creator", "Copilot" }; + + Assert.True(Evaluate("Copilot, Work Order Creator", AgentChainModes.Contains, chain).Passed); + Assert.False(Evaluate("Copilot, Work Order Creator", AgentChainModes.Ordered, chain).Passed); + } + + [Fact] + public void Ordered_matches_a_repeated_agent_against_a_return_hop() + { + // The runner collapses only CONSECUTIVE repeats, so a genuine return hop survives in the + // chain -- and this is the assertion that reads it. Requiring Copilot twice must match + // Copilot -> WO -> Copilot and not Copilot -> WO. + Assert.True(Evaluate( + "Copilot, Work Order Creator, Copilot", AgentChainModes.Ordered, + "Copilot", "Work Order Creator", "Copilot").Passed); + + Assert.False(Evaluate( + "Copilot, Work Order Creator, Copilot", AgentChainModes.Ordered, + "Copilot", "Work Order Creator").Passed); + } + + [Fact] + public void Exact_rejects_an_extra_agent_that_ordered_would_allow() + { + var chain = new[] { "Copilot", "Diagnosis", "Work Order Creator" }; + + Assert.True(Evaluate("Copilot, Work Order Creator", AgentChainModes.Ordered, chain).Passed); + Assert.False(Evaluate("Copilot, Work Order Creator", AgentChainModes.Exact, chain).Passed); + } + + [Fact] + public void Exact_on_a_single_agent_is_how_isolation_is_asserted() + { + // An Agent case is supposed to measure one agent with the router out of the picture, but + // route_to_agent stays on the allow list during it, so a leaf agent that routes onward turns + // the case into a multi-agent one with nothing to show it. An Exact chain of one agent is the + // assertion that catches that. + Assert.True(Evaluate("Work Order Creator", AgentChainModes.Exact, "Work Order Creator").Passed); + Assert.False(Evaluate( + "Work Order Creator", AgentChainModes.Exact, "Work Order Creator", "Diagnosis").Passed); + } + + [Fact] + public void Agent_names_are_matched_case_insensitively_and_trimmed() + { + // Authors type agent names by hand and copy them out of the UI, so casing and stray spaces + // around the commas are not a reason to fail a case. Mirrors routedToAgent, which has always + // compared OrdinalIgnoreCase. + var result = Evaluate( + " copilot , WORK ORDER CREATOR ", AgentChainModes.Ordered, "Copilot", "Work Order Creator"); + + Assert.True(result.Passed); + } + + [Fact] + public void The_actual_chain_is_reported_in_a_readable_form() + { + // The chain is what the author has to reason about when the assertion fails, and a bare + // list would be rendered by the UI as an opaque blob. Arrow-joined mirrors how the hand-off + // reads in the conversation. + var result = Evaluate("Nobody", AgentChainModes.Contains, "Copilot", "Work Order Creator"); + + Assert.Equal("Copilot -> Work Order Creator", result.Actual); + } + + [Fact] + public void An_empty_chain_fails_every_mode_but_never_throws() + { + // A case whose agent never answered (or one that errored before any turn ran) leaves the + // chain empty. That has to be an ordinary failing assertion, not an exception that turns the + // case into an infrastructure Error and hides the real problem. + foreach (var mode in AgentChainModes.All) + { + var result = Evaluate("Copilot", mode); + + Assert.False(result.Passed); + Assert.Equal(string.Empty, result.Actual); + } + } + + [Fact] + public void Save_time_validation_requires_the_expected_list() + { + // The same guard as at evaluation time, but at case create/update, so a chain assertion with + // nothing to compare against is rejected while its author is still looking at it. + Assert.NotNull(AssertionValidation.Validate( + new TestAssertion { Type = AssertionTypes.AgentChain, Target = AgentChainModes.Ordered })); + + Assert.Null(AssertionValidation.Validate( + new TestAssertion { Type = AssertionTypes.AgentChain, Expected = "Copilot" })); + } + + private static AssertionContext ChainOf(params string[] idAndName) => new() + { + // Pairs, flattened: ("id1", "Name 1", "id2", "Name 2"). + AgentChain = Enumerable.Range(0, idAndName.Length / 2) + .Select(i => new AgentChainHop { Id = idAndName[i * 2], Name = idAndName[i * 2 + 1] }) + .ToList() + }; + + [Fact] + public void An_expected_agent_may_be_given_as_an_id_instead_of_a_name() + { + // The id is exactly what an author copies out of the agent list, and the first real routing + // case did precisely that: it asserted routedToAgent against a guid while the chain reported + // a display name, so it could never pass no matter what the agent did. + var context = ChainOf("0fe3905d-75f1-4e2e-8e54-ec3d33d6b6f0", "WO Cancellation"); + + var byId = AssertionEvaluator.Evaluate( + new TestAssertion + { + Type = AssertionTypes.RoutedToAgent, + Expected = "0fe3905d-75f1-4e2e-8e54-ec3d33d6b6f0" + }, + context); + + var byName = AssertionEvaluator.Evaluate( + new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "WO Cancellation" }, + context); + + Assert.True(byId.Passed); + Assert.True(byName.Passed); + + // Either way the reported actual is the readable name, never the guid. + Assert.Equal("WO Cancellation", byId.Actual); + } + + [Fact] + public void An_agent_chain_accepts_ids_names_and_a_mixture_of_both() + { + // Authors paste whichever identifier is in front of them, and a chain listing several agents + // is the most likely place to end up with a mixture. + var context = ChainOf( + "2cd4b805-7078-4405-87e9-2ec9aadf8a11", "Lessen Copilot", + "0fe3905d-75f1-4e2e-8e54-ec3d33d6b6f0", "WO Cancellation"); + + var result = AssertionEvaluator.Evaluate( + new TestAssertion + { + Type = AssertionTypes.AgentChain, + Target = AgentChainModes.Ordered, + Expected = "2cd4b805-7078-4405-87e9-2ec9aadf8a11, WO Cancellation" + }, + context); + + Assert.True(result.Passed); + } + + [Fact] + public void A_wrong_id_still_fails() + { + // Accepting either identifier must not become accepting anything: an id belonging to some + // other agent has to fail exactly as a wrong name does. + var result = AssertionEvaluator.Evaluate( + new TestAssertion + { + Type = AssertionTypes.RoutedToAgent, + Expected = "11111111-2222-3333-4444-555555555555" + }, + ChainOf("0fe3905d-75f1-4e2e-8e54-ec3d33d6b6f0", "WO Cancellation")); + + Assert.False(result.Passed); + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestCaseRunnerTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestCaseRunnerTests.cs index 373aecf63..529c8b5d9 100644 --- a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestCaseRunnerTests.cs +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestCaseRunnerTests.cs @@ -10,6 +10,8 @@ using BotSharp.Plugin.AgentTesting.Services; using BotSharp.Plugin.AgentTesting.Models; using Xunit; +using BotSharp.Abstraction.Conversations; +using BotSharp.Abstraction.Conversations.Models; namespace BotSharp.Core.UnitTests.AgentTesting; @@ -32,6 +34,51 @@ private sealed class FakeDriver : IAgentConversationDriver public string? RoutedAgent { get; set; } public TimeSpan SendDelay { get; set; } = TimeSpan.Zero; + /// + /// The agents that answer each turn, one entry per SendAsync in call order. Lets a test model + /// a hand-off inside a single turn (["Copilot", "WorkOrder"]), one agent answering several + /// times in a row (["A", "A"]), or a turn that produced no answer at all ([]) -- none of + /// which can express. A turn with no entry here is answered by + /// RoutedAgent, which is what leaves the pre-existing single-agent tests untouched. + /// + public List AgentsPerTurn { get; } = []; + + /// + /// The accumulated assistant-message sequence, exactly as the real driver reads it back out + /// of the dialog store: never sliced per turn and never de-duplicated, because slicing and + /// collapsing are the runner's job and that is what these tests are checking. + /// + private readonly List _agentSequence = []; + + /// Everything InjectHistoryAsync was handed, in order. + public List InjectedHistory { get; } = []; + + /// + /// Overrides what InjectHistoryAsync reports as written, to simulate the real failure mode: + /// AppendConversationDialogs is an UpdateOne with no upsert, so it silently writes nothing + /// when the dialog document is missing. Null reports every message as written. + /// + public int? HistoryWriteCount { get; set; } + + /// + /// Agent the authored assistant history is attributed to. Set it to something the turns do + /// NOT use to prove the history is excluded from the chain rather than merely collapsed into + /// the first turn's entry. + /// + public string? HistoryAgentName { get; set; } + + /// + /// Runs at the start of each SendAsync. Lets a test move an external counter -- the token + /// meter -- during the case rather than only before or after it, which is the only way to + /// exercise the delta. + /// + public Action? OnSend { get; set; } + + /// Which agent each call was made against, so a test can prove which one won. + public string? PreparedAgentId { get; private set; } + public string? CanaryAgentId { get; private set; } + public List SentAgentIds { get; } = []; + // Lets the empty-Turns guard test assert that the seam was never touched at all, not merely // that no message was sent. public bool PrepareCalled { get; private set; } @@ -56,6 +103,7 @@ private sealed class FakeDriver : IAgentConversationDriver public Task PrepareAsync(string conversationId, string agentId, IReadOnlyList initialStates) { PrepareCalled = true; + PreparedAgentId = agentId; return Task.CompletedTask; } @@ -70,6 +118,18 @@ public Task PrepareAsync(string conversationId, string agentId, IReadOnlyList SendAsync(string conversationId, string agentId, string userMessage, CancellationToken ct) { Sent.Add(userMessage); + SentAgentIds.Add(agentId); + OnSend?.Invoke(); + + // Appended before any of the failure simulations below, like the real thing: BotSharp + // has already written the assistant dialogs by the time a later step throws. + string[] turnAgents = AgentsPerTurn.Count >= Sent.Count + ? AgentsPerTurn[Sent.Count - 1] + : RoutedAgent == null ? [] : [RoutedAgent]; + // Id mirrors the name here, because the runner collapses consecutive hops BY ID: giving + // every agent the same blank id would collapse a genuine hand-off away. + _agentSequence.AddRange( + turnAgents.Select(name => new AgentChainHop { Id = name, Name = name })); if (BlockedOnFirstSend.Count > 0 && Sent.Count == 1) { @@ -114,23 +174,62 @@ public Task PrepareAsync(string conversationId, string agentId, IReadOnlyList 0 ? Replies.Dequeue() : string.Empty; } + public Task InjectHistoryAsync( + string conversationId, string agentId, IReadOnlyList history) + { + InjectedHistory.AddRange(history); + + // Mirrors the real driver: an authored assistant message becomes a stored assistant + // dialog, so it turns up in the sequence and the runner has to exclude it from the chain. + // A fake that skipped this would let a broken offset pass. + foreach (var message in history) + { + if (string.Equals(message.Role, HistoryRoles.Assistant, StringComparison.OrdinalIgnoreCase)) + { + var name = HistoryAgentName ?? agentId; + _agentSequence.Add(new AgentChainHop { Id = name, Name = name }); + } + } + + return Task.FromResult(HistoryWriteCount ?? history.Count); + } + public Task RunCanaryAsync(string conversationId, string agentId, CancellationToken ct) { CanaryCalled = true; + CanaryAgentId = agentId; return Task.FromResult(CanaryResult); } public Task> ReadStatesAsync(string conversationId) => Task.FromResult>(States); - public Task ReadRoutedAgentNameAsync(string conversationId) - => Task.FromResult(RoutedAgent); + public Task> ReadAssistantAgentSequenceAsync(string conversationId) + => Task.FromResult>(_agentSequence.ToList()); + } + + /// + /// Reports whatever it is told to. Only Total and AccumulatedCost are read by the runner -- the + /// rest of ITokenStatistics exists for the completion providers that feed it. + /// + private sealed class FakeTokens : ITokenStatistics + { + public long Total { get; set; } + public float AccumulatedCost { get; set; } + public float Cost => AccumulatedCost; + + public Task AddToken(TokenStatsModel stats, RoleDialogModel message) => Task.CompletedTask; + public void PrintStatistics() { } + public void StartTimer() { } + public void StopTimer() { } } - private static AgentTestCaseRunner Build(FakeDriver driver, out AgentTestRunRegistry registry) + private static AgentTestCaseRunner Build( + FakeDriver driver, out AgentTestRunRegistry registry, ITokenStatistics? tokens = null) { registry = new AgentTestRunRegistry(); - return new AgentTestCaseRunner(registry, driver, NullLogger.Instance); + return new AgentTestCaseRunner( + registry, driver, NullLogger.Instance, judge: null, tokens: tokens); } private static AgentTestSuite Suite(int timeoutSeconds = 120) => new() @@ -557,4 +656,397 @@ public void Log( Entries.Add((logLevel, exception, formatter(state, exception))); } } + + [Fact] + public async Task The_cases_entry_agent_overrides_the_suites_on_every_driver_call() + { + // This one value decides whether the router is part of what the case measures, because + // BotSharp dispatches on the agent's own type (ConversationService.SendMessage: Routing -> + // InstructLoop and can hand off, anything else -> InstructDirect and cannot). It has to win + // on all three calls -- a case that prepares and sends against the entry agent but runs its + // canary against the suite's would prove the seam live on the wrong conversation shape. + var driver = new FakeDriver { RoutedAgent = "Work Order Creator" }; + driver.Replies.Enqueue("done"); + var runner = Build(driver, out _); + + await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + EntryAgentId = "copilot-entry", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal("copilot-entry", driver.PreparedAgentId); + Assert.Equal("copilot-entry", driver.CanaryAgentId); + Assert.Equal(["copilot-entry"], driver.SentAgentIds); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public async Task A_case_with_no_entry_agent_still_uses_the_suites(string? entryAgentId) + { + // Every case authored before EntryAgentId existed deserialises with it null, so the fallback + // is what keeps those cases running against the same agent they always did. Blank strings go + // the same way: a UI that sends "" for an untouched field must not silently retarget a case + // at an agent id of "". + var driver = new FakeDriver(); + driver.Replies.Enqueue("done"); + var runner = Build(driver, out _); + + await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + EntryAgentId = entryAgentId, + Turns = [new TestTurn { Index = 0, UserMessage = "a" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal("agent-1", driver.PreparedAgentId); + Assert.Equal(["agent-1"], driver.SentAgentIds); + } + + [Fact] + public async Task The_agent_chain_collapses_consecutive_repeats_but_keeps_a_return_hop() + { + // A -> A is one agent sending two messages, not a hand-off, so it collapses. The second A is + // a real return hop and must survive: flattening the chain to distinct agents would make an + // ordered agentChain assertion unable to see the conversation come back. + var driver = new FakeDriver(); + driver.AgentsPerTurn.Add(["Copilot", "Copilot", "Work Order Creator", "Copilot"]); + driver.Replies.Enqueue("done"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(["Copilot", "Work Order Creator", "Copilot"], result.AgentChain); + } + + [Fact] + public async Task Each_turns_chain_is_only_that_turns_slice() + { + // The driver hands back the whole conversation every time, so a turn's chain is the runner's + // own slice. The distinction is load-bearing here: turn 2's chain is ["Work Order Creator"] + // even though the case-level chain collapses that agent together with turn 1's trailing + // entry, so a per-turn chain cannot be derived from the case-level one. + var driver = new FakeDriver(); + driver.AgentsPerTurn.Add(["Copilot", "Work Order Creator"]); + driver.AgentsPerTurn.Add(["Work Order Creator"]); + driver.Replies.Enqueue("one"); + driver.Replies.Enqueue("two"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = + [ + new TestTurn { Index = 0, UserMessage = "a" }, + new TestTurn { Index = 1, UserMessage = "b" } + ] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(["Copilot", "Work Order Creator"], result.Turns[0].AgentChain); + Assert.Equal(["Work Order Creator"], result.Turns[1].AgentChain); + Assert.Equal(["Copilot", "Work Order Creator"], result.AgentChain); + } + + [Fact] + public async Task A_turn_that_produced_no_answer_routes_to_nobody() + { + // Turn-level routedToAgent reads that turn's own last agent, not the conversation's. Were it + // the conversation's, this second turn would inherit turn 1's agent and pass an assertion + // about a turn that never routed anywhere at all. + var driver = new FakeDriver(); + driver.AgentsPerTurn.Add(["Work Order Creator"]); + driver.AgentsPerTurn.Add([]); + driver.Replies.Enqueue("one"); + driver.Replies.Enqueue("two"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = + [ + new TestTurn + { + Index = 0, + UserMessage = "a", + Assertions = + [ + new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "Work Order Creator" } + ] + }, + new TestTurn + { + Index = 1, + UserMessage = "b", + Assertions = + [ + new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "Work Order Creator" } + ] + } + ] + }, "run-1", model: null, CancellationToken.None); + + Assert.True(result.Turns[0].Assertions[0].Passed); + Assert.False(result.Turns[1].Assertions[0].Passed); + Assert.Empty(result.Turns[1].AgentChain); + } + + [Fact] + public async Task An_agent_chain_assertion_sees_a_hand_off_that_routed_to_agent_cannot() + { + // The reason agentChain exists. Control reaches the work order agent and comes back, so the + // last agent to speak is the entry agent -- routedToAgent reports Copilot and a correctly + // routed case looks like a routing failure. The chain still shows the hand-off happened. + var driver = new FakeDriver(); + driver.AgentsPerTurn.Add(["Copilot", "Work Order Creator", "Copilot"]); + driver.Replies.Enqueue("done"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }], + Assertions = + [ + new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "Work Order Creator" }, + new TestAssertion + { + Type = AssertionTypes.AgentChain, + Target = AgentChainModes.Ordered, + Expected = "Copilot, Work Order Creator" + } + ] + }, "run-1", model: null, CancellationToken.None); + + Assert.False(result.Assertions[0].Passed); + Assert.Equal("Copilot", result.Assertions[0].Actual); + Assert.True(result.Assertions[1].Passed); + } + + [Fact] + public async Task The_case_type_is_stamped_on_the_result_even_when_the_case_never_ran() + { + // Routing accuracy is aggregated from the result rows, so a row that cannot say what type of + // case it came from would silently drop out of the figure. The no-turns guard returns before + // the driver is touched at all, which is exactly the path most likely to forget the stamp. + var driver = new FakeDriver(); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + CaseType = CaseTypes.Routing, + Turns = [] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Error, result.Status); + Assert.Equal(CaseTypes.Routing, result.CaseType); + Assert.False(driver.PrepareCalled); + } + + [Fact] + public async Task Authored_history_is_written_before_the_first_turn_is_sent() + { + // The whole point is that the model sees the exchange as the conversation's opening context. + // Written after the first turn it would be context for the second turn onwards, which is a + // different scenario from the one the author wrote. + var driver = new FakeDriver { RoutedAgent = "Work Order Creator" }; + driver.Replies.Enqueue("done"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + History = + [ + new TestHistoryMessage { Role = HistoryRoles.User, Content = "my fridge is leaking" }, + new TestHistoryMessage { Role = HistoryRoles.Assistant, Content = "I raised work order B123." } + ], + Turns = [new TestTurn { Index = 0, UserMessage = "when is someone coming?" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Passed, result.Status); + Assert.Equal(2, driver.InjectedHistory.Count); + Assert.Equal("my fridge is leaking", driver.InjectedHistory[0].Content); + Assert.Equal(["when is someone coming?"], driver.Sent); + } + + [Fact] + public async Task A_history_write_that_silently_did_nothing_errors_the_case() + { + // AppendConversationDialogs is an UpdateOne with no upsert, so a missing dialog document + // makes it a no-op that reports success. Left unchecked, the case would run with no context + // at all and report an ordinary pass or fail about a scenario that never existed -- the same + // "executed nothing, reports green" family as the canary and the no-turns guard. + var driver = new FakeDriver { HistoryWriteCount = 1 }; + driver.Replies.Enqueue("done"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + History = + [ + new TestHistoryMessage { Role = HistoryRoles.User, Content = "a" }, + new TestHistoryMessage { Role = HistoryRoles.Assistant, Content = "b" } + ], + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Error, result.Status); + Assert.Contains("1 of 2 history messages", result.Error); + // And it stopped there rather than running the case anyway. + Assert.Empty(driver.Sent); + } + + [Fact] + public async Task Authored_history_is_excluded_from_the_agent_chain() + { + // Authored history is not something the agent under test did. Counting it would break the + // one assertion that most needs to be trustworthy: an exact chain of a single agent is how a + // case asserts nothing routed away, and a fabricated preamble in the chain would fail it for + // a reason the author never caused. + var driver = new FakeDriver { HistoryAgentName = "Copilot" }; + driver.AgentsPerTurn.Add(["Work Order Creator"]); + driver.Replies.Enqueue("done"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + History = [new TestHistoryMessage { Role = HistoryRoles.Assistant, Content = "earlier answer" }], + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }], + Assertions = + [ + new TestAssertion + { + Type = AssertionTypes.AgentChain, + Target = AgentChainModes.Exact, + Expected = "Work Order Creator" + } + ] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(["Work Order Creator"], result.AgentChain); + Assert.Equal(["Work Order Creator"], result.Turns[0].AgentChain); + Assert.True(result.Assertions[0].Passed); + } + + [Fact] + public async Task Token_usage_is_recorded_as_a_delta_not_an_absolute_reading() + { + // ITokenStatistics is scoped and the queue opens a scope per case, so on that path the + // baseline is zero. But a scope that outlived an earlier case would otherwise have this case + // billed for that one's tokens too, and a cost figure that charges the wrong case is worse + // than none. + var driver = new FakeDriver(); + driver.Replies.Enqueue("done"); + var tokens = new FakeTokens { Total = 500, AccumulatedCost = 0.05f }; + var runner = Build(driver, out _, tokens); + + // Simulates the conversation consuming 300 tokens during the case. + driver.OnSend = () => { tokens.Total = 800; tokens.AccumulatedCost = 0.08f; }; + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(300, result.TotalTokens); + Assert.Equal(0.03, result.Cost, precision: 4); + } + + [Fact] + public async Task Usage_is_still_recorded_when_the_case_times_out() + { + // A run that fell over having burned the budget is exactly the run whose cost matters, so the + // reading happens in the finally block rather than on the success path. + var driver = new FakeDriver { SendDelay = TimeSpan.FromSeconds(5) }; + var tokens = new FakeTokens(); + var runner = Build(driver, out _, tokens); + driver.OnSend = () => { tokens.Total = 120; tokens.AccumulatedCost = 0.01f; }; + + var result = await runner.RunAsync(Suite(timeoutSeconds: 1), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Error, result.Status); + Assert.Equal(120, result.TotalTokens); + } + + [Fact] + public async Task Model_duration_covers_the_agent_call_and_not_the_harness() + { + // A latency gate read against the case's whole wall clock also measures the canary, the mock + // lookups and the conversation reads. On a fast case that overhead is a large enough share to + // move a percentile, so the two figures are kept apart. + var driver = new FakeDriver { SendDelay = TimeSpan.FromMilliseconds(120) }; + driver.Replies.Enqueue("one"); + driver.Replies.Enqueue("two"); + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = + [ + new TestTurn { Index = 0, UserMessage = "a" }, + new TestTurn { Index = 1, UserMessage = "b" } + ] + }, "run-1", model: null, CancellationToken.None); + + // Two turns of ~120ms each, summed onto the case and recorded per turn. + Assert.True(result.ModelDurationMs >= 200, $"was {result.ModelDurationMs}"); + Assert.True(result.Turns[0].ModelDurationMs >= 100, $"was {result.Turns[0].ModelDurationMs}"); + Assert.Equal( + result.ModelDurationMs, + result.Turns.Sum(t => t.ModelDurationMs)); + + // And it is never larger than the case's own wall clock, which contains it. + Assert.True(result.DurationMs >= result.ModelDurationMs); + } + + [Fact] + public async Task A_case_that_never_reached_the_model_reports_zero_model_time() + { + // What keeps a crashed run from looking like the fastest one on record: the executor drops + // these from the latency percentile, and it can only do that because the figure is zero + // rather than the wall clock of the failure. + var driver = new FakeDriver { CanaryResult = false }; + var runner = Build(driver, out _); + + var result = await runner.RunAsync(Suite(), new AgentTestCase + { + Id = "case-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "a" }] + }, "run-1", model: null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Error, result.Status); + Assert.Equal(0, result.ModelDurationMs); + } } diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestControllerTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestControllerTests.cs index bac90f1ab..ffb6ef79d 100644 --- a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestControllerTests.cs +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestControllerTests.cs @@ -5,6 +5,7 @@ using System.Security.Claims; using System.Threading.Tasks; using BotSharp.Abstraction.Agents; +using BotSharp.Abstraction.Agents.Models; using BotSharp.Abstraction.Infrastructures.Attributes; using BotSharp.Abstraction.MLTasks; using BotSharp.Abstraction.MLTasks.Settings; @@ -85,8 +86,26 @@ public Task> ListRunsByStatusAsync(string status) => Task.FromResult(Runs.Values.Where(r => r.Status == status).ToList()); public Task UpdateRunAsync(AgentTestRun run) { Runs[run.Id] = run; return Task.CompletedTask; } + /// + /// Cascades exactly as the real repository does. A fake that dropped only the run would let + /// a test pass while production left unreachable result rows behind. + /// + public Task DeleteRunAsync(string id) + { + var removed = Results.RemoveAll(r => r.RunId == id); + Runs.Remove(id); + return Task.FromResult((long)removed); + } + public Task AddCaseResultAsync(AgentTestCaseResult result) => Task.CompletedTask; - public Task> ListCaseResultsAsync(string runId) => Task.FromResult(new List()); + /// + /// Real storage rather than an always-empty list, so the delete cascade is actually + /// observable: a fake that never held results could not tell a cascade from a no-op. + /// + public List Results { get; } = []; + + public Task> ListCaseResultsAsync(string runId) + => Task.FromResult(Results.Where(r => r.RunId == runId).ToList()); } private sealed class RecordingQueue : IAgentTestRunQueue @@ -110,8 +129,26 @@ private static ILlmProviderService ProviderServiceKnowing(params string[] provid return mock.Object; } + /// + /// An IAgentService that resolves exactly the given ids. The default Mock.Of<IAgentService> + /// resolves nothing, which is correct for every test that leaves EntryAgentId blank (the check + /// short-circuits there) but would reject any test that sets one. + /// + private static IAgentService AgentServiceKnowing(params string[] agentIds) + { + var known = new HashSet(agentIds, StringComparer.OrdinalIgnoreCase); + var mock = new Mock(); + mock.Setup(x => x.GetAgent(It.IsAny())) + .ReturnsAsync((string id) => known.Contains(id) ? new Agent { Id = id, Name = id } : null!); + return mock.Object; + } + private static AgentTestController BuildController( - InMemoryRepo repo, out RecordingQueue queue, ILlmProviderService? llmProviders = null) + InMemoryRepo repo, + out RecordingQueue queue, + ILlmProviderService? llmProviders = null, + IAgentService? agents = null, + ICaseAuthor? author = null) { var recorder = new AgentTestRecorder( Mock.Of(), @@ -120,7 +157,8 @@ private static AgentTestController BuildController( queue = new RecordingQueue(); var controller = new AgentTestController( - repo, queue, Mock.Of(), recorder, llmProviders ?? ProviderServiceKnowing()); + repo, queue, agents ?? Mock.Of(), recorder, author ?? Mock.Of(), + llmProviders ?? ProviderServiceKnowing()); // TriggerRun reads User.FindFirstValue(ClaimTypes.NameIdentifier) -- a directly-constructed // controller (no MVC pipeline/TestServer) has no HttpContext at all by default, and @@ -610,4 +648,993 @@ public async Task CancelRun_still_accepts_a_run_that_has_not_finished(string liv Assert.IsType(result); Assert.True(repo.Runs["run-1"].CancelRequested); } + + private static AgentTestCaseUpsertRequest RoutingCaseRequest( + int turns = 1, params TestAssertion[] assertions) => new() + { + SuiteId = "suite-1", + Name = "routing case", + CaseType = CaseTypes.Routing, + Turns = Enumerable.Range(0, turns) + .Select(i => new TestTurn { Index = i, UserMessage = "hi" }) + .ToList(), + Assertions = assertions.ToList() + }; + + private static InMemoryRepo RepoWithSuite() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = new AgentTestSuite { Id = "suite-1", AgentId = "agent-1", Name = "s" }; + return repo; + } + + [Fact] + public async Task An_unknown_case_type_is_rejected_rather_than_stored_as_the_default() + { + // Silently storing "Rounting" as Agent would leave the author with a case that reads + // routing-shaped in the UI and is never counted towards routing accuracy -- the failure mode + // is a gate figure quietly measuring fewer cases than anyone thinks. + var controller = BuildController(RepoWithSuite(), out _); + + var request = RoutingCaseRequest( + assertions: new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "WO" }); + request.CaseType = "Rounting"; + + var response = await controller.CreateCase(request); + + var bad = Assert.IsType(response.Result); + Assert.Contains("caseType", bad.Value?.ToString()); + } + + [Theory] + [InlineData("routing")] + [InlineData("ROUTING")] + public async Task A_case_type_is_stored_in_its_canonical_casing(string sent) + { + // Every comparison against CaseTypes.Routing is Ordinal, so storing the caller's casing would + // leave a case that never matches -- it would run, and then not be counted. + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + var request = RoutingCaseRequest( + assertions: new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "WO" }); + request.CaseType = sent; + + await controller.CreateCase(request); + + Assert.Equal(CaseTypes.Routing, Assert.Single(repo.Cases.Values).CaseType); + } + + [Fact] + public async Task A_case_that_omits_the_type_is_an_agent_case() + { + // Backward compatibility for every client written before the field existed, and for the + // documents already in the store: both have to keep meaning Agent. + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }); + + Assert.Equal(CaseTypes.Agent, Assert.Single(repo.Cases.Values).CaseType); + } + + [Theory] + [InlineData(0)] + [InlineData(2)] + public async Task A_routing_case_must_have_exactly_one_turn(int turns) + { + // Routing is a single-turn question: which agent picks this message up. A second turn asks + // something else, and its verdict would still land in the routing accuracy figure. + var controller = BuildController(RepoWithSuite(), out _); + + var response = await controller.CreateCase(RoutingCaseRequest( + turns, new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "WO" })); + + var bad = Assert.IsType(response.Result); + Assert.Contains("exactly one turn", bad.Value?.ToString()); + } + + [Fact] + public async Task A_routing_case_without_a_routing_assertion_is_rejected() + { + // Otherwise the case counts towards routing accuracy while asserting nothing about routing: + // it reports Passed for having successfully said anything at all. + var controller = BuildController(RepoWithSuite(), out _); + + var response = await controller.CreateCase(RoutingCaseRequest( + assertions: new TestAssertion { Type = AssertionTypes.OutputContains, Expected = "hello" })); + + var bad = Assert.IsType(response.Result); + Assert.Contains(AssertionTypes.RoutedToAgent, bad.Value?.ToString()); + } + + [Fact] + public async Task Either_routing_assertion_type_satisfies_that_requirement() + { + // agentChain is the assertion that can actually describe a hand-off, so it has to count -- + // requiring routedToAgent specifically would force every routing case onto the weaker one. + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + var response = await controller.CreateCase(RoutingCaseRequest( + assertions: new TestAssertion + { + Type = AssertionTypes.AgentChain, + Target = AgentChainModes.Exact, + Expected = "Work Order Creator" + })); + + Assert.Null(response.Result); + Assert.Equal(CaseTypes.Routing, Assert.Single(repo.Cases.Values).CaseType); + } + + [Fact] + public async Task A_turn_level_routing_assertion_counts_too() + { + // A single-turn routing case can just as reasonably put its assertion on the turn as at case + // level; looking only at case-level assertions would reject a perfectly good case. + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + var response = await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + CaseType = CaseTypes.Routing, + Turns = + [ + new TestTurn + { + Index = 0, + UserMessage = "hi", + Assertions = [new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "WO" }] + } + ] + }); + + Assert.Null(response.Result); + Assert.Single(repo.Cases.Values); + } + + [Fact] + public async Task A_routing_case_cannot_use_the_llm_judge() + { + // Routing is scored purely as expected-agent == actual-agent. An llmJudge would also make the + // routing figure depend on a vendor call, so a vendor outage would read as a routing + // regression. + var controller = BuildController(RepoWithSuite(), out _); + + var response = await controller.CreateCase(RoutingCaseRequest( + 1, + new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "WO" }, + new TestAssertion { Type = AssertionTypes.LlmJudge, Expected = "is polite", MinScore = 4 })); + + var bad = Assert.IsType(response.Result); + Assert.Contains(AssertionTypes.LlmJudge, bad.Value?.ToString()); + } + + [Fact] + public async Task An_agent_case_is_not_held_to_the_routing_rules() + { + // Those rules are Routing-only. An Agent case is normally multi-turn and may well use + // llmJudge; applying routing's constraints to it would break every case already stored. + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + var response = await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + CaseType = CaseTypes.Agent, + Turns = + [ + new TestTurn { Index = 0, UserMessage = "a" }, + new TestTurn { Index = 1, UserMessage = "b" } + ], + Assertions = [new TestAssertion { Type = AssertionTypes.LlmJudge, Expected = "is polite", MinScore = 4 }] + }); + + Assert.Null(response.Result); + Assert.Single(repo.Cases.Values); + } + + [Fact] + public async Task An_entry_agent_that_does_not_exist_is_rejected_at_save_time() + { + // A typo here would otherwise turn every run of the case into an opaque infrastructure Error: + // the canary fails against an agent BotSharp cannot load, and its message says nothing about + // the real cause. + var controller = BuildController(RepoWithSuite(), out _, agents: AgentServiceKnowing("copilot-entry")); + + var request = RoutingCaseRequest( + assertions: new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "WO" }); + request.EntryAgentId = "coplot-entry"; + + var response = await controller.CreateCase(request); + + var bad = Assert.IsType(response.Result); + Assert.Contains("coplot-entry", bad.Value?.ToString()); + } + + [Fact] + public async Task A_known_entry_agent_is_stored_trimmed() + { + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _, agents: AgentServiceKnowing("copilot-entry")); + + var request = RoutingCaseRequest( + assertions: new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "WO" }); + request.EntryAgentId = " copilot-entry "; + + await controller.CreateCase(request); + + Assert.Equal("copilot-entry", Assert.Single(repo.Cases.Values).EntryAgentId); + } + + [Fact] + public async Task A_blank_entry_agent_is_stored_as_null_and_needs_no_lookup() + { + // Null is what the runner's "fall back to the suite's agent" check reads. A UI posting "" for + // an untouched field must not retarget the case at an agent id of "", and saving a case must + // not require knowing any agent id at all -- note this controller resolves no agents. + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + var request = RoutingCaseRequest( + assertions: new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "WO" }); + request.EntryAgentId = " "; + + await controller.CreateCase(request); + + Assert.Null(Assert.Single(repo.Cases.Values).EntryAgentId); + } + + [Fact] + public async Task E2E_is_no_longer_an_accepted_case_type() + { + // Dropped by project owner decision: a multi-agent journey is an Agent case whose agentChain + // assertion describes the hand-offs, so a third type bought nothing but a third branch in + // every validation and aggregation path. + var controller = BuildController(RepoWithSuite(), out _); + + var request = RoutingCaseRequest( + assertions: new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "WO" }); + request.CaseType = "E2E"; + + var response = await controller.CreateCase(request); + + var bad = Assert.IsType(response.Result); + Assert.Contains("caseType", bad.Value?.ToString()); + } + + [Theory] + [InlineData("system")] + [InlineData("function")] + [InlineData("tool")] + [InlineData("")] + public async Task An_unsupported_history_role_is_rejected(string role) + { + // system would compete with the agent's own instruction, and function would fake a tool call + // -- letting a case claim a tool ran when nothing did. Both are worse than a save error. + var controller = BuildController(RepoWithSuite(), out _); + + var response = await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + History = [new TestHistoryMessage { Role = role, Content = "hello" }], + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }); + + var bad = Assert.IsType(response.Result); + Assert.Contains("history message 1", bad.Value?.ToString()); + } + + [Fact] + public async Task A_history_message_with_no_content_is_rejected() + { + // BotSharp's own dialog storage drops elements with blank content, so it would not be there + // at run time, and the runner's write-count check would then fail the case with a confusing + // message about the write having vanished. + var controller = BuildController(RepoWithSuite(), out _); + + var response = await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + History = [new TestHistoryMessage { Role = HistoryRoles.User, Content = " " }], + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }); + + var bad = Assert.IsType(response.Result); + Assert.Contains("no content", bad.Value?.ToString()); + } + + [Fact] + public async Task A_history_role_is_stored_in_its_canonical_casing() + { + // The driver and every later comparison use the lowercase constants. + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + History = [new TestHistoryMessage { Role = "ASSISTANT", Content = "hello" }], + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }); + + var stored = Assert.Single(repo.Cases.Values); + Assert.Equal(HistoryRoles.Assistant, Assert.Single(stored.History).Role); + } + + [Fact] + public async Task History_does_not_count_towards_a_routing_cases_one_turn_limit() + { + // Replaying a prior exchange and then asking one question is still a single routing decision, + // and it is the most realistic way to test routing that depends on context. Counting history + // as turns would make that impossible to express. + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + var request = RoutingCaseRequest( + assertions: new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "WO" }); + request.History = + [ + new TestHistoryMessage { Role = HistoryRoles.User, Content = "my fridge is leaking" }, + new TestHistoryMessage { Role = HistoryRoles.Assistant, Content = "I raised work order B123." } + ]; + + var response = await controller.CreateCase(request); + + Assert.Null(response.Result); + Assert.Equal(2, Assert.Single(repo.Cases.Values).History.Count); + } + + /// + /// A case with every field populated, so a copy test fails when a field is dropped rather than + /// only when the obvious ones are. + /// + private static AgentTestCase FullyPopulatedCase() => new() + { + Id = "case-1", + SuiteId = "suite-1", + Name = "asking for an ETA", + Enabled = true, + CaseType = CaseTypes.Routing, + EntryAgentId = "copilot-entry", + History = [new TestHistoryMessage { Role = HistoryRoles.User, Content = "my fridge is leaking" }], + Turns = + [ + new TestTurn + { + Index = 0, + UserMessage = "when is someone coming?", + Assertions = [new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "WO", Fatal = true }] + } + ], + Assertions = + [ + new TestAssertion + { + Type = AssertionTypes.AgentChain, + Target = AgentChainModes.Ordered, + Expected = "Copilot, WO" + } + ], + InitialStates = [new TestState { Key = "wo_num", Value = "B123", ActiveRounds = 3, Global = true }], + Mocks = + [ + new TestToolMock + { + FunctionName = "get_estimate_arrival_time", + ArgsMatchJson = "{\"wo_num\":\"B123\"}", + CallIndex = 1, + ResultContent = "tomorrow 9am", + StopCompletion = true, + StateWrites = [new TestState { Key = "eta", Value = "tomorrow" }] + } + ], + UnmockedToolPolicy = UnmockedToolPolicies.Block, + SourceConversationId = "conv-9" + }; + + [Fact] + public async Task Copying_a_case_carries_every_field() + { + // The reason this endpoint exists server-side at all. A client that rebuilds the payload from + // its own form drops whatever it does not know about, and a copy missing its mocks looks + // identical in the list right up to the run where it blocks every tool. + var repo = RepoWithSuite(); + var source = FullyPopulatedCase(); + repo.Cases[source.Id] = source; + var controller = BuildController(repo, out _); + + var response = await controller.CopyCase("case-1"); + + var copy = Assert.IsType(response.Value); + Assert.Equal(source.SuiteId, copy.SuiteId); + Assert.Equal(source.CaseType, copy.CaseType); + Assert.Equal(source.EntryAgentId, copy.EntryAgentId); + Assert.Equal(source.UnmockedToolPolicy, copy.UnmockedToolPolicy); + Assert.Equal(source.SourceConversationId, copy.SourceConversationId); + + Assert.Equal("my fridge is leaking", Assert.Single(copy.History).Content); + Assert.Equal("when is someone coming?", Assert.Single(copy.Turns).UserMessage); + Assert.True(Assert.Single(Assert.Single(copy.Turns).Assertions).Fatal); + Assert.Equal(AgentChainModes.Ordered, Assert.Single(copy.Assertions).Target); + + var state = Assert.Single(copy.InitialStates); + Assert.Equal("wo_num", state.Key); + Assert.Equal(3, state.ActiveRounds); + Assert.True(state.Global); + + var mock = Assert.Single(copy.Mocks); + Assert.Equal("get_estimate_arrival_time", mock.FunctionName); + Assert.Equal("{\"wo_num\":\"B123\"}", mock.ArgsMatchJson); + Assert.Equal(1, mock.CallIndex); + Assert.True(mock.StopCompletion); + Assert.Equal("eta", Assert.Single(mock.StateWrites!).Key); + } + + [Fact] + public async Task A_copy_lands_disabled_even_when_the_source_was_enabled() + { + // An exact duplicate joining the next run measures the same thing twice: it pads the + // pass-rate denominator, and for a routing case it double-weights one routing decision. The + // copy waits for the edit it was made for. + var repo = RepoWithSuite(); + repo.Cases["case-1"] = FullyPopulatedCase(); + var controller = BuildController(repo, out _); + + var response = await controller.CopyCase("case-1"); + + Assert.False(Assert.IsType(response.Value).Enabled); + // And the source is untouched. + Assert.True(repo.Cases["case-1"].Enabled); + } + + [Fact] + public async Task A_copy_gets_its_own_id_and_does_not_overwrite_the_source() + { + // The BSON round trip copies the source's _id, so failing to blank it would turn the copy + // into a full overwrite of the original -- the worst possible outcome for a copy button. + var repo = RepoWithSuite(); + repo.Cases["case-1"] = FullyPopulatedCase(); + var controller = BuildController(repo, out _); + + var response = await controller.CopyCase("case-1"); + + var copy = Assert.IsType(response.Value); + Assert.NotEqual("case-1", copy.Id); + Assert.NotEmpty(copy.Id); + Assert.Equal(2, repo.Cases.Count); + Assert.Equal("asking for an ETA", repo.Cases["case-1"].Name); + } + + [Fact] + public async Task Editing_a_copy_does_not_reach_back_into_the_source() + { + // A shallow clone would have both documents sharing the same Turns/Mocks list instances, so + // the first edit to the copy would silently rewrite the case it came from. + var repo = RepoWithSuite(); + repo.Cases["case-1"] = FullyPopulatedCase(); + var controller = BuildController(repo, out _); + + var copy = Assert.IsType((await controller.CopyCase("case-1")).Value); + + copy.Turns[0].UserMessage = "changed"; + copy.Mocks[0].ResultContent = "changed"; + copy.History[0].Content = "changed"; + + var source = repo.Cases["case-1"]; + Assert.Equal("when is someone coming?", source.Turns[0].UserMessage); + Assert.Equal("tomorrow 9am", source.Mocks[0].ResultContent); + Assert.Equal("my fridge is leaking", source.History[0].Content); + } + + [Fact] + public async Task Repeated_copies_get_distinguishable_names() + { + // Two rows both called "x (copy)" cannot be told apart in the list, which is the one place + // copies are managed. + var repo = RepoWithSuite(); + repo.Cases["case-1"] = FullyPopulatedCase(); + var controller = BuildController(repo, out _); + + var first = Assert.IsType((await controller.CopyCase("case-1")).Value); + var second = Assert.IsType((await controller.CopyCase("case-1")).Value); + var third = Assert.IsType((await controller.CopyCase("case-1")).Value); + + Assert.Equal("asking for an ETA (copy)", first.Name); + Assert.Equal("asking for an ETA (copy 2)", second.Name); + Assert.Equal("asking for an ETA (copy 3)", third.Name); + } + + [Fact] + public async Task A_copy_of_a_copy_is_named_from_the_case_it_was_copied_from() + { + // Not "x (copy) (copy)": the suffix is appended to whatever the source is called, and the + // collision check is what keeps the result unique. + var repo = RepoWithSuite(); + repo.Cases["case-1"] = FullyPopulatedCase(); + var controller = BuildController(repo, out _); + + var first = Assert.IsType((await controller.CopyCase("case-1")).Value); + var nested = Assert.IsType((await controller.CopyCase(first.Id)).Value); + + Assert.Equal("asking for an ETA (copy) (copy)", nested.Name); + } + + [Fact] + public async Task A_copied_name_stays_short_enough_to_edit() + { + // A name the case editor's own input cannot hold would have to be trimmed by hand before any + // other change to the copy could be saved. + var repo = RepoWithSuite(); + var source = FullyPopulatedCase(); + source.Name = new string('x', 200); + repo.Cases["case-1"] = source; + var controller = BuildController(repo, out _); + + var copy = Assert.IsType((await controller.CopyCase("case-1")).Value); + + Assert.True(copy.Name.Length <= 200, $"name was {copy.Name.Length} characters"); + Assert.EndsWith(" (copy)", copy.Name); + } + + [Fact] + public async Task A_copy_does_not_inherit_the_sources_create_date() + { + // The round trip copies it, which would have the copy claim to be as old as the case it came + // from -- and the case list is sorted newest first, so a fresh copy would appear buried. + var repo = RepoWithSuite(); + var source = FullyPopulatedCase(); + source.CreateDate = new DateTime(2020, 1, 1, 0, 0, 0, DateTimeKind.Utc); + repo.Cases["case-1"] = source; + var controller = BuildController(repo, out _); + + var copy = Assert.IsType((await controller.CopyCase("case-1")).Value); + + Assert.True(copy.CreateDate > new DateTime(2021, 1, 1, 0, 0, 0, DateTimeKind.Utc)); + } + + [Fact] + public async Task Copying_a_case_that_does_not_exist_is_a_404() + { + var controller = BuildController(RepoWithSuite(), out _); + + var response = await controller.CopyCase("nope"); + + Assert.IsType(response.Result); + } + + // ------------------------------------------------------------- governance metadata + + [Theory] + [InlineData("P3")] + [InlineData("high")] + public async Task An_unknown_priority_is_rejected(string priority) + { + // Priority decides the batch, and a batch decides whether a failure stops the evaluation. + // Storing "high" would leave a case that matches no priority and lands in the default batch + // while its author believes it is stop-loss. + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + var response = await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + Priority = priority, + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }); + + var bad = Assert.IsType(response.Result); + Assert.Contains("priority", bad.Value?.ToString()); + } + + [Fact] + public async Task An_unknown_severity_is_rejected() + { + var controller = BuildController(RepoWithSuite(), out _); + + var response = await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + Severity = "critical", + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }); + + var bad = Assert.IsType(response.Result); + Assert.Contains("severity", bad.Value?.ToString()); + } + + [Fact] + public async Task An_out_of_range_batch_is_rejected_rather_than_clamped() + { + // Clamping 4 to 3 would file the case somewhere its author never asked for, silently, in the + // batch that does not block a release. + var controller = BuildController(RepoWithSuite(), out _); + + var response = await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + Batch = 4, + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }); + + var bad = Assert.IsType(response.Result); + Assert.Contains("batch", bad.Value?.ToString()); + } + + [Fact] + public async Task Priority_and_severity_are_stored_in_canonical_casing() + { + // Every comparison against CasePriorities.P0 is Ordinal, so storing "p0" would leave a case + // that runs and is then filed in the wrong batch. + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + Priority = "p0", + Severity = "s0", + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }); + + var stored = Assert.Single(repo.Cases.Values); + Assert.Equal(CasePriorities.P0, stored.Priority); + Assert.Equal(CaseSeverities.S0, stored.Severity); + } + + [Fact] + public async Task A_case_that_omits_the_governance_fields_gets_the_untriaged_defaults() + { + // P1 and S1 for every case stored before these fields existed: P0/S0 would make each of them + // an immediate no-go, and P2/S2 would drop them out of the mandatory batches and let a real + // failure read as an experience nit. + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }); + + var stored = Assert.Single(repo.Cases.Values); + Assert.Equal(CasePriorities.P1, stored.Priority); + Assert.Equal(CaseSeverities.S1, stored.Severity); + Assert.Null(stored.Batch); + Assert.False(stored.CrossCutting); + Assert.Empty(stored.InvolvedAgents); + Assert.Null(stored.LastReviewedDate); + } + + [Fact] + public async Task Involved_agents_are_trimmed_and_deduplicated() + { + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + InvolvedAgents = [" agent-a ", "AGENT-A", "", "agent-b"], + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }); + + Assert.Equal(["agent-a", "agent-b"], Assert.Single(repo.Cases.Values).InvolvedAgents); + } + + [Fact] + public async Task Saving_a_case_never_stamps_the_reviewed_date() + { + // A case can be edited many times and still rest on an assumption nobody has questioned in a + // year. Stamping this on every write would hide exactly that. + var repo = RepoWithSuite(); + var controller = BuildController(repo, out _); + + await controller.CreateCase(new AgentTestCaseUpsertRequest + { + SuiteId = "suite-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }); + + Assert.Null(Assert.Single(repo.Cases.Values).LastReviewedDate); + } + + // ------------------------------------------------------------- scope selection + + private static InMemoryRepo RepoWithScopedCases() + { + var repo = new InMemoryRepo(); + repo.Suites["suite-1"] = new AgentTestSuite { Id = "suite-1", AgentId = "agent-a", Name = "A suite" }; + repo.Cases["on-target"] = new AgentTestCase + { + Id = "on-target", SuiteId = "suite-1", Name = "on target", EntryAgentId = "agent-a" + }; + repo.Cases["off-target"] = new AgentTestCase + { + Id = "off-target", SuiteId = "suite-1", Name = "off target", EntryAgentId = "agent-b" + }; + repo.Cases["safety"] = new AgentTestCase + { + Id = "safety", SuiteId = "suite-1", Name = "safety", EntryAgentId = "agent-b", CrossCutting = true + }; + repo.Cases["draft"] = new AgentTestCase + { + Id = "draft", SuiteId = "suite-1", Name = "draft", EntryAgentId = "agent-a", Enabled = false + }; + return repo; + } + + [Fact] + public async Task A_scope_with_no_targets_and_no_platform_flag_is_rejected() + { + // It would narrow to nothing, and an empty scope reported as a successful plan is the single + // most dangerous answer this endpoint could give: it reads as "nothing needs testing". + var controller = BuildController(RepoWithScopedCases(), out _); + + var response = await controller.SelectScope(new ScopeSelectionRequest()); + + Assert.IsType(response.Result); + } + + [Fact] + public async Task A_scope_reports_both_halves_with_a_reason_for_each() + { + // The excluded half is the one worth reading: an excluded case produces no result to notice, + // so the only defence is being able to see what was left out and why. + var controller = BuildController(RepoWithScopedCases(), out _); + + var response = await controller.SelectScope(new ScopeSelectionRequest + { + TargetAgentIds = ["agent-a"] + }); + + var scope = Assert.IsType(response.Value); + Assert.Equal(4, scope.TotalCases); + + Assert.Equal( + ["on-target", "safety"], + scope.Included.Select(c => c.CaseId).OrderBy(id => id).ToList()); + Assert.Equal(ScopeReasons.TargetAgent, scope.Included.Single(c => c.CaseId == "on-target").Reason); + Assert.Equal(ScopeReasons.CrossCutting, scope.Included.Single(c => c.CaseId == "safety").Reason); + + Assert.Equal(ScopeReasons.NotInvolved, scope.Excluded.Single(c => c.CaseId == "off-target").Reason); + Assert.Equal(ScopeReasons.Disabled, scope.Excluded.Single(c => c.CaseId == "draft").Reason); + } + + [Fact] + public async Task A_platform_wide_scope_includes_every_enabled_case() + { + // Narrowing switches off: there is no agent a foundation model swap demonstrably does not + // touch. Disabled cases stay out, because no run would execute them. + var controller = BuildController(RepoWithScopedCases(), out _); + + var response = await controller.SelectScope(new ScopeSelectionRequest { FullPlatform = true }); + + var scope = Assert.IsType(response.Value); + Assert.Equal(3, scope.Included.Count); + Assert.Equal("draft", Assert.Single(scope.Excluded).CaseId); + } + + [Fact] + public async Task A_scope_carries_the_metadata_the_decision_was_made_from() + { + // A verdict on its own cannot be reviewed. The involved set and the effective batch are what + // let someone check the plan rather than trust it. + var controller = BuildController(RepoWithScopedCases(), out _); + + var response = await controller.SelectScope(new ScopeSelectionRequest + { + TargetAgentIds = ["agent-a"] + }); + + var scope = Assert.IsType(response.Value); + var onTarget = scope.Included.Single(c => c.CaseId == "on-target"); + + Assert.Equal(["agent-a"], onTarget.InvolvedAgentIds); + Assert.Equal(CaseBatches.Mandatory, onTarget.Batch); + Assert.Equal("A suite", onTarget.SuiteName); + // Cross-cutting forces batch 1, and the response has to agree with that. + Assert.Equal(CaseBatches.StopLoss, scope.Included.Single(c => c.CaseId == "safety").Batch); + } + + [Fact] + public async Task A_scope_can_be_narrowed_to_one_batch() + { + var controller = BuildController(RepoWithScopedCases(), out _); + + var response = await controller.SelectScope(new ScopeSelectionRequest + { + TargetAgentIds = ["agent-a"], + Batch = CaseBatches.StopLoss + }); + + var scope = Assert.IsType(response.Value); + Assert.Equal("safety", Assert.Single(scope.Included).CaseId); + Assert.Equal(ScopeReasons.OtherBatch, scope.Excluded.Single(c => c.CaseId == "on-target").Reason); + } + + [Fact] + public async Task An_out_of_range_batch_on_a_scope_is_rejected() + { + var controller = BuildController(RepoWithScopedCases(), out _); + + var response = await controller.SelectScope(new ScopeSelectionRequest + { + TargetAgentIds = ["agent-a"], + Batch = 7 + }); + + Assert.IsType(response.Result); + } + + // ------------------------------------------------------------- clearing run history + + private static InMemoryRepo RepoWithRuns() + { + var repo = RepoWithSuite(); + repo.Runs["done"] = new AgentTestRun + { + Id = "done", SuiteId = "suite-1", Status = AgentTestStatus.Passed + }; + repo.Runs["failed"] = new AgentTestRun + { + Id = "failed", SuiteId = "suite-1", Status = AgentTestStatus.Failed + }; + repo.Runs["live"] = new AgentTestRun + { + Id = "live", SuiteId = "suite-1", Status = AgentTestStatus.Running + }; + repo.Results.Add(new AgentTestCaseResult { Id = "r1", RunId = "done", CaseId = "c1" }); + repo.Results.Add(new AgentTestCaseResult { Id = "r2", RunId = "done", CaseId = "c2" }); + repo.Results.Add(new AgentTestCaseResult { Id = "r3", RunId = "live", CaseId = "c1" }); + return repo; + } + + [Fact] + public async Task Deleting_a_run_takes_its_case_results_with_it() + { + // Results are keyed by run id and reachable no other way, so dropping the run alone would + // leave rows nothing can ever list, read or clean up again. + var repo = RepoWithRuns(); + var controller = BuildController(repo, out _); + + var response = await controller.DeleteRuns(new AgentTestRunDeleteRequest { RunIds = ["done"] }); + + var result = Assert.IsType(response.Value); + Assert.Equal(["done"], result.DeletedRunIds); + Assert.Equal(2, result.DeletedResultCount); + Assert.DoesNotContain("done", repo.Runs.Keys); + Assert.DoesNotContain(repo.Results, r => r.RunId == "done"); + + // And it touched nothing else. + Assert.Contains(repo.Results, r => r.RunId == "live"); + } + + [Fact] + public async Task A_running_run_is_refused_rather_than_deleted() + { + // Deleting it would not stop it: the queue keeps driving cases, keeps spending tokens, and + // keeps writing results for a run id that no longer exists. + var repo = RepoWithRuns(); + var controller = BuildController(repo, out _); + + var response = await controller.DeleteRuns(new AgentTestRunDeleteRequest { RunIds = ["live"] }); + + var result = Assert.IsType(response.Value); + Assert.Empty(result.DeletedRunIds); + Assert.Contains("cancel it before deleting", Assert.Single(result.Skipped).Reason); + Assert.Contains("live", repo.Runs.Keys); + Assert.Contains(repo.Results, r => r.RunId == "live"); + } + + [Fact] + public async Task One_running_run_does_not_block_the_rest_of_the_batch() + { + // Selecting everything and clearing is the normal way this is used, and a live run in the + // list is common. Refusing the whole call would make the feature unusable exactly when it is + // most wanted. + var repo = RepoWithRuns(); + var controller = BuildController(repo, out _); + + var response = await controller.DeleteRuns(new AgentTestRunDeleteRequest + { + RunIds = ["done", "live", "failed"] + }); + + var result = Assert.IsType(response.Value); + Assert.Equal(["done", "failed"], result.DeletedRunIds); + Assert.Equal("live", Assert.Single(result.Skipped).RunId); + Assert.Equal(["live"], repo.Runs.Keys.ToList()); + } + + [Fact] + public async Task A_run_that_is_already_gone_is_reported_not_an_error() + { + // Two people clearing the same history is a race, not a failure worth refusing a batch over. + var repo = RepoWithRuns(); + var controller = BuildController(repo, out _); + + var response = await controller.DeleteRuns(new AgentTestRunDeleteRequest + { + RunIds = ["done", "never-existed"] + }); + + var result = Assert.IsType(response.Value); + Assert.Equal(["done"], result.DeletedRunIds); + Assert.Equal("already deleted", Assert.Single(result.Skipped).Reason); + } + + [Fact] + public async Task Duplicate_ids_are_deleted_once() + { + // A select-all plus a row click can send the same id twice; the second pass would otherwise + // report it as "already deleted" and make the summary read as a partial failure. + var repo = RepoWithRuns(); + var controller = BuildController(repo, out _); + + var response = await controller.DeleteRuns(new AgentTestRunDeleteRequest + { + RunIds = ["done", "done"] + }); + + var result = Assert.IsType(response.Value); + Assert.Equal(["done"], result.DeletedRunIds); + Assert.Empty(result.Skipped); + } + + [Fact] + public async Task An_empty_delete_is_rejected() + { + // An empty list is far more likely to be a UI bug -- a select-all that selected nothing -- + // than a deliberate no-op, and answering 200 with "deleted nothing" hides it. + var controller = BuildController(RepoWithRuns(), out _); + + var response = await controller.DeleteRuns(new AgentTestRunDeleteRequest()); + + Assert.IsType(response.Result); + } + + [Fact] + public async Task A_delete_of_nothing_but_blanks_is_rejected_too() + { + // Same bug wearing a different shape: a client that posts one empty row per unchecked box. + var controller = BuildController(RepoWithRuns(), out _); + + var response = await controller.DeleteRuns(new AgentTestRunDeleteRequest + { + RunIds = ["", " "] + }); + + Assert.IsType(response.Result); + } + + [Fact] + public void Clearing_run_history_requires_an_admin() + { + // Runs are the record of whether an agent change was evaluated at all, so removing them is at + // least as consequential as creating them -- the same gate triggering sits behind. + var method = typeof(AgentTestController).GetMethod(nameof(AgentTestController.DeleteRuns)); + + Assert.NotNull(method); + Assert.NotEmpty(method!.GetCustomAttributes(typeof(BotSharpAuthAttribute), inherit: true)); + } } diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestJudgeTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestJudgeTests.cs new file mode 100644 index 000000000..84592a82e --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestJudgeTests.cs @@ -0,0 +1,231 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using BotSharp.Plugin.AgentTesting.Models; +using BotSharp.Plugin.AgentTesting.Runtime; +using BotSharp.Plugin.AgentTesting.Services; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// llmJudge is the one assertion type that is not a pure function, and that makes two things worth +/// pinning here. +/// +/// First, the model's answer is extended no trust: a blank reply, a non-JSON reply, malformed JSON or +/// a score off the 1-5 scale are all "no verdict", never a coerced pass or fail. A judge that ignored +/// the rubric has graded nothing, and reading pass/fail out of that is reading meaning into noise. +/// +/// Second, and more important, every way the judge can fail to reach a verdict has to land on Error, +/// not Failed. A vendor timeout, an unconfigured judge model or a rate limit says nothing about the +/// agent under test -- if any of them surfaced as a failing assertion, provider noise would be +/// indistinguishable from an agent regression, which is the whole reason the harness keeps Failed and +/// Error apart. +/// +public class AgentTestJudgeTests +{ + // ---- ParseVerdict: extend the model's answer no trust --------------------------------------- + + [Fact] + public void Parses_a_plain_json_verdict() + { + var verdict = LlmAgentTestJudge.ParseVerdict("""{"score": 4, "reason": "asks for the work order number"}"""); + + Assert.Equal(4, verdict.Score); + Assert.Equal("asks for the work order number", verdict.Reason); + } + + [Fact] + public void Parses_a_verdict_wrapped_in_a_code_fence_or_prose() + { + // Models add fences and preambles despite being told not to. Rejecting those would make the + // feature fail for a formatting habit rather than for a real problem. + var verdict = LlmAgentTestJudge.ParseVerdict( + "Sure, here is my assessment:\n```json\n{\"score\": 5, \"reason\": \"clear\"}\n```\n"); + + Assert.Equal(5, verdict.Score); + Assert.Equal("clear", verdict.Reason); + } + + [Fact] + public void Accepts_property_names_in_any_case() + { + var verdict = LlmAgentTestJudge.ParseVerdict("""{"Score": 3, "Reason": "partial"}"""); + + Assert.Equal(3, verdict.Score); + Assert.Equal("partial", verdict.Reason); + } + + [Fact] + public void Accepts_a_verdict_with_no_reason() + { + // The reason is for a human reading the result afterwards; its absence must not invalidate a + // score the model did give. + var verdict = LlmAgentTestJudge.ParseVerdict("""{"score": 5}"""); + + Assert.Equal(5, verdict.Score); + Assert.Null(verdict.Reason); + } + + [Theory] + [InlineData("")] + [InlineData("I would rate this a 4 out of 5.")] // no JSON at all + [InlineData("{\"score\": }")] // malformed + [InlineData("{\"reason\": \"good\"}")] // no score -> defaults to 0, off scale + public void Rejects_a_reply_it_cannot_read_as_a_score(string raw) + { + Assert.Throws(() => LlmAgentTestJudge.ParseVerdict(raw)); + } + + [Theory] + [InlineData(0)] + [InlineData(6)] + [InlineData(-1)] + [InlineData(100)] + public void Rejects_a_score_outside_the_scale_instead_of_clamping_it(double score) + { + // Clamping would silently turn "did not follow the rubric" into a grade. 6 clamped to 5 + // would even pass, which is the worst of the available outcomes. + var ex = Assert.Throws( + () => LlmAgentTestJudge.ParseVerdict($"{{\"score\": {score}}}")); + + Assert.Contains("1-5", ex.Message); + } + + // ---- JudgeAsync guards: everything decidable without a vendor ------------------------------- + + [Fact] + public async Task Refuses_to_judge_when_the_suite_has_no_judge_model() + { + // The reason this check exists at all: BotSharp's InstructService silently falls back to + // openai/gpt-4o when no provider and model are given. Inheriting that would score cases with + // a model nobody chose, and the run would still look conclusive. + var ex = await Assert.ThrowsAsync( + () => Judge().JudgeAsync(Assertion(), Context("we need your work order number"), new AgentTestSuite + { + Id = "suite-1", + AgentId = "agent-1", + Name = "s" + }, CancellationToken.None)); + + // The message has to name the fix -- whoever sees this in the UI is the person who has to + // configure the suite. + Assert.Contains("judgeProvider", ex.Message); + Assert.Contains("judgeModel", ex.Message); + } + + [Fact] + public async Task Refuses_to_judge_an_assertion_with_no_criterion() + { + // AssertionValidation already rejects this at save time. Repeated in the judge because a case + // stored before that rule existed would otherwise reach the vendor with an empty criterion + // and come back with a meaningless score. + await Assert.ThrowsAsync( + () => Judge().JudgeAsync( + new TestAssertion { Type = AssertionTypes.LlmJudge, Expected = " " }, + Context("anything"), + ConfiguredSuite(), + CancellationToken.None)); + } + + [Fact] + public async Task Refuses_to_judge_when_the_agent_produced_no_reply() + { + // Not a failing verdict: there is nothing to grade. Whatever went wrong upstream is the real + // finding, and this case's other assertions report it far more usefully than a fabricated + // score would. + await Assert.ThrowsAsync( + () => Judge().JudgeAsync(Assertion(), Context(null), ConfiguredSuite(), CancellationToken.None)); + } + + [Fact] + public async Task Reports_an_unregistered_judge_provider_as_no_verdict() + { + var ex = await Assert.ThrowsAsync( + () => Judge().JudgeAsync(Assertion(), Context("some reply"), ConfiguredSuite(provider: "not-installed"), + CancellationToken.None)); + + Assert.Contains("not-installed", ex.Message); + } + + // ---- The Failed/Error split, end to end through the runner ---------------------------------- + + [Fact] + public async Task An_unjudgeable_case_is_Error_not_Failed() + { + // The single most important behaviour in this file. If this ever reports Failed, every + // vendor hiccup starts reading as an agent regression, and a run's Failed count stops + // meaning anything. + var driver = new StubDriver("we need your work order number"); + var runner = new AgentTestCaseRunner( + new AgentTestRunRegistry(), + driver, + NullLogger.Instance, + judge: null); // no judge registered at all + + var result = await runner.RunAsync(ConfiguredSuite(), new AgentTestCase + { + Id = "case-1", + SuiteId = "suite-1", + Name = "c", + Turns = [new TestTurn { Index = 0, UserMessage = "where is my work order" }], + Assertions = [Assertion()] + }, "run-1", null, CancellationToken.None); + + Assert.Equal(AgentTestStatus.Error, result.Status); + Assert.Contains("llmJudge", result.Error); + } + + // ---- helpers ------------------------------------------------------------------------------- + + private static LlmAgentTestJudge Judge() + => new(new ServiceCollection().BuildServiceProvider(), NullLogger.Instance); + + private static TestAssertion Assertion() => new() + { + Type = AssertionTypes.LlmJudge, + Expected = "the reply asks the user for a work order number" + }; + + private static AssertionContext Context(string? output) => new() { Output = output }; + + private static AgentTestSuite ConfiguredSuite(string provider = "openai") => new() + { + Id = "suite-1", + AgentId = "agent-1", + Name = "s", + CaseTimeoutSeconds = 120, + JudgeProvider = provider, + JudgeModel = "gpt-4o" + }; + + /// Minimal driver: a live seam and one canned reply, so the runner reaches the judge. + private sealed class StubDriver : IAgentConversationDriver + { + private readonly string _reply; + + public StubDriver(string reply) => _reply = reply; + + public Task PrepareAsync(string conversationId, string agentId, IReadOnlyList initialStates) + => Task.CompletedTask; + + public Task SendAsync(string conversationId, string agentId, string userMessage, CancellationToken ct) + => Task.FromResult(_reply); + + public Task RunCanaryAsync(string conversationId, string agentId, CancellationToken ct) + => Task.FromResult(true); + + public Task> ReadStatesAsync(string conversationId) + => Task.FromResult>(new Dictionary()); + + public Task> ReadAssistantAgentSequenceAsync(string conversationId) + => Task.FromResult>([]); + + public Task InjectHistoryAsync( + string conversationId, string agentId, IReadOnlyList history) + => Task.FromResult(history.Count); + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestRunExecutorTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestRunExecutorTests.cs index eee4129f6..b2527f583 100644 --- a/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestRunExecutorTests.cs +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/AgentTestRunExecutorTests.cs @@ -8,6 +8,9 @@ using BotSharp.Plugin.AgentTesting.Services; using BotSharp.Plugin.AgentTesting.Models; using Xunit; +using BotSharp.Abstraction.MLTasks; +using BotSharp.Abstraction.MLTasks.Settings; +using Moq; namespace BotSharp.Core.UnitTests.AgentTesting; @@ -40,6 +43,11 @@ private sealed class InMemoryRepo : IAgentTestRepository public Task> ListRunsByStatusAsync(string status) => Task.FromResult(Run.Status == status ? new List { Run } : []); public Task UpdateRunAsync(AgentTestRun run) { Run = run; return Task.CompletedTask; } + public Task DeleteRunAsync(string id) + { + var removed = Results.RemoveAll(r => r.RunId == id); + return Task.FromResult((long)removed); + } public Task AddCaseResultAsync(AgentTestCaseResult result) { Results.Add(result); return Task.CompletedTask; } public Task> ListCaseResultsAsync(string runId) => Task.FromResult(Results); } @@ -82,6 +90,11 @@ private sealed class CloningRunRepo : IAgentTestRepository public Task> ListRunsByStatusAsync(string status) => Task.FromResult(_stored.Status == status ? new List { Clone(_stored) } : []); public Task UpdateRunAsync(AgentTestRun run) { _stored = Clone(run); return Task.CompletedTask; } + public Task DeleteRunAsync(string id) + { + var removed = Results.RemoveAll(r => r.RunId == id); + return Task.FromResult((long)removed); + } public Task AddCaseResultAsync(AgentTestCaseResult result) { Results.Add(result); return Task.CompletedTask; } public Task> ListCaseResultsAsync(string runId) => Task.FromResult(Results); @@ -113,10 +126,30 @@ private static AgentTestRunExecutor Build(InMemoryRepo repo, Func BuildWithRunner(repo, run).Executor; private static (AgentTestRunExecutor Executor, DelegatingCaseRunner Runner) BuildWithRunner( - InMemoryRepo repo, Func run) + InMemoryRepo repo, + Func run, + ILlmProviderService? llmProviders = null) { var runner = new DelegatingCaseRunner(run); - return (new AgentTestRunExecutor(repo, runner, NullLogger.Instance), runner); + return ( + new AgentTestRunExecutor(repo, runner, NullLogger.Instance, llmProviders), + runner); + } + + /// An ILlmProviderService that knows one model's text token unit costs. + private static ILlmProviderService PricingFor(string provider, string model, float input, float output) + { + var mock = new Mock(); + mock.Setup(x => x.GetSetting(It.IsAny(), It.IsAny())) + .Returns((string p, string m) => + string.Equals(p, provider) && string.Equals(m, model) + ? new LlmModelSetting + { + Name = m, + Cost = new LlmCostSetting { TextInputCost = input, TextOutputCost = output } + } + : null); + return mock.Object; } private sealed class DelegatingCaseRunner(Func run) : ICaseRunner @@ -124,15 +157,25 @@ private sealed class DelegatingCaseRunner(FuncEvery (case, model) pair the executor asked for, in the order it asked. public List<(string CaseId, string? Model)> Invocations { get; } = []; + /// + /// Set instead of relying on the constructor delegate when a test's outcome has to depend on + /// WHICH model ran the case -- the only way to exercise a per-model figure such as routing + /// accuracy without coupling the test to the executor's (case, model) iteration order. + /// + public Func? RunWithModel { get; set; } + public Task RunAsync( AgentTestSuite suite, AgentTestCase testCase, string runId, TestModel? model, CancellationToken ct) { Invocations.Add((testCase.Id, model?.Model)); - var result = run(testCase); + var result = RunWithModel != null ? RunWithModel(testCase, model) : run(testCase); // The real runner stamps these onto the result; mirror it so tests can assert that a // result can be attributed back to the model that produced it. result.Provider ??= model?.Provider; result.Model ??= model?.Model; + // Same reason: the real runner copies the case type onto the result, and the executor's + // routing tally reads it back off the result rather than off the case. + result.CaseType = testCase.CaseType; return Task.FromResult(result); } } @@ -411,4 +454,244 @@ public async Task A_cancel_that_arrives_while_the_only_case_is_executing_still_s // happened. Assert.True(repo.Stored.CancelRequested); } + + private static AgentTestCase RoutingCase(string id) => new() + { + Id = id, SuiteId = "suite-1", Name = id, + CaseType = CaseTypes.Routing, + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }; + + [Fact] + public async Task Routing_accuracy_counts_only_routing_cases() + { + // The evaluation framework gates routing accuracy separately from the agent pass rate, so + // folding an agent case into this figure makes the gate unreadable -- and it is the easiest + // mistake to make, since every case result flows through the same tally. + var repo = new InMemoryRepo { Cases = [RoutingCase("r1"), RoutingCase("r2"), CaseNamed("a1")] }; + var executor = Build(repo, c => new AgentTestCaseResult + { + CaseId = c.Id, + Status = c.Id == "r2" ? AgentTestStatus.Failed : AgentTestStatus.Passed + }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + var accuracy = Assert.Single(repo.Run.RoutingAccuracies); + Assert.Equal(2, accuracy.CaseCount); + Assert.Equal(1, accuracy.PassedCount); + + // The agent case still counts in the run's own totals; it is only absent from the routing + // figure. + Assert.Equal(3, repo.Run.TotalCount); + Assert.Equal(2, repo.Run.PassedCount); + } + + [Fact] + public async Task An_errored_routing_case_counts_against_accuracy_rather_than_being_skipped() + { + // "Could not tell" is not "routed correctly". Leaving Error rows out of CaseCount would let a + // harness that times out on nine cases in ten report perfect routing accuracy off the one + // that ran. + var repo = new InMemoryRepo { Cases = [RoutingCase("r1"), RoutingCase("r2")] }; + var executor = Build(repo, c => new AgentTestCaseResult + { + CaseId = c.Id, + Status = c.Id == "r1" ? AgentTestStatus.Passed : AgentTestStatus.Error, + Error = c.Id == "r1" ? null : "the case timed out after 120s" + }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + var accuracy = Assert.Single(repo.Run.RoutingAccuracies); + Assert.Equal(2, accuracy.CaseCount); + Assert.Equal(1, accuracy.PassedCount); + } + + [Fact] + public async Task Routing_accuracy_is_kept_per_model_so_a_comparison_run_stays_readable() + { + // The whole point of sweeping several models is the difference between them. One run-wide + // figure would average the candidate together with the baseline and hide exactly what the run + // exists to measure. + var repo = new InMemoryRepo { Cases = [RoutingCase("r1"), RoutingCase("r2")] }; + repo.Run.Models = + [ + new TestModel { Provider = "openai", Model = "gpt-4o" }, + new TestModel { Provider = "openai", Model = "gpt-4o-mini" } + ]; + + var (executor, runner) = BuildWithRunner(repo, _ => new AgentTestCaseResult()); + runner.RunWithModel = (c, model) => new AgentTestCaseResult + { + CaseId = c.Id, + Status = model?.Model == "gpt-4o-mini" && c.Id == "r2" + ? AgentTestStatus.Failed + : AgentTestStatus.Passed + }; + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Equal(2, repo.Run.RoutingAccuracies.Count); + + var full = repo.Run.RoutingAccuracies.Single(a => a.Model == "gpt-4o"); + Assert.Equal(2, full.CaseCount); + Assert.Equal(2, full.PassedCount); + + var mini = repo.Run.RoutingAccuracies.Single(a => a.Model == "gpt-4o-mini"); + Assert.Equal(2, mini.CaseCount); + Assert.Equal(1, mini.PassedCount); + Assert.Equal("openai", mini.Provider); + } + + [Fact] + public async Task A_run_with_no_routing_cases_records_no_accuracy_rows() + { + // An empty list is what lets a caller tell "this run measured no routing" apart from "this + // run measured routing and got nothing right" -- a 0/0 row would read as the latter. + var repo = new InMemoryRepo { Cases = [CaseNamed("a1")] }; + var executor = Build(repo, c => new AgentTestCaseResult { CaseId = c.Id, Status = AgentTestStatus.Passed }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Empty(repo.Run.RoutingAccuracies); + } + + private static AgentTestCaseResult Timed(string caseId, long modelMs, long tokens, double cost) => new() + { + CaseId = caseId, + Status = AgentTestStatus.Passed, + ModelDurationMs = modelMs, + TotalTokens = tokens, + Cost = cost + }; + + [Fact] + public async Task Latency_percentiles_are_nearest_rank_and_never_invent_a_duration() + { + // An interpolated percentile returns a number no case actually took, which is indefensible + // the moment someone asks which case was the slow one -- and with the handful of cases a real + // suite starts with, interpolation would be inventing most of the answer. + var durations = new long[] { 100, 200, 300, 400, 500 }; + var repo = new InMemoryRepo + { + Cases = durations.Select((_, i) => CaseNamed($"c{i}")).ToList() + }; + + var byId = durations + .Select((ms, i) => (Id: $"c{i}", Ms: ms)) + .ToDictionary(x => x.Id, x => x.Ms); + + var executor = Build(repo, c => Timed(c.Id, byId[c.Id], tokens: 10, cost: 0.001)); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + var summary = Assert.Single(repo.Run.PerformanceSummaries); + Assert.Equal(5, summary.CaseCount); + // ceil(0.5 * 5) - 1 = 2 -> the third value. + Assert.Equal(300, summary.LatencyP50Ms); + // ceil(0.95 * 5) - 1 = 4 -> the slowest. + Assert.Equal(500, summary.LatencyP95Ms); + Assert.Contains(summary.LatencyP50Ms, durations); + Assert.Contains(summary.LatencyP95Ms, durations); + } + + [Fact] + public async Task A_case_that_never_reached_the_model_is_left_out_of_the_percentile() + { + // Otherwise a run that mostly crashed reports the best latency anyone has ever seen: the + // failures contribute zero, and zero drags a percentile down hard. + var repo = new InMemoryRepo { Cases = [CaseNamed("ok"), CaseNamed("dead")] }; + var executor = Build(repo, c => c.Id == "ok" + ? Timed("ok", modelMs: 400, tokens: 10, cost: 0.001) + : new AgentTestCaseResult + { + CaseId = "dead", + Status = AgentTestStatus.Error, + Error = "the mock seam is not live", + ModelDurationMs = 0, + TotalTokens = 7, + Cost = 0.0005 + }); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + var summary = Assert.Single(repo.Run.PerformanceSummaries); + Assert.Equal(1, summary.CaseCount); + Assert.Equal(400, summary.LatencyP50Ms); + + // Tokens and cost still count every result: the failed case spent what it spent, and hiding + // that would understate the run's real cost. + Assert.Equal(17, summary.TotalTokens); + Assert.Equal(0.0015, summary.TotalCost, precision: 6); + } + + [Fact] + public async Task Performance_is_summarised_per_model() + { + // Same reason routing accuracy is: one figure covering every model averages the candidate + // together with the baseline and hides the difference the run exists to measure. + var repo = new InMemoryRepo { Cases = [CaseNamed("a")] }; + repo.Run.Models = + [ + new TestModel { Provider = "openai", Model = "gpt-4o" }, + new TestModel { Provider = "openai", Model = "gpt-4o-mini" } + ]; + + var (executor, runner) = BuildWithRunner(repo, _ => new AgentTestCaseResult()); + runner.RunWithModel = (c, model) => new AgentTestCaseResult + { + CaseId = c.Id, + Status = AgentTestStatus.Passed, + ModelDurationMs = model?.Model == "gpt-4o" ? 900 : 300, + TotalTokens = 100, + Cost = 0.01 + }; + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + Assert.Equal(2, repo.Run.PerformanceSummaries.Count); + Assert.Equal(900, repo.Run.PerformanceSummaries.Single(s => s.Model == "gpt-4o").LatencyP95Ms); + Assert.Equal(300, repo.Run.PerformanceSummaries.Single(s => s.Model == "gpt-4o-mini").LatencyP95Ms); + } + + [Fact] + public async Task The_pricing_behind_a_cost_figure_is_snapshotted() + { + // A cost figure cannot be compared with any other run's without it: a provider price change + // would otherwise surface as a cost regression with nothing to point at. Recording the numbers + // rather than a version string is what makes two runs' pricing checkably the same. + var repo = new InMemoryRepo { Cases = [CaseNamed("a")] }; + repo.Run.Models = [new TestModel { Provider = "openai", Model = "gpt-4o" }]; + + var (executor, _) = BuildWithRunner( + repo, + c => Timed(c.Id, 100, 50, 0.005), + PricingFor("openai", "gpt-4o", input: 2.5f, output: 10f)); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + var pricing = Assert.Single(repo.Run.ModelPricing); + Assert.Equal("gpt-4o", pricing.Model); + Assert.Equal(2.5f, pricing.TextInputCost); + Assert.Equal(10f, pricing.TextOutputCost); + } + + [Fact] + public async Task Unknown_pricing_is_recorded_as_unknown_rather_than_as_free() + { + // Zero would read as "this model costs nothing", which is a claim. Null reads as "nobody + // knows", which is the truth and stops the figure being compared. + var repo = new InMemoryRepo { Cases = [CaseNamed("a")] }; + repo.Run.Models = [new TestModel { Provider = "openai", Model = "gpt-9-imaginary" }]; + + var (executor, _) = BuildWithRunner( + repo, c => Timed(c.Id, 100, 50, 0.005), PricingFor("openai", "gpt-4o", 2.5f, 10f)); + + await executor.ExecuteAsync("run-1", CancellationToken.None); + + var pricing = Assert.Single(repo.Run.ModelPricing); + Assert.Null(pricing.TextInputCost); + Assert.Null(pricing.TextOutputCost); + } } diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/AssertionEvaluatorTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/AssertionEvaluatorTests.cs index fc9f01c11..88b911bcf 100644 --- a/tests/BotSharp.Core.UnitTests/AgentTesting/AssertionEvaluatorTests.cs +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/AssertionEvaluatorTests.cs @@ -24,7 +24,10 @@ private static AssertionContext Context( Output = output, ToolCalls = calls ?? [], States = states ?? new Dictionary(), - RoutedToAgent = routedTo + // routedToAgent reads the chain's last entry, so a one-hop chain IS "routed to this + // agent". Given a name only, the id is left blank -- which is also what an agent whose + // record cannot be loaded looks like. + AgentChain = routedTo == null ? [] : [new AgentChainHop { Id = string.Empty, Name = routedTo }] }; [Fact] @@ -170,14 +173,18 @@ public void An_unknown_assertion_type_fails_loudly() } [Fact] - public void Llm_judge_is_reported_as_unavailable_in_p1_rather_than_silently_passing() + public void Llm_judge_fails_loudly_here_rather_than_silently_passing() { + // llmJudge is scored by IAgentTestJudge, because it needs a model call and this evaluator is + // pure and synchronous. Reaching this branch means somebody evaluated assertions without + // going through the runner, and the verdict they get back must be one they cannot mistake + // for a pass -- a silent pass would show a case that verified nothing as green. var result = AssertionEvaluator.Evaluate( - new TestAssertion { Type = AssertionTypes.LlmJudge, Expected = "confirms the address before quoting", MinScore = 0.8 }, + new TestAssertion { Type = AssertionTypes.LlmJudge, Expected = "confirms the address before quoting", MinScore = 4 }, Context(output: "whatever")); Assert.False(result.Passed); - Assert.Contains("not available in P1", result.Message!); + Assert.Contains("IAgentTestJudge", result.Message!); } /// diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/CaseAuthoringTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/CaseAuthoringTests.cs new file mode 100644 index 000000000..342283938 --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/CaseAuthoringTests.cs @@ -0,0 +1,522 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using BotSharp.Plugin.AgentTesting.Models; +using BotSharp.Plugin.AgentTesting.Services; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// Authoring a case by conversation hands a model write access to a document a human is editing. The +/// four things stopping that from going wrong are all in and +/// , and this is where they are pinned: +/// +/// 1. a field the model does not declare cannot be deleted by being left out of its answer; +/// 2. a mock or tool assertion naming a function the agent cannot call is removed, not stored; +/// 3. a draft that would fail on save comes back labelled, not presented as progress; +/// 4. the diff shown to the user is computed from the two drafts, never read off the model's own +/// account of what it did. +/// +/// No vendor is involved: both methods are pure, which is why they are public. +/// +public class CaseAuthoringTests +{ + private static AgentTestSuite Suite(string? judgeProvider = null, string? judgeModel = null) => new() + { + Id = "suite-1", + AgentId = "agent-1", + Name = "suite", + Enabled = true, + JudgeProvider = judgeProvider, + JudgeModel = judgeModel + }; + + private static List Targets(params string[] names) + => names.Select(n => new MockTargetInfo(n, null, null)).ToList(); + + /// A saved case with one turn and one mock -- the thing an authoring turn can damage. + private static AgentTestCaseUpsertRequest Baseline() => new() + { + SuiteId = "suite-1", + Name = "where is my technician", + Turns = [new TestTurn { Index = 0, UserMessage = "where is my tech" }], + Mocks = [new TestToolMock { FunctionName = "get_eta", ResultContent = """{"eta":"2pm"}""" }] + }; + + private static AuthorAttempt Attempt( + string[] declared, + AgentTestCaseUpsertRequest? draft, + string reply = "Added a turn.", + string[]? rejected = null) + => new(reply, declared.ToList(), (rejected ?? []).ToList(), draft, "{}"); + + private static AgentTestAuthorResponse Run( + AuthorAttempt attempt, + AgentTestCaseUpsertRequest? baseline = null, + List? targets = null, + List? existing = null, + AgentTestSuite? suite = null) + => LlmCaseAuthor.Assemble( + baseline ?? Baseline(), + attempt, + targets ?? Targets("get_eta", "reschedule"), + existing ?? [], + suite ?? Suite()); + + // ---- 1. A field the model does not declare survives ------------------------------------- + + [Fact] + public void An_undeclared_field_is_kept_even_when_the_model_returns_it_empty() + { + // The whole reason for the declared-fields whitelist. A model that returns the document with + // `mocks` missing is the normal failure of asking for a full document back, and taking its + // answer wholesale would delete a mock nobody asked it to touch -- silently, because an + // absent mock only shows up as the case failing on its next run. + var modelDraft = new AgentTestCaseUpsertRequest + { + Name = "where is my technician", + Turns = + [ + new TestTurn { Index = 0, UserMessage = "where is my tech" }, + new TestTurn { Index = 1, UserMessage = "and the address?" } + ], + Mocks = [] + }; + + var response = Run(Attempt(["turns"], modelDraft)); + + Assert.Single(response.Draft.Mocks); + Assert.Equal("get_eta", response.Draft.Mocks[0].FunctionName); + Assert.Equal(2, response.Draft.Turns.Count); + } + + [Fact] + public void A_declared_field_is_applied() + { + var modelDraft = new AgentTestCaseUpsertRequest + { + Turns = + [ + new TestTurn { Index = 0, UserMessage = "where is my tech" }, + new TestTurn { Index = 1, UserMessage = "and the address?" } + ] + }; + + var response = Run(Attempt(["turns"], modelDraft)); + + Assert.True(response.DraftChanged); + Assert.Contains(response.Changes, c => c.Field == "turns"); + Assert.Equal("1 -> 2 item(s)", response.Changes.Single(c => c.Field == "turns").Detail); + } + + [Fact] + public void A_field_the_model_may_not_change_is_ignored_and_reported() + { + // AuthorFields.Normalize returns null for anything unwritable, and Parse routes those into + // RejectedFields. Reported rather than dropped in silence: "I renamed the suite for you" + // needs to be visibly untrue. + var response = Run(Attempt([], null, reply: "Renamed the suite.", rejected: ["suiteId"])); + + Assert.False(response.DraftChanged); + Assert.Contains(response.Warnings, w => w.Contains("suiteId")); + } + + [Fact] + public void The_unmocked_tool_policy_cannot_be_moved_off_block() + { + // Not a writable field, so a model proposing Passthrough gets Block anyway. Worth its own + // test because Passthrough is the one setting under which toolNotCalled passes vacuously + // against a tool that really executed. + var modelDraft = new AgentTestCaseUpsertRequest + { + Turns = [new TestTurn { Index = 0, UserMessage = "where is my tech" }], + UnmockedToolPolicy = "Passthrough" + }; + + var response = Run(Attempt(["turns"], modelDraft)); + + Assert.Equal(UnmockedToolPolicies.Block, response.Draft.UnmockedToolPolicy); + Assert.Empty(response.ValidationErrors); + } + + [Fact] + public void Declaring_a_field_without_returning_a_draft_changes_nothing() + { + var response = Run(Attempt(["turns"], null)); + + Assert.False(response.DraftChanged); + Assert.Single(response.Draft.Turns); + Assert.Contains(response.Warnings, w => w.Contains("returned no draft")); + } + + // ---- 2. Never a function the agent cannot call ------------------------------------------ + + [Fact] + public void A_mock_for_an_unknown_function_is_dropped() + { + var modelDraft = new AgentTestCaseUpsertRequest + { + Mocks = + [ + new TestToolMock { FunctionName = "get_eta", ResultContent = "{}" }, + new TestToolMock { FunctionName = "send_sms_to_resident", ResultContent = "{}" } + ] + }; + + var response = Run(Attempt(["mocks"], modelDraft)); + + Assert.Single(response.Draft.Mocks); + Assert.Equal("get_eta", response.Draft.Mocks[0].FunctionName); + Assert.Contains(response.Warnings, w => w.Contains("send_sms_to_resident")); + } + + [Fact] + public void A_tool_assertion_on_an_unknown_function_is_dropped() + { + // Kept, it would never match at run time, so the case would fail forever for a reason that + // reads as an agent regression. + var modelDraft = new AgentTestCaseUpsertRequest + { + Assertions = + [ + new TestAssertion { Type = AssertionTypes.ToolCalled, Target = "get_eta" }, + new TestAssertion { Type = AssertionTypes.ToolCalled, Target = "get_estimated_arrival" } + ] + }; + + var response = Run(Attempt(["assertions"], modelDraft)); + + Assert.Single(response.Draft.Assertions); + Assert.Equal("get_eta", response.Draft.Assertions[0].Target); + Assert.Contains(response.Warnings, w => w.Contains("get_estimated_arrival")); + } + + [Fact] + public void A_non_tool_assertion_is_never_dropped_for_its_target() + { + // stateEquals also carries a Target, and it is a state key, not a function name. + var modelDraft = new AgentTestCaseUpsertRequest + { + Assertions = [new TestAssertion { Type = AssertionTypes.StateEquals, Target = "wo_num", Expected = "B1" }] + }; + + var response = Run(Attempt(["assertions"], modelDraft)); + + Assert.Single(response.Draft.Assertions); + } + + [Fact] + public void An_unfamiliar_state_key_is_kept_but_flagged() + { + // The asymmetry with function names is deliberate: nothing in this system enumerates the keys + // an agent writes, so the "known" list is only what other cases happen to use. Dropping on + // absence from an admittedly incomplete list would delete correct work. + var modelDraft = new AgentTestCaseUpsertRequest + { + Assertions = [new TestAssertion { Type = AssertionTypes.StateEquals, Target = "invented_key", Expected = "x" }] + }; + + var response = Run(Attempt(["assertions"], modelDraft)); + + Assert.Single(response.Draft.Assertions); + Assert.Contains(response.Warnings, w => w.Contains("invented_key")); + } + + [Fact] + public void A_state_key_another_case_already_uses_is_not_flagged() + { + var existing = new List + { + new() + { + Id = "case-9", + SuiteId = "suite-1", + Name = "other", + InitialStates = [new TestState { Key = "wo_num", Value = "B1" }] + } + }; + + var modelDraft = new AgentTestCaseUpsertRequest + { + Assertions = [new TestAssertion { Type = AssertionTypes.StateEquals, Target = "wo_num", Expected = "B1" }] + }; + + var response = Run(Attempt(["assertions"], modelDraft), existing: existing); + + Assert.DoesNotContain(response.Warnings, w => w.Contains("wo_num")); + } + + [Fact] + public void An_llm_judge_assertion_on_a_suite_with_no_judge_model_is_flagged() + { + // It would not fail validation -- it would save cleanly and then fail every run, which is the + // sort of thing worth saying at authoring time. + var modelDraft = new AgentTestCaseUpsertRequest + { + Assertions = [new TestAssertion { Type = AssertionTypes.LlmJudge, Expected = "polite and specific" }] + }; + + var withoutJudge = Run(Attempt(["assertions"], modelDraft)); + Assert.Contains(withoutJudge.Warnings, w => w.Contains("judge model")); + + var withJudge = Run(Attempt(["assertions"], modelDraft), suite: Suite("openai", "gpt-4o")); + Assert.DoesNotContain(withJudge.Warnings, w => w.Contains("judge model")); + } + + // ---- 3. An invalid draft is labelled, not presented as progress ------------------------- + + [Fact] + public void A_routing_case_the_model_gave_two_turns_comes_back_with_the_validation_error() + { + var modelDraft = new AgentTestCaseUpsertRequest + { + CaseType = CaseTypes.Routing, + Turns = + [ + new TestTurn { Index = 0, UserMessage = "my fridge leaks" }, + new TestTurn { Index = 1, UserMessage = "when will someone come" } + ], + Assertions = [new TestAssertion { Type = AssertionTypes.RoutedToAgent, Expected = "Work Order Agent" }] + }; + + var response = Run(Attempt(["caseType", "turns", "assertions"], modelDraft)); + + Assert.NotEmpty(response.ValidationErrors); + Assert.Contains("exactly one turn", response.ValidationErrors[0]); + } + + [Fact] + public void A_routing_case_that_asserts_no_routing_outcome_is_rejected() + { + var modelDraft = new AgentTestCaseUpsertRequest + { + CaseType = CaseTypes.Routing, + Turns = [new TestTurn { Index = 0, UserMessage = "my fridge leaks" }], + Assertions = [new TestAssertion { Type = AssertionTypes.OutputContains, Expected = "sorry" }] + }; + + var response = Run(Attempt(["caseType", "turns", "assertions"], modelDraft)); + + Assert.NotEmpty(response.ValidationErrors); + } + + [Fact] + public void Turns_are_renumbered_from_zero() + { + // A model inserting a turn tends to renumber badly or not at all, and the runner reads turns + // in order -- so a stale Index would reorder the case rather than fail it. + var modelDraft = new AgentTestCaseUpsertRequest + { + Turns = + [ + new TestTurn { Index = 5, UserMessage = "first" }, + new TestTurn { Index = 5, UserMessage = "second" } + ] + }; + + var response = Run(Attempt(["turns"], modelDraft)); + + Assert.Equal([0, 1], response.Draft.Turns.Select(t => t.Index)); + } + + // ---- 4. The diff is computed, not claimed ----------------------------------------------- + + [Fact] + public void A_declared_field_that_did_not_actually_change_is_not_reported_as_changed() + { + // The model claims two fields; only one differs. What the user sees has to be the second + // number, or the change list stops being evidence of anything. + var modelDraft = new AgentTestCaseUpsertRequest + { + Name = "where is my technician", + Turns = + [ + new TestTurn { Index = 0, UserMessage = "where is my tech" }, + new TestTurn { Index = 1, UserMessage = "and the address?" } + ] + }; + + var response = Run(Attempt(["name", "turns"], modelDraft)); + + Assert.Contains(response.Changes, c => c.Field == "turns"); + Assert.DoesNotContain(response.Changes, c => c.Field == "name"); + } + + [Fact] + public void An_answer_that_declares_nothing_is_a_question_not_a_no_op_edit() + { + // The clarifying-question path. It has to leave the draft untouched and still deliver the + // reply, or the model cannot ask anything without also having to invent an edit. + var response = Run(Attempt([], null, reply: "Which work order should the case use?")); + + Assert.False(response.DraftChanged); + Assert.Empty(response.Changes); + Assert.Equal("Which work order should the case use?", response.Reply); + Assert.Single(response.Draft.Turns); + Assert.Single(response.Draft.Mocks); + } + + [Fact] + public void The_suite_id_always_comes_from_the_request() + { + var modelDraft = new AgentTestCaseUpsertRequest + { + SuiteId = "some-other-suite", + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }; + + var response = Run(Attempt(["turns"], modelDraft)); + + Assert.Equal("suite-1", response.Draft.SuiteId); + } + + // ---- Parse: reading the envelope -------------------------------------------------------- + + [Fact] + public void Parse_normalises_declared_field_names_and_separates_the_unwritable_ones() + { + var attempt = LlmCaseAuthor.Parse( + """{"reply":"ok","changedFields":["Turns","lastReviewedDate","turns"],"draft":{"name":"x"}}"""); + + Assert.Equal("ok", attempt.Reply); + Assert.Equal(["turns"], attempt.DeclaredFields); + Assert.Equal(["lastReviewedDate"], attempt.RejectedFields); + Assert.Equal("x", attempt.Draft?.Name); + } + + [Fact] + public void Parse_tolerates_a_code_fence_and_surrounding_prose() + { + // The commonest and most harmless way for a model to disobey "JSON only" -- the segmenter + // takes the same view. + var attempt = LlmCaseAuthor.Parse( + "Here you go:\n```json\n{\"reply\":\"ok\",\"changedFields\":[],\"draft\":null}\n```"); + + Assert.Equal("ok", attempt.Reply); + Assert.Empty(attempt.DeclaredFields); + } + + [Fact] + public void Parse_treats_a_missing_changed_fields_list_as_no_change() + { + var attempt = LlmCaseAuthor.Parse("""{"reply":"I need to know the work order number first."}"""); + + Assert.Empty(attempt.DeclaredFields); + Assert.Null(attempt.Draft); + } + + [Theory] + [InlineData("I could not do that.")] + [InlineData("")] + [InlineData("{ not json at all ")] + public void Parse_rejects_an_answer_it_cannot_read(string raw) + { + // No draft was produced, which is a different outcome from an invalid draft: the caller + // should retry, not review. + Assert.Throws(() => LlmCaseAuthor.Parse(raw)); + } + + // ---- Parse: coercing a JSON-as-text field written as a nested object ------------------- + // + // The exact shape reported in production: a model writes argsMatchJson as a real object because + // that is the natural way to express "match these arguments", even though the field is a string + // holding escaped JSON. Both forms are syntactically valid JSON, so nothing before deserialization + // can reject this as malformed -- it has to be normalised before the strongly-typed model ever + // sees it, or every case-level and turn-level argsMatchJson blows up the whole reply. + + [Fact] + public void An_object_valued_argsMatchJson_on_a_case_level_assertion_is_coerced_to_its_json_text() + { + var attempt = LlmCaseAuthor.Parse( + """ + {"reply":"ok","changedFields":["assertions"],"draft":{"assertions":[ + {"type":"toolCalled","target":"get_eta","argsMatchJson":{"work_order_id":"12345"}} + ]}} + """); + + var assertion = Assert.Single(attempt.Draft!.Assertions); + Assert.Equal("""{"work_order_id":"12345"}""", assertion.ArgsMatchJson); + } + + [Fact] + public void An_object_valued_argsMatchJson_on_a_turn_level_assertion_is_also_coerced() + { + var attempt = LlmCaseAuthor.Parse( + """ + {"reply":"ok","changedFields":["turns"],"draft":{"turns":[ + {"index":0,"userMessage":"where is my tech","assertions":[ + {"type":"toolCalled","target":"get_eta","argsMatchJson":{"wo_num":"B1"}} + ]} + ]}} + """); + + var assertion = Assert.Single(attempt.Draft!.Turns[0].Assertions); + Assert.Equal("""{"wo_num":"B1"}""", assertion.ArgsMatchJson); + } + + [Fact] + public void An_object_valued_mock_result_content_and_args_match_are_both_coerced() + { + var attempt = LlmCaseAuthor.Parse( + """ + {"reply":"ok","changedFields":["mocks"],"draft":{"mocks":[ + {"functionName":"get_eta","argsMatchJson":{"wo_num":"B1"},"resultContent":{"eta":"2pm"}} + ]}} + """); + + var mock = Assert.Single(attempt.Draft!.Mocks); + Assert.Equal("""{"wo_num":"B1"}""", mock.ArgsMatchJson); + Assert.Equal("""{"eta":"2pm"}""", mock.ResultContent); + } + + [Fact] + public void An_array_valued_mock_state_write_value_is_coerced() + { + var attempt = LlmCaseAuthor.Parse( + """ + {"reply":"ok","changedFields":["mocks"],"draft":{"mocks":[ + {"functionName":"get_eta","stateWrites":[{"key":"items","value":[1,2,3]}]} + ]}} + """); + + Assert.Equal("[1,2,3]", attempt.Draft!.Mocks[0].StateWrites![0].Value); + } + + [Fact] + public void An_object_valued_initial_state_value_is_coerced() + { + var attempt = LlmCaseAuthor.Parse( + """ + {"reply":"ok","changedFields":["initialStates"],"draft":{"initialStates":[ + {"key":"customer","value":{"name":"Jane"}} + ]}} + """); + + Assert.Equal("""{"name":"Jane"}""", attempt.Draft!.InitialStates[0].Value); + } + + [Fact] + public void A_string_valued_argsMatchJson_passes_through_unchanged() + { + // The correct, already-escaped form must not be touched -- only the wrong shape is coerced. + var attempt = LlmCaseAuthor.Parse( + """ + {"reply":"ok","changedFields":["assertions"],"draft":{"assertions":[ + {"type":"toolCalled","target":"get_eta","argsMatchJson":"{\"wo_num\":\"B1\"}"} + ]}} + """); + + Assert.Equal("""{"wo_num":"B1"}""", attempt.Draft!.Assertions[0].ArgsMatchJson); + } + + [Fact] + public void A_genuine_json_syntax_error_still_fails_instead_of_being_silently_swallowed() + { + // The coercion pass must not mask a real syntax error by quietly returning the input + // unchanged and hoping for the best -- it has to fall through to the same rejection an + // unrecoverable reply always gets. + Assert.Throws(() => LlmCaseAuthor.Parse( + """{"reply":"ok","changedFields":["assertions"],"draft":{"assertions":[{,}]}}""")); + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/CaseScopeTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/CaseScopeTests.cs new file mode 100644 index 000000000..89855118b --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/CaseScopeTests.cs @@ -0,0 +1,245 @@ +using System.Collections.Generic; +using System.Linq; +using BotSharp.Plugin.AgentTesting.Models; +using BotSharp.Plugin.AgentTesting.Services; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// Scope narrowing is the one feature here whose failure mode is silence. A case wrongly INCLUDED +/// costs tokens and is obvious. A case wrongly EXCLUDED produces no result at all, and once the +/// numbers reach a report "not run" is indistinguishable from "passed" -- so every rule below is +/// checked in the direction of including, and the exclusions are checked for having a reason someone +/// can review. +/// +public class CaseScopeTests +{ + private const string CopilotId = "2cd4b805-7078-4405-87e9-2ec9aadf8a11"; + private const string WoCancellationId = "0fe3905d-75f1-4e2e-8e54-ec3d33d6b6f0"; + private const string DiagnosisId = "11111111-2222-3333-4444-555555555555"; + + private static AgentTestCase Case( + string? entryAgentId = null, + bool enabled = true, + bool crossCutting = false, + string priority = CasePriorities.P1, + int? batch = null, + params string[] involved) => new() + { + Id = "case-1", + SuiteId = "suite-1", + Name = "c", + Enabled = enabled, + CrossCutting = crossCutting, + Priority = priority, + Batch = batch, + EntryAgentId = entryAgentId, + InvolvedAgents = involved.ToList(), + Turns = [new TestTurn { Index = 0, UserMessage = "hi" }] + }; + + private static ScopeQuery Changed(params string[] targetAgentIds) + => new() { TargetAgentIds = targetAgentIds }; + + // ------------------------------------------------------------------ involved agents + + [Fact] + public void An_authored_involved_list_wins_over_the_entry_agent() + { + // The reason the field exists: for a routing case the entry agent is the router, and the + // agents that matter are the ones downstream of it. Those cannot be derived from the case + // definition at all. + var involved = CaseScope.InvolvedAgentIds( + Case(entryAgentId: CopilotId, involved: [WoCancellationId, DiagnosisId]), "suite-agent"); + + Assert.Equal([WoCancellationId, DiagnosisId], involved); + } + + [Fact] + public void With_no_authored_list_the_entry_agent_is_the_involved_agent() + { + // Definitionally true and already known, so an Agent case is picked up by a change to the + // agent it runs against without anyone maintaining a list. + Assert.Equal([CopilotId], CaseScope.InvolvedAgentIds(Case(entryAgentId: CopilotId), "suite-agent")); + } + + [Fact] + public void With_no_entry_agent_the_suites_agent_is_used() + { + // Which is exactly what the runner does when it opens the conversation, so the scope is + // computed against the agent the case really runs on. + Assert.Equal(["suite-agent"], CaseScope.InvolvedAgentIds(Case(), "suite-agent")); + } + + [Fact] + public void Blank_entries_in_an_authored_list_are_ignored() + { + // A UI that posts an empty row must not contribute an agent id of "" that matches nothing and + // silently narrows the scope. + var involved = CaseScope.InvolvedAgentIds( + Case(involved: [CopilotId, "", " "]), "suite-agent"); + + Assert.Equal([CopilotId], involved); + } + + // ------------------------------------------------------------------ rules + + [Fact] + public void Rule_3_includes_a_case_that_touches_a_changed_agent() + { + var decision = CaseScope.Decide(Case(entryAgentId: CopilotId), "suite-agent", Changed(CopilotId)); + + Assert.True(decision.Included); + Assert.Equal(ScopeReasons.TargetAgent, decision.Reason); + } + + [Fact] + public void Rule_4_excludes_a_case_that_touches_none_of_them() + { + // The entire point of narrowing: a change to one agent cannot affect a case that never goes + // near it, and running it anyway buys nothing. + var decision = CaseScope.Decide(Case(entryAgentId: CopilotId), "suite-agent", Changed(DiagnosisId)); + + Assert.False(decision.Included); + Assert.Equal(ScopeReasons.NotInvolved, decision.Reason); + } + + [Fact] + public void Agent_ids_are_matched_case_insensitively() + { + var decision = CaseScope.Decide( + Case(entryAgentId: CopilotId.ToUpperInvariant()), "suite-agent", Changed(CopilotId)); + + Assert.True(decision.Included); + } + + [Fact] + public void Rule_1_includes_a_cross_cutting_case_no_matter_what_changed() + { + // Narrowing exists to skip cases a change cannot affect, and "this change cannot affect + // safety" is precisely the claim not to accept untested. + var decision = CaseScope.Decide( + Case(entryAgentId: CopilotId, crossCutting: true), "suite-agent", Changed(DiagnosisId)); + + Assert.True(decision.Included); + Assert.Equal(ScopeReasons.CrossCutting, decision.Reason); + } + + [Fact] + public void Rule_2_includes_everything_for_a_platform_wide_change() + { + // A foundation model or provider swap has no agent it demonstrably does not touch, so + // narrowing switches off rather than being applied against an empty target list. + var decision = CaseScope.Decide( + Case(entryAgentId: CopilotId), "suite-agent", + new ScopeQuery { FullPlatform = true }); + + Assert.True(decision.Included); + Assert.Equal(ScopeReasons.FullPlatform, decision.Reason); + } + + [Fact] + public void A_case_with_no_known_agents_is_included_rather_than_dropped() + { + // Fail open. An unknown involved set means the harness cannot show the change does not affect + // this case; excluding it on that basis would produce no result to notice. + var decision = CaseScope.Decide(Case(), suiteAgentId: null, Changed(CopilotId)); + + Assert.True(decision.Included); + Assert.Equal(ScopeReasons.UnknownAgents, decision.Reason); + } + + [Fact] + public void A_disabled_case_is_excluded_and_says_so() + { + // The executor skips it, so calling it in scope would overstate coverage by exactly the cases + // nobody is running. Its own reason, so a disabled cross-cutting safety case stands out + // instead of blending in with cases the change genuinely cannot affect. + var decision = CaseScope.Decide( + Case(entryAgentId: CopilotId, enabled: false, crossCutting: true), "suite-agent", + Changed(CopilotId)); + + Assert.False(decision.Included); + Assert.Equal(ScopeReasons.Disabled, decision.Reason); + } + + // ------------------------------------------------------------------ batches + + [Theory] + [InlineData(CasePriorities.P0, CaseBatches.StopLoss)] + [InlineData(CasePriorities.P1, CaseBatches.Mandatory)] + [InlineData(CasePriorities.P2, CaseBatches.Optional)] + public void Priority_derives_the_batch(string priority, int expected) + { + Assert.Equal(expected, CaseBatches.Effective(Case(priority: priority))); + } + + [Fact] + public void A_cross_cutting_case_is_batch_one_whatever_its_priority() + { + // A safety case that only runs once everything else has passed cannot stop anything, which is + // the entire job of batch 1. + Assert.Equal( + CaseBatches.StopLoss, + CaseBatches.Effective(Case(priority: CasePriorities.P2, crossCutting: true))); + } + + [Fact] + public void An_explicit_batch_overrides_both() + { + Assert.Equal( + CaseBatches.Optional, + CaseBatches.Effective(Case(priority: CasePriorities.P0, crossCutting: true, batch: CaseBatches.Optional))); + } + + [Fact] + public void An_out_of_range_explicit_batch_falls_back_to_the_derivation() + { + // Rejected at save time, so this is only reachable for a document written before the check or + // edited around the API. Falling back beats filing the case in a batch that does not exist, + // where nothing would ever run it. + Assert.Equal(CaseBatches.StopLoss, CaseBatches.Effective(Case(priority: CasePriorities.P0, batch: 9))); + } + + [Fact] + public void Narrowing_to_a_batch_excludes_the_others_with_their_own_reason() + { + // Scheduling, not scoping: batches exist to run in order and stop early, so a case left out + // of this batch is not out of scope -- it just runs later, and the reason has to say that. + var decision = CaseScope.Decide( + Case(entryAgentId: CopilotId, priority: CasePriorities.P2), "suite-agent", + new ScopeQuery { TargetAgentIds = [CopilotId], Batch = CaseBatches.StopLoss }); + + Assert.False(decision.Included); + Assert.Equal(ScopeReasons.OtherBatch, decision.Reason); + } + + [Fact] + public void A_cross_cutting_case_still_belongs_to_batch_one_when_narrowing_by_batch() + { + // The two axes have to agree: cross-cutting forces batch 1, so asking for batch 1 has to + // return it. If they disagreed, the stop-loss batch would run without its safety cases. + var decision = CaseScope.Decide( + Case(entryAgentId: DiagnosisId, crossCutting: true, priority: CasePriorities.P2), + "suite-agent", + new ScopeQuery { TargetAgentIds = [CopilotId], Batch = CaseBatches.StopLoss }); + + Assert.True(decision.Included); + Assert.Equal(ScopeReasons.CrossCutting, decision.Reason); + Assert.Equal(CaseBatches.StopLoss, decision.Batch); + } + + [Fact] + public void The_decision_reports_the_set_it_was_made_against() + { + // A scope nobody can explain is a scope nobody can review, and reviewing it is the only + // defence against signing off a change against a set that quietly left out the interesting + // case. + var decision = CaseScope.Decide( + Case(entryAgentId: CopilotId, involved: [WoCancellationId]), "suite-agent", Changed(CopilotId)); + + Assert.False(decision.Included); + Assert.Equal([WoCancellationId], decision.InvolvedAgentIds); + } +} diff --git a/tests/BotSharp.Core.UnitTests/AgentTesting/SyntheticConversationExemptionTests.cs b/tests/BotSharp.Core.UnitTests/AgentTesting/SyntheticConversationExemptionTests.cs new file mode 100644 index 000000000..e4ce5e12a --- /dev/null +++ b/tests/BotSharp.Core.UnitTests/AgentTesting/SyntheticConversationExemptionTests.cs @@ -0,0 +1,286 @@ +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using BotSharp.Abstraction.Agents.Enums; +using BotSharp.Abstraction.Conversations; +using BotSharp.Abstraction.Conversations.Models; +using BotSharp.Abstraction.Conversations.Settings; +using BotSharp.Abstraction.Repositories.Filters; +using BotSharp.Abstraction.Users; +using BotSharp.Abstraction.Utilities; +using BotSharp.Logger.Hooks; +using BotSharp.Plugin.AgentTesting.Runtime; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace BotSharp.Core.UnitTests.AgentTesting; + +/// +/// BotSharp's rate limiting counts human behaviour, and a regression suite does not look like one: it +/// opens a conversation per case per model and drives the turns as fast as the model answers. +/// +/// Two of its three guards therefore have to stand aside for a harness conversation, and BOTH of them +/// matter -- the conversation quota is the one that surfaced first, but the two-second gap between +/// messages blocks any case with authored history outright, because injected history lands with the +/// current timestamp and the first real turn follows it immediately. +/// +/// The direction of these tests matters as much as the assertions: a probe that wrongly answers yes +/// exempts real user traffic from the limits it is meant to be held to, so every "real traffic" case +/// below is checked for still being limited. +/// +public class SyntheticConversationExemptionTests +{ + private const string ConversationId = "conv-under-test"; + + private static RateLimitConversationHook BuildHook( + IServiceProvider services, List dialogs) + { + var hook = new RateLimitConversationHook( + services, NullLogger.Instance); + + // Dialogs is only reachable through the base class's own loader, which is also how the real + // conversation path populates it. + hook.OnDialogsLoaded(dialogs).GetAwaiter().GetResult(); + return hook; + } + + /// + /// Everything OnMessageReceived reaches for. null registers none at all, + /// which is the normal deployment and must exempt nobody. + /// + private static IServiceProvider BuildServices( + ISyntheticConversationProbe? probe, + int conversationsToday, + string conversationId = ConversationId) + { + var services = new ServiceCollection(); + + services.AddSingleton(new ConversationSetting + { + RateLimit = new RateLimitSetting + { + MaxConversationPerDay = 100, + MaxInputLengthPerRequest = 1024, + MinTimeSecondsBetweenMessages = 2 + } + }); + + var states = new Mock(); + // Blank, exactly as a harness conversation leaves it: the guards are skipped for Phone, Email + // and Database channels, and the harness deliberately seeds no channel because production + // routing branches on that key. + states.Setup(x => x.GetState(It.IsAny(), It.IsAny())).Returns(string.Empty); + // The hook takes the conversation id from state rather than from IConversationService. + states.Setup(x => x.GetConversationId()).Returns(conversationId); + services.AddSingleton(states.Object); + + // Only reached on the over-long-message path, where the hook stores the message before + // replacing it. Registered for every case so a test that trips that guard does not fail on a + // missing service instead of on what it is checking. + services.AddSingleton(Mock.Of()); + + var identity = new Mock(); + // Empty, as it is in a BackgroundService with no HTTP context. Note the Mongo filter drops an + // empty UserId rather than matching on it, so this counts every conversation in the instance. + identity.Setup(x => x.Id).Returns(string.Empty); + services.AddSingleton(identity.Object); + + var conversations = new Mock(); + conversations + .Setup(x => x.GetConversations(It.IsAny())) + .ReturnsAsync(new PagedItems + { + Count = conversationsToday, + Items = [] + }); + services.AddSingleton(conversations.Object); + + if (probe != null) + { + services.AddSingleton(probe); + } + + return services.BuildServiceProvider(); + } + + private static List TwoUserMessagesOneSecondApart() => + [ + new RoleDialogModel(AgentRole.User, "earlier") { CreatedAt = DateTime.UtcNow.AddSeconds(-1) }, + new RoleDialogModel(AgentRole.User, "now") { CreatedAt = DateTime.UtcNow } + ]; + + // ------------------------------------------------------------------ the probe + + [Fact] + public void The_probe_recognises_a_conversation_the_harness_registered() + { + var registry = new AgentTestRunRegistry(); + registry.Register(new ActiveTestRun { ConversationId = ConversationId, CaseId = "case-1" }); + + var probe = new AgentTestSyntheticConversationProbe(registry); + + Assert.True(probe.IsSynthetic(ConversationId)); + } + + [Theory] + [InlineData("some-other-conversation")] + [InlineData("")] + [InlineData(null)] + public void The_probe_answers_no_for_anything_it_does_not_recognise(string? conversationId) + { + // Answering yes here would exempt real user traffic from the limits it exists to be held to, + // which is a far worse failure than a test being rate limited. + var registry = new AgentTestRunRegistry(); + registry.Register(new ActiveTestRun { ConversationId = ConversationId, CaseId = "case-1" }); + + var probe = new AgentTestSyntheticConversationProbe(registry); + + Assert.False(probe.IsSynthetic(conversationId!)); + } + + [Fact] + public void The_probe_stops_recognising_a_conversation_once_the_case_finishes() + { + // The runner unregisters in a finally block. If the probe kept saying yes afterwards, that + // conversation id would stay exempt from rate limiting for the life of the process. + var registry = new AgentTestRunRegistry(); + registry.Register(new ActiveTestRun { ConversationId = ConversationId, CaseId = "case-1" }); + var probe = new AgentTestSyntheticConversationProbe(registry); + + registry.Unregister(ConversationId); + + Assert.False(probe.IsSynthetic(ConversationId)); + } + + // ------------------------------------------------------------------ the quota guard + + [Fact] + public async Task Real_traffic_over_the_quota_is_still_stopped() + { + // The guard has to keep working. With no probe registered -- the normal deployment -- nothing + // is exempt. + var services = BuildServices(probe: null, conversationsToday: 107); + var hook = BuildHook(services, []); + var message = new RoleDialogModel(AgentRole.User, "hello"); + + await hook.OnMessageReceived(message); + + Assert.True(message.StopCompletion); + Assert.Contains("exceeds the system maximum of 100", message.Content); + } + + [Fact] + public async Task A_harness_conversation_over_the_quota_runs_anyway() + { + // The reported failure. 107 conversations across the whole instance in 24 hours, of which the + // harness opened 30 -- and because an empty UserId drops the filter rather than matching on + // it, the harness was measured against all 107 and every case failed. + var registry = new AgentTestRunRegistry(); + registry.Register(new ActiveTestRun { ConversationId = ConversationId, CaseId = "case-1" }); + + var services = BuildServices( + new AgentTestSyntheticConversationProbe(registry), conversationsToday: 107); + var hook = BuildHook(services, []); + var message = new RoleDialogModel(AgentRole.User, "hello"); + + await hook.OnMessageReceived(message); + + Assert.False(message.StopCompletion); + Assert.Equal("hello", message.Content); + } + + [Fact] + public async Task A_conversation_the_probe_does_not_claim_is_still_limited() + { + // A harness being active must not exempt everything else running at the same time. + var registry = new AgentTestRunRegistry(); + registry.Register(new ActiveTestRun { ConversationId = "a-different-case", CaseId = "case-1" }); + + var services = BuildServices( + new AgentTestSyntheticConversationProbe(registry), conversationsToday: 107); + var hook = BuildHook(services, []); + var message = new RoleDialogModel(AgentRole.User, "hello"); + + await hook.OnMessageReceived(message); + + Assert.True(message.StopCompletion); + } + + // ------------------------------------------------------------------ the frequency guard + + [Fact] + public async Task Real_traffic_sending_faster_than_the_minimum_gap_is_still_stopped() + { + var services = BuildServices(probe: null, conversationsToday: 1); + var hook = BuildHook(services, TwoUserMessagesOneSecondApart()); + var message = new RoleDialogModel(AgentRole.User, "hello"); + + await hook.OnMessageReceived(message); + + Assert.True(message.StopCompletion); + Assert.Contains("frequency", message.Content); + } + + [Fact] + public async Task A_harness_conversation_is_not_held_to_the_minimum_gap() + { + // This is the guard that blocks any case with authored history: injected history lands with + // the current timestamp, so the first real turn follows the last history message by about zero + // seconds and trips a two-second minimum every time. It would also make multi-turn cases + // flaky whenever the model happens to answer quickly. + var registry = new AgentTestRunRegistry(); + registry.Register(new ActiveTestRun { ConversationId = ConversationId, CaseId = "case-1" }); + + var services = BuildServices( + new AgentTestSyntheticConversationProbe(registry), conversationsToday: 1); + var hook = BuildHook(services, TwoUserMessagesOneSecondApart()); + var message = new RoleDialogModel(AgentRole.User, "hello"); + + await hook.OnMessageReceived(message); + + Assert.False(message.StopCompletion); + } + + // ------------------------------------------------------------------ the input length guard + + [Fact] + public async Task An_over_long_message_is_still_rejected_in_a_harness_conversation() + { + // Kept in force on purpose. That guard is about one message being too large for the model, + // which is a real condition a test should surface rather than be excused from -- unlike the + // two volume guards, which measure how much a human has been using the system. + var registry = new AgentTestRunRegistry(); + registry.Register(new ActiveTestRun { ConversationId = ConversationId, CaseId = "case-1" }); + + var services = BuildServices( + new AgentTestSyntheticConversationProbe(registry), conversationsToday: 1); + var hook = BuildHook(services, []); + var message = new RoleDialogModel(AgentRole.User, new string('x', 2000)); + + await hook.OnMessageReceived(message); + + Assert.True(message.StopCompletion); + Assert.Contains("characters", message.Content); + } + + // ------------------------------------------------------------------ a misbehaving probe + + [Fact] + public async Task A_probe_that_throws_fails_closed() + { + // Failing open would let a bug in a probe silently lift the limits for real traffic. The worst + // case here is a harness message being rate limited, which is visible and recoverable. + var probe = new Mock(); + probe.Setup(x => x.IsSynthetic(It.IsAny())).Throws(new InvalidOperationException("boom")); + + var services = BuildServices(probe.Object, conversationsToday: 107); + var hook = BuildHook(services, []); + var message = new RoleDialogModel(AgentRole.User, "hello"); + + await hook.OnMessageReceived(message); + + Assert.True(message.StopCompletion); + } +} diff --git a/tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj b/tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj index 4dd287f90..030ffeaa7 100644 --- a/tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj +++ b/tests/BotSharp.Core.UnitTests/BotSharp.Core.UnitTests.csproj @@ -29,6 +29,8 @@ + + diff --git a/tests/UnitTest/MainTest.cs b/tests/UnitTest/MainTest.cs index a02276bde..e3f2dc462 100644 --- a/tests/UnitTest/MainTest.cs +++ b/tests/UnitTest/MainTest.cs @@ -37,6 +37,12 @@ public void TestConversationHookProvider() class TestHookA : ConversationHookBase { + // Empty rather than an id: ConversationHookBase.IsMatch is + // IsNullOrEmpty(SelfId) || SelfId == agentId, and this test resolves hooks with + // GetHooksOrderByPriority(string.Empty) and asserts all three come back. Any non-empty + // value here would match nothing and the count assertion would fail. + public override string SelfId => string.Empty; + public TestHookA() { Priority = 1; @@ -45,6 +51,8 @@ public TestHookA() class TestHookB : ConversationHookBase { + public override string SelfId => string.Empty; + public TestHookB() { Priority = 2; @@ -53,6 +61,8 @@ public TestHookB() class TestHookC : ConversationHookBase { + public override string SelfId => string.Empty; + public TestHookC() { Priority = 3;