Yicong-Huang commented on code in PR #6213:
URL: https://github.com/apache/texera/pull/6213#discussion_r3741174400
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -391,6 +416,112 @@ export class WorkflowEditorComponent implements OnInit,
AfterViewInit, OnDestroy
});
}
+ /**
+ * Drives the performance heat-map overlay (Layers > Performance). The
overlay colors only the
+ * operator body fill, so it coexists with the execution-status border.
Colors are derived from
+ * the WorkflowStatusService performance metrics via the pure
heatmap-scoring helpers, so both the
+ * main canvas and the mini-map (shared model) update.
+ */
+ private handleHeatmapOverlay(): void {
+ // Repaint whenever the active view or the metrics change (only while a
view is active).
+ combineLatest([this.wrapper.getHeatmapViewStream(),
this.workflowStatusService.getPerformanceMetricsStream()])
+ .pipe(untilDestroyed(this))
+ .subscribe(([view]) => {
+ if (view !== null) {
+ this.repaintHeatmapColors(view);
+ }
+ });
+
+ // Restore default fills when the overlay is turned off.
+ this.wrapper
+ .getHeatmapViewStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(view => {
+ if (view === null) {
+ this.restoreAllOperatorFills();
+ this.heatmapTooltip = null;
+ }
+ });
+
+ // Paint newly (re)added operators when the overlay is active (e.g. after
reload).
Review Comment:
`changeOperatorDisableStatus` (`joint-ui.service.ts:502`, driven from
`:713`) writes the same `rect.body/fill` this overlay owns, and nothing here
repaints afterwards. With the overlay on, disabling an operator paints it
`#E0E0E0` and re-enabling `#FFFFFF` — neither is on the ramp. It recovers only
on the next metrics tick, view switch, or operator add, so an idle workflow
stays mis-colored.
The Regions layer closes its equivalent path at `:594`. Fix: subscribe
`getDisabledOperatorsChangedStream()` beside the operator-add subscription
below.
##########
frontend/src/app/workspace/component/heatmap-legend/heatmap-legend.component.scss:
##########
@@ -0,0 +1,54 @@
+/**
+ * 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.
+ */
+
+.heatmap-legend {
+ position: absolute;
+ // Bottom-left, lifted above the mini-map preview toggle in the corner. The
mini-map
+ // itself occupies the bottom-right of the canvas.
+ bottom: 72px;
+ left: 0px;
+ z-index: 10;
+ padding: 8px 10px;
+ font-size: 12px;
+ background: rgba(255, 255, 255, 0.95);
+ border: 1px solid #d9d9d9;
+ border-radius: 4px;
+ box-shadow: 0 1px 4px rgba(0, 0, 0, 0.15);
+ pointer-events: none;
+
+ &__title {
+ font-weight: 600;
+ margin-bottom: 4px;
+ }
+
+ &__bar {
+ width: 140px;
+ height: 10px;
+ border-radius: 2px;
+ // Mirrors the cold -> hot stops in heatmap-color.ts (scoreToColor).
+ background: linear-gradient(to right, #5b9bd5, #ffffbf, #e05a52);
Review Comment:
These are `COLD`/`MID`/`HOT` from `heatmap-color.ts:27-29`, re-declared. All
copies agree today, so nothing is broken now. But a future ramp edit recolors
the canvas and leaves this bar on the old gradient. `heatmap-color.spec.ts`
won't catch it — it asserts the TS constants, not the CSS.
The legend is a component, so it can bind the gradient from
`scoreToColor(0)`, `scoreToColor(0.5)`, `scoreToColor(1)` and drop the literals.
##########
frontend/src/app/workspace/service/heatmap/heatmap-scoring.ts:
##########
@@ -0,0 +1,157 @@
+/**
+ * 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 { OperatorPerformanceMetrics } from
"../workflow-status/performance-metrics";
+
+/**
+ * The three heat-map views. Each answers a different "where should I look?"
+ * question; see {@link rawMetricForView} for the per-operator cost each uses.
+ * String-valued so the selection serializes readably (e.g. to localStorage).
+ */
+export enum HeatmapView {
+ Runtime = "runtime",
+ Throughput = "throughput",
Review Comment:
This view's metric is seconds per row (`:50-51`) — the reciprocal of
throughput, growing as throughput falls. Yet the legend and tooltip print
"Throughput" above a hot end that means *low* throughput. The naming
conventions reserve that word for the rate itself, so something directional
like `TimePerRow` would read the way the ramp behaves.
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.scss:
##########
@@ -19,12 +19,41 @@
#workflow-editor-wrapper {
height: 100%;
+ // Anchors absolutely-positioned canvas overlays (e.g. the heat-map legend).
+ position: relative;
}
#workflow-editor {
height: 100%;
}
+// Smoothly animate operator body fill changes (e.g. heat-map recolor on view
switch / toggle).
+::ng-deep #workflow-editor .body {
+ transition: fill 0.25s ease;
Review Comment:
`::ng-deep` plus a global id selector makes this canvas-wide: every operator
fill change now animates, including the disable/enable and validation paths,
for users who never enable the overlay.
If the animation is meant for the heat-map, scope it to a class the overlay
toggles. If it is meant for everything, it deserves a line in the description,
since it changes canvas behavior beyond this feature.
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -391,6 +416,112 @@ export class WorkflowEditorComponent implements OnInit,
AfterViewInit, OnDestroy
});
}
+ /**
+ * Drives the performance heat-map overlay (Layers > Performance). The
overlay colors only the
+ * operator body fill, so it coexists with the execution-status border.
Colors are derived from
+ * the WorkflowStatusService performance metrics via the pure
heatmap-scoring helpers, so both the
+ * main canvas and the mini-map (shared model) update.
+ */
+ private handleHeatmapOverlay(): void {
+ // Repaint whenever the active view or the metrics change (only while a
view is active).
+ combineLatest([this.wrapper.getHeatmapViewStream(),
this.workflowStatusService.getPerformanceMetricsStream()])
+ .pipe(untilDestroyed(this))
+ .subscribe(([view]) => {
+ if (view !== null) {
+ this.repaintHeatmapColors(view);
+ }
+ });
+
+ // Restore default fills when the overlay is turned off.
+ this.wrapper
+ .getHeatmapViewStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(view => {
+ if (view === null) {
+ this.restoreAllOperatorFills();
+ this.heatmapTooltip = null;
+ }
+ });
+
+ // Paint newly (re)added operators when the overlay is active (e.g. after
reload).
+ this.workflowActionService
+ .getTexeraGraph()
+ .getOperatorAddStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(() => {
+ const view = this.wrapper.getHeatmapView();
+ if (view !== null) {
+ this.repaintHeatmapColors(view);
+ }
+ });
+ }
+
+ private heatmapScores(view: HeatmapView): Record<string, number> {
+ const metrics = this.workflowStatusService.getCurrentPerformanceMetrics();
+ const rawById: Record<string, number> = {};
+ for (const operatorId of Object.keys(metrics)) {
+ rawById[operatorId] = rawMetricForView(metrics[operatorId], view);
+ }
+ return normalizeScores(rawById);
+ }
+
+ private repaintHeatmapColors(view: HeatmapView): void {
+ const scores = this.heatmapScores(view);
+ this.workflowActionService
+ .getTexeraGraph()
+ .getAllOperators()
+ .forEach(op => this.jointUIService.applyHeatmapColor(this.paper,
op.operatorID, scores[op.operatorID]));
+ }
+
+ private restoreAllOperatorFills(): void {
+ this.workflowActionService
+ .getTexeraGraph()
+ .getAllOperators()
+ .forEach(op => this.jointUIService.restoreOperatorFill(this.paper, op));
+ }
+
+ /**
+ * Shows a small tooltip with the hovered operator's metric value and heat
score for the active
+ * view. Only active while the heat-map overlay is on.
+ */
+ private handleHeatmapHover(): void {
+ fromJointPaperEvent(this.paper, "element:mouseenter")
+ .pipe(untilDestroyed(this))
+ .subscribe(([elementView, evt]) => {
+ const view = this.wrapper.getHeatmapView();
+ if (view === null) {
+ return;
+ }
+ const operatorId = elementView.model.id.toString();
+ if
(!this.workflowActionService.getTexeraGraph().hasOperator(operatorId)) {
+ return;
+ }
+ const metrics =
this.workflowStatusService.getCurrentPerformanceMetrics()[operatorId];
+ const score = this.heatmapScores(view)[operatorId];
+ const rect = this.editor.getBoundingClientRect();
+ const mouseEvent = evt as unknown as MouseEvent;
+ // Missing metrics render as "—" for both fields, so "no data yet" is
not
+ // confused with a genuine zero value.
+ this.heatmapTooltip = {
+ x: mouseEvent.clientX - rect.left + 12,
+ y: mouseEvent.clientY - rect.top + 12,
+ title: heatmapViewTitle(view),
+ metricLabel: metrics ? formatMetricForView(rawMetricForView(metrics,
view), view) : "—",
+ heatLabel: score === undefined ? "—" : `${Math.round(score * 100)}%`,
+ };
+ // JointJS paper events fire outside Angular's zone, so trigger change
detection
+ // for the tooltip to render (mirrors the chat-popover handling).
+ this.changeDetectorRef.detectChanges();
+ });
+
+ fromJointPaperEvent(this.paper, "element:mouseleave")
Review Comment:
The `mouseenter` branch returns early when no view is active; this one
doesn't. Every canvas mouse-out therefore clears the tooltip and forces a
synchronous change-detection pass, including for users who never open the
overlay. Guarding on `this.wrapper.getHeatmapView() === null` before touching
`heatmapTooltip` keeps the feature inert when it is off.
##########
frontend/src/app/workspace/service/heatmap/heatmap-color.spec.ts:
##########
@@ -0,0 +1,64 @@
+/**
+ * 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 { HEATMAP_NO_DATA_COLOR, scoreToColor } from "./heatmap-color";
+
+describe("scoreToColor", () => {
+ it("maps 0 to the cold stop (blue)", () => {
+ expect(scoreToColor(0)).toBe("#5b9bd5");
+ });
+
+ it("maps 0.5 to the mid stop (pale yellow)", () => {
+ expect(scoreToColor(0.5)).toBe("#ffffbf");
+ });
+
+ it("maps 1 to the hot stop (red)", () => {
+ expect(scoreToColor(1)).toBe("#e05a52");
+ });
+
+ it("clamps values below 0 to the cold stop", () => {
+ expect(scoreToColor(-0.5)).toBe(scoreToColor(0));
+ });
+
+ it("clamps values above 1 to the hot stop", () => {
+ expect(scoreToColor(2)).toBe(scoreToColor(1));
+ });
+
+ it("treats a non-finite score as cold rather than emitting an invalid
color", () => {
Review Comment:
The second assertion pins `Infinity` to the hot stop, so the name describes
only the NaN half.
```suggestion
it("maps non-finite scores to the ramp endpoints (NaN -> cold, Infinity ->
hot)", () => {
```
##########
frontend/src/app/workspace/component/heatmap-legend/heatmap-legend.component.scss:
##########
@@ -0,0 +1,54 @@
+/**
+ * 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.
+ */
+
+.heatmap-legend {
+ position: absolute;
+ // Bottom-left, lifted above the mini-map preview toggle in the corner. The
mini-map
+ // itself occupies the bottom-right of the canvas.
Review Comment:
The mini-map toggle is not in this corner — all of its buttons are `bottom:
0; right: 0..90px` (`mini-map.component.scss:20-45`), and the next sentence
says so. Worth naming whatever the 72px offset actually clears.
```suggestion
// Bottom-left of the canvas; the mini-map and its toggle buttons sit in
the
// bottom-right corner (see mini-map.component.scss).
```
--
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]
