Skip to content

Feature/agent test set p1 - #1398

Merged
iceljc merged 7 commits into
SciSharp:masterfrom
yuyixg:feature/agent-test-set-p1
Aug 18, 2026
Merged

Feature/agent test set p1#1398
iceljc merged 7 commits into
SciSharp:masterfrom
yuyixg:feature/agent-test-set-p1

Conversation

@yuyixg

@yuyixg yuyixg commented Aug 18, 2026

Copy link
Copy Markdown

Agent regression test harness
Adds a per-agent regression test harness as a new plugin, plus the one extension point in BotSharp.Core it needs. Scripted multi-turn cases, mocked tools, deterministic assertions, recording cases from real conversations, comparison runs across several models, and AI-assisted case extraction.

The problem it solves: there is currently no way to tell whether editing an agent's instruction, swapping its model, or changing a tool broke something, other than talking to it by hand. That is not repeatable, and for agents with side-effecting tools it is not safe either.

Two separable parts

  1. An extension point in the core (6 existing files, +93/-11).

FunctionExecutorFactory was internal static, so nothing outside BotSharp.Core could take part in deciding who executes a function name. It is now a service behind a new IFunctionExecutorFactory, and asks any registered IFunctionExecutorProvider first (ascending Order, so resolution never depends on DI registration order).

BotSharp.Core.Rules' ToolCallAction used to look up IFunctionCallback itself and invoke it directly, bypassing the factory entirely. It now resolves through the factory, which closes the only path by which a function could execute without a provider getting a say. Its original case-insensitive name matching is preserved: registered callbacks are still used to resolve the canonical name, and only the execution moves to the factory.

Compatibility. With no IFunctionExecutorProvider registered, resolution is byte-for-byte what it was: the same three-stage chain in the same order with the same semantics. That is asserted directly by FunctionExecutorFactoryTests, which exists specifically to pin it, since this is the one way this change could introduce a silent regression on the core path.

Two guards came out of testing rather than review: a null/blank function name used to be harmless (the old IsEqualTo lookup simply matched nothing) but would now reach IFunctionExecutorProvider.TryResolve, whose contract declares a non-null string. A provider doing a dictionary lookup by name would throw ArgumentNullException instead of failing gracefully, so both FunctionExecutorFactory.Create and ToolCallAction reject it up front.

  1. BotSharp.Plugin.AgentTesting (new, ~7k lines including tests).

Self-contained: depends on BotSharp.Abstraction only, keeps its own four Mongo collections behind AgentTestMongoDbContext, and mirrors BotSharp.Plugin.MongoStorage's conventions — same Database:BotSharpMongoDb setting, same rule for deriving the database name, same TablePrefix convention — so a host that already has Mongo storage configured needs no additional configuration.

Deliberately not added to IBotSharpRepository: four new collections there would mean roughly twenty new members that FileRepository and BotSharpDbContext would each have to implement, for data no other feature reads.

How a test run avoids touching the real world
TestMockExecutorProvider takes over every function inside a conversation under test, so an unmocked tool is blocked rather than executed. Five explicit control-flow functions are allowed through -- notably not by a util- prefix, because util-email-handle_email_sender, util-twilio-outbound_phone_call, util-twilio-text_message, util-http-handle_http_request and util-db-sql_select all start with it and all have real side effects. Allowing by prefix would mean one test run really sends the emails and really places the calls; a test asserts that the allow list is exactly those five names, and fails under a prefix rule.

Interception is keyed by conversationId in a registry, not by AsyncLocal. AsyncLocal depends on ExecutionContext flowing and is silently lost across a background-queue or SideCar boundary, and losing it does not produce a failing test — it produces unmocked tools executing for real.

Before any user message is sent, each case calls a canary function and verifies the content that comes back. If the seam is not live (a build resolving an older BotSharp.Core without provider support, say), mocking fails silently, and the canary turns that into an explicit Error on the case instead of a green run against real tools.

A blocked tool fails the case. Blocking is the seam working correctly, but it also stops that turn, so everything the agent would have done next never happened and every later assertion is evaluated against a conversation that ended early. Reporting Passed there is the same "executed nothing, reports green" defect the no-turns guard and the canary exist to prevent.

