[PG1204]

What changes were proposed in this PR?

The heat-map overlay's pure logic is well covered, but the editor-side wiring shipped without specs in #6213. This adds them. Test-only: no production code changes, so no behaviour change and nothing to screenshot.

22 tests covering handleHeatmapOverlay, repaintHeatmapColors, restoreAllOperatorFills and handleHeatmapHover, in four groups:

Group	Covers
Painting	Spread on view select, repaint on new metrics, inert while off, no-metrics and not-measurable-for-view operators painted as no-data, operators added while the overlay is on and off
Turning off	Fills restored respecting each operator's disabled state, .heatmap-active toggled, an open tooltip cleared
Disable/enable repaint	The #6213 regression, both directions, plus the overlay-off case
Hover tooltip	Metric and score, inert while off, non-operator cells ignored, em dash for missing metrics and for an unmeasurable view, position relative to the editor, mouseleave clearing, both change-detection skips

The tests drive the real paper and the real JointUIService and assert actual rect.body/fill values against scoreToColor and HEATMAP_NO_DATA_COLOR. A spy on applyHeatmapColor would pass even when changeOperatorDisableStatus overwrites the fill afterwards, which is the exact failure the disable/enable regression exists to catch. Metrics are published through statisticsSubject rather than pushed onto the derived stream, so the scoring under test is the one the service actually derives.

The ngOnDestroy view reset already has a spec and is not duplicated here.

Any related issues, documentation, discussions?

Closes #7491. Follow-up from #6213. Part of umbrella #5772.

How was this PR tested?

The specs are the change. Each one was verified to fail for its own reason by deleting the behaviour it covers and re-running:

Deleted from workflow-editor.component.ts	Tests that failed
The getDisabledOperatorsChangedStream repaint	2
The operator-add repaint	1
restoreAllOperatorFills() on overlay off	1
The .heatmap-active class toggle	1
The hover hasOperator guard	1
The tooltip clear on overlay off	1

cd frontend && ng test --watch=false --include "**/workflow-editor.component.spec.ts"

139 passing, up from 117. Statement coverage of the wiring (lines 429-564) is 46/46, and the file overall is 626/627.

Full frontend suite: 6203 passing, 1 skipped. tsc --noEmit (strict), eslint ./src and Prettier are clean.

Was this PR authored or co-authored using generative AI tooling?

This PR was co-authored by Claude, in compliance with ASF.

[github-actions]

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

• Contributors with relevant context: @yangzhang75
  You can notify them by mentioning @yangzhang75 in a comment.

[PG1204]

/request-review @Yicong-Huang

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.10%. Comparing base (d79ac16) to head (1ecdc24).

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8629      +/-   ##
============================================
+ Coverage     92.05%   92.10%   +0.05%     
- Complexity     5039     5040       +1     
============================================
  Files          1286     1286              
  Lines         55928    55928              
  Branches       7138     7138              
============================================
+ Hits          51483    51515      +32     
+ Misses         2693     2663      -30     
+ Partials       1752     1750       -2     

Flag	Coverage Δ		*Carryforward flag
access-control-service	77.44% <ø> (ø)		Carriedforward from 40f75ab
agent-service	99.16% <ø> (ø)		Carriedforward from 40f75ab
amber	86.56% <ø> (+0.01%)	⬆️	Carriedforward from 40f75ab
computing-unit-managing-service	64.02% <ø> (ø)		Carriedforward from 40f75ab
config-service	87.61% <ø> (ø)		Carriedforward from 40f75ab
file-service	82.29% <ø> (ø)		Carriedforward from 40f75ab
frontend	96.60% <ø> (+0.11%)	⬆️
notebook-migration-service	83.77% <ø> (ø)		Carriedforward from 40f75ab
pyamber	98.54% <ø> (ø)		Carriedforward from 40f75ab
workflow-compiling-service	74.25% <ø> (ø)		Carriedforward from 40f75ab

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[PG1204]

/request-review @Ma77Ball

[Ma77Ball]

Review summary

