fix: deterministic DOM widget clip-path for flaky screenshot test#9400
fix: deterministic DOM widget clip-path for flaky screenshot test#9400christian-byrne merged 3 commits intomainfrom
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (3)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a post-drag mouse move in a browser interaction test to avoid hover highlight at drop, and refactors DomWidget.vue to introduce a composeStyle helper and a clippingStyle watcher so composed styles are reapplied after asynchronous clip-path updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 549 passed, 0 failed · 5 flaky📊 Browser Reports
|
📦 Bundle: 4.49 MB gzip 🔴 +21 BDetailsSummary
Category Glance App Entry Points — 17.8 kB (baseline 17.8 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 913 kB (baseline 913 kB) • 🔴 +144 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.4 kB (baseline 72.4 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
User & Accounts — 16.1 kB (baseline 16.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Editors & Dialogs — 779 B (baseline 779 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
UI Components — 47 kB (baseline 47 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 2.73 MB (baseline 2.73 MB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 56.6 kB (baseline 56.6 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.86 MB (baseline 8.86 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.92 MB (baseline 7.92 MB) • ⚪ 0 BBundles that do not match a named category
|
⚡ Performance ReportNo regressions detected. All metrics
Historical variance (last 5 runs)
Raw data{
"timestamp": "2026-03-05T05:28:34.342Z",
"gitSha": "fe8bed3ce70fac103a3861a8b9ce1c9ed1ed01aa",
"branch": "ci/fix-flaky-test",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2038.3249999999862,
"styleRecalcs": 124,
"styleRecalcDurationMs": 24.282999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 441.1039999999999,
"heapDeltaBytes": -1616204
},
{
"name": "canvas-idle",
"durationMs": 2016.4629999999875,
"styleRecalcs": 121,
"styleRecalcDurationMs": 19.849999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 379.84299999999996,
"heapDeltaBytes": -3188600
},
{
"name": "canvas-idle",
"durationMs": 2049.6720000000437,
"styleRecalcs": 126,
"styleRecalcDurationMs": 25.953999999999997,
"layouts": 1,
"layoutDurationMs": 0.308,
"taskDurationMs": 406.70099999999996,
"heapDeltaBytes": -2940044
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1971.347000000037,
"styleRecalcs": 182,
"styleRecalcDurationMs": 57.844,
"layouts": 12,
"layoutDurationMs": 3.75,
"taskDurationMs": 890.463,
"heapDeltaBytes": -2902412
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1892.3340000000053,
"styleRecalcs": 172,
"styleRecalcDurationMs": 49.891000000000005,
"layouts": 12,
"layoutDurationMs": 3.221,
"taskDurationMs": 825.1429999999999,
"heapDeltaBytes": -3120496
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1894.6599999999876,
"styleRecalcs": 172,
"styleRecalcDurationMs": 48.307,
"layouts": 12,
"layoutDurationMs": 3.2739999999999996,
"taskDurationMs": 793.6590000000001,
"heapDeltaBytes": -2896556
},
{
"name": "dom-widget-clipping",
"durationMs": 550.6709999999657,
"styleRecalcs": 40,
"styleRecalcDurationMs": 14.068,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 356.974,
"heapDeltaBytes": 7715576
},
{
"name": "dom-widget-clipping",
"durationMs": 546.4709999999968,
"styleRecalcs": 40,
"styleRecalcDurationMs": 12.847000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 333.178,
"heapDeltaBytes": 6697492
},
{
"name": "dom-widget-clipping",
"durationMs": 581.5680000000043,
"styleRecalcs": 43,
"styleRecalcDurationMs": 13.136999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 355.742,
"heapDeltaBytes": 7739480
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 586.6170000000466,
"styleRecalcs": 73,
"styleRecalcDurationMs": 15.386000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 418.62999999999994,
"heapDeltaBytes": -7905356
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 576.3979999999833,
"styleRecalcs": 72,
"styleRecalcDurationMs": 14.037999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 404.35800000000006,
"heapDeltaBytes": -8774336
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 596.3380000000029,
"styleRecalcs": 73,
"styleRecalcDurationMs": 15.147999999999998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 418.509,
"heapDeltaBytes": -9309416
},
{
"name": "subgraph-idle",
"durationMs": 2023.0730000000108,
"styleRecalcs": 122,
"styleRecalcDurationMs": 20.898,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 391.10200000000003,
"heapDeltaBytes": -3275712
},
{
"name": "subgraph-idle",
"durationMs": 2005.5389999999989,
"styleRecalcs": 121,
"styleRecalcDurationMs": 21.171000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 391.73900000000003,
"heapDeltaBytes": -2136200
},
{
"name": "subgraph-idle",
"durationMs": 2003.341999999975,
"styleRecalcs": 121,
"styleRecalcDurationMs": 20.610000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 382.47900000000004,
"heapDeltaBytes": -2751796
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1714.9969999999826,
"styleRecalcs": 156,
"styleRecalcDurationMs": 46.397,
"layouts": 16,
"layoutDurationMs": 4.407,
"taskDurationMs": 747.22,
"heapDeltaBytes": -4017768
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 2018.478000000016,
"styleRecalcs": 174,
"styleRecalcDurationMs": 55.931999999999995,
"layouts": 16,
"layoutDurationMs": 4.675000000000001,
"taskDurationMs": 985.344,
"heapDeltaBytes": -4601768
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 2012.8520000000094,
"styleRecalcs": 173,
"styleRecalcDurationMs": 56.458000000000006,
"layouts": 16,
"layoutDurationMs": 4.634,
"taskDurationMs": 1017.9909999999999,
"heapDeltaBytes": -2846068
}
]
} |
|
Updating Playwright Expectations |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/graph/widgets/DomWidget.vue`:
- Around line 121-131: The watch that currently observes [() => widgetState,
left, top] must also include enableDomClipping (use enableDomClipping.value) so
toggling Comfy.DOMClippingEnabled retriggers the callback; update the same for
the second watch that observes clippingStyle so it also lists enableDomClipping
as a dependency. In practice, add enableDomClipping to both watch dependency
arrays so when the toggle changes the callback runs and calls composeStyle()
(and updateDomClipping() when enabled), ensuring clippingStyle and DOM clipping
are recomputed immediately.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b5f83e4a-fb2a-461f-9107-958094b2e6b0
⛔ Files ignored due to path filters (1)
browser_tests/tests/interaction.spec.ts-snapshots/dragged-node1-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (1)
src/components/graph/widgets/DomWidget.vue
622c903 to
17fe167
Compare
|
/update-playwright |
…eview Two flaky screenshot tests were identified during the PR #9400 snapshot update, where baselines regenerated without any code changes affecting them: 1. added-node-no-connection (nodeSearchBox.spec.ts): Root cause: fillAndSelectFirstNode('KSampler') selected nth(0) from the dropdown, but search result ordering is non-deterministic when the search box opens via link release (with filter chips). Sometimes 'Preview Image' appeared first instead of 'KSampler'. Fix: Add exact:true option that uses aria-label selector to click the specific matching result instead of blindly selecting the first item. 2. no-workflow-webp (loadWorkflowInMedia.spec.ts): Root cause: Dropping no_workflow.webp (no embedded workflow) creates a LoadImage node and uploads the file. The image preview then loads asynchronously via /view API. The screenshot was taken before the upload and preview load completed, causing the preview to sometimes show the image and sometimes show a black/empty area. Fix: Wait for the upload response and subsequent /view response before taking the screenshot. Both baselines are deleted for CI regeneration. Fixes #4658 Amp-Thread-ID: https://ampcode.com/threads/T-019cbc87-ce1f-7338-93c7-d0677a632805
…#9426) ## Summary Fixes flaky screenshot test that was identified during the PR #9400 snapshot update, where baselines regenerated without any code changes affecting them. **Root cause:** `fillAndSelectFirstNode('KSampler')` selected `nth(0)` from the autocomplete dropdown, but search result ordering is non-deterministic when the search box opens via link release with filter chips. Sometimes 'Preview Image' appeared as the first result instead of 'KSampler', causing a completely different node to be added. **Fix:** Added an `exact: true` option to `fillAndSelectFirstNode` that uses an `aria-label` selector to click the specific matching result instead of blindly selecting the first item. - Fixes #4658 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9426-fix-stabilize-flaky-screenshot-tests-for-search-results-and-image-preview-31a6d73d365081598167ce285416995c) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com>
) ## Summary Fix deterministic DOM widget clip-path rendering to resolve the flaky "Can drag node" screenshot test. ## Root Cause `useDomClipping.updateClipPath()` schedules clip-path calculation in a `requestAnimationFrame`, but `DomWidget.vue`'s watcher reads `clippingStyle.value` synchronously before the RAF fires. The stale clip-path gets baked into `style.value` and never updated when the RAF completes, causing the textarea DOM widget to non-deterministically render in front of or behind the canvas-drawn node selection border. ## Fix - Extract `composeStyle()` function and add a dedicated watcher on `clippingStyle` that recomposes the final inline style whenever the RAF-deferred clip-path updates - Add `enableDomClipping` to the main watcher dependency array so toggling the clipping setting immediately recomposes the style - Add `moveMouseToEmptyArea()` call in the test as a secondary stabilizer against hover highlight non-determinism - Delete stale snapshot so CI regenerates it with correct clip-path behavior - Fixes #4658 --------- Co-authored-by: github-actions <github-actions@github.com>
Summary
Fix deterministic DOM widget clip-path rendering to resolve the flaky "Can drag node" screenshot test.
Root Cause
useDomClipping.updateClipPath()schedules clip-path calculation in arequestAnimationFrame, butDomWidget.vue's watcher readsclippingStyle.valuesynchronously before the RAF fires. The stale clip-path gets baked intostyle.valueand never updated when the RAF completes, causing the textarea DOM widget to non-deterministically render in front of or behind the canvas-drawn node selection border.Fix
Extract
composeStyle()function and add a dedicated watcher onclippingStylethat recomposes the final inline style whenever the RAF-deferred clip-path updatesAdd
enableDomClippingto the main watcher dependency array so toggling the clipping setting immediately recomposes the styleAdd
moveMouseToEmptyArea()call in the test as a secondary stabilizer against hover highlight non-determinismDelete stale snapshot so CI regenerates it with correct clip-path behavior
Fixes [test] Fix flaky playwright tests in CI #4658