Results are attributable
Failed and Error are kept strictly apart: Failed means it ran and an assertion did not hold, Error means it never got that far (timeout, dead canary, a case with no turns, a case filter that matched nothing, a host restart mid-run). Collapsing them would make "the harness broke" read as "the agent regressed", and every run-level Error carries a reason worded for whoever has to act on it, because such a run can produce zero case results and then that field is the only place the reason exists.

Multi-model runs sweep one suite across several models in a single run (cases x models), each result tagged with the model that produced it, so pass rates and durations compare side by side. The override rides on the existing IAgentHook, rewriting agent.LlmConfig as the agent loads. That timing is forced rather than chosen: RoutingService.InvokeAgent passes agent.LlmConfig's provider/model to CompletionProvider explicitly, and GetProviderAndModel only consults the conversation-state override when what it was passed is empty — so seeding provider/model into conversation state has no effect on the main path.

Recording and AI extraction
Recording converts a real conversation into an editable draft case: real function returns become mocks, real state deltas become state writes and initial states, and the two stable assertion kinds (toolCalled, stateEquals) are generated. Output-text assertions are deliberately never generated — using the model's exact wording as a baseline is brittle enough that any rephrasing goes red.

AI extraction splits one conversation into a case per scenario it covers. The model decides only where to cut and what to name each case; mock return values, assertions and state still come verbatim from the conversation. Letting a model author mocks would stop a case being a replay of a call that really happened, and letting it author assertions would reintroduce exactly the brittle output-text assertions above. The segmenter rejects any segmentation it cannot fully verify -- gaps, overlaps, out-of-range or uncovered turns -- because a half-correct one looks entirely normal in the UI and only misbehaves when someone runs it. Only user messages and tool names are sent to the model; tool arguments and results, where the densest PII sits, are withheld.

Every recorded draft lands disabled and has to be reviewed and enabled by hand before it joins a run.

Testing
194 unit tests in tests/BotSharp.Core.UnitTests, of which 153 cover this feature and the rest are the pre-existing suite. The two seam test classes are the ones worth reviewing: they pin no-provider compatibility on the core path and the case-insensitive name matching on the rule path.

Verified end to end against a live host: multi-model comparison across two models produced per-model attributed results with differing durations; an intentionally invalid model name is rejected at trigger time rather than failing every case with an opaque error; AI extraction split a three-topic conversation into three correctly named cases with earlier-turn state carried into each.

Notes for review
The plugin exposes a REST controller. Every endpoint requires authentication, and the two cost/PII-sensitive ones (recording, triggering a run) additionally require admin/root: recording copies raw conversation content into the test store with no ownership check on the conversation id, and triggering really spends token quota with no throttling anywhere.
Runs execute serially. That is a safety choice, not a performance oversight -- cases share external dependencies and would pollute each other's state concurrently.
The run queue is in-process, so a restart abandons runs in flight. They are swept to Error on the next start rather than left claiming to be Running.
llmJudge is accepted as an assertion type but always fails, with a message saying so, rather than passing silently and showing a case that verified nothing as green.

marsyusms and others added 7 commits August 12, 2026 22:44
…hes the factory

A missing function_name now short-circuits to the existing "unable to find
function" result before IFunctionExecutorFactory.Create/IFunctionExecutorProvider
.TryResolve are ever called. Previously the null traveled into those seams' non-
nullable string parameter; the factory's own built-in fallthrough tolerates it
today, but a plausible future provider (e.g. a Dictionary-keyed mock/blocking
lookup) throws ArgumentNullException instead of failing gracefully. Restores the
pre-refactor null-safe behavior of the old IsEqualTo-based lookup.

Adds a regression test with a Dictionary-backed provider that reproduces the
crash against the unguarded code and passes once the guard is in place.
…on name

RoutingService.InvokeFunction calls this factory with no guard of its own, and a
registered IFunctionExecutorProvider keyed by a case-insensitive Dictionary (a real
shape, see ToolCallActionTests.NullIntolerantProvider) throws ArgumentNullException on
a null key even for a read-only lookup. The factory is documented as the single
trusted seam every function-call path must go through, so it should not depend on
every caller pre-validating for it. Fail closed (return null) instead.
Per-agent regression test sets: scripted multi-turn cases, mocked tools,
deterministic assertions, recording from real conversations, multi-model
comparison runs, and AI-assisted case extraction. Developed and verified
against a live host in the onebrain repo; this is where it belongs, so it moves
here in full.

