You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@texera.apache.org by gi...@apache.org on 2026/08/07 11:17:20 UTC
(texera) 01/01: test(agent-service): cover TexeraAgent's step tree, settings, and lifecycle (#7385)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7385-e6b93a7d637861ea94658f9e7e6de4e7f115fec9
in repository https://gitbox.apache.org/repos/asf/texera.git
commit d72b8be6ce2124c403e0b20dcb63a71546e3e20a
Author: Xinyuan Lin <xi...@uci.edu>
AuthorDate: Fri Aug 7 04:16:50 2026 -0700
test(agent-service): cover TexeraAgent's step tree, settings, and lifecycle (#7385)
### What changes were proposed in this PR?
`TexeraAgent` had no spec, and it is the largest untested file in the
service. Most of the class is bookkeeping that `sendMessage` maintains
but that needs no model to exercise — the constructor only stores the
model reference, so a placeholder builds a real agent.
Adds 26 tests. The part worth the effort is that the ReAct step tree is
a real **tree**, not a list: each step carries a `parentId` and a `head`
pointer selects the active branch. Regenerating an answer leaves the
abandoned branch in `stepsById`, so the two readers disagree on purpose:
```
INITIAL ── a ── b head = b
└─ c
getVisibleReActSteps() -> [a, b] the conversation the user sees
getAllSteps() -> [a, b, c] everything retained
```
`getReActStepsByOperatorIds` then attributes each step to the operators
it touched by reading the tool results, gated on the tool's **name**
agreeing with its message — so a delete tool echoing "Added operator X"
cannot claim X. The structured-JSON path uses a stricter exact-name gate
than the text path does; that asymmetry is now pinned in both directions
so narrowing or widening either is a visible change.
Also covered: partial `updateSettings` (only `allowedOperatorTypes`
triggers a prompt rebuild), the defensive copy in `getSettings`, the
disabled-tool flag surfacing in `getSystemInfo`, and `clearHistory` /
`stop` / `destroy` / the step callback.
**Verified by mutation**, all reverted (production diff empty):
| Mutation | Result |
|---|---|
| build the ancestor path in reverse | red |
| ignore the explicit `stepId` argument | red |
| stop hiding the synthetic initial step | red |
| keep the initial step in `getAllSteps` | red |
| make an empty operator filter match nothing | red |
| stop skipping a failed tool result | red |
| drop the tool-name gate on the add path | red |
| drop the tool-name gate on the modify path | red |
| widen the JSON add path to any add-ish name | red |
| return settings by reference | red |
| stop rebuilding the prompt on an allowed-types change | red |
| ignore disabled tools in the system info | red |
| leave `head` dangling in `clearHistory` | red |
| leave the state unchanged on `stop` | red |
| drop the derived agent-name default | red |
The modify-gate mutation initially **survived** — the suite gated the
add path but not the symmetric modify path. Added the missing case.
The LLM loop in `sendMessage` is deliberately out of scope: it needs the
AI SDK stubbed and deserves its own pass rather than being half-covered
here.
No production file is touched.
### Any related issues, documentation, discussions?
Closes #7382
### How was this PR tested?
```
bun test
```
```
228 pass
0 fail
Ran 228 tests across 16 files.
```
`bun run typecheck` and `bun run format:check` both pass.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
---------
Signed-off-by: Meng Wang <me...@uci.edu>
Co-authored-by: Meng Wang <me...@uci.edu>
Co-authored-by: Copilot Autofix powered by AI <17...@users.noreply.github.com>
---
agent-service/src/agent/texera-agent.spec.ts | 299 +++++++++++++++++++++++++++
1 file changed, 299 insertions(+)
diff --git a/agent-service/src/agent/texera-agent.spec.ts b/agent-service/src/agent/texera-agent.spec.ts
new file mode 100644
index 0000000000..27aa37b813
--- /dev/null
+++ b/agent-service/src/agent/texera-agent.spec.ts
@@ -0,0 +1,299 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { beforeEach, describe, expect, test } from "bun:test";
+import { TexeraAgent } from "./texera-agent";
+import { AgentState, INITIAL_STEP_ID, type ReActStep } from "../types/agent";
+
+/**
+ * These tests cover the agent's bookkeeping — the ReAct step tree, settings, and client set — which
+ * `sendMessage` maintains but which is entirely separate from talking to a model. The constructor
+ * only stores the model reference, so a placeholder is enough to build a real agent.
+ */
+function makeAgent(): TexeraAgent {
+ return new TexeraAgent({
+ model: {} as any,
+ modelType: "test-model",
+ agentId: "agent-1",
+ });
+}
+
+function makeStep(id: string, parentId: string | undefined, extra: Partial<ReActStep> = {}): ReActStep {
+ return {
+ id,
+ parentId,
+ messageId: "msg-1",
+ stepId: 0,
+ timestamp: 0,
+ role: "agent",
+ content: "",
+ isBegin: false,
+ isEnd: false,
+ ...extra,
+ };
+}
+
+/** `addStep` and `head` are the agent's own internals; sendMessage is the only public writer. */
+function addStep(agent: TexeraAgent, step: ReActStep): void {
+ (agent as any).addStep(step);
+}
+
+function setHead(agent: TexeraAgent, id: string): void {
+ (agent as any).head = id;
+}
+
+/** A step whose tool call reported a result, as the LLM loop would record it. */
+function toolStep(id: string, toolName: string, output: unknown, isError = false): ReActStep {
+ return makeStep(id, INITIAL_STEP_ID, {
+ toolCalls: [{ toolName, toolCallId: "call-1", input: {} }],
+ toolResults: [{ toolCallId: "call-1", output, isError }],
+ });
+}
+
+describe("TexeraAgent", () => {
+ let agent: TexeraAgent;
+
+ beforeEach(() => {
+ agent = makeAgent();
+ });
+
+ describe("construction", () => {
+ test("starts available, at the initial step, with no steps of its own", () => {
+ expect(agent.getState()).toBe(AgentState.AVAILABLE);
+ expect(agent.getHead()).toBe(INITIAL_STEP_ID);
+ expect(agent.getAllSteps()).toEqual([]);
+ expect(agent.getVisibleReActSteps()).toEqual([]);
+ });
+
+ test("falls back to a name derived from the id", () => {
+ expect(makeAgent().agentName).toBe("Agent-agent-1");
+ expect(new TexeraAgent({ model: {} as any, modelType: "m", agentId: "a", agentName: "Named" }).agentName).toBe(
+ "Named"
+ );
+ });
+ });
+
+ describe("getAncestorPath", () => {
+ test("walks from the root down to the requested step", () => {
+ addStep(agent, makeStep("a", INITIAL_STEP_ID));
+ addStep(agent, makeStep("b", "a"));
+
+ expect(agent.getAncestorPath("b")).toEqual([INITIAL_STEP_ID, "a", "b"]);
+ });
+
+ test("defaults to the current head", () => {
+ addStep(agent, makeStep("a", INITIAL_STEP_ID));
+ setHead(agent, "a");
+
+ expect(agent.getAncestorPath()).toEqual([INITIAL_STEP_ID, "a"]);
+ });
+
+ test("returns an unknown id on its own rather than an empty path", () => {
+ expect(agent.getAncestorPath("ghost")).toEqual(["ghost"]);
+ });
+ });
+
+ describe("getVisibleReActSteps", () => {
+ test("shows only the branch the head sits on", () => {
+ // Two branches off the same parent: regenerating an answer leaves the abandoned branch in
+ // stepsById, and it must not reappear in the conversation the user sees.
+ addStep(agent, makeStep("a", INITIAL_STEP_ID));
+ addStep(agent, makeStep("b", "a"));
+ addStep(agent, makeStep("c", "a"));
+
+ setHead(agent, "b");
+ expect(agent.getVisibleReActSteps().map(s => s.id)).toEqual(["a", "b"]);
+
+ setHead(agent, "c");
+ expect(agent.getVisibleReActSteps().map(s => s.id)).toEqual(["a", "c"]);
+ });
+
+ test("hides the synthetic initial step", () => {
+ addStep(agent, makeStep("a", INITIAL_STEP_ID));
+ setHead(agent, "a");
+
+ expect(agent.getVisibleReActSteps().map(s => s.id)).not.toContain(INITIAL_STEP_ID);
+ });
+
+ test("getAllSteps keeps the abandoned branch that getVisibleReActSteps drops", () => {
+ addStep(agent, makeStep("a", INITIAL_STEP_ID));
+ addStep(agent, makeStep("b", "a"));
+ addStep(agent, makeStep("c", "a"));
+ setHead(agent, "b");
+
+ expect(
+ agent
+ .getAllSteps()
+ .map(s => s.id)
+ .sort()
+ ).toEqual(["a", "b", "c"]);
+ });
+ });
+
+ describe("getReActStepsByOperatorIds", () => {
+ test("returns every step when no operator is named", () => {
+ // An empty filter means "no filter", not "match nothing" — the opposite reading would make
+ // the UI show an empty history whenever no operator is selected.
+ addStep(agent, toolStep("s1", "addOperator", "Added operator op-7"));
+
+ expect(agent.getReActStepsByOperatorIds([]).map(s => s.id)).toEqual(["s1"]);
+ });
+
+ test("matches an operator added through the tool's text output", () => {
+ addStep(agent, toolStep("s1", "addOperator", "Added operator op-7"));
+ addStep(agent, toolStep("s2", "addOperator", "Added operator op-8"));
+
+ expect(agent.getReActStepsByOperatorIds(["op-7"]).map(s => s.id)).toEqual(["s1"]);
+ });
+
+ test("matches an operator modified through the tool's text output", () => {
+ addStep(agent, toolStep("s1", "modifyOperator", "Operator op-8 modified"));
+
+ expect(agent.getReActStepsByOperatorIds(["op-8"]).map(s => s.id)).toEqual(["s1"]);
+ });
+
+ test("matches an operator reported as structured JSON", () => {
+ addStep(agent, toolStep("s1", "addCodeOperator", JSON.stringify({ operatorId: "op-9" })));
+
+ expect(agent.getReActStepsByOperatorIds(["op-9"]).map(s => s.id)).toEqual(["s1"]);
+ });
+
+ test("ignores a tool result that failed", () => {
+ addStep(agent, toolStep("s1", "addOperator", "Added operator op-7", true));
+
+ expect(agent.getReActStepsByOperatorIds(["op-7"])).toEqual([]);
+ });
+
+ test("ignores an add message produced by a tool that does not add", () => {
+ // The operator id is only trusted when the tool name agrees with the message, so a delete
+ // tool echoing "Added operator" in its output cannot claim that operator.
+ addStep(agent, toolStep("s1", "deleteOperator", "Added operator op-7"));
+
+ expect(agent.getReActStepsByOperatorIds(["op-7"])).toEqual([]);
+ });
+
+ test("ignores a modify message produced by a tool that does not modify", () => {
+ addStep(agent, toolStep("s1", "deleteOperator", "Operator op-8 modified"));
+
+ expect(agent.getReActStepsByOperatorIds(["op-8"])).toEqual([]);
+ });
+
+ test("accepts JSON only from the exactly-named add and modify tools", () => {
+ // The text path accepts any tool whose name contains "add", but the JSON path requires an
+ // exact name. Pinning the asymmetry so that narrowing or widening one path is a visible change.
+ addStep(agent, toolStep("s1", "addSomethingElse", JSON.stringify({ operatorId: "op-9" })));
+
+ expect(agent.getReActStepsByOperatorIds(["op-9"])).toEqual([]);
+ });
+
+ test("skips a step with no tool results at all", () => {
+ addStep(agent, makeStep("s1", INITIAL_STEP_ID, { content: "just talking" }));
+
+ expect(agent.getReActStepsByOperatorIds(["op-7"])).toEqual([]);
+ });
+ });
+
+ describe("settings", () => {
+ test("applies only the fields given and leaves the rest alone", () => {
+ const before = agent.getSettings();
+
+ agent.updateSettings({ maxSteps: 99 });
+
+ const after = agent.getSettings();
+ expect(after.maxSteps).toBe(99);
+ expect(after.toolTimeoutMs).toBe(before.toolTimeoutMs);
+ expect(after.maxOperatorResultCharLimit).toBe(before.maxOperatorResultCharLimit);
+ });
+
+ test("returns a new settings object (mutating primitive fields does not stick)", () => {
+ agent.getSettings().maxSteps = 12345;
+
+ expect(agent.getSettings().maxSteps).not.toBe(12345);
+ });
+
+ test("a disabled tool is reported as disabled in the system info", () => {
+ const toolName = agent.getSystemInfo().tools[0].name;
+
+ agent.updateSettings({ disabledTools: new Set([toolName]) });
+
+ const info = agent.getSystemInfo().tools.find(t => t.name === toolName);
+ expect(info?.enabled).toBe(false);
+ });
+
+ test("restricting the allowed operator types rewrites the system prompt", () => {
+ const before = agent.getSystemInfo().systemPrompt;
+
+ agent.updateSettings({ allowedOperatorTypes: ["CSVFileScan"] });
+
+ expect(agent.getSystemInfo().systemPrompt).not.toBe(before);
+ expect(agent.getSettings().allowedOperatorTypes).toEqual(["CSVFileScan"]);
+ });
+ });
+
+ describe("clients", () => {
+ test("tracks and drops subscribers", () => {
+ const ws = { id: 1 };
+
+ agent.addClient(ws);
+ expect(agent.getClients().has(ws)).toBe(true);
+
+ agent.removeClient(ws);
+ expect(agent.getClients().has(ws)).toBe(false);
+ });
+ });
+
+ describe("lifecycle", () => {
+ test("a new step is announced to the registered callback", () => {
+ const seen: string[] = [];
+ agent.setStepCallback(step => seen.push(step.id));
+
+ addStep(agent, makeStep("a", INITIAL_STEP_ID));
+
+ expect(seen).toEqual(["a"]);
+ });
+
+ test("clearHistory rewinds to the initial step but keeps the agent usable", () => {
+ addStep(agent, makeStep("a", INITIAL_STEP_ID));
+ setHead(agent, "a");
+
+ agent.clearHistory();
+
+ expect(agent.getHead()).toBe(INITIAL_STEP_ID);
+ expect(agent.getAllSteps()).toEqual([]);
+ expect(agent.getAncestorPath()).toEqual([INITIAL_STEP_ID]);
+ });
+
+ test("stop moves the agent into the stopping state", () => {
+ agent.stop();
+
+ expect(agent.getState()).toBe(AgentState.STOPPING);
+ });
+
+ test("destroy drops the steps and the subscribers", () => {
+ addStep(agent, makeStep("a", INITIAL_STEP_ID));
+ agent.addClient({ id: 1 });
+
+ agent.destroy();
+
+ expect(agent.getAllSteps()).toEqual([]);
+ expect(agent.getReActSteps()).toEqual([]);
+ expect(agent.getClients().size).toBe(0);
+ });
+ });
+});