test(agent-service): drive sendMessage with a stand-in language model - #7487
Open
aglinxinyuan wants to merge 2 commits into
Open
test(agent-service): drive sendMessage with a stand-in language model#7487aglinxinyuan wants to merge 2 commits into
aglinxinyuan wants to merge 2 commits into
Conversation
sendMessage was the largest untested region in the service: 238 of the 292 uncovered lines in texera-agent.ts, and nothing exercised it because the existing spec stops at the model boundary. Nothing there actually needs the network - ai/test ships a MockLanguageModelV4 that satisfies the same LanguageModel type the constructor takes. Adds 25 tests across two blocks. The first drives the ReAct loop itself: branch bookkeeping, per-step and summed usage, tool projection, the maxSteps cap, turn chaining, and every failure path - a thrown model, a non-Error throw, cancellation, and stop() mid-run. The second covers delegate mode through fetch: the one-time backend refresh, auto-execution after a tool call and where its result is keyed, the guards that suppress it, and the debounced auto-persist. texera-agent.ts goes from 51.01% to 99.82% lines. The one line left is getStepsById, which has no call site. No production file is touched.
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7487 +/- ##
============================================
+ Coverage 85.69% 86.38% +0.69%
Complexity 4168 4168
============================================
Files 1169 1169
Lines 46739 46694 -45
Branches 5203 5203
============================================
+ Hits 40052 40337 +285
+ Misses 4974 4646 -328
+ Partials 1713 1711 -2
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
agent-service/src/agent/texera-agent.spec.ts:996
- This replaces a process-wide singleton, but restoration occurs only at the end of the test. Any failed assertion or rejected await before line 1039 leaves the test metadata installed for subsequent tests and can create cascading, order-dependent failures. Restore the singleton (and destroy the agent) in a
finallyblock or anafterEachcleanup that runs even when the test fails.
const saved = (WorkflowSystemMetadata as any).instance;
(WorkflowSystemMetadata as any).instance = undefined;
Comment on lines
+787
to
+789
| * rather than being seeded on the agent. And every test here ends with `agent.destroy()`, | ||
| * because the auto-persist debounce would otherwise fire after the fetch spy is restored and | ||
| * issue a real request. |
Per-test cleanup ran after the assertions, so a failing test skipped it and leaked state into the tests that follow: - agents built by makeAgentWith are now tracked and destroyed from the root afterEach, before the fetch spy is restored, so a pending auto-persist debounce can no longer fire a real request; the per-test destroy() calls are gone - the WorkflowSystemMetadata singleton swap and the two validator spies are restored in finally blocks
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
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.
What changes were proposed in this PR?
sendMessageheld 238 of the 292 uncovered lines intexera-agent.ts, and the existing spec stopsat the model boundary. Nothing in there actually needs the network:
ai/testships aMockLanguageModelV4satisfying the sameLanguageModeltype the constructor already takes, so theloop runs in-process.
fetchis spied on as a tripwire, and the no-delegate tests assert that not asingle call escapes.
Adds 25 tests in two blocks:
sendMessagemaxStepscap, turn chaining, an abandoned branch staying invisible to the model, DAG compilation feeding schemas and cached results into the promptsendMessagefailuresErrorthrow stringified, a failed turn staying on the branch, cancellation reported as stopped, anAbortError-named provider error read as a user stop,stop()mid-run preventing the next call,GENERATINGfor the durationdelegate modemodifyOperatorand where its result is keyed, the two guards that suppress it,buildExecutionConfig, and the debounced auto-persist plus its failure pathtexera-agent.tsgoes from 51.01% to 99.82% lines (80.43% → 98.21% funcs).On what these tests actually pin
Line coverage overstates this, and it is worth being precise.
buncredits a whole function bodyonce it is entered, so the first test alone takes the uncovered count from 292 to 27. Most of the
remaining 24 buy no additional lines — they exist because each kills a mutation nothing else kills.
They are mutation guards, not coverage.
The matrix ran 44 mutations against a pristine source with a checkout-and-verify between each. Six
were re-run independently after the tests were merged into the existing spec:
operatorIdstill auto-executes[ERROR]tool result no longer suppresses the follow-up runEXECUTE_AFTER_TOOLSfilter droppedtotalUsagepreferred overusagecontent: text || ""fallbackThe last two are listed deliberately. They survive because they are unobservable in
ai@7.0.48—totalUsageandusageare the same object, and the SDK already hands""to a text-less step.No test claims to pin them, and no test was written to cement them.
Five further fragments are line-covered but not behaviourally pinned, for the same reason:
lastPreparedMessages = undefined(re-assigned before every step),isError: !!(tr.output)?.error(no tool ever returns an object), the
?? finalUsage?.promptTokens/?? completionTokensarms(v4-era key names that no longer exist), and the delegate guard at 408–410 (masked by the catch
below it).
Deliberately not included
getStepsById(line 261) — the one line left uncovered. No call site anywhere in the repo;server.tsusesgetReActSteps/getAllSteps/getVisibleReActSteps. It also hands out thelive private
Mapby reference. Deleting it beats testing it, and that belongs in its own change.currentMessageId— five writes, zero reads. An assertion on it would cement dead state.maxSteps: 0— writing that test hangs the suite rather than failing it. Filed as maxSteps of 0 silently disables the agent's step cap #7484.Two defects surfaced while writing these and are filed rather than fixed here, since this PR touches no production code: #7484 (a
maxStepsof 0 silently disables the step cap) and #7485 (a falsy throw from the model makessendMessagereject instead of reporting an error step).No production file is touched.
Any related issues, documentation, discussions?
Closes #7486
How was this PR tested?
25 new on top of the existing 228.
bun run typecheckpasses.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)