Skip to content

Commit 66d9785

Browse files
committed
refactor: remove legacy result marker fields
1 parent a3b179d commit 66d9785

9 files changed

Lines changed: 59 additions & 81 deletions

File tree

agent-service/src/agent/tools/result-formatting.spec.ts

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -29,24 +29,19 @@ import {
2929
type SampleRow,
3030
} from "../../types/execution";
3131

32-
// Convert flat test rows (with an optional embedded __row_index__) into the
33-
// structured SampleRow[] the summary now carries.
3432
function toSampleRows(rows: Record<string, any>[]): SampleRow[] {
35-
return rows.map((row, i) => {
36-
const { __row_index__, ...tuple } = row;
37-
return { rowIndex: typeof __row_index__ === "number" ? __row_index__ : i, tuple };
38-
});
33+
return rows.map((tuple, rowIndex) => ({ rowIndex, tuple }));
3934
}
4035

41-
// Test convenience: accept the (old) flat fields and assemble the structured
42-
// OperatorExecutionSummary, so the cases below stay terse.
4336
interface OpInfoOverrides {
4437
state?: OperatorState;
4538
error?: string;
4639
outputTuples?: number;
4740
tuplesCount?: number;
4841
warnings?: string[];
4942
result?: Record<string, any>[];
43+
sampleTuples?: SampleRow[];
44+
resultMode?: OperatorResultMode;
5045
}
5146

