Copilot commented on code in PR #5751:
URL: https://github.com/apache/texera/pull/5751#discussion_r3487127776
##########
frontend/src/app/workspace/service/agent/agent.service.ts:
##########
@@ -1311,7 +1274,10 @@ export class AgentService {
}
/**
- * Fetch operator results from the backend (fallback if WebSocket data not
available).
+ * Pull the agent's latest operator result summaries from the backend and
push
+ * them to `operatorResultSummaries$`. Called on demand when the UI needs to
+ * show results (e.g. opening an operator's popover); results are no longer
+ * pushed over the WebSocket.
*/
Review Comment:
This doc comment says operator results are no longer pushed over the
WebSocket, but the service still consumes `message.operatorResults` in the
legacy `headChange` WS frame. Either clarify that the “no longer pushed”
guarantee applies to snapshot/step/status frames only, or remove
`operatorResults` from `headChange` as part of the deprecation cleanup.
##########
agent-service/src/server.ts:
##########
@@ -523,21 +492,23 @@ export function buildApp() {
return;
}
- let msg: WsMessage;
+ let msg: WsClientRequest;
try {
- msg = typeof messageData === "string" ? JSON.parse(messageData) :
(messageData as WsMessage);
+ msg = typeof messageData === "string" ? JSON.parse(messageData) :
(messageData as WsClientRequest);
} catch {
ws.send(JSON.stringify({ type: "error", error: "Invalid message
format" }));
return;
}
- if (msg.type === "stop") {
- agent.stop();
- broadcastToAgent(agentId, { type: "state", state: "STOPPING" });
+ if (msg.type === "command") {
+ if (msg.commandType === "stop") {
+ agent.stop();
+ broadcastToAgent(agentId, { type: "status", state:
AgentState.STOPPING });
+ }
return;
}
Review Comment:
The WS handler silently ignores unknown `commandType` values (and any
unexpected `msg.type` values), which makes client/server mismatches hard to
diagnose and can lead to no-op behavior on bad input. Since frames are parsed
from untrusted JSON, it would be safer to validate the discriminator(s) at
runtime and return an explicit error frame for unknown types/commands.
##########
agent-service/src/types/ws/server.ts:
##########
@@ -0,0 +1,110 @@
+/**
+ * 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.
+ */
+
+// Server -> client WebSocket frames for this service's protocol
+// (`/agents/:id/react`). Modeled as a discriminated union on `type`, so each
+// message kind declares exactly the fields it sends.
+
+import type { AgentState, ReActStep } from "../agent";
+import type { WorkflowContent } from "../workflow";
+
+/**
+ * Wire projection of one operator's execution result, summarized for the
+ * client: counts and a small record sample instead of full payloads.
+ */
+export interface OperatorResultSummaryWs {
+ state: string;
+ inputTuples: number;
+ outputTuples: number;
+ inputPortShapes?: { portIndex: number; rows: number; columns: number }[];
+ outputColumns?: number;
+ error?: string;
+ warnings?: string[];
+ consoleLogCount?: number;
+ totalRowCount?: number;
+ sampleRecords?: Record<string, unknown>[];
+ resultStatistics?: Record<string, string>;
+}
+
+/** Per-operator result summaries, keyed by operator id. */
+type OperatorResults = Record<string, OperatorResultSummaryWs>;
+
+/** Shared discriminator base; every server frame sets a unique `type`. */
+interface WsServerMessageBase {
+ type: "snapshot" | "step" | "status" | "error" | "headChange";
+}
+
+/**
+ * Full state pushed once when a client connects: the agent's current lifecycle
+ * state, the complete step list, and the HEAD pointer. Operator results are
not
+ * included — they are pulled on demand via `GET /operator-results`.
+ */
+export interface WsServerSnapshotMessage extends WsServerMessageBase {
+ type: "snapshot";
+ state: AgentState;
+ steps: ReActStep[];
+ headId: string;
+}
+
+/**
+ * A single ReAct step, streamed live as the agent runs.
+ */
+export interface WsServerStepMessage extends WsServerMessageBase {
+ type: "step";
+ step: ReActStep;
+}
+
+/**
+ * An agent lifecycle transition (e.g. GENERATING when a run starts, the
resting
+ * state when it ends, STOPPING on stop).
+ */
+export interface WsServerStatusMessage extends WsServerMessageBase {
+ type: "status";
+ state: AgentState;
+}
+
+/** An error surfaced to the client (agent not found, bad request, failed
run). */
+export interface WsServerErrorMessage extends WsServerMessageBase {
+ type: "error";
+ error: string;
+}
+
+/**
+ * Emitted after a checkout: HEAD moved, carrying the full step list and the
+ * workflow snapshot at the new head.
+ *
+ * @deprecated Redundant and unused — the checkout flow that produces this
frame
+ * is unreachable in the product (nothing invokes the client's
`checkoutStep()`).
+ * Scheduled for removal (see #5930); do not build new code on it.
+ */
+export interface WsServerHeadChangeMessage extends WsServerMessageBase {
+ type: "headChange";
+ headId: string;
+ steps: ReActStep[];
+ workflowContent?: WorkflowContent;
+ operatorResults: OperatorResults;
Review Comment:
`WsServerHeadChangeMessage` still includes `operatorResults`, which
contradicts the PR description/issue goal that operator result summaries are no
longer pushed over the WebSocket. If `headChange` is truly
deprecated/unreachable, consider removing `operatorResults` from this frame
(and from the checkout broadcast + frontend handler) so the protocol is
consistent and the results transport is purely HTTP.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
