mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-11 00:10:40 +00:00
b593496e3ef1e034482b9e43784c306026e14e67
7584 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
dante01yoon
|
b593496e3e |
Merge remote-tracking branch 'origin/main' into feat/tooltip-migrate-node-tooltips
# Conflicts: # src/App.vue # src/components/TopMenuSection.vue # src/components/queue/JobHistoryActionsMenu.test.ts # src/components/queue/QueueOverlayActive.test.ts # src/components/queue/QueueOverlayHeader.test.ts # src/renderer/extensions/vueNodes/components/NodeHeader.test.ts # src/renderer/extensions/vueNodes/components/NodeWidgets.vue |
||
|
Dante
|
2d50cc2d76 |
feat: show success toast after ComfyHub publish (#11316)
## Summary
Adds a success toast in the ComfyHub publish flow so users get explicit
confirmation that the workflow was published before the dialog closes.
## Changes
- **What**: `ComfyHubPublishDialog.handlePublish()` calls `toast.add({
severity: 'success', ... })` after `submitToComfyHub()` resolves and
before `onClose()` runs. Adds two i18n keys (`publishSuccessTitle`,
`publishSuccessDescription`) and an assertion in the existing
success-path test.
## Review Focus
- This is the lightweight stop-gap discussed in [Slack
thread](https://comfy-organization.slack.com/archives/C0AEPRS8N74/p1776370871654139?thread_ts=1776362591.237159&cid=C0AEPRS8N74)
while the larger published-state design is still pending phase-2 work.
Symmetric with the existing `publishFailedTitle/Description` error
toast.
- `submitToComfyHub` is synchronous (asset uploads happen inside it), so
a successful resolve means the workflow is live.
- `<Toast>` is mounted in `GlobalToast.vue`, so it persists after
`onClose()` destroys the dialog.
## Screenshots (if applicable)
<img width="1135" height="634" alt="Screenshot 2026-04-17 at 8 11 34 AM"
src="https://github.com/user-attachments/assets/a71400a7-2055-4c2a-a761-9298cfa24e9a"
/>
n/a — toast text only.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11316-feat-show-success-toast-after-ComfyHub-publish-3446d73d365081a7bbb3ca29ca3bb618)
by [Unito](https://www.unito.io)
|
||
|
Kelly Yang
|
89c11c9aa9 |
test: add unit test suite for apps/desktop-ui (#11275)
## Summary
This is a follow-up PR of #11102
| Requirement | Status | Implementation |
| :--- | :--- | :--- |
| Add vitest configuration for desktop-ui workspace | ✅ Done | Added
`apps/desktop-ui/vitest.config.mts` with `happy-dom` environment, `@`
alias, and `setupFiles` pointing to `src/test/setup.ts` (registers
`@testing-library/jest-dom` matchers) |
| Add test:unit script to package.json | ✅ Done | Added `"test:unit":
"vitest run --config vitest.config.mts"` to
`apps/desktop-ui/package.json` |
| stores/maintenanceTaskStore.ts | ✅ Done | 34 tests covering task state
machine, IPC integration, executeTask flow, and error handling via
`@pinia/testing` |
| utils/electronMirrorCheck.ts | ✅ Done | 5 tests covering URL
validation, canAccessUrl delegation, and true/false return logic |
| utils/refUtil.ts (useMinLoadingDurationRef) | ✅ Done | 7 tests
covering initial state, timing behavior using `vi.useFakeTimers`, and
computed ref input |
| utils/envUtil.ts | ✅ Done | 7 tests covering electronAPI detection and
fallback behavior |
| constants/desktopDialogs.ts | ✅ Done | 8 tests covering dialog
structure and field contracts |
| constants/desktopMaintenanceTasks.ts | ✅ Done | 5 tests covering
`pythonPackages.execute` success/failure return values, and URL-opening
tasks calling `window.open` |
| composables/bottomPanelTabs/useTerminal.ts | ✅ Done | 7 tests covering
key event handler: Ctrl/Meta+C with/without selection, Ctrl/Meta+V,
non-keydown events, and unrelated keys — mocked xterm with Vitest
v4-compatible function constructors |
| composables/bottomPanelTabs/useTerminalBuffer.ts | ✅ Done | 2 tests
for `copyTo`: verifies serialized buffer content is written to
destination terminal |
| utils/validationUtil.ts | ⛔ Skipped | The current file contains only a
`ValidationState` enum with no logic. There is no behavior to test
without writing a change-detector test (asserting enum values), which
violates project testing guidelines |
**Additional config changes (not in issue but required to make tests
work):**
| Change | Reason |
| :--- | :--- |
| Added `"vitest.config.mts"` to `apps/desktop-ui/tsconfig.json` include
| Required for ESLint's TypeScript parser to process the config file
without a parsing error |
| Removed 6 redundant test devDependencies from
`apps/desktop-ui/package.json` | `vitest`, `@testing-library/*`,
`@pinia/testing`, `happy-dom` are already declared at the root and
hoisted by pnpm — re-declaring them in the sub-package is unnecessary |
## Changes
- Add vitest.config.mts with happy-dom environment and path aliases
- Add src/test/setup.ts to register @testing-library/jest-dom matchers
- Add test:unit script to package.json
- Add vitest.config.mts to tsconfig.json include for ESLint
compatibility
- Remove redundant test devDependencies already declared at root
- Add 132 tests across 16 files covering stores, composables, utils, and
constants
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Test- and config-only changes; main risk is CI/build instability from
new Vitest configuration or brittle mocks, with no runtime behavior
changes shipped to users.
>
> **Overview**
> Adds a dedicated Vitest setup for `apps/desktop-ui` (new
`vitest.config.mts` using `happy-dom`, aliases, and a `jest-dom` setup
file) and wires it into the workspace via a new `test:unit` script plus
`tsconfig.json` inclusion.
>
> Introduces a broad set of new unit tests for desktop UI components,
composables, constants, utilities, and the `maintenanceTaskStore`
(mocking Electron/PrimeVue/Xterm as needed) to validate state
transitions, validation flows, and key UI behaviors without changing
production logic.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Kelly Yang
|
29d6263fb9 |
test: add Preview3D execution flow E2E tests (#11014)
## Summary
Adds Playwright coverage for `Preview3D execution` and persistence :
real queue execution against a `Load3D → Preview3D` workflow, plus `save
/ full reload / reopen` from the sidebar.
## What these tests do
**Fixture** (every test)
Turns on Vue Nodes, uses the sidebar for workflows, loads a Load3D →
Preview3D workflow, waits for nodes, then clears saved workflows after
the test so runs stay isolated.
**Test 1 — execution updates Preview3D**
Uploads `cube.obj`(the existing test file in the merged version) to
Load3D, runs `Queue Prompt`, then checks that Preview3D’s model_file and
Last Time Model File match and the canvas has non-zero size. No 3D
screenshots (GPU flakiness).
**Test 2 — persistence after reload**
Same upload + queue, then saves the workflow, reloads the page,
re-applies the same UI settings, opens the saved workflow, and checks
the same model path and camera state (with a small numeric tolerance).
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Adds new slow, WebGL-dependent E2E tests and fixtures, which can
increase CI runtime and introduce flakiness due to timing/graphics
variability, but does not change production logic.
>
> **Overview**
> Adds a new `Load3D → Preview3D` workflow asset and a dedicated
Playwright fixture (`Preview3DPipelineFixture`) to drive real queue
execution, upload a 3D model, and interact with the 3D canvases (orbit
drags) while asserting `model_file`/`Last Time Model File` and camera
state via node properties.
>
> Introduces camera-state comparison helpers with explicit numeric
tolerances, and adds a new `preview3dExecution.spec.ts` suite that
validates (1) Preview3D updates from execution output and (2) model +
camera persistence across save, full page reload, and reopening the
workflow from the sidebar.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
jaeone94
|
a1e6fb36d2 |
refactor: harden ChangeTracker lifecycle with self-defending API (#10816)
## Summary Harden the `ChangeTracker` lifecycle to eliminate the class of bugs where an inactive workflow's tracker silently captures the wrong graph state. Renames `checkState()` to `captureCanvasState()` with a self-defending assertion, introduces `deactivate()` and `prepareForSave()` lifecycle methods, and closes a latent undo-history corruption bug discovered during code review. ## Background ComfyUI supports multiple workflows open as tabs, but only one canvas (`app.rootGraph`) exists at a time. When the user switches tabs, the old workflow's graph is unloaded and the new one is loaded into this shared canvas. The old `checkState()` method serialized `app.rootGraph` into `activeState` to track changes for undo/redo. It had no awareness of *which* workflow it belonged to -- if called on an inactive tab's tracker, it would capture the active tab's graph data and silently overwrite the inactive workflow's state. This caused permanent data loss (fixed in PR #10745 with caller-side `isActive` guards). The caller-side guards were fragile: every new call site had to remember to add the guard, and forgetting would reintroduce the same silent data corruption. Additionally, `beforeLoadNewGraph` only called `store()` (viewport/outputs) without `checkState()`, meaning canvas state could be stale if a tab switch happened without a preceding mouseup event. ### Before (fragile) ``` saveWorkflow(workflow): if (isActive(workflow)) <-- caller must remember this guard workflow.changeTracker.checkState() <-- name implies "read", actually writes ... beforeLoadNewGraph(): activeWorkflow.changeTracker.store() <-- only saves viewport, NOT graph state ``` ### After (self-defending) ``` saveWorkflow(workflow): workflow.changeTracker.prepareForSave() <-- handles active/inactive internally ... beforeLoadNewGraph(): activeWorkflow.changeTracker.deactivate() <-- captures graph + viewport together ``` ## Changes - Rename `checkState` to `captureCanvasState` with active-tracker assertion - Add `deactivate()` and `prepareForSave()` lifecycle methods - Fix undo-history corruption: `captureCanvasState()` guarded by `_restoringState` - Fix viewport regression during undo: `deactivate()` skips `captureCanvasState()` during undo/redo but always calls `store()` to preserve viewport (regression from PR #10247) - Log inactive tracker warnings unconditionally at warn level (not DEV-only) - Deprecated `checkState()` wrapper for extension compatibility - Rename `checkState` to `captureCanvasState` in `useWidgetSelectActions` composable - Add `appModeStore.ts` to manual call sites documentation - Add `checkState()` deprecation note to architecture docs - Add 16 unit tests covering all guard conditions, lifecycle methods, and undo behavior - Add E2E test: "Undo preserves viewport offset" ## New ChangeTracker Public API | Method | Caller | Purpose | |--------|--------|---------| | `captureCanvasState()` | Event handlers, UI interactions | Snapshots canvas into activeState, pushes undo. Asserts active tracker. | | `deactivate()` | `beforeLoadNewGraph` only | `captureCanvasState()` (skipped during undo/redo) + `store()`. Freezes state for tab switch. | | `prepareForSave()` | Save paths only | Active: `captureCanvasState()`. Inactive: no-op. | | `checkState()` | **Deprecated** -- extensions only | Wrapper that delegates to `captureCanvasState()` with deprecation warning. | | `store()` | Internal to `deactivate()` | Saves viewport, outputs, subgraph navigation. | | `restore()` | `afterLoadNewGraph` | Restores viewport, outputs, subgraph navigation. | | `reset()` | `afterLoadNewGraph`, save | Resets initial state (marks as "clean"). | ## Test plan - [x] Unit tests: 16 tests covering all guard conditions, state capture, undo queue behavior - [x] E2E test: "Undo preserves viewport offset" verifies no viewport drift on undo - [x] E2E test: "Prevents captureCanvasState from corrupting workflow state during tab switch" - [x] Existing E2E: "Closing an inactive tab with save preserves its own content" - [ ] Manual: rapidly switch tabs during undo/redo, verify no viewport drift - [ ] Manual: verify extensions calling `checkState()` see deprecation warning in console |
||
|
jaeone94
|
394e36984f |
fix: re-sync collapsed node slot positions after subgraph fitView (#11240)
## Summary Fix collapsed node connection links rendering at wrong positions when entering a subgraph for the first time. `fitView()` (added in #10995) changes canvas scale/offset, invalidating cached slot positions for collapsed nodes. ## Changes - **What**: Schedule `requestSlotLayoutSyncForAllNodes()` on the next frame after `fitView()` in `restoreViewport()` so collapsed node slot positions are re-measured against the updated transform. Inner RAF guarded against mid-frame graph changes. - **Test coverage**: - Unit tests in `subgraphNavigationStore.viewport.test.ts` verify the RAF chain calls `requestSlotLayoutSyncForAllNodes` after `fitView`, and skip the re-sync when the active graph changes between frames. - E2E screenshot test (`@screenshot` tag) validates correct link rendering on first subgraph entry using a new fixture with a pre-collapsed inner node. ## Review Focus The nested `requestAnimationFrame` is intentional: the outer RAF runs `fitView()`, which updates `ds.scale`/`ds.offset` and triggers a CSS transform update on `TransformPane`. The inner RAF ensures the DOM has reflowed with the new transform before `requestSlotLayoutSyncForAllNodes()` measures `getBoundingClientRect()` on slot elements. --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
Dante
|
19fff29204 |
test: backfill e2e coverage gaps for toolkit widgets, minimap, mask editor, painter (#11183)
## Summary Backfills missing e2e test coverage identified in the [FixIt Burndown](https://www.notion.so/comfy-org/FixIt-Burndown-32e6d73d365080609a81cdc9bc884460) audit. Adds 39 new behavioral tests across 5 spec files with zero test-code overlap. ## Changes - **What**: New e2e specs for Image Crop (6 tests) and Curve Widget (6 tests). Deepened coverage for Minimap (+6), Mask Editor (+10), Painter (+11). - **New fixtures**: `curve_widget.json`, updated `image_crop_widget.json` ## Test Inventory | Spec | New tests | Coverage area | |---|---|---| | `imageCrop.spec.ts` | 6 | Empty state, bounding box inputs, ratio selector/presets, lock toggle, programmatic value update | | `curveWidget.spec.ts` | 6 | SVG render, click-to-add point, drag-to-reshape, Ctrl+click remove, interpolation mode switch, min-2 guard | | `minimap.spec.ts` | +6 | Click-to-pan, drag-to-pan, zoom viewport shrink, node count changes, workflow reload, pan state reflection | | `maskEditor.spec.ts` | +10 | Brush drawing, undo/redo, clear, cancel, invert, Ctrl+Z, tool panel/switching, brush settings, save with mock, eraser | | `painter.spec.ts` | +11 | Clear, eraser, control visibility toggle, brush size slider, stroke width comparison, canvas dimensions, background color, multi-stroke accumulate, color picker, opacity, partial erase | ## Review Focus - Mask editor tests use `.maskEditor_toolPanelContainer` class selectors — may need test-id hardening later - Painter slider interaction tests could be flaky if slider layout changes - All canvas pixel-count assertions use `expect.poll()` with timeouts for reliability ## Test plan - [ ] CI passes all new/modified specs - [ ] No duplicate coverage with existing tests (verified via grep before writing) - [ ] No `waitForTimeout` usage (confirmed) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11183-test-backfill-e2e-coverage-gaps-for-toolkit-widgets-minimap-mask-editor-painter-3416d73d3650819ca33edd1f27b9651a) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Johnpaul Chiwetelu
|
b3b895a2a9 |
refactor(test): use canvasOps.clickEmptySpace in copyPaste spec (#10991)
## Summary
Replace two hardcoded blank-canvas click positions in
`copyPaste.spec.ts` with the existing
`comfyPage.canvasOps.clickEmptySpace()` helper.
## Changes
- **What**: Both `{ x: 50, y: 500 }` click literals in the `Copy paste
node, image paste onto LoadImage, image paste on empty canvas` test now
use `canvasOps.clickEmptySpace()` (which wraps
`DefaultGraphPositions.emptySpaceClick = { x: 35, y: 31 }`). Redundant
`await nextFrame()` calls dropped — the helper already awaits a frame
internally.
## Review Focus
Draft PR — need CI to confirm `(35, 31)` is a valid blank-canvas click
for the `load_image_with_ksampler` workflow used by this test. The
workflow places `LoadImage` at `[50, 50]` and `KSampler` at `[500, 50]`,
so `(35, 31)` should be clear of both. Locally the test was already
failing on `main` (pre-existing, unrelated), so CI is the source of
truth here. If CI fails, the fallback is to add a dedicated named
constant `emptyCanvasClick: { x: 50, y: 500 }` to
`DefaultGraphPositions` as originally proposed in the issue.
Fixes #10330
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10991-refactor-test-use-canvasOps-clickEmptySpace-in-copyPaste-spec-33d6d73d3650817aa3ccea44cb48c0ae)
by [Unito](https://www.unito.io)
|
||
|
Dante
|
e5c81488e4 |
fix: include focusMode in splitter refresh key to prevent panel resize (#11295)
## Summary When the properties panel is open, toggling focus mode on then off causes the panel to resize unexpectedly. The root cause is that `splitterRefreshKey` in `LiteGraphCanvasSplitterOverlay.vue` does not include `focusMode`, so the PrimeVue Splitter component instance is reused across focus mode transitions and restores stale panel sizes from localStorage. Fix: add `focusMode` to `splitterRefreshKey` so the Splitter is recreated when focus mode toggles. ## Red-Green Verification | Commit | CI Status | Purpose | |--------|-----------|---------| | `test: add failing test for focus mode toggle resizing properties panel` | 🔴 Red | Proves the test catches the bug | | `fix: include focusMode in splitter refresh key to prevent panel resize` | 🟢 Green | Proves the fix resolves the bug | ## demo ### AS IS https://github.com/user-attachments/assets/95f6a9e3-e4c7-4aba-8e17-0eee11f70491 ### TO BE https://github.com/user-attachments/assets/595eafcd-6a80-443d-a6f3-bb7605ed0758 ## Test Plan - [ ] CI red on test-only commit - [ ] CI green on fix commit - [ ] E2E regression test added in `browser_tests/tests/focusMode.spec.ts` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11295-fix-include-focusMode-in-splitter-refresh-key-to-prevent-panel-resize-3446d73d365081b7bc3ac65338e17a8f) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
5c07198acb |
fix: add validation to E2E coverage shard merge (#11290)
## Summary Add a validation step after merging E2E coverage shards to detect data loss and improve observability. ## Changes - **What**: After `lcov -a` merges shard LCOVs, a new step parses merged + per-shard stats (source files, lines hit) and writes them to the **GitHub Actions job summary** as a markdown table. If merged `LH` (lines hit) is less than any single shard's `LH`, an error annotation is emitted — this invariant should never be violated since merging should only add coverage. - Helps diagnose the 68% → 42% E2E coverage drop after sharding was introduced. ## Review Focus The step is informational — it emits `::error::` annotations but does not `exit 1`, so it won't block the workflow. We can make it a hard failure once we're confident the merge is stable. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11290-fix-add-validation-to-E2E-coverage-shard-merge-3446d73d365081c8a942e92deba92006) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
6fb90b224d |
fix(load3d): restore missed hover state when viewer init is async (#11265)
## Summary followup https://github.com/Comfy-Org/ComfyUI_frontend/pull/9520 mouseenter fires before load3d is created during async init (getLoad3dAsync), so the STATUS_MOUSE_ON_VIEWER flag is never set. This causes isActive() to return false after INITIAL_RENDER_DONE, stopping the animation loop from calling controlsManager.update() and making OrbitControls unresponsive on first open. Track hover state in the composable and sync it to load3d after creation. |
||
|
pythongosssss
|
a8e1fa8bef |
test: add regression test for WEBP RIFF padding (#8527) (#11267)
## Summary Add a regression test for #8527 (handle RIFF padding for odd-sized WEBP chunks). The fix added + (chunk_length % 2) to the chunk-stride calculation in getWebpMetadata so EXIF chunks following an odd-sized chunk are still located correctly. There was no existing unit test covering getWebpMetadata, so without a regression test the fix could silently break in a future refactor. ## Changes - **What**: - New unit test file src/scripts/pnginfo.test.ts covering getWebpMetadata's RIFF chunk traversal. - Helpers build a minimal in-memory WEBP with one VP8 chunk of configurable length followed by an EXIF chunk encoding workflow:<json>. - Odd-length case (regression for #8527): without the % 2 padding adjustment, the parser walks one byte short and returns {}. - Even-length case: guards against an over-correction that always adds 1. - Verified RED→GREEN locally. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11267-test-add-regression-test-for-WEBP-RIFF-padding-8527-3436d73d36508117a66edf3cb108ded0) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
83ceef8cb3 |
test: add regression test for non-string serverLogs (#8460) (#11268)
## Summary Add a regression test for #8460 (handle non-string `serverLogs` in error report). The fix added `typeof error.serverLogs === 'string' ? ... : JSON.stringify(...)` in `errorReportUtil.ts` so object-shaped logs no longer render as `[object Object]`. There was no existing unit test for `generateErrorReport`, so this regression could silently return. ## Changes - **What**: New unit test file `src/utils/errorReportUtil.test.ts` covering `generateErrorReport`'s `serverLogs` rendering. - String case: verifies plain-string logs still appear verbatim and `[object Object]` is absent. - Object case (regression for #8460): verifies object logs are JSON-stringified instead of coerced to `[object Object]`. - Verified RED→GREEN locally. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11268-test-add-regression-test-for-non-string-serverLogs-8460-3436d73d36508195a32fc559ab7ce5bb) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
4885ef856c |
[chore] Update Comfy Registry API types from comfy-api@113318d (#11261)
## Automated API Type Update This PR updates the Comfy Registry API types from the latest comfy-api OpenAPI specification. - API commit: 113318d - Generated on: 2026-04-15T04:26:33Z These types are automatically generated using openapi-typescript. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11261-chore-Update-Comfy-Registry-API-types-from-comfy-api-113318d-3436d73d3650816784d4efd98d6a665a) by [Unito](https://www.unito.io) Co-authored-by: bigcat88 <13381981+bigcat88@users.noreply.github.com> |
||
|
Christian Byrne
|
873a75d607 |
test: add unit tests for usePainter composable (#11137)
## Summary Add 25 behavioral unit tests for `usePainter` composable, bringing coverage from 0% to ~35% lines / ~57% functions. ## Changes - **What**: New test file `src/composables/painter/usePainter.test.ts` covering widget sync, settings persistence, canvas sizing, brush display scaling, serialization, restore, pointer event guards, and cursor visibility. ## Review Focus - Mock patterns: singleton factory mocks for stores, wrapper component for lifecycle hooks - Test coverage prioritization: focused on mount-time sync, reactive watchers, and computed behavior rather than canvas pixel output ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11137-test-add-unit-tests-for-usePainter-composable-33e6d73d36508147bde7e9c349c743ca) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
ecb6fbe8fb |
test: Add waitForWorkflowIdle & remove redundant nextFrame (#11264)
## Summary More cleanup and reliability ## Changes - **What**: - Add wait for idle - Remove redundant nextFrames ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11264-test-Add-waitForWorkflowIdle-remove-redundant-nextFrame-3436d73d3650812c837ac7503ce0947b) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Alexander Brown
|
52ccd9ed1a |
refactor: internalize nextFrame() into fixture/helper methods (#11166)
## Summary
Internalize `nextFrame()` calls into fixture/helper methods so spec
authors don't need to remember to call it after common operations.
`nextFrame()` waits for one `requestAnimationFrame` (~16ms) — an extra
call is always safe, making this a low-risk refactor.
## Changes
### Phase 1: `SettingsHelper.setSetting()`
`setSetting()` now calls `nextFrame()` internally. Removed 15 redundant
calls across 7 files.
### Phase 2: `CommandHelper.executeCommand()`
`executeCommand()` now calls `nextFrame()` internally. Removed 15
redundant calls across 7 files, including the now-redundant call in
`AppModeHelper.toggleAppMode()`.
### Phase 3: `WorkflowHelper.loadGraphData()`
New helper wraps `page.evaluate(loadGraphData)` + `nextFrame()`.
Migrated `SubgraphHelper.serializeAndReload()` and `groupNode.spec.ts`.
### Phase 4: `NodeReference` cleanup
Removed redundant `nextFrame()` from `copy()`, `convertToGroupNode()`,
`resizeNode()`, `dragTextEncodeNode2()`, and
`convertDefaultKSamplerToSubgraph()`. Removed 6 spec-level calls after
`node.click('title')`.
### Phase 5: `KeyboardHelper.press()` and `delete()`
New convenience methods that press a key and wait one frame. Converted
40 `canvas.press(key)` + `nextFrame()` pairs across 13 spec files.
### Phase 6: `ComfyPage.expectScreenshot()`
New helper combines `nextFrame()` + `toHaveScreenshot()`. Converted 45
pairs across 12 spec files.
## Total impact
- **~130 redundant `nextFrame()` calls eliminated** across ~35
spec/helper files
- **3 new helper methods** added (`loadGraphData`, `press`/`delete`,
`expectScreenshot`)
- **2 existing methods** enhanced (`setSetting`, `executeCommand`)
## What was NOT changed
- `performance.spec.ts` frame-counting loops (intentional)
- `ComfyMouse.ts` / `CanvasHelper.ts` (already internalized)
- `SubgraphHelper.packAllInteriorNodes()` (deliberate orchestration)
- Builder helpers (already internalized)
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11166-refactor-internalize-nextFrame-into-fixture-helper-methods-33f6d73d3650817bb5f6fb46e396085e)
by [Unito](https://www.unito.io)
---------
Co-authored-by: Amp <amp@ampcode.com>
|
||
|
Kelly Yang
|
92ad6fc798 |
test: address review nits for image compare E2E (#11260)
## Summary
A follow-up PR of #11196.
| # | Nit | Action | Reason |
| :--- | :--- | :--- | :--- |
| 1 | Replace `page.on('pageerror')` with request-wait | **Left as-is**
| The `pageErrors` array is an accumulator checked at the end via
`expect(pageErrors).toHaveLength(0)` – the goal is to assert that broken
image URLs don't surface as uncaught JS exceptions during the test run.
A request-wait can't substitute for that behavioral assertion, so the
listener pattern is intentional here. |
| 2 | Move helpers to a `vueNodes.getImageCompareHelper()` subclass |
**Left as-is** | Helpers such as `setImageCompareValue` and
`moveToPercentage` are only used in this file, making local
encapsulation enough. Extracting them to a page object would increase
the file/interface surface area and violate YAGNI; additionally,
`AGENTS.md` clearly states to "minimize the exported values of each
module. |
| 3 | Use `TestIds` enum for test ID strings | **Fixed** – added
`imageCompare` section to `TestIds` in `selectors.ts`; replaced all 8
inline string IDs in `imageCompare.spec.ts` with
`TestIds.imageCompare.*` references | The project already has a
`TestIds` convention for centralizing test IDs. Inline strings create
drift risk between the Vue component and the test file. |
| 4 | Move `expect.poll` bounding box check to helper/page object |
**Left as-is** | This logic already lives inside `moveToPercentage`,
which is a local helper. Moving it further to a page object is the same
refactor as #2 above. |
| 5 | Remove `// ---` style section header comments | **Fixed** –
removed all 8 divider blocks from `imageCompare.spec.ts` | Consistent
with project guidelines and your explicit preference. Test names already
describe what each block does. |
| 6 | Name magic numbers `400` and `350` | **Fixed** – introduced
`minWidth = 400` and `minHeight = 350` constants in the test |
Descriptive names make the constraint self-documenting and easier to
update if the workflow asset changes. |
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: changes are limited to Playwright E2E test code and shared
selector constants, with no production logic impacted.
>
> **Overview**
> **E2E Image Compare tests now use centralized selectors.** Adds an
`imageCompare` section to `TestIds` and updates `imageCompare.spec.ts`
to reference `TestIds.imageCompare.*` instead of inline `data-testid`
strings.
>
> Cleans up the spec by removing divider comments and naming the minimum
size magic numbers (`minWidth`, `minHeight`) used in the node sizing
assertion.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
pythongosssss
|
06686a1f50 |
test: App mode - additional app mode coverage (#11194)
## Summary Adds additional test coverage for empty state/welcome screen/connect outputs/vue nodes auto switch ## Changes - **What**: - add tests ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11194-test-App-mode-additional-app-mode-coverage-3416d73d365081ca91d0ed61de19f840) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
693b8383d6 |
fix: missing-asset correctness follow-ups from #10856 (#11233)
Follow-up to #10856. Four correctness issues and their regression tests. ## Bugs fixed ### 1. ErrorOverlay model count reflected node selection `useErrorGroups` exposed `filteredMissingModelGroups` under the public name `missingModelGroups`. `ErrorOverlay.vue` read that alias to compute its model count label, so selecting a node shrank the overlay total. The overlay must always show the whole workflow's errors. Exposed both shapes explicitly: `missingModelGroups` / `missingMediaGroups` (unfiltered totals) and `filteredMissingModelGroups` / `filteredMissingMediaGroups` (selection-scoped). `TabErrors.vue` destructures the filtered variant with an alias. Before https://github.com/user-attachments/assets/eb848c5f-d092-4a4f-b86f-d22bb4408003 After https://github.com/user-attachments/assets/75e67819-c9f2-45ec-9241-74023eca6120 ### 2. Bypass → un-bypass dropped url/hash metadata Realtime `scanNodeModelCandidates` only reads widget values, so un-bypass produced a fresh candidate without the url that `enrichWithEmbeddedMetadata` had previously attached from `graphData.models`. `MissingModelRow`'s download/copy-url buttons disappeared after a bypass/un-bypass cycle. Added `enrichCandidateFromNodeProperties` that copies `url`/`hash`/`directory` from the node's own `properties.models` — which persists across mode toggles — into each scanned candidate. Applied to every call site of the per-node scan. A later fix in the same branch also enforces directory agreement to prevent a same-name / different-directory collision from stamping the wrong metadata. Before https://github.com/user-attachments/assets/39039d83-4d55-41a9-9d01-dec40843741b After https://github.com/user-attachments/assets/047a603b-fb52-4320-886d-dfeed457d833 ### 3. Initial full scan surfaced interior errors of a muted/bypassed subgraph container `scanAllModelCandidates`, `scanAllMediaCandidates`, and the JSON-based missing-node scan only check each node's own mode. Interior nodes whose parent container was bypassed passed the filter. Added `isAncestorPathActive(rootGraph, executionId)` to `graphTraversalUtil` and post-filter the three pipelines in `app.ts` after the live rootGraph is configured. The filter uses the execution-ID path (`"65:63"` → check node 65's mode) so it handles both live-scan-produced and JSON-enrichment-produced candidates. Before https://github.com/user-attachments/assets/3032d46b-81cd-420e-ab8e-f58392267602 After https://github.com/user-attachments/assets/02a01931-951d-4a48-986c-06424044fbf8 ### 4. Bypassed subgraph entry re-surfaced interior errors `useGraphNodeManager` replays `graph.onNodeAdded` for each existing interior node when the Vue node manager initializes on subgraph entry. That chain reached `scanSingleNodeErrors` via `installErrorClearingHooks`' `onNodeAdded` override. Each interior node's own mode was active, so the caller guards passed and the scan re-introduced the error that the initial pipeline had correctly suppressed. Added an ancestor-activity gate at the top of `scanSingleNodeErrors`, the single entry point shared by paste, un-bypass, subgraph entry, and subgraph container activation. A later commit also hardens this guard against detached nodes (null execution ID → skip) and applies the same ancestor check to `isCandidateStillActive` in the realtime verification callback. Before https://github.com/user-attachments/assets/fe44862d-f1d6-41ed-982d-614a7e83d441 After https://github.com/user-attachments/assets/497a76ce-3caa-479f-9024-4cd0f7bd20a4 ## Tests - 6 unit tests for `isAncestorPathActive` (root, active, immediate-bypass, deep-nested mute, unresolvable ancestor, null rootGraph) - 4 unit tests for `enrichCandidateFromNodeProperties` (enrichment, no-overwrite, name mismatch, directory mismatch) - 1 unit test for `scanSingleNodeErrors` ancestor guard (subgraph entry replaying onNodeAdded) - 2 unit tests for `useErrorGroups` dual export + ErrorOverlay contract - 4 E2E tests: - ErrorOverlay model count stays constant when a node is selected (new fixture `missing_models_distinct.json`) - Bypass/un-bypass cycle preserves Copy URL button (uses `missing_models_from_node_properties`) - Loading a workflow with bypassed subgraph suppresses interior missing model error (new fixture `missing_models_in_bypassed_subgraph.json`) - Entering a bypassed subgraph does not resurface interior missing model error (shares the above fixture) `pnpm typecheck`, `pnpm lint`, 206 related unit tests passing. ## Follow-up Several items raised by code review are deferred as pre-existing tech debt or scope-avoided refactors. Tracked via comments on #11215 and #11216. --- Follows up on #10856. |
||
|
Christian Byrne
|
033b3dad3a |
feat: add Slack notification workflow for coverage improvements (#10977)
## Summary
Adds a GitHub Actions workflow + TypeScript script that posts to Slack
when a merged PR improves unit or E2E test coverage.
## Changes
- **What**: New `coverage-slack-notify.yaml` workflow triggered on push
to main. Compares current coverage against previous baselines, generates
Slack Block Kit payload with progress bars and milestone celebrations,
posts to `#p-frontend-automated-testing`.
- **Script**: `scripts/coverage-slack-notify.ts` — parses lcov files,
computes deltas, detects milestone crossings (every 5%), builds Slack
payload. Pure functions exported for testability.
- **Tests**: 26 unit tests in `scripts/coverage-slack-notify.test.ts`
covering all pure functions including edge cases (malformed lcov, exact
boundaries, zero coverage).
### Security hardening
- All `${{ }}` expressions moved from `run:` blocks to `env:` variables
- `SLACK_BOT_TOKEN` passed via env var, not inline
- Unique heredoc delimiter (timestamp-based) prevents payload injection
- `parseInt` fallback (`|| 0`) guards against malformed lcov
- PR regex anchored to first line of commit message
### Robustness
- `continue-on-error: true` on Slack post step (outage does not fail the
job)
- Baseline save guarded by `steps.unit-tests.outcome == success`
(prevents corrupt baselines on test failure)
- Channel ID commented for maintainability
- Top-level `text` field added for Slack mobile push notifications
- Author linked to GitHub profile instead of bare `@username`
## Review Focus
- Workflow step ordering and conditional logic
- Security of expression handling and secret management
- Slack payload structure and Block Kit formatting
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10977-feat-add-Slack-notification-workflow-for-coverage-improvements-33d6d73d3650819c8950f483c83f297c)
by [Unito](https://www.unito.io)
---------
Co-authored-by: GitHub Action <action@github.com>
|
||
|
Kelly Yang
|
66e8d570e7 |
test: expand Image Compare E2E and stabilize widget selectors (#11196)
## Summary
This PR extends **Image Compare** browser tests, adds **stable
`data-testid` hooks** on `WidgetImageCompare.vue`, and aligns slider
interactions with the **same viewport element** used by
`useMouseInElement`, with **layout polling** to reduce flakes.
## What this PR does
- [x] Adds **`data-testid="image-compare-viewport"`** on the compare
area root (the `containerRef` div) so E2E targets the real slider hit
region instead of a long Tailwind class chain or the `<img>` box alone.
- [x] Adds **`data-testid="image-compare-empty"`** on the no-images
branch so the empty state can be asserted **without hard-coded English**
UI text.
- [x] Adds a **smoke** test that the widget **shows both images and the
drag handle** after value injection, with **`waitForImagesLoaded`** (no
extra full-node screenshot to avoid duplicating the default-50 golden).
- [x] Extends slider coverage to **clamp at both edges** (**0%** and
**~100%**) using the viewport locator and **`expect.poll`** until
**`boundingBox()`** is valid before reading coordinates.
- [x] Updates **`moveToPercentage`** to **`expect.poll`** until the
target locator has a **non-empty layout box** before moving the mouse.
- [x] Routes **hover**, **preserve position**, and **25% / 75%
screenshot** mouse moves through **`image-compare-viewport`**
(consistent with `containerRef`).
- [x] Adds an E2E assertion that the **workflow ImageCompare node** size
is **at least 400×350** (matches the widget workflow asset).
- [x] Hardens the **broken image** case: **`page.on('pageerror')`** /
**`page.off`** in **`finally`**, **`http://127.0.0.1:1/...`** URLs for
fast failure, **`expect.soft`** on key UI invariants, and a hard
assertion that **no page errors** were recorded.
- [x] Extends the **large batch (20×20)** test to **page to the last
index** and assert **counters `20 / 20`**, **previous enabled**, and
**next disabled** on both sides.
- [x] Renames the clamp test title to use an **ASCII hyphen** (`0-100%`)
for easier grepping.
## Out of scope (unchanged in this PR)
- [ ] Replacing **`setImageCompareValue`**’s **`page.evaluate`** setup
with a full UI-driven path (would be a larger follow-up).
## Suggested title
`test: expand Image Compare E2E and stabilize widget selectors`
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Primarily test and selector-hook changes; low production risk, with
only minor DOM attribute additions that could affect external test
tooling if relied upon.
>
> **Overview**
> Improves Image Compare Playwright coverage and reduces flakiness by
driving slider interactions through a new stable
`data-testid="image-compare-viewport"` hook and polling for a valid
layout box before mouse moves.
>
> Updates assertions to avoid localized text (new
`data-testid="image-compare-empty"`), adds smoke coverage that
images/handle render after value injection, validates slider clamping at
both 0% and ~100%, and extends screenshot tests to use the same viewport
target.
>
> Hardens edge-case tests by ensuring broken image loads don’t raise
uncaught `pageerror`s, adds a minimum node size assertion, and extends
large-batch navigation checks through the final index and
disabled/enabled nav states.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Yourz
|
121ceda66d |
feat: implement website layout and homepage (#11168)
## Summary Implement the website layout system and homepage with all sections, reusable components, scroll-driven animations, and routing. ## Changes - **What**: - Reorganize components into `common/`, `home/`, `company/`, `product/` directories - Add `BaseLayout` with shared `SiteNav` and `SiteFooter` - Implement homepage sections: Hero, SocialProofBar, ProductShowcase, UseCase, GetStarted, ProductCards, CaseStudySpotlight, BuildWhat - Add reusable components: BrandButton, NodeBadge, ProductCard, FooterLinkColumn, NavDesktopLink, MobileMenu - Add PPFormula font family, client logos, and icon assets - Add hero/footer logo frame sequences for scroll-driven animations - Add `useFrameScrub` composable and `smoothScroll` (Lenis + GSAP ScrollTrigger) - Add route config, nav config, and placeholder pages for all routes - Add Playwright e2e tests for homepage and navigation - **Dependencies**: gsap, lenis, @astrojs/check desktop  mobile  ## Review Focus - Component structure and naming conventions under `apps/website/` - Scroll-driven animation approach (GSAP ScrollTrigger + Lenis smooth scroll) - Mobile responsive behavior (MobileMenu, ScrollTrigger matchMedia) --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: DrJKL <DrJKL0424@gmail.com> Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Dante
|
634d57809b |
test: add E2E tests for bottom panel core behaviors (#10814)
## Summary - Add `browser_tests/tests/bottomPanel.spec.ts` with tests for behaviors not covered by existing `bottomPanelLogs` and `bottomPanelShortcuts` specs - Tests cover: close button (X), tab persistence on re-open, resize gutter visibility and drag, canvas interaction when panel is closed, cross-panel switching (terminal <-> shortcuts), and registered tab enumeration - Extend `BottomPanel` fixture with `closeButton` and `resizeGutter` locators ## Test plan - [ ] `pnpm test:browser:local` passes all new tests in `bottomPanel.spec.ts` - [ ] Existing `bottomPanelLogs.spec.ts` and `bottomPanelShortcuts.spec.ts` are unaffected - [ ] `pnpm typecheck:browser` passes - [ ] `pnpm lint` passes ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10814-test-add-E2E-tests-for-bottom-panel-core-behaviors-3366d73d365081ea9b90c643897845fa) by [Unito](https://www.unito.io) --------- Co-authored-by: dante <dante@danteui-MacStudio.local> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Dante
|
5807e03c74 |
test: expand E2E coverage for toolbox actions (#10968)
## Summary - Adds `selectionToolboxMoreActions.spec.ts` with E2E coverage for previously untested toolbox actions - Covers: pin/unpin, minimize/expand, adjust size, copy, duplicate, refresh button, align (top/left), distribute (horizontal/vertical), alignment options hidden for single selection, multi-node bypass toggle - Part of the FixIt Burndown test coverage initiative (toolbox actions) ## Test plan - [ ] All new tests pass in CI - [ ] No regressions in existing selectionToolbox tests ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10968-test-expand-E2E-coverage-for-toolbox-actions-33c6d73d3650811286cefdd0eb4f5242) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Christian Byrne
|
5c6be4ed66 |
chore: add .editorconfig for cross-editor consistency (#11128)
## Summary Add `.editorconfig` for cross-editor formatting consistency, matching existing oxfmt settings. ## Changes - **What**: New `.editorconfig` with 2-space indent, LF line endings, UTF-8, trailing whitespace trimming (except `.md`) Fixes #11073 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11128-chore-add-editorconfig-for-cross-editor-consistency-33e6d73d365081faabf6fb996144940d) by [Unito](https://www.unito.io) |
||
|
Dante
|
6847c7ba2d |
fix: store promoted widget values per SubgraphNode instance (#10849)
## Summary - Multiple SubgraphNode instances of the same blueprint share inner nodes, causing promoted widget values to collide — the last configured instance overwrites all previous values - Add per-instance value storage (`_instanceWidgetValues`) on SubgraphNode so each instance preserves its own promoted widget values independently - Restore `widgets_values` from serialized data into this per-instance map after promoted views are created during configure - Fixes #10146 ## Root Cause When loading a workflow with multiple SubgraphNode instances of the same blueprint: 1. `LGraph.configure()` creates ONE shared Subgraph per blueprint (line 2625) 2. Each SubgraphNode instance calls `configure(instanceData)` sequentially 3. `PromotedWidgetView.value` setter writes to the **shared inner node's widget** (`promotedWidgetView.ts:199`) 4. The last instance's `configure()` overwrites all previous instances' values **Regression**: Introduced by PR #8594 (WidgetValueStore, v1.41.3) which centralized widget state without per-instance scoping for shared blueprints. ## Fix - **SubgraphNode**: Add `_instanceWidgetValues` Map and `_pendingWidgetsValues` for configure-time restoration - **PromotedWidgetView getter**: Check instance map first before falling back to widget store / inner node - **PromotedWidgetView setter**: Write to instance map to avoid shared inner node mutation - **_internalConfigureAfterSlots**: Apply serialized `widgets_values` to per-instance map after promoted views are created ## Red-Green Verification | Commit | CI Status | Purpose | |--------|-----------|---------| | `test: add failing tests for multi-instance subgraph widget value collision` | 🔴 Red | Proves widget values collide across instances | | `fix: store promoted widget values per SubgraphNode instance` | 🟢 Green | Per-instance storage prevents collision | ## Test Plan - [x] CI red on test-only commit - [x] CI green on fix commit - [x] Unit test: `preserves promoted widget values after configure with different widgets_values` - [x] All 253 existing subgraph tests pass - [ ] Manual: load workflow from issue image → verify 3 subgraph instances produce different results ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10849-fix-store-promoted-widget-values-per-SubgraphNode-instance-3386d73d3650815a8544f54adcc0d504) by [Unito](https://www.unito.io) --------- Co-authored-by: dante <dante@danteui-MacStudio.local> |
||
|
AustinMroz
|
988a546721 |
Add missing dialog tests (#11133)
Leveraging the fancy coverage functionality of #10930, this PR aims to add coverage to missing dialogue models. This has proven quite constructive as many of the dialogues have since been shown to be bugged. - The APINodes sign in dialog that displays when attempting to run a workflow containing Partner nodes while not logged in was intended to display a list of nodes required to execute the workflow. The import for this component was forgotten in the original commit (#3532) and the backing component was later knipped - Error dialogs resulting are intended to display the file responsible for the error, but the prop was accidentally left out during the refactoring of #3265 - ~~The node library migration (#8548) failed to include the 'Edit Blueprint' button, and had incorrect sizing and color on the 'Delete Blueprint' button.~~ - On request, the library button changes were spun out to a separate PR ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11133-Add-missing-dialog-tests-33e6d73d3650812cb142d610461adcd4) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
AustinMroz
|
25ac047b58 |
Fix node library action buttons (#11232)
The node library migration (#8548) failed to include the 'Edit Blueprint' button, and had incorrect sizing and color on the 'Delete Blueprint' button. - Re-add edit blueprint which was missed in the migration - Fix incorrect sizing on delete blueprint - Fix color (lucide uses background, not text) - Migrate all action buttons use our capital 'B' Button component and use standardized variants where possible Spun out of #11133 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11232-Fix-node-library-action-buttons-3426d73d365081339cafc7457c0b5421) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
165984fe4c |
test: Improve speed of app mode input corruption test (#11236)
## Summary Speeds up test that was timing out https://9b579efd.comfyui-playwright-chromium.pages.dev/#?testId=b97e313f05078cede9be-5e6b75a76880fb6a5d96 ## Changes - **What**: - load prebuilt workflows to reduce test time (17s -> 11s) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11236-test-Improve-speed-of-app-mode-input-corruption-test-3426d73d3650815b9475ec96dfbd7ad5) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
34a02a29c9 |
test: Remove unnecessary setup, UseNewMenu and waitForNodes calls (#11237)
## Summary More simplification ## Changes - **What**: - Remove more UseNewMenu settings calls - Remove `await comfyPage.setup()` - Remove `waitForNodes` in vue node tagged tests ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11237-test-Remove-unnecessary-setup-UseNewMenu-and-waitForNodes-calls-3426d73d36508198a100c218420d479c) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
4bd655f625 |
feat: add PreToolUse hooks to enforce pnpm scripts (#11201)
## Summary Add Claude Code PreToolUse hooks to block agents from bypassing package.json scripts with raw tool invocations. ## Changes - **What**: 15 PreToolUse hooks in `.claude/settings.json` that intercept `npx`/`pnpx`/bare invocations of tsc, vitest, eslint, prettier, oxlint, stylelint, and knip — redirecting agents to the correct `pnpm` script (`pnpm typecheck`, `pnpm test:unit`, `pnpm lint`, etc.) - Also removes stale `permissions.allow` entries left over from a debugging session ## Review Focus - Pattern coverage: are there common agent invocations we're missing? - The `if` field only supports simple `*` globs (no alternation), so each pattern needs its own hook entry ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11201-feat-add-PreToolUse-hooks-to-enforce-pnpm-scripts-3416d73d365081a59a38c86ee4669aee) by [Unito](https://www.unito.io) Co-authored-by: Amp <amp@ampcode.com> |
||
|
pythongosssss
|
a09bb81b98 |
test: Auto wait for nodes after loadWorkflow in vue-node tests (#11238)
## Summary Updates tests to auto wait for vue-nodes when loading a workflow in a test with the vue-nodes tag ## Changes - **What**: - If tag includes vue-nodes, wait - Remove all load->wait calls ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11238-test-Auto-wait-for-nodes-after-loadWorkflow-in-vue-node-tests-3426d73d3650810e8760c5601186fde8) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
aeedb60628 |
fix(ci): resolve pnpm version in release workflow for frontend/ checkout path (#11224)
The release workflow checks out to `frontend/` subdirectory, but `pnpm/action-setup` looks for `package.json` at the repo root by default. This causes `No pnpm version is specified` failures. Adds `package_json_file: frontend/package.json` so the action can read the `packageManager` field. Same pattern used in #10972 for the version-bump workflow. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11224-fix-ci-resolve-pnpm-version-in-release-workflow-for-frontend-checkout-path-3426d73d365081c28d16cb01bf8218ef) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Christian Byrne
|
43fb5a8b19 |
ops: add patch release support to ComfyUI release workflow (#11223)
## Summary Add `release_type` input (`minor`/`patch`) to the release workflow so patch releases can target the current production branch instead of always preferring the next minor. ## Problem When a patch release is needed for `core/1.42` but `core/1.43` already exists, the resolver always prefers `1.43`. There was no way to do a patch release with PyPI publish + ComfyUI PR for the current production version. ## Changes - Rename workflow from "Release: Bi-weekly ComfyUI" → "Release: ComfyUI" (serves both cadences) - Add `release_type` choice input: `minor` (default, bi-weekly) vs `patch` (hotfix for current production version) - Update `resolve-comfyui-release.ts` to read `RELEASE_TYPE` env var for branch targeting - Scheduled runs continue to work as before (default to `minor`) ## Usage ```bash # Bi-weekly minor release (or just let the schedule run) gh workflow run release-biweekly-comfyui.yaml --ref main # Patch release for current production version gh workflow run release-biweekly-comfyui.yaml --ref main --field release_type=patch ``` ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11223-ops-add-patch-release-support-to-ComfyUI-release-workflow-3426d73d365081498c15ff978a7f1116) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
c484c3984f |
fix: remove appendTo override from FormDropdown to restore correct positioning (#11147)
## Problem PR #10338 added `useTransformCompatOverlayProps()` to FormDropdown and FormDropdownMenuActions, which sets `appendTo: 'self'` in graph mode. This breaks PrimeVue Popover positioning inside CSS-transformed containers — the dropdown appears at incorrect Y positions. ## Root Cause PrimeVue Popover with `appendTo: 'self'` renders the overlay inside the component's DOM, inheriting parent CSS transforms. This causes the popover to miscalculate its position when the parent has `transform: scale()` or `translate()`. ## Fix Remove the `appendTo` override from both FormDropdown and FormDropdownMenuActions. PrimeVue defaults to `appendTo: 'body'`, which teleports the popover to `<body>` — correctly positioning it outside any CSS transform context. - **Graph mode**: restores pre-#10338 behavior (`appendTo: 'body'` default) - **App mode**: unaffected — `'body'` is exactly what app mode needs (prevents sidebar overflow clipping) ## Testing - Existing unit tests pass (5/5) - Typecheck clean - Lint clean - **E2E test rationale**: No E2E test added — this is a pure removal of a prop override (reverting to PrimeVue defaults). The positioning bug requires CSS transforms at specific viewport scales which are impractical to assert reliably in Playwright. The existing `subgraph-dom-widget-clipping` perf test exercises dropdown rendering in transformed contexts and shows no regression. Fixes #10499 Supersedes #11001 (temporary hotfix for backport) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Dante
|
2524846f5c |
fix: guard progress_text before canvas init (#11174)
## Summary Prevent early `progress_text` websocket events from throwing before the graph canvas is initialized. ## Changes - **What**: Guard `handleProgressText()` until `canvasStore.canvas` exists, and add a regression test for a startup-time `progress_text` event arriving before `GraphCanvas` finishes initialization. ## Review Focus Confirm this is the right guard point for the startup race between `GraphView` websocket binding and `GraphCanvas` async setup, and that progress text behavior is unchanged once the canvas is ready. ## Validation - `pnpm exec eslint src/stores/executionStore.ts src/stores/executionStore.test.ts` - `pnpm exec vitest run src/stores/executionStore.test.ts -t "should ignore progress_text before the canvas is initialized"` - `pnpm test:unit -- --run src/stores/executionStore.test.ts` still reports one unrelated isolated-file failure in `nodeLocatorIdToExecutionId` on current `main` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11174-fix-guard-progress_text-before-canvas-init-3406d73d3650813dad23d511fb51add5) by [Unito](https://www.unito.io) |
||
|
Comfy Org PR Bot
|
12f578870e |
1.44.4 (#11177)
Patch version increment to 1.44.4 **Base branch:** `main` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11177-1-44-4-3416d73d365081c0a2e0def7071c1441) by [Unito](https://www.unito.io) --------- Co-authored-by: christian-byrne <72887196+christian-byrne@users.noreply.github.com> Co-authored-by: github-actions <github-actions@github.com>v1.44.4 |
||
|
Alexander Brown
|
72eed86cea |
test: remove redundant setup/settings now handled by @vue-nodes fixture (#11195)
## Summary Follow-up cleanup for #11184 — removes redundant test setup calls that the `@vue-nodes` fixture now handles. ## Changes - **What**: Remove 40 lines of redundant `setSetting`, `setup()`, and `waitForNodes()` calls across 11 test files - `UseNewMenu: 'Top'` calls (already fixture default) - `setup()` + `waitForNodes()` on default workflow (fixture already does this for `@vue-nodes`) - Page reload in `subgraphZeroUuid` (fixture applies VueNodes.Enabled server-side before navigation) ## Review Focus Each removal was verified against the fixture's `setupSettings()` defaults (ComfyPage.ts:420-442) and the `@vue-nodes` auto-setup (lines 454-456). Tests that call `setup()`/`waitForNodes()` after `loadWorkflow()` or `page.evaluate()` were intentionally kept. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11195-test-remove-redundant-setup-settings-now-handled-by-vue-nodes-fixture-3416d73d36508154827df116a97e9130) by [Unito](https://www.unito.io) Co-authored-by: Amp <amp@ampcode.com> |
||
|
Benjamin Lu
|
719ed16d32 |
fix: track workspace subscription success on immediate subscribe (#11130)
## Summary Track GTM `subscription_success` when a workspace subscription completes synchronously in the dialog. The async billing-operation path already emitted telemetry; the missing gap was the immediate `subscribed` response. ## Changes - **What**: Add the missing GTM success emission to both synchronous workspace subscribe success branches while preserving the existing toast, billing refresh, and dialog close behavior. ## Review Focus Verify the synchronous `response.status === "subscribed"` workspace dialog paths are the only missing frontend success emissions, while the async billing-operation telemetry path remains unchanged. This PR intentionally stays minimal. It does not add new browser coverage yet; the previous component-level unit test was more implementation-coupled than this fix justified, and a better long-term test would be a higher-level workspace billing flow test once we have a cleaner harness. |
||
|
pythongosssss
|
5899a9392e |
test: Simplify vue node/menu test setup (#11184)
## Summary Simplifies test setup for common settings ## Changes - **What**: - add vue-nodes tag to auto enable nodes 2.0 - remove UseNewMenu Top as this is default ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11184-test-Simplify-vue-node-menu-test-setup-3416d73d3650815487e0c357d28761fe) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
e39468567a |
fix: check server feature flags for progress_text binary format (#10996)
## Problem API node generation status text (sent via `progress_text` WebSocket binary messages) was not showing on local ComfyUI, but worked on cloud. ## Root Cause The binary decoder for `progress_text` messages (eventType 3) checked `getClientFeatureFlags()?.supports_progress_text_metadata` — the **client's own flags** — to decide whether to parse the new format with `prompt_id`. Since the client always advertises `supports_progress_text_metadata: true`, it always tried to parse the new wire format: ``` [4B event_type][4B prompt_id_len][prompt_id][4B node_id_len][node_id][text] ``` But the backend PR that adds `prompt_id` to the binary message ([ComfyUI#12540](https://github.com/Comfy-Org/ComfyUI/pull/12540)) was **closed without merging**, so local ComfyUI still sends the legacy format: ``` [4B event_type][4B node_id_len][node_id][text] ``` The decoder misinterpreted the `node_id_len` as `prompt_id_len`, consuming the actual node_id bytes as a prompt_id, then producing garbled `nodeId` and `text` — silently dropping all progress text updates via the catch handler. Cloud worked because the cloud backend supports and echoes the feature flag. ## Fix One-line change: check `serverFeatureFlags.value` (what the server echoed back) instead of `getClientFeatureFlags()` (what the client advertises). ## Tests Added 3 tests covering: - Legacy format parsing when server doesn't support the flag - New format parsing when server does support the flag - Corruption regression test: client advertises support but server doesn't ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10996-fix-check-server-feature-flags-for-progress_text-binary-format-33d6d73d365081449a0dc918358799de) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
pythongosssss
|
a373633ab2 |
refactor: fix lint errors in tests (#11182)
## Summary Fix tests failing lint ## Changes - **What**: - Fix relative imports - Fix test not using comfyPage ## Review Focus <!-- Critical design decisions or edge cases that need attention --> <!-- If this PR fixes an issue, uncomment and update the line below --> <!-- Fixes #ISSUE_NUMBER --> ## Screenshots (if applicable) <!-- Add screenshots or video recording to help explain your changes --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11182-refactor-fix-lint-errors-in-tests-3416d73d3650812fbf1bc88554c57de2) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
521019d173 |
fix: exclude muted/bypassed nodes from missing asset detection (#10856)
## Summary Muted and bypassed nodes are excluded from execution but were still triggering missing model/media/node warnings. This PR makes the error system mode-aware: muted/bypassed nodes no longer produce missing asset errors, and all error lifecycle events (mode toggle, deletion, paste, undo, tab switch) are handled consistently. - Fixes Comfy-Org/ComfyUI#13256 ## Behavioral notes - **Tab switch overlay suppression (intentional)**: Switching back to a workflow with missing assets no longer re-shows the error overlay. This reverses the behavior introduced in #10190. The error state is still restored silently in the errors tab — users can access it via the properties panel without being interrupted by the overlay on every tab switch. ## Changes ### 1. Scan filtering - `scanAllModelCandidates`, `scanAllMediaCandidates`, `scanMissingNodes`: skip nodes with `mode === NEVER || BYPASS` - `collectMissingNodes` (serialized data): skip error reporting for muted/bypassed nodes while still calling `sanitizeNodeName` for safe `configure()` - `collectEmbeddedModelsWithSource`: skip muted/bypassed nodes; workflow-level `graphData.models` only create candidates when active nodes exist - `enrichWithEmbeddedMetadata`: filter unmatched workflow-level models when all referencing nodes are inactive ### 2. Realtime mode change handling - `useErrorClearingHooks.ts` chains `graph.onTrigger` to detect `node:property:changed` (mode) - Deactivation (active → muted/bypassed): remove missing model/media/node errors for the node - Activation (muted/bypassed → active): scan the node and add confirmed errors, show overlay - Subgraph container deactivation: remove all interior node errors (execution ID prefix match) - Subgraph container activation: scan all active interior nodes recursively - Subgraph interior mode change: resolve node via `localGraph.getNodeById()` then compute execution ID from root graph ### 3. Node deletion - `graph.onNodeRemoved`: remove missing model/media/node errors for the deleted node - Handle `node.graph === null` at callback time by using `String(node.id)` for root-level nodes ### 4. Node paste/duplicate - `graph.onNodeAdded`: scan via `queueMicrotask` (deferred until after `node.configure()` restores widget values) - Guard: skip during `ChangeTracker.isLoadingGraph` (undo/redo/tab switch handled by pipeline) - Guard: skip muted/bypassed nodes ### 5. Workflow tab switch optimization - `skipAssetScans` option in `loadGraphData`: skip full pipeline on tab switch - Cache missing model/media/node state per workflow via `PendingWarnings` - `beforeLoadNewGraph`: save current store state to outgoing workflow's `pendingWarnings` - `showPendingWarnings`: restore cached errors silently (no overlay), always sync missing nodes store (even when null) - Preserve UI state (`fileSizes`, `urlInputs`) on tab switch by using `setMissingModels([])` instead of `clearMissingModels()` - `MissingModelRow.vue`: fetch file size on mount via `fetchModelMetadata` memory cache ### 6. Undo/redo overlay suppression - `silentAssetErrors` option propagated through pipeline → `surfaceMissingModels`/`surfaceMissingMedia` `{ silent }` option - `showPendingWarnings` `{ silent }` option for missing nodes overlay - `changeTracker.ts`: pass `silentAssetErrors: true` on undo/redo ### 7. Error tab node filtering - Selected node filters missing model/media card contents (not just group visibility) - `isAssetErrorInSelection`: resolve execution ID → graph node for selection matching - Missing nodes intentionally unfiltered (pack-level scope) - `hasMissingMediaSelected` added to `RightSidePanel.vue` error tab visibility - Download All button: show only when 2+ downloadable models exist ### 8. New store functions - `missingModelStore`: `addMissingModels`, `removeMissingModelsByNodeId` - `missingMediaStore`: `addMissingMedia`, `removeMissingMediaByNodeId` - `missingNodesErrorStore`: `removeMissingNodesByNodeId` - `missingModelScan`: `scanNodeModelCandidates` (extracted single-node scan) - `missingMediaScan`: `scanNodeMediaCandidates` (extracted single-node scan) ### 9. Test infrastructure improvements - `data-testid` on `RightSidePanel.vue` tabs (`panel-tab-{value}`) - Error-related TestIds moved from `dialogs` to `errorsTab` namespace in `selectors.ts` - Removed unused `TestIdValue` type - Extracted `cleanupFakeModel` to shared `ErrorsTabHelper.ts` - Renamed `openErrorsTabViaSeeErrors` → `loadWorkflowAndOpenErrorsTab` - Added `aria-label` to pencil edit button and subgraph toggle button ## Test plan ### Unit tests (41 new) - Store functions: `addMissing*`, `removeMissing*ByNodeId` - `executionErrorStore`: `surfaceMissing*` silent option - Scan functions: muted/bypassed filtering, `scanNodeModelCandidates`, `scanNodeMediaCandidates` - `workflowService`: `showPendingWarnings` silent, `beforeLoadNewGraph` caching ### E2E tests (17 new in `errorsTabModeAware.spec.ts`) **Missing nodes** - [x] Deleting a missing node removes its error from the errors tab - [x] Undo after bypass restores error without showing overlay **Missing models** - [x] Loading a workflow with all nodes bypassed shows no errors - [x] Bypassing a node hides its error, un-bypassing restores it - [x] Deleting a node with missing model removes its error - [x] Undo after bypass restores error without showing overlay - [x] Pasting a node with missing model increases referencing node count - [x] Pasting a bypassed node does not add a new error - [x] Selecting a node filters errors tab to only that node **Missing media** - [x] Loading a workflow with all nodes bypassed shows no errors - [x] Bypassing a node hides its error, un-bypassing restores it - [x] Pasting a bypassed node does not add a new error - [x] Selecting a node filters errors tab to only that node **Subgraph** - [x] Bypassing a subgraph hides interior errors, un-bypassing restores them - [x] Bypassing a node inside a subgraph hides its error, un-bypassing restores it **Workflow switching** - [x] Does not resurface error overlay when switching back to workflow with missing nodes - [x] Restores missing nodes in errors tab when switching back to workflow # Screenshots https://github.com/user-attachments/assets/e0a5bcb8-69ba-4120-ab7f-5c83e4cfc3c5 ## Follow-up work - Extract error-detection computed properties from `RightSidePanel.vue` into a composable (e.g. `useErrorsTabVisibility`) --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Kelly Yang
|
bd82c855e0 |
test: add minimap E2E tests for graph content and click-to-navigate (#10738)
## Summary
Adds Playwright E2E tests verifying that
1. the minimap canvas renders node content
2. clears when the graph is empty
3. correctly navigates the main canvas on click
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10738-test-add-minimap-E2E-tests-for-graph-content-and-click-to-navigate-3336d73d365081eb955ce711b3efc57f)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: changes are limited to adding `data-testid` attributes to
the minimap UI and expanding Playwright E2E assertions, with no
production behavior changes expected.
>
> **Overview**
> Strengthens minimap E2E coverage by switching existing assertions from
CSS selectors to new `data-testid`-based selectors and adding helper
utilities for canvas/overlay interactions.
>
> Adds new Playwright tests that verify the minimap canvas renders
content when nodes exist, clears when the graph is emptied, and that
clicking the minimap pans the main canvas (including a
post-`fitViewToSelectionAnimated` tolerance check).
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Kelly Yang
|
5b7ef3fe21 |
test: Painter Widget E2E Test Plan (#10846)
### Summary of Improvements
* **Custom Test Coverage Extension**: Enhanced the Painter widget E2E
test suite by refactoring logic for better maintainability and
robustness.
* **Stable Component Targeting**: Introduced
`data-testid="painter-dimension-text"` to `WidgetPainter.vue`, providing
a reliable, non-CSS-dependent locator for canvas size verification.
* **Improved Test Organization**: Reorganized existing test scenarios
into logical categories using `test.describe` blocks (Drawing, Brush
Settings, Canvas Size Controls, etc.).
* **Asynchronous Helper Integration**: Converted `hasCanvasContent` to
an asynchronous helper and unified its usage across multiple test cases
to eliminate redundant pixel-checking logic.
* **Locator Resilience**: Updated Reka UI slider interaction logic to
use more precise targeting (`:not([data-slot])`), preventing ambiguity
and improving test stability.
* **Scenario Refinement**: Updated the `pointerup` test logic to
accurately reflect pointer capture behavior when interactions occur
outside the canvas boundaries.
* **Enhanced Verification Feedback**: Added descriptive error messages
to `expect.poll` assertions to provide clearer context on potential
failure points.
* **Standardized Tagging**: Restored the original tagging strategy
(including `@smoke` and `@screenshot` tags) to ensure tests are
categorized correctly for CI environments.
### Red-Green Verification
| Commit | CI Status | Purpose |
| :--- | :--- | :--- |
| `test: refactor painter widget e2e tests and address review findings`
| 🟢 Green | Addresses all E2E test quality and stability issues from
review findings. |
### Test Plan
- [x] **Quality Checks**: `pnpm format`, `pnpm lint`, and `pnpm
typecheck` verified as passing.
- [x] **Component Integration**: `WidgetPainter.vue` `data-testid`
correctly applied and used in tests.
- [x] **Helper Reliability**: `hasCanvasContent` correctly identifies
colored pixels and returns a promise for `expect.poll`.
- [x] **Locator Robustness**: Verified Reka slider locators correctly
exclude internal thumb spans.
- [x] **Boundary Interaction**: Verified `pointerup` correctly ends
strokes when triggered outside the viewport.
- [x] **Tagging Consistency**: Verified `@smoke` and `@screenshot` tags
are present in the final test suite.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10846-test-Painter-Widget-E2E-Test-Plan-3386d73d365081deb70fe4afbd417efb)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Primarily adds/refactors Playwright E2E tests and stable `data-testid`
hooks, with no changes to Painter drawing logic. Risk is limited to
potential test brittleness or minor UI attribute changes.
>
> **Overview**
> Expands the Painter widget Playwright suite with new grouped scenarios
covering drawing/erasing behavior, tool switching, brush inputs, canvas
resizing (including preserving drawings), clear behavior, and
serialization/upload flows (including failure toast).
>
> Refactors the tests to use a shared `@e2e/helpers/painter` module
(`drawStroke`, `hasCanvasContent`, `triggerSerialization`), improves
stability via role/testid-based locators and clearer `expect.poll`
messaging, and adds `data-testid` attributes (e.g.,
`painter-clear-button`, `painter-*-row`, `painter-dimension-text`) to
`WidgetPainter.vue` to avoid CSS-dependent selectors.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Kelly Yang
|
85de833776 |
test: add E2E tests for ImageCompare widget (#10767)
## Summary
Add E2E tests for ImageCompare widget
Covers slider interaction, batch navigation, single-image modes, visual
regression screenshots, and edge cases for the ImageCompare Vue node
widget.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10767-test-add-E2E-tests-for-ImageCompare-widget-3346d73d365081c6bfc6fbd97fa04e4d)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Adds Playwright E2E coverage and screenshot assertions only; main risk
is increased CI runtime/flakiness due to additional image-loading and
hover/position polling.
>
> **Overview**
> Adds a new Playwright E2E suite for the ImageCompare Vue widget
(tagged `@widget`) that programmatically sets widget values and asserts
rendering for empty, single-image, and dual-image states.
>
> Expands coverage to **slider behavior** (default 50%, hover movement,
clamping, persistence) using polling on inline `clip-path`/handle
position, and adds **batch navigation** tests for multi-image
before/after sets.
>
> Introduces **visual regression screenshots** at default and specific
slider positions, plus edge-case tests for broken URLs, rapid updates
resetting batch index, legacy string values, and custom alt text.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Kelly Yang
|
cab46567c0 |
test: add E2E tests for ImageCropV2 widget (#10737)
## Summary
Adds Playwright E2E tests for the ImageCropV2 widget covering
1. the empty state (no source image)
2. default control rendering
3. source image display with crop overlay
4. drag-to-reposition behavior.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10737-test-add-E2E-tests-for-ImageCropV2-widget-3336d73d365081b28ed9db63e5df383e)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: primarily adds Playwright E2E coverage and introduces
`data-testid` attributes for more stable selectors, with no changes to
core crop behavior.
>
> **Overview**
> Adds new Playwright E2E coverage for the `ImageCropV2` Vue-node
widget, including workflows/fixtures for a disconnected input and a
`LoadImage -> ImageCropV2 -> PreviewImage` pipeline.
>
> Tests validate the empty state and default controls, verify the crop
overlay renders after execution with screenshot assertions, and exercise
drag-to-reposition by dispatching pointer events and asserting the
widget’s crop value updates.
>
> Updates `WidgetImageCrop.vue` to add `data-testid` hooks (empty
state/icon and crop overlay) to make the E2E selectors stable.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Comfy Org PR Bot
|
63435bdb34 |
1.44.3 (#11170)
Patch version increment to 1.44.3 **Base branch:** `main` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11170-1-44-3-3406d73d365081799aa4e189009d123b) by [Unito](https://www.unito.io) Co-authored-by: christian-byrne <72887196+christian-byrne@users.noreply.github.com>v1.44.3 |
||
|
Kelly Yang
|
20255da61f |
feat(load3d): add optional HDRI environment lighting to 3D preview nodes (#10818)
## Summary Adds `HDRIManager` to load `.hdr/.exr` files as equirectangular environment maps via **three.js** `RGBELoader/EXRLoader` - Uploads HDRI files to the server via `/upload/image` API so they persist across page reloads - Restores HDRI state (enabled, **intensity**, **background**) from node properties on reload - Auto-enables "**Show as Background**" on successful upload for immediate visual feedback - Hides standard directional lights when HDRI is active; restores them when disabled - Hides the Light Intensity control while HDRI is active (lights have no effect when HDRI overrides scene lighting) - Limits HDRI availability to PBR-capable formats (.gltf, .glb, .fbx, .obj); automatically disables when switching to an incompatible model - Adds intensity slider and "**Show as Background**" toggle to the HDRI panel ## How to Use HDRI Environment Lighting 1. Load a 3D model using a Load3D or Load3DViewer node (supported formats: .gltf, .glb, .fbx, .obj) 2. Open the control panel → go to the Light tab 3. Click the globe icon to open the **HDRI panel** 4. Click Upload HDRI and select a` .hdr` or `.exr` file 5. The environment lighting applies automatically — the scene background also updates to preview the panorama 6. Use the intensity slider to adjust the strength of the environment lighting 7. Toggle Show as Background to show or hide the HDRI panorama behind the model ## Screenshots https://github.com/user-attachments/assets/1ec56ef0-853e-452f-ae2b-2474c9d0d781 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10818-feat-load3d-add-optional-HDRI-environment-lighting-to-3D-preview-nodes-3366d73d365081ea8c7ad9226b8b1e2f) by [Unito](https://www.unito.io) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Adds new HDRI loading/rendering path and persists new `LightConfig.hdri` state, touching Three.js rendering, file uploads, and node property restoration. Risk is moderate due to new async flows and potential compatibility/performance issues with model switching and renderer settings. > > **Overview** > Adds optional **HDRI environment lighting** to Load3D previews, including a new `HDRIManager` that loads `.hdr`/`.exr` files into Three.js environment/background and exposes controls for enable/disable, background display, and intensity. > > Extends `LightConfig` with an `hdri` block that is persisted on nodes and restored on reload; `useLoad3d` now uploads HDRI files, loads them into `Load3d`, maps scene light intensity to HDRI intensity, and auto-disables HDRI when the current model format doesn’t support it. > > Updates the UI to include embedded HDRI controls under the Light panel (with dismissable overlays and icon updates), adjusts light intensity behavior when HDRI is active, and adds tests/strings/utilities (`getFilenameExtension`, `mapSceneLightIntensityToHdri`, new constants) to support the feature. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit b12c9722dc54a227004bdc383e3bf583504ebdce. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Terry Jia <terryjia88@gmail.com> |