Test-only PR: it adds 22 specs for the heat-map overlay wiring in workflow-editor.component.ts (view painting, overlay-off restore, the #6213 disable/enable repaint, and the hover tooltip), with no production change. I traced every assertion back to the production code it exercises and they line up. The approach of driving the real WorkflowStatusService and real JointUIService and asserting concrete rect.body/fill values, rather than spying on applyHeatmapColor, is the right call: it is what actually catches the disable/enable regression that a spy would miss. No blocking issues, and nothing I would hold merge on.

Action items: Nothing blocking; no changes requested. The notes below are verification only.

---

Findings

0 total: 0 P0, 0 P1, 0 P2, 0 P3.

No correctness, contract, or coverage problems found. No inline comments.

---

Verification

Checked statically against the fork head (4327053); I did not run the suite locally, so the pass counts and the delete-to-fail checks in the description are the author's. What I confirmed by reading the sources the specs touch:

• Hard-coded fills match production exactly. ENABLED_FILL / DISABLED_FILL ("#FFFFFF" / "#E0E0E0") are the two values JointUIService.getOperatorFillColor returns (joint-ui.service.ts:982), set on rect.body/fill by both restoreOperatorFill (:551) and changeOperatorDisableStatus (:531). So the restore-respecting-disabled-state and disable/enable specs assert the real values, not guesses.
• Heat and no-data colors cannot drift from production casing. HOT / COLD / HEATMAP_NO_DATA_COLOR in the specs come from the same scoreToColor / HEATMAP_NO_DATA_COLOR that applyHeatmapColor uses (joint-ui.service.ts:542), so a lowercase-hex vs uppercase mismatch is impossible.
• publishStatistics drives the real derivation. statisticsSubject.next(...) flows through buildPerformanceMetrics into the performanceMetricsSubject BehaviorSubject (workflow-status.service.ts:52, :120), which is exactly what the overlay's combineLatest repaint and getCurrentPerformanceMetrics() read. The "repaint on new metrics" and "no view leaves fills untouched" specs therefore exercise the real stream, not a shortcut.
• The Runtime numbers check out. Control time defaults to 0 (performance-metrics.ts), so Runtime raw = dataNs = 1e9, giving the "1.00 s" metric and a 100% heat label for the hottest operator.
• The no-data and em-dash paths are genuine. rawMetricForView returns undefined for I/O imbalance when inputRows === 0 (heatmap-scoring.ts:66), which is what makes both the "painted as no-data" and the "em-dash metric" specs assert the not-measurable branch rather than a measured zero. An operator absent from the metrics map is likewise scored undefined, so it does not anchor the scale minimum (the point the "no metrics as no-data rather than coldest" spec pins).
• The tooltip-position spec is not brittle under jsdom. It reads the same editor.getBoundingClientRect() the handler reads (workflow-editor.component.ts:563), so a zero rect in jsdom cancels on both sides.

One observation, not a request: this describe block has no afterEach destroying the fixture, but that is the block's existing pattern (resetTestingModule() in beforeEach with a fresh paper per test), not something this PR introduces, so the hover-event listeners added here do not leak across tests.

[Ma77Ball]

@PG1204, Overall it LGTM and appears to have no blocking issues!

[PG1204]

/request-review @xuang7

@Ma77Ball has approved the changes, requesting your review for merge.

[Yicong-Huang]

🟡 0 must-fix · 1 advisory · 0 polish: solid test-only PR with real-collaborator specs; one optional assertion tightening.

Simplifications (1)

• workflow-editor.component.spec.ts:2517: negated check would also pass on an unpainted operator (advisory, see inline)

[Yicong-Huang]

🟢 1 resolved · 0 open · 0 new (0 new = 0 newly introduced · 0 late catches)

Verification trace

The prior advisory is addressed in the tree: the measurable operator's fill in the unmeasurable-view spec is now pinned exactly with toEqual(HOT). In that spec the scan operator is unmeasurable for I/O imbalance (zero input rows), so it drops out of normalization, and the result operator is the sole measurable one with a positive raw metric, so it scores 1 and paints scoreToColor(1). The new assertion fails if the operator goes unpainted or no-data, which the old negated check could not catch. The two removed negated checks were implied by their adjacent toEqual(HOT) assertions.