Self-contained: depends on BotSharp.Abstraction only, carries its own four
Mongo collections behind AgentTestMongoDbContext, and mirrors
BotSharp.Plugin.MongoStorage's conventions (same Database:BotSharpMongoDb
setting, same TablePrefix rule) so a host with Mongo storage configured needs
no extra configuration. Deliberately NOT added to IBotSharpRepository -- that
would mean ~20 new members FileRepository and BotSharpDbContext would each have
to implement for data no other feature reads.

How it stops a test run from touching the real world: TestMockExecutorProvider
implements IFunctionExecutorProvider and takes over every function inside a
conversation under test, so an unmocked tool is blocked rather than executed.
Only five explicit control-flow functions are let through -- notably NOT by a
`util-` prefix, since util-email-handle_email_sender, util-twilio-*, util-http-*
and util-db-sql_select all start with it and all have real side effects. The
seam is proven live per case by a canary function before any user message is
sent, because a silently dead seam means real emails and real phone calls
rather than a failing test.

Multi-model runs sweep one suite across several models in a single run (cases x
models), each result tagged with the model that produced it, so response times
and pass rates compare side by side. The override rides on IAgentHook, rewriting
agent.LlmConfig as the agent loads -- the only point that works, since
RoutingService.InvokeAgent passes provider/model to CompletionProvider
explicitly and the conversation-state override is therefore never consulted.

AI extraction splits a recorded conversation into one case per scenario. The
model decides only where to cut and what to name each case; mock return values,
assertions and state still come verbatim from the conversation, and only user
messages and tool names are sent to the vendor. The segmenter rejects any
segmentation it cannot fully verify, because a half-correct one looks normal in
the UI and only misbehaves when someone runs it.

151 unit tests in tests/BotSharp.Core.UnitTests/AgentTesting.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A run can fail with zero case results -- the suite was deleted or disabled, the
selected cases were all disabled, the host restarted mid-run, or execution
crashed outright. AgentTestRun had nowhere to put the reason, so all five of
those paths only wrote to the server log. The API returned status=Error with
0/0/0/0 counts and an empty result list, and no client could say why.

Observed: a run naming one disabled case came back Error with nothing at all to
show. The reason ("none of them matched an enabled case in suite ...") existed
only in the log.

AgentTestRun.Error now carries it, set at every one of those five sites, and
worded for whoever has to act on it rather than for whoever wrote the code --
the disabled-cases one says to enable them and run again; the crash one carries
the exception message, since that is the case where nothing else survives.

Distinct from AgentTestCaseResult.Error, which explains one case. This one
explains why there are no cases to explain.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…to English

Blocked tools now fail the case. Blocking is the mock seam working correctly --
the agent reached for a tool the case does not mock, and executing it for real
could have sent an email or created a work order. But the block also stops that
turn, so everything the agent would have done next never happened and every
later assertion is evaluated against a conversation that ended early. Reporting
Passed there was the same "executed nothing, reports green" defect the no-turns
guard, the canary and the CaseIds-matched-nothing guard all exist to prevent.
Observed: a run whose only tool call came back Blocked still reported Passed.

Modelled as a synthetic case-level assertion rather than as result.Error, so it
renders in the ordinary assertion table with expected/actual and the existing
all-assertions-passed rule decides the status with no special case. Error stays
reserved for "the harness itself did not work", which is the opposite of what
happened. Its type, AssertionTypes.NoBlockedTools, is result-only: never
authored on a case, never evaluated, and deliberately absent from
AssertionValidation's map of the eight authorable types.

Also translates every comment introduced by this branch from Chinese to
English -- 309 lines across the plugin, the IFunctionExecutorProvider seam and
the tests. Content is preserved rather than summarised: these comments carry the
reasoning behind the safety-critical decisions (why the allow list must not
become a `util-` prefix match, why the driver must not wrap its Task in
WaitAsync, why each case needs its own DI scope), and losing that would cost
more than the Chinese did.

One fixture changed rather than translated: AgentTestDocumentTests used a
Chinese case name to prove the Mongo serialiser does not reshape user text. It
is now a non-Chinese but still non-ASCII string, so that coverage survives, with
a comment saying why it is not plain ASCII.

194 BotSharp unit tests pass, 153 of them agent-testing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@yileicn
yileicn requested a review from iceljc August 18, 2026 08:00
@iceljc
iceljc merged commit 3fa34a9 into SciSharp:master Aug 18, 2026
1 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants