Skip to content

refactor(agent-service): redesign sync-execution result and error model #6009

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bobbai00 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

refactor(agent-service): redesign sync-execution result and error model #6009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
464381c
refactor(agent-service): redesign sync-execution result and error model
bobbai00 Jun 29, 2026
e2163bc
refactor(frontend): consume execution summaries directly
bobbai00 Jun 29, 2026
9389f3e
refactor(agent-service): keep operator result summary name
bobbai00 Jun 29, 2026
84022c9
refactor: align execution result summary contract
bobbai00 Jun 29, 2026
43166b9
refactor: remove legacy result marker fields
bobbai00 Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion agent-service/src/agent/texera-agent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ export class TexeraAgent {
const result = new Map<string, string>();
const visible = this.workflowResultState.getAllVisible();
for (const [operatorId, entry] of visible) {
result.set(operatorId, formatOperatorResult(operatorId, entry.operatorInfo, this.workflowState));
result.set(operatorId, formatOperatorResult(operatorId, entry.operatorInfo));
}
return result;
}
Expand Down
247 changes: 92 additions & 155 deletions agent-service/src/agent/tools/result-formatting.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,104 +19,118 @@

import { describe, expect, test } from "bun:test";
import { formatOperatorResult } from "./result-formatting";
import { WorkflowState } from "../workflow-state";
import type { OperatorInfo } from "../../types/execution";
import type { OperatorPredicate, OperatorLink, PortDescription } from "../../types/workflow";
import {
ConsoleMessageType,
OperatorState,
OperatorResultMode,
WorkflowFatalErrorType,
type OperatorExecutionSummary,
type WorkflowFatalError,
type SampleRow,
} from "../../types/execution";

function toSampleRows(rows: Record<string, any>[]): SampleRow[] {
return rows.map((tuple, rowIndex) => ({ rowIndex, tuple }));
}

