Yicong-Huang commented on code in PR #8301:
URL: https://github.com/apache/texera/pull/8301#discussion_r3927209232
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts:
##########
@@ -898,21 +899,15 @@ describe("WorkflowEditorComponent", () => {
* default (gray) when the user navigates away from and back to a workflow
* that has already finished executing. Both the operator-add stream and
* the validation stream route their final border decision through
- * applyOperatorBorder, which encodes the priority: invalid > cached
+ * applyOperatorStateAndBorder, which encodes the priority: invalid >
cached
* execution state > default valid. These tests assert the operator's
* actual final rect.body/stroke on the paper, so they pin down the visible
* outcome rather than the internal helper calls.
*/
describe("operator border restoration after navigation", () => {
let workflowStatusService: WorkflowStatusService;
const cachedStatus = (operatorState: OperatorState) => ({
Review Comment:
`cachedStatus` builds a `Record<string, OperatorState>` and is mocked onto
`getCurrentState`, and the five titles below it (`:920`, `:930`, `:942`,
`:952`, `:962`) say "cached ... status".
The PR deliberately keeps "status" for the bundled concept —
`handleOperatorStatusUpdate`, `OperatorRuntimeStatus` — which is what makes it
the wrong word for the state half here. `cachedState` and "cached ... state"
match what the helper actually returns.
##########
frontend/src/app/workspace/types/execute-workflow.interface.ts:
##########
@@ -93,9 +92,21 @@ export interface OperatorStatistics
aggregatedIdleTime?: number;
}> {}
+/**
+ * Wire shape of one operator's entry in OperatorStatisticsUpdateEvent. The
Review Comment:
This doc makes `OperatorRuntimeStatus` the wire shape, but
`performance-metrics.ts:26` still calls `OperatorStatistics` "the raw ... the
backend streams over the websocket". After the split that type is the
frontend-side projection, so the two comments now disagree about what comes off
the socket.
I cleared this in round 1 because every field `OperatorStatistics` declares
is still backend-streamed — that reasoning does not survive the doc you added
here.
--
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]