5247
function makeExecutionFailure(message: string): WorkflowFatalError {
@@ -66,11 +61,13 @@ function makeOpInfo(overrides: OpInfoOverrides = {}): OperatorExecutionSummary {
6661
errorMessages: overrides.error ? [makeExecutionFailure(overrides.error)] : [],
6762
};
6863
// The result summary is present only when the operator produced a result.
69-
if (overrides.result !== undefined) {
64+
if (overrides.result !== undefined || overrides.sampleTuples !== undefined) {
7065
summary.resultSummary = {
71-
resultMode: OperatorResultMode.TABLE,
66+
resultMode: overrides.resultMode ?? OperatorResultMode.TABLE,
7267
// Non-arrays are passed through to exercise the "(no result data)" guard.
73-
sampleTuples: Array.isArray(overrides.result) ? toSampleRows(overrides.result) : (overrides.result as any),
68+
sampleTuples:
69+
overrides.sampleTuples ??
70+
(Array.isArray(overrides.result) ? toSampleRows(overrides.result) : (overrides.result as any)),
7471
tuplesCount: overrides.tuplesCount ?? overrides.outputTuples ?? 0,
7572
};
7673
}
@@ -125,16 +122,15 @@ describe("formatOperatorResult - table shape and metadata", () => {
125122
expect(out).toContain("Output table shape: (999, 2)");
126123
});
127124

128-
test("filters internal __is_visualization__ key from outer column count", () => {
125+
test("counts every result tuple key as a column", () => {
129126
const out = formatOperatorResult(
130127
"op1",
131128
makeOpInfo({
132129
outputTuples: 1,
133-
result: [{ __is_visualization__: true, "html-content": "<x/>" }],
130+
result: [{ "html-content": "<x/>", label: "chart" }],
134131
})
135132
);
136-
// 1 visible column ("html-content") since __is_visualization__ is filtered.
137-
expect(out).toContain("Output table shape: (1, 1)");
133+
expect(out).toContain("Output table shape: (1, 2)");
138134
});
139135

140136
test("appends warnings after metadata lines", () => {
@@ -155,14 +151,14 @@ describe("formatOperatorResult - table shape and metadata", () => {
155151
});
156152

157153
describe("formatOperatorResult - visualization rows", () => {
158-
test("strips html-content and json-content payloads when row is flagged as visualization", () => {
154+
test("strips html-content and json-content payloads when result mode is visualization", () => {
159155
const out = formatOperatorResult(
160156
"op1",
161157
makeOpInfo({
162158
outputTuples: 1,
159+
resultMode: OperatorResultMode.VISUALIZATION,
163160
result: [
164161
{
165-
__is_visualization__: true,
166162
"html-content": "<div>hidden</div>",
167163
"json-content": '{"big":1}',
168164
label: "chart",
@@ -176,32 +172,32 @@ describe("formatOperatorResult - visualization rows", () => {
176172
expect(out).toContain("chart");
177173
});
178174

179-
test("__is_visualization__ false leaves the visualization-only fields untouched", () => {
175+
test("table result mode leaves visualization payload fields untouched", () => {
180176
const out = formatOperatorResult(
181177
"op1",
182178
makeOpInfo({
183179
outputTuples: 1,
184-
result: [{ __is_visualization__: false, "html-content": "<keep/>" }],
180+
resultMode: OperatorResultMode.TABLE,
181+
result: [{ "html-content": "<keep/>" }],
185182
})
186183
);
187184
expect(out).toContain("<keep/>");
188185
expect(out).not.toContain("<skipped: visualization content>");
189186
});
190187

191-
test("__is_visualization__ column is excluded from rendered table body and shape agrees", () => {
188+
test("table rows render all tuple columns and shape agrees", () => {
192189
const out = formatOperatorResult(
193190
"op1",
194191
makeOpInfo({
195192
outputTuples: 1,
196-
result: [{ __is_visualization__: false, value: 1 }],
193+
result: [{ value: 1 }],
197194
})
198195
);
199196
const lines = out.split("\n");
200197
expect(out).toContain("Output table shape: (1, 1)");
201198
// Header line is the third line (after brief summary and shape line).
202199
expect(lines[2]).toBe("\tvalue");
203200
expect(lines[3]).toBe("0\t1");
204-
expect(out).not.toContain("__is_visualization__");
205201
});
206202
});
207203

@@ -240,14 +236,14 @@ describe("jsonToTableFormat - cell coercion via formatOperatorResult", () => {
240236
});
241237

242238
describe("jsonToTableFormat - row index gaps", () => {
243-
test("inserts ... separator when __row_index__ skips ahead", () => {
239+
test("inserts ... separator when rowIndex skips ahead", () => {
244240
const out = formatOperatorResult(
245241
"op1",
246242
makeOpInfo({
247243
outputTuples: 2,
248-
result: [
249-
{ __row_index__: 0, v: "a" },
250-
{ __row_index__: 5, v: "b" },
244+
sampleTuples: [
245+
{ rowIndex: 0, tuple: { v: "a" } },
246+
{ rowIndex: 5, tuple: { v: "b" } },
251247
],
252248
})
253249
);
@@ -259,22 +255,25 @@ describe("jsonToTableFormat - row index gaps", () => {
259255
expect(lines[lines.length - 1]).toBe("5\tb");
260256
});
261257

262-
test("no separator is emitted between consecutive __row_index__ values", () => {
258+
test("no separator is emitted between consecutive rowIndex values", () => {
263259
const out = formatOperatorResult(
264260
"op1",
265261
makeOpInfo({
266262
outputTuples: 2,
267-
result: [
268-
{ __row_index__: 0, v: "a" },
269-
{ __row_index__: 1, v: "b" },
263+
sampleTuples: [
264+
{ rowIndex: 0, tuple: { v: "a" } },
265+
{ rowIndex: 1, tuple: { v: "b" } },
270266
],
271267
})
272268
);
273269
expect(out).not.toContain("...\t...");
274270
});
275271

276-
test("non-zero starting __row_index__ does not emit a leading gap marker", () => {
277-
const out = formatOperatorResult("op1", makeOpInfo({ outputTuples: 1, result: [{ __row_index__: 9, v: "z" }] }));
272+
test("non-zero starting rowIndex does not emit a leading gap marker", () => {
273+
const out = formatOperatorResult(
274+
"op1",
275+
makeOpInfo({ outputTuples: 1, sampleTuples: [{ rowIndex: 9, tuple: { v: "z" } }] })
276+
);
278277
expect(out).not.toContain("...\t...");
279278
expect(out.endsWith("9\tz")).toBe(true);
280279
});

agent-service/src/agent/tools/result-formatting.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
* under the License.
1818
*/
1919

20-
import type { OperatorExecutionSummary, SampleRow } from "../../types/execution";
20+
import { OperatorResultMode, type OperatorExecutionSummary, type SampleRow } from "../../types/execution";
2121
import { formatExecuteOperatorResult, getOperatorWarnings, getVisibleResultHeaders } from "./tools-utility";
2222

2323
export function formatOperatorResult(operatorId: string, opInfo: OperatorExecutionSummary): string {
@@ -31,15 +31,11 @@ export function formatOperatorResult(operatorId: string, opInfo: OperatorExecuti
3131
return "(no result data)";
3232
}
3333

34-
const headers = sampleTuples.length > 0 ? getVisibleResultHeaders(sampleTuples[0].tuple) : [];
35-
const columns = headers.length;
36-
37-
const isViz = sampleTuples.length > 0 && sampleTuples[0].tuple["__is_visualization__"] === true;
34+
const isViz = opInfo.resultSummary?.resultMode === OperatorResultMode.VISUALIZATION;
3835
const rows: SampleRow[] = isViz
3936
? sampleTuples.map(({ rowIndex, tuple }) => {
4037
const cleaned: Record<string, any> = {};
4138
for (const key of Object.keys(tuple)) {
42-
if (key === "__is_visualization__") continue;
4339
if (key === "html-content" || key === "json-content") {
4440
cleaned[key] = "<skipped: visualization content>";
4541
} else {
@@ -50,6 +46,9 @@ export function formatOperatorResult(operatorId: string, opInfo: OperatorExecuti
5046
})
5147
: sampleTuples;
5248

49+
const headers = rows.length > 0 ? getVisibleResultHeaders(rows[0].tuple) : [];
50+
const columns = headers.length;
51+
5352
const dataString = jsonToTableFormat(rows);
5453

5554
// Output shape only; input-port shapes are derivable by the agent from the DAG

agent-service/src/agent/tools/tools-utility.spec.ts

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,37 +29,17 @@ import {
2929
} from "./tools-utility";
3030

3131
describe("getVisibleResultHeaders", () => {
32-
test("returns every key when no internal columns are present", () => {
32+
test("returns every key", () => {
3333
expect(getVisibleResultHeaders({ a: 1, b: 2 })).toEqual(["a", "b"]);
3434
});
3535

36-
test("strips __row_index__ from the result", () => {
37-
expect(getVisibleResultHeaders({ __row_index__: 0, a: 1 })).toEqual(["a"]);
38-
});
39-
40-
test("strips __is_visualization__ from the result", () => {
41-
expect(getVisibleResultHeaders({ __is_visualization__: true, a: 1 })).toEqual(["a"]);
42-
});
43-
44-
test("strips every known internal column at once", () => {
45-
expect(getVisibleResultHeaders({ __row_index__: 0, __is_visualization__: true, a: 1, b: 2 })).toEqual(["a", "b"]);
46-
});
47-
4836
test("preserves visible column order", () => {
49-
expect(getVisibleResultHeaders({ z: 1, __row_index__: 0, a: 2, __is_visualization__: true, m: 3 })).toEqual([
50-
"z",
51-
"a",
52-
"m",
53-
]);
37+
expect(getVisibleResultHeaders({ z: 1, a: 2, m: 3 })).toEqual(["z", "a", "m"]);
5438
});
5539

5640
test("returns an empty array for an empty row", () => {
5741
expect(getVisibleResultHeaders({})).toEqual([]);
5842
});
59-
60-
test("returns an empty array when only internal columns are present", () => {
61-
expect(getVisibleResultHeaders({ __row_index__: 0, __is_visualization__: true })).toEqual([]);
62-
});
6343
});
6444

6545
describe("createToolResult", () => {

agent-service/src/agent/tools/tools-utility.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,8 @@
1919

2020
import type { OperatorExecutionSummary } from "../../types/execution";
2121

22-
export const INTERNAL_RESULT_KEYS: ReadonlySet<string> = new Set(["__row_index__", "__is_visualization__"]);
23-
2422
export function getVisibleResultHeaders(row: Record<string, any>): string[] {
25-
return Object.keys(row).filter(k => !INTERNAL_RESULT_KEYS.has(k));
23+
return Object.keys(row);
2624
}
2725

2826
// Warnings are the console messages the engine tags with a "WARNING: " title

agent-service/src/types/execution.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,7 @@ export interface ConsoleMessage {
8585
message: string;
8686
}
8787

88-
// One sampled output row: its original position plus the row's columns. (A viz
89-
// payload's tuple still carries an `__is_visualization__` marker.)
88+
// One sampled output row: its original position plus the row's columns.
9089
export interface SampleRow {
9190
rowIndex: number;
9291
tuple: Record<string, unknown>;

amber/src/main/scala/org/apache/texera/web/resource/SyncExecutionResource.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,6 @@ class SyncExecutionResource extends LazyLogging {
551551
if (totalCount == 1 && isVisualizationTuple(firstTuple)) {
552552
val jsonResults =
553553
ExecutionResultService.convertTuplesToJson(List(firstTuple), isVisualization = true)
554-
jsonResults.foreach(_.put("__is_visualization__", true))
555554
val rows = jsonResults.zipWithIndex.map { case (json, idx) => SampleRow(idx, json) }
556555
return ("visualization", Some(rows), Some(totalCount), Some(1), Some(false))
557556
}

frontend/src/app/workspace/component/agent/agent-interaction/agent-interaction.component.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import { NzTooltipDirective } from "ng-zorro-antd/tooltip";
4040
import { NzInputDirective, NzAutosizeDirective } from "ng-zorro-antd/input";
4141
import { NzButtonComponent } from "ng-zorro-antd/button";
4242
import { NzWaveDirective } from "ng-zorro-antd/core/wave";
43-
import { SampleRow } from "../../../service/agent/agent.service";
43+
import { OperatorResultMode, SampleRow } from "../../../service/agent/agent.service";
4444

4545
/**
4646
* AgentInteractionComponent provides a compact interface for users to send feedback
@@ -77,6 +77,7 @@ export class AgentInteractionComponent implements OnInit, OnChanges {
7777
@Input() operatorId!: string;
7878
@Input() operatorDisplayName?: string;
7979
@Input() sampleTuples?: SampleRow[];
80+
@Input() resultMode?: OperatorResultMode;
8081

8182
public availableAgents: Array<{ id: string; name: string; isConnected: boolean }> = [];
8283
public selectedAgentId: string | null = null;
@@ -102,10 +103,9 @@ export class AgentInteractionComponent implements OnInit, OnChanges {
102103
}
103104

104105
ngOnChanges(changes: SimpleChanges): void {
105-
if (changes["sampleTuples"]) {
106+
if (changes["sampleTuples"] || changes["resultMode"]) {
106107
// Only update cached visualization HTML when the actual content changes
107-
const newRows = changes["sampleTuples"].currentValue as SampleRow[] | undefined;
108-
const htmlContent = newRows?.[0]?.tuple["html-content"];
108+
const htmlContent = this.sampleTuples?.[0]?.tuple["html-content"];
109109
const newHtml = typeof htmlContent === "string" ? htmlContent : null;
110110
if (newHtml !== this.cachedVisualizationRawHtml) {
111111
this.cachedVisualizationRawHtml = newHtml;
@@ -177,12 +177,8 @@ export class AgentInteractionComponent implements OnInit, OnChanges {
177177
return !!this.selectedAgentId && !!this.feedbackMessage.trim();
178178
}
179179

180-
/**
181-
* Check if sample rows represent a visualization (has __is_visualization__ flag).
182-
*/
183180
public isVisualization(): boolean {
184-
if (!this.sampleTuples || this.sampleTuples.length === 0) return false;
185-
return this.sampleTuples[0].tuple["__is_visualization__"] === true;
181+
return this.resultMode === OperatorResultMode.VISUALIZATION;
186182
}
187183

188184
/**
@@ -197,7 +193,7 @@ export class AgentInteractionComponent implements OnInit, OnChanges {
197193
*/
198194
public getSampleColumns(): string[] {
199195
if (!this.sampleTuples || this.sampleTuples.length === 0) return [];
200-
return Object.keys(this.sampleTuples[0].tuple).filter(k => k !== "__is_visualization__");
196+
return Object.keys(this.sampleTuples[0].tuple);
201197
}
202198

203199
/**

frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@
4040
<texera-agent-interaction
4141
[operatorId]="chatPopoverOperator.operatorId"
4242
[operatorDisplayName]="chatPopoverOperator.displayName"
43-
[sampleTuples]="getOperatorSampleTuples(chatPopoverOperator.operatorId)">
43+
[sampleTuples]="getOperatorSampleTuples(chatPopoverOperator.operatorId)"
44+
[resultMode]="getOperatorResultMode(chatPopoverOperator.operatorId)">
4445
</texera-agent-interaction>
4546
</div>
4647
</div>

frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,12 @@ import { isDefined } from "../../../common/util/predicate";
4343
import { GuiConfigService } from "../../../common/service/gui-config.service";
4444
import { line, curveCatmullRomClosed } from "d3-shape";
4545
import concaveman from "concaveman";
46-
import { AgentService, OperatorExecutionSummary, SampleRow } from "../../service/agent/agent.service";
46+
import {
47+
AgentService,
48+
OperatorExecutionSummary,
49+
OperatorResultMode,
50+
SampleRow,
51+
} from "../../service/agent/agent.service";
4752
import { NzNoAnimationDirective } from "ng-zorro-antd/core/animation";
4853
import { ContextMenuComponent } from "./context-menu/context-menu/context-menu.component";
4954
import { NgIf } from "@angular/common";
@@ -1700,10 +1705,12 @@ export class WorkflowEditorComponent implements OnInit, AfterViewInit, OnDestroy
17001705
return this.operatorSummaries.get(operatorId)?.resultSummary?.sampleTuples;
17011706
}
17021707

1708+
getOperatorResultMode(operatorId: string): OperatorResultMode | undefined {
1709+
return this.operatorSummaries.get(operatorId)?.resultSummary?.resultMode;
1710+
}
1711+
17031712
isOperatorVisualization(operatorId: string): boolean {
1704-
return (
1705-
this.operatorSummaries.get(operatorId)?.resultSummary?.sampleTuples?.[0]?.tuple["__is_visualization__"] === true
1706-
);
1713+
return this.getOperatorResultMode(operatorId) === OperatorResultMode.VISUALIZATION;
17071714
}
17081715

17091716
/**

0 commit comments

Comments
 (0)