Agent test harness: routing cases, authored history, scope narrowing,… - #1409
Merged
Conversation
… metrics
Everything in one commit because no smaller one builds. AgentTestCaseRunner.cs and
AgentTestingPlugin.cs each carry changes from several of the features below, and
splitting by file leaves an intermediate that fails to compile -- the runner would
reference an interface member that no longer exists, or the plugin would register a
judge whose implementation is not yet present. That also folds in the llmJudge work
that was already sitting uncommitted in this worktree.
Routing and agent chains
------------------------
Cases now declare what they verify (CaseType: Routing or Agent) and where the
conversation opens (EntryAgentId, defaulting to the suite own agent). That one value
decides whether the router is part of what a case measures: ConversationService
sends a routing-type agent through InstructLoop, which can hand off, and everything
else through InstructDirect, which cannot.
route_to_agent is on the mock allow list and so never reaches MockFunctionExecutor,
the only caller that records an observed tool call -- routing decisions were
therefore invisible. The chain is now reconstructed from the conversation own
assistant dialogs, each of which carries the agent that wrote it, and reported per
turn and per case with consecutive repeats collapsed by agent id.
A new agentChain assertion compares that chain in three modes: contains, ordered
(the hand-off assertion) and exact (which is how a case asserts nothing routed
away). An unrecognised mode fails rather than falling back to the loosest check.
Both agentChain and routedToAgent accept an agent id or its display name: the id is
what an author copies out of the agent list, and asserting an id against a name
could never pass no matter what the agent did.
routedToAgent now reads the chain last entry instead of a separate field, so the
two cannot disagree, and a turn-level assertion sees that turn own slice -- a turn
that produced no answer no longer inherits the previous turn agent.
Routing accuracy is tallied per model on the run, counting only Routing cases.
Errored cases count against it: "could not tell" is not "routed correctly".
E2E was dropped as a case type. A journey across several agents is an Agent case
whose agentChain assertion describes the hand-offs, and a third type bought only a
third branch in every validation and aggregation path.
Authored history
----------------
A case can carry prior turns, written into the conversation before it runs, so a
real exchange becomes a fixed starting context. Not driven through the model: no
token cost, and the preamble cannot itself become a source of flakiness.
AppendConversationDialogs is an UpdateOne with no upsert, so it silently writes
nothing when the conversation dialog document does not exist -- and PrepareAsync
deliberately does not create it. The conversation is therefore created first,
through the same call SendMessage uses, and the write is read back and counted. A
short count errors the case: running without the context it was written around would
otherwise report an ordinary pass or fail about a scenario that never existed.
Authored history is excluded from the agent chain. It is not something the agent did,
and letting it in would fail an exact chain assertion for a reason the author never
caused.
Copying a case
--------------
POST cases/{id}/copy duplicates a case inside its suite. Server-side because the
copy has to carry every field: a client that rebuilds the payload from its own form
drops what it does not know about, and a copy missing its mocks is indistinguishable
in the list until the run where it blocks every tool. Cloned by a BSON round trip
for the same reason -- a hand-written clone would silently omit the next field added.
The copy lands disabled whatever the source was. An exact duplicate joining the next
run measures the same thing twice, and for a routing case it double-weights one
routing decision.
Scope narrowing
---------------
Cases carry the registration a change-scoped evaluation needs: Priority, Severity,
Batch (derived from priority, with cross-cutting forced to batch 1), CrossCutting,
InvolvedAgents, BusinessDomain, ExpectedOutcome and LastReviewedDate. Existing cases
read back as P1/S1 -- mandatory but not stop-loss, which is the honest position for
a case nobody has triaged.
POST scope answers which cases a change needs to run. Every rule resolves towards
including, because the two failure directions are not symmetrical: a case wrongly
included costs tokens and is obvious, while one wrongly excluded produces no result
at all, and "not run" is indistinguishable from "passed" once the numbers are in a
report. Unknown involved agents therefore fail open, and both halves of the decision
are returned with the rule that produced them.
InvolvedAgents falls back to the case entry agent when unauthored, which is
definitionally involved and already known -- so an Agent case is picked up by a
change to its own agent without anyone maintaining a list.
Latency, tokens and cost
------------------------
Each turn is timed around the agent call alone, and the case reports that separately
from its wall clock, which also contains the canary and the conversation reads. The
run summarises P50 and P95 per model at completion, nearest-rank so every figure is
a duration some case actually took. Cases that never reached the model are excluded
from the percentiles -- otherwise a run that mostly crashed reports the best latency
on record -- but still counted in tokens and cost, which they really did spend.
Token usage is read as a delta across the case rather than as an absolute, so a
reused scope cannot bill one case for another tokens, and it is read in a finally
block so a timed-out case still reports what it cost. Total only: the input/output
split lives in TokenStatistics private fields and is not reachable through
ITokenStatistics.
The run also snapshots each model configured unit costs. A cost figure is not
comparable with another run without them, and recording only a version string
would leave nobody able to check whether two versions differ.
Rate limiting
-------------
BotSharp rate limiting counts human behaviour, and a regression suite does not
look like one: it opens a conversation per case per model, drives turns as fast as
the model answers, and runs in a BackgroundService whose user identity is empty --
which, because the Mongo filter drops an empty UserId rather than matching on it,
measured the harness against every conversation in the instance. Every case failed
with a message about conversation quotas that said nothing about the agent.
ISyntheticConversationProbe (new, in Abstraction) lets a harness declare which
conversations are its own, and RateLimitConversationHook stands aside for those on
its two volume guards. The plugin answers from the run registry rather than from the
conversation tag, because the tag is written only after the first message has
already passed the hook. The input-length guard still applies: that one is about a
single message being too large, which a test should surface rather than be excused
from. A probe that throws fails closed, so a bug there cannot lift the limits for
real traffic.
Tests
-----
336 unit tests, up from 194. The ones worth reviewing are CaseScopeTests, which pins
every narrowing rule in the direction of including, and
SyntheticConversationExemptionTests, which checks that real traffic is still limited
in every case where the harness is not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Runs accumulate forever -- one per case per model per run, and nothing ever removed
them. Adds a way to clear them, with the guards the operation needs.
POST runs/delete takes a list. Bulk-only, and the single-row button 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 worth having in exactly one place.
Deleting a run deletes its case results too. 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 -- and every later aggregate that scans results would keep counting them.
Results go first, so a process death between the two deletes leaves an orphaned RUN,
which is visible and deletable again, rather than orphaned RESULTS, which are not
reachable at all.
A run that is still executing is refused. 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. Cancel it first -- and on a live row the UI offers only Cancel, since the
other button would not do what it says.
One live run does not fail the batch. Selecting everything and clearing is the normal way
this gets used, and a running row in the list is common; refusing the whole call would
make the feature useless exactly when it is most wanted. Skipped runs come back with a
reason and the UI shows each one, because "deleted 2" while a third row silently stays is
how someone concludes the button is broken. The count on the button excludes running
runs, so it never promises a delete that will not happen.
Behind the same admin gate as triggering a run: runs are the record of whether an agent
change was evaluated at all, so removing them is at least as consequential as creating
them.
An empty list is a 400 rather than a successful no-op -- far more likely a select-all
that selected nothing than a deliberate request.
Not touched: the conversations these runs created. They live in BotSharp's own store and
are the only forensic record of what an agent actually said when an assertion failed, so
they are not something to remove as a side effect of tidying a list. The harness still
never cleans them up, which is a separate decision to make.
8 tests: the cascade, the running-run refusal, one live run not blocking a batch, an
already-deleted run being reported rather than erroring, duplicate ids deleting once, two
shapes of empty request, and the admin gate. 344 unit tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI failed to compile tests/UnitTest: TestHookA, TestHookB and TestHookC do not implement
ConversationHookBase.SelfId.
Not caused by this branch. master added `public abstract string SelfId { get; }` to
ConversationHookBase when it started implementing IHookBase, and updated every
implementation in the tree except those three -- the same break is present on
SciSharp/BotSharp master right now, so anything merged into it fails the same way. This
branch only surfaced it, because CI builds the merge.
SelfId is string.Empty for all three, and that is required rather than conventional:
IsMatch is IsNullOrEmpty(SelfId) || SelfId == agentId, and the test resolves hooks with
GetHooksOrderByPriority(string.Empty) then asserts all three come back. Any non-empty
value would match nothing and fail the count assertion.
The merge also brought two changes to RateLimitConversationHook, which this branch edits:
master added its own SelfId and moved the conversation id to
IConversationStateService.GetConversationId(), having previously resolved
IConversationStorage up front. Git merged the file cleanly, and both sides survived --
checked rather than assumed, since a clean merge is exactly how the scope panel ended up
half-restyled in the UI repo. The synthetic-conversation check now takes that same
conversation id as an argument instead of resolving IConversationService for itself, so
the hook reads from one source.
Verified: the whole BotSharp solution builds with zero compiler errors, tests/UnitTest
passes 4, and BotSharp.Core.UnitTests passes 344. Building only the projects this branch
touches is what let the original failure through, so the solution build is the check that
matters here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…uthor) Lets a QA/PM write or edit a test case by chat instead of hand-writing turns, mocks and assertions. Stateless on the server -- the whole authoring conversation and the current draft travel with each request -- and it never saves: the draft still goes through the same create/update endpoints and the same CaseValidation those already ran, now shared instead of duplicated. Four guards on what a model may do to someone's draft: only fields it declares in changedFields are taken from its answer (an omitted field is kept, never deleted); a mock or toolCalled/toolNotCalled naming a function the agent cannot call is dropped rather than stored; a draft that fails validation gets one repair round against the real error text, and if that still fails the original draft comes back untouched; the change list shown to the user is diffed from the two drafts, never read off the model's own account. Also fixes a real failure: models routinely write argsMatchJson/resultContent/ a state value as a nested object instead of a JSON-string, which is valid JSON overall and so slipped past the existing "is this JSON" check and only broke at strong-typed deserialization. Normalises that shape deterministically before parsing, and separately gives a genuinely unparseable reply one retry (the validation-repair round already had one; this one didn't). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
… metrics
Everything in one commit because no smaller one builds. AgentTestCaseRunner.cs and AgentTestingPlugin.cs each carry changes from several of the features below, and splitting by file leaves an intermediate that fails to compile -- the runner would reference an interface member that no longer exists, or the plugin would register a judge whose implementation is not yet present. That also folds in the llmJudge work that was already sitting uncommitted in this worktree.
Routing and agent chains
Cases now declare what they verify (CaseType: Routing or Agent) and where the conversation opens (EntryAgentId, defaulting to the suite own agent). That one value decides whether the router is part of what a case measures: ConversationService sends a routing-type agent through InstructLoop, which can hand off, and everything else through InstructDirect, which cannot.
route_to_agent is on the mock allow list and so never reaches MockFunctionExecutor, the only caller that records an observed tool call -- routing decisions were therefore invisible. The chain is now reconstructed from the conversation own assistant dialogs, each of which carries the agent that wrote it, and reported per turn and per case with consecutive repeats collapsed by agent id.
A new agentChain assertion compares that chain in three modes: contains, ordered (the hand-off assertion) and exact (which is how a case asserts nothing routed away). An unrecognised mode fails rather than falling back to the loosest check. Both agentChain and routedToAgent accept an agent id or its display name: the id is what an author copies out of the agent list, and asserting an id against a name could never pass no matter what the agent did.
routedToAgent now reads the chain last entry instead of a separate field, so the two cannot disagree, and a turn-level assertion sees that turn own slice -- a turn that produced no answer no longer inherits the previous turn agent.
Routing accuracy is tallied per model on the run, counting only Routing cases. Errored cases count against it: "could not tell" is not "routed correctly".
E2E was dropped as a case type. A journey across several agents is an Agent case whose agentChain assertion describes the hand-offs, and a third type bought only a third branch in every validation and aggregation path.
Authored history
A case can carry prior turns, written into the conversation before it runs, so a real exchange becomes a fixed starting context. Not driven through the model: no token cost, and the preamble cannot itself become a source of flakiness.
AppendConversationDialogs is an UpdateOne with no upsert, so it silently writes nothing when the conversation dialog document does not exist -- and PrepareAsync deliberately does not create it. The conversation is therefore created first, through the same call SendMessage uses, and the write is read back and counted. A short count errors the case: running without the context it was written around would otherwise report an ordinary pass or fail about a scenario that never existed.
Authored history is excluded from the agent chain. It is not something the agent did, and letting it in would fail an exact chain assertion for a reason the author never caused.
Copying a case
POST cases/{id}/copy duplicates a case inside its suite. Server-side because the copy has to carry every field: a client that rebuilds the payload from its own form drops what it does not know about, and a copy missing its mocks is indistinguishable in the list until the run where it blocks every tool. Cloned by a BSON round trip for the same reason -- a hand-written clone would silently omit the next field added.
The copy lands disabled whatever the source was. An exact duplicate joining the next run measures the same thing twice, and for a routing case it double-weights one routing decision.
Scope narrowing
Cases carry the registration a change-scoped evaluation needs: Priority, Severity, Batch (derived from priority, with cross-cutting forced to batch 1), CrossCutting, InvolvedAgents, BusinessDomain, ExpectedOutcome and LastReviewedDate. Existing cases read back as P1/S1 -- mandatory but not stop-loss, which is the honest position for a case nobody has triaged.
POST scope answers which cases a change needs to run. Every rule resolves towards including, because the two failure directions are not symmetrical: a case wrongly included costs tokens and is obvious, while one wrongly excluded produces no result at all, and "not run" is indistinguishable from "passed" once the numbers are in a report. Unknown involved agents therefore fail open, and both halves of the decision are returned with the rule that produced them.
InvolvedAgents falls back to the case entry agent when unauthored, which is definitionally involved and already known -- so an Agent case is picked up by a change to its own agent without anyone maintaining a list.
Latency, tokens and cost
Each turn is timed around the agent call alone, and the case reports that separately from its wall clock, which also contains the canary and the conversation reads. The run summarises P50 and P95 per model at completion, nearest-rank so every figure is a duration some case actually took. Cases that never reached the model are excluded from the percentiles -- otherwise a run that mostly crashed reports the best latency on record -- but still counted in tokens and cost, which they really did spend.
Token usage is read as a delta across the case rather than as an absolute, so a reused scope cannot bill one case for another tokens, and it is read in a finally block so a timed-out case still reports what it cost. Total only: the input/output split lives in TokenStatistics private fields and is not reachable through ITokenStatistics.
The run also snapshots each model configured unit costs. A cost figure is not comparable with another run without them, and recording only a version string would leave nobody able to check whether two versions differ.
Rate limiting
BotSharp rate limiting counts human behaviour, and a regression suite does not look like one: it opens a conversation per case per model, drives turns as fast as the model answers, and runs in a BackgroundService whose user identity is empty -- which, because the Mongo filter drops an empty UserId rather than matching on it, measured the harness against every conversation in the instance. Every case failed with a message about conversation quotas that said nothing about the agent.
ISyntheticConversationProbe (new, in Abstraction) lets a harness declare which conversations are its own, and RateLimitConversationHook stands aside for those on its two volume guards. The plugin answers from the run registry rather than from the conversation tag, because the tag is written only after the first message has already passed the hook. The input-length guard still applies: that one is about a single message being too large, which a test should surface rather than be excused from. A probe that throws fails closed, so a bug there cannot lift the limits for real traffic.
Tests
336 unit tests, up from 194. The ones worth reviewing are CaseScopeTests, which pins every narrowing rule in the direction of including, and SyntheticConversationExemptionTests, which checks that real traffic is still limited in every case where the harness is not.