function makeOpInfo(overrides: Partial<OperatorInfo> = {}): OperatorInfo {
return {
state: "completed",
inputTuples: 0,
outputTuples: 0,
resultMode: "table",
...overrides,
};
interface OpInfoOverrides {
state?: OperatorState;
error?: string;
outputTuples?: number;
tuplesCount?: number;
warnings?: string[];
result?: Record<string, any>[];
sampleTuples?: SampleRow[];
resultMode?: OperatorResultMode;
}

function makeOperator(id: string, inputPortIDs: string[] = []): OperatorPredicate {
const inputPorts: PortDescription[] = inputPortIDs.map((portID, i) => ({
portID,
displayName: `Input ${i}`,
}));
function makeExecutionFailure(message: string): WorkflowFatalError {
return {
operatorID: id,
operatorType: "TestOp",
operatorVersion: "1.0",
operatorProperties: {},
inputPorts,
outputPorts: [{ portID: "output-0", displayName: "Output 0" }],
showAdvanced: false,
type: { name: WorkflowFatalErrorType.EXECUTION_FAILURE },
timestamp: { seconds: 0, nanos: 0 },
message,
details: "",
operatorId: "",
workerId: "",
};
}

function makeLink(linkID: string, source: [string, string], target: [string, string]): OperatorLink {
return {
linkID,
source: { operatorID: source[0], portID: source[1] },
target: { operatorID: target[0], portID: target[1] },
function makeOpInfo(overrides: OpInfoOverrides = {}): OperatorExecutionSummary {
const summary: OperatorExecutionSummary = {
state: overrides.state ?? OperatorState.COMPLETED,
errorMessages: overrides.error ? [makeExecutionFailure(overrides.error)] : [],
};
// The result summary is present only when the operator produced a result.
if (overrides.result !== undefined || overrides.sampleTuples !== undefined) {
summary.resultSummary = {
resultMode: overrides.resultMode ?? OperatorResultMode.TABLE,
// Non-arrays are passed through to exercise the "(no result data)" guard.
sampleTuples:
overrides.sampleTuples ??
(Array.isArray(overrides.result) ? toSampleRows(overrides.result) : (overrides.result as any)),
tuplesCount: overrides.tuplesCount ?? overrides.outputTuples ?? 0,
};
}
if (overrides.warnings) {
// Warnings are derived from console messages whose title is "WARNING: ...".
summary.consoleLogsSummary = {
messages: overrides.warnings.map(w => ({ msgType: ConsoleMessageType.PRINT, title: w, message: "" })),
};
}
return summary;
}

const EMPTY_STATE = new WorkflowState();

describe("formatOperatorResult - early returns", () => {
test("returns [ERROR] prefix when error field is set", () => {
const out = formatOperatorResult("op1", makeOpInfo({ error: "boom" }), EMPTY_STATE);
const out = formatOperatorResult("op1", makeOpInfo({ error: "boom" }));
expect(out).toBe("[ERROR] boom");
});

test("treats empty-string error as falsy and continues to result path", () => {
const out = formatOperatorResult("op1", makeOpInfo({ error: "" }), EMPTY_STATE);
const out = formatOperatorResult("op1", makeOpInfo({ error: "" }));
expect(out).not.toContain("[ERROR]");
expect(out).toContain("(no result data)");
});

test("returns (no result data) when result is undefined", () => {
const out = formatOperatorResult("op1", makeOpInfo(), EMPTY_STATE);
const out = formatOperatorResult("op1", makeOpInfo());
expect(out).toBe("(no result data)");
});

test("returns (no result data) when result is not an array", () => {
const out = formatOperatorResult(
"op1",
makeOpInfo({ result: { rows: [] } as unknown as Record<string, any>[] }),
EMPTY_STATE
);
const out = formatOperatorResult("op1", makeOpInfo({ result: { rows: [] } as unknown as Record<string, any>[] }));
expect(out).toBe("(no result data)");
});

test("empty array result emits brief summary plus zero-column shape only", () => {
const out = formatOperatorResult("op1", makeOpInfo({ result: [], outputTuples: 0 }), EMPTY_STATE);
const out = formatOperatorResult("op1", makeOpInfo({ result: [], outputTuples: 0 }));
expect(out.split("\n")).toEqual(["Executed operator op1", "Output table shape: (0, 0)"]);
});
});

describe("formatOperatorResult - table shape and metadata", () => {
test("uses outputTuples for row count when totalRowCount missing", () => {
const out = formatOperatorResult("op1", makeOpInfo({ outputTuples: 7, result: [{ a: 1, b: 2 }] }), EMPTY_STATE);
test("uses outputTuples for row count when tuplesCount missing", () => {
const out = formatOperatorResult("op1", makeOpInfo({ outputTuples: 7, result: [{ a: 1, b: 2 }] }));
expect(out).toContain("Output table shape: (7, 2)");
});

test("totalRowCount overrides outputTuples in output shape", () => {
test("tuplesCount overrides outputTuples in output shape", () => {
const out = formatOperatorResult(
"op1",
makeOpInfo({ outputTuples: 7, totalRowCount: 999, result: [{ a: 1, b: 2 }] }),
EMPTY_STATE
makeOpInfo({ outputTuples: 7, tuplesCount: 999, result: [{ a: 1, b: 2 }] })
);
expect(out).toContain("Output table shape: (999, 2)");
});

test("filters internal __is_visualization__ key from outer column count", () => {
test("counts every result tuple key as a column", () => {
const out = formatOperatorResult(
"op1",
makeOpInfo({
outputTuples: 1,
result: [{ __is_visualization__: true, "html-content": "<x/>" }],
}),
EMPTY_STATE
result: [{ "html-content": "<x/>", label: "chart" }],
})
);
// 1 visible column ("html-content") since __is_visualization__ is filtered.
expect(out).toContain("Output table shape: (1, 1)");
expect(out).toContain("Output table shape: (1, 2)");
});

test("appends warnings after metadata lines", () => {
Expand All @@ -125,145 +139,71 @@ describe("formatOperatorResult - table shape and metadata", () => {
makeOpInfo({
outputTuples: 1,
result: [{ a: 1 }],
warnings: ["truncated to 1 row", "something else"],
}),
EMPTY_STATE
warnings: ["WARNING: truncated to 1 row", "WARNING: something else"],
})
);
const lines = out.split("\n");
expect(lines[0]).toBe("Executed operator op1");
expect(lines[1]).toBe("Output table shape: (1, 1)");
expect(lines[2]).toBe("truncated to 1 row");
expect(lines[3]).toBe("something else");
});
});

describe("formatOperatorResult - input port metadata", () => {
test("omits input metadata when inputPortShapes is missing", () => {
const out = formatOperatorResult("op1", makeOpInfo({ outputTuples: 1, result: [{ a: 1 }] }), EMPTY_STATE);
expect(out).not.toContain("Input operator");
});

test("omits input metadata when inputPortShapes is empty", () => {
const out = formatOperatorResult(
"op1",
makeOpInfo({ outputTuples: 1, result: [{ a: 1 }], inputPortShapes: [] }),
EMPTY_STATE
);
expect(out).not.toContain("Input operator");
});

test("falls back to inputN placeholder when no upstream link matches the port", () => {
const out = formatOperatorResult(
"op1",
makeOpInfo({
outputTuples: 1,
result: [{ a: 1 }],
inputPortShapes: [{ portIndex: 0, rows: 5, columns: 3 }],
}),
EMPTY_STATE
);
expect(out).toContain("Input operator(table shape): input0(5, 3)");
});

test("uses upstream operator id when an input link matches the port", () => {
const state = new WorkflowState();
state.addOperator(makeOperator("upstream"));
state.addOperator(makeOperator("op1", ["input-0"]));
state.addLink(makeLink("l1", ["upstream", "output-0"], ["op1", "input-0"]));

const out = formatOperatorResult(
"op1",
makeOpInfo({
outputTuples: 4,
result: [{ a: 1, b: 2 }],
inputPortShapes: [{ portIndex: 0, rows: 10, columns: 2 }],
}),
state
);
expect(out).toContain("Input operator(table shape): upstream(10, 2)");
});

test("sorts multiple input ports by portIndex regardless of input order", () => {
const state = new WorkflowState();
state.addOperator(makeOperator("up0"));
state.addOperator(makeOperator("up1"));
state.addOperator(makeOperator("op1", ["input-0", "input-1"]));
state.addLink(makeLink("l0", ["up0", "output-0"], ["op1", "input-0"]));
state.addLink(makeLink("l1", ["up1", "output-0"], ["op1", "input-1"]));

const out = formatOperatorResult(
"op1",
makeOpInfo({
outputTuples: 1,
result: [{ a: 1 }],
inputPortShapes: [
{ portIndex: 1, rows: 2, columns: 2 },
{ portIndex: 0, rows: 1, columns: 1 },
],
}),
state
);
expect(out).toContain("Input operator(table shape): up0(1, 1), up1(2, 2)");
expect(lines[2]).toBe("WARNING: truncated to 1 row");
expect(lines[3]).toBe("WARNING: something else");
});
});

describe("formatOperatorResult - visualization rows", () => {
test("strips html-content and json-content payloads when row is flagged as visualization", () => {
test("strips html-content and json-content payloads when result mode is visualization", () => {
const out = formatOperatorResult(
"op1",
makeOpInfo({
outputTuples: 1,
resultMode: OperatorResultMode.VISUALIZATION,
result: [
{
__is_visualization__: true,
"html-content": "<div>hidden</div>",
"json-content": '{"big":1}',
label: "chart",
},
],
}),
EMPTY_STATE
})
);
expect(out).toContain("<skipped: visualization content>");
expect(out).not.toContain("<div>hidden</div>");
expect(out).not.toContain('{"big":1}');
expect(out).toContain("chart");
});

test("__is_visualization__ false leaves the visualization-only fields untouched", () => {
test("table result mode leaves visualization payload fields untouched", () => {
const out = formatOperatorResult(
"op1",
makeOpInfo({
outputTuples: 1,
result: [{ __is_visualization__: false, "html-content": "<keep/>" }],
}),
EMPTY_STATE
resultMode: OperatorResultMode.TABLE,
result: [{ "html-content": "<keep/>" }],
})
);
expect(out).toContain("<keep/>");
expect(out).not.toContain("<skipped: visualization content>");
});

test("__is_visualization__ column is excluded from rendered table body and shape agrees", () => {
test("table rows render all tuple columns and shape agrees", () => {
const out = formatOperatorResult(
"op1",
makeOpInfo({
outputTuples: 1,
result: [{ __is_visualization__: false, value: 1 }],
}),
EMPTY_STATE
result: [{ value: 1 }],
})
);
const lines = out.split("\n");
expect(out).toContain("Output table shape: (1, 1)");
// Header line is the third line (after brief summary and shape line).
expect(lines[2]).toBe("\tvalue");
expect(lines[3]).toBe("0\t1");
expect(out).not.toContain("__is_visualization__");
});
});

describe("jsonToTableFormat - cell coercion via formatOperatorResult", () => {
function tableLines(opInfo: Partial<OperatorInfo>): string[] {
const out = formatOperatorResult("op1", makeOpInfo({ outputTuples: 1, ...opInfo }), EMPTY_STATE);
function tableLines(opInfo: OpInfoOverrides): string[] {
const out = formatOperatorResult("op1", makeOpInfo({ outputTuples: 1, ...opInfo }));
// Skip brief summary + shape line.
return out.split("\n").slice(2);
}
Expand Down Expand Up @@ -296,17 +236,16 @@ describe("jsonToTableFormat - cell coercion via formatOperatorResult", () => {
});

describe("jsonToTableFormat - row index gaps", () => {
test("inserts ... separator when __row_index__ skips ahead", () => {
test("inserts ... separator when rowIndex skips ahead", () => {
const out = formatOperatorResult(
"op1",
makeOpInfo({
outputTuples: 2,
result: [
{ __row_index__: 0, v: "a" },
{ __row_index__: 5, v: "b" },
sampleTuples: [
{ rowIndex: 0, tuple: { v: "a" } },
{ rowIndex: 5, tuple: { v: "b" } },
],
}),
EMPTY_STATE
})
);
const lines = out.split("\n");
// header, row0, gap marker, row5
Expand All @@ -316,26 +255,24 @@ describe("jsonToTableFormat - row index gaps", () => {
expect(lines[lines.length - 1]).toBe("5\tb");
});

test("no separator is emitted between consecutive __row_index__ values", () => {
test("no separator is emitted between consecutive rowIndex values", () => {
const out = formatOperatorResult(
"op1",
makeOpInfo({
outputTuples: 2,
result: [
{ __row_index__: 0, v: "a" },
{ __row_index__: 1, v: "b" },
sampleTuples: [
{ rowIndex: 0, tuple: { v: "a" } },
{ rowIndex: 1, tuple: { v: "b" } },
],
}),
EMPTY_STATE
})
);
expect(out).not.toContain("...\t...");
});

test("non-zero starting __row_index__ does not emit a leading gap marker", () => {
test("non-zero starting rowIndex does not emit a leading gap marker", () => {
const out = formatOperatorResult(
"op1",
makeOpInfo({ outputTuples: 1, result: [{ __row_index__: 9, v: "z" }] }),
EMPTY_STATE
makeOpInfo({ outputTuples: 1, sampleTuples: [{ rowIndex: 9, tuple: { v: "z" } }] })
);
expect(out).not.toContain("...\t...");
expect(out.endsWith("9\tz")).toBe(true);
Expand Down
Loading
Loading