mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-24 14:45:36 +00:00
858fe2385914842413402cdf8a8c60b3dfbe0933
322 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Alexander Brown
|
858fe23859 | Merge branch 'main' into drjkl/world-consolidation-phase2 | ||
|
Alexander Brown
|
73d4e24ffa |
revert: roll back #10849 + #11697 (per-instance promoted widget values) (#11790)
## Summary Reverts #10849 (per-instance promoted widget value storage) and its companion test-pinning PR #11697. The fix in #10849 caused regressions in promoted-widget serialization (notably the Z-Image-Turbo template, see #10146 follow-up). A replacement fix is being developed on `fix/subgraph-promoted-widget-inline-state` and will land separately. ## Changes - **Revert #11697** — drops the `it.fails`-marked tests that pin the #10849 corruption symptom. With #10849 reverted, those markers would falsely flip to passing. - **Revert #10849** — removes per-instance `_instanceWidgetValues` map, `_pendingWidgetsValues` configure-time hydration, the `widgets_values` write path in `SubgraphNode.serialize()`, the `sourceSerialize` field on `PromotedWidgetView`, the multi-instance Vitest suite, and the multi-instance E2E test + asset. - **Conflict resolution** in `browser_tests/tests/subgraph/subgraphSerialization.spec.ts`: kept the restored test coverage from #11579 (which is post-#10849 on main) and removed only the now-unreachable multi-instance test, its helper, and its workflow constant. Auto-merge with #11698 (`incrementVersion`) and #11699 (ID type aliases) was clean. ## Review Focus - Confirm no other on-main code path has come to depend on `PromotedWidgetView.sourceSerialize` or `SubgraphNode._instanceWidgetValues` since #10849 (grep is clean locally). - Confirm we want to land this revert before the replacement fix on `fix/subgraph-promoted-widget-inline-state` is ready — this leaves the original #10146 (multi-instance widget value collision) unfixed in the meantime. - The retained #11579 test coverage now exercises pre-#10849 behavior; some of those assertions were written expecting the #10849 code path. CI will surface any that need adjustment. ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11790-revert-roll-back-10849-11697-per-instance-promoted-widget-values-3536d73d3650814094abd58b6b712d8d) by [Unito](https://www.unito.io) |
||
|
DrJKL
|
0e206118d3 |
refactor(world): split WidgetValue into per-aspect components
- Replace WidgetValueComponent with four components (Value, Display, Schema, Serialize) defined via new defineComponentKeys factory. - Add slot() phantom-typed helper so factory can recover TData/TEntity per slot and emit string-literal-typed names. - widgetValueStore now stores per-aspect data and returns delegating WidgetState views built by buildView(); identity is no longer preserved across getWidget calls (data round-trips, identity does not). - Add WidgetRegistration input shape (carries name/nodeId for id construction) distinct from the returned WidgetState view. - Add getNodeWidgetsByName() that derives names from WidgetEntityId via new parseWidgetEntityId() helper. - entityIds: add parseWidgetEntityId, isNodeIdForGraph, isWidgetIdForGraph; clearGraph and stores use the predicates instead of duplicating prefix logic. - world: introduce getBucket/getOrCreateBucket internals, key buckets by ComponentKey reference identity (documented), confine the existential cast to a single eraseKey boundary. - Update tests for componentKey factory, entityIds parsers, world bucket semantics, widgetValueStore view semantics, and adjust BaseWidget/useUpstreamValue/useImageCrop tests accordingly. - docs: refresh ECS pattern survey appendix to reflect new layout. Amp-Thread-ID: https://ampcode.com/threads/T-019de010-ead4-7627-9552-3d44d7a46726 Co-authored-by: Amp <amp@ampcode.com> |
||
|
pythongosssss
|
b8dfbfc0bb |
fix: ensure escape key/graph navigation cancels ghost node placement (#11779)
## Summary When inside a subgraph, the parent key handler intercepts the escape key before `processKey` is called, causing a stale ghost node to be added to the inner subgraph when the graph is changed to the parent. ## Changes - **What**: - move escape key handler to document, prevent propagating the event - add saftey net cancel in setGraph - tests ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11779-fix-ensure-escape-key-graph-navigation-cancels-ghost-node-placement-3526d73d3650812292e4ca10d384f783) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
9d61b4df06 |
feat: ECS Phase 0b — ID type aliases (FE-166/475/476/477) (#11699)
## Summary
ECS Phase 0b — type-only ID aliases. Builds on FE-165 (centralized
version counter, base of this PR) and adds a tranche of named ID aliases
plus mechanical adoption at known call sites.
This PR now covers four child tickets:
- **FE-166** — adds `GroupId` (in `LGraphGroup.ts`) and `SlotIndex` (in
`interfaces.ts`); re-exported from the litegraph barrel.
- **FE-475** — mechanical adoption of existing aliases (`NodeId`,
`LinkId`, `RerouteId`, `GroupId`, `SlotIndex`, `ExecutionId`) across
litegraph at the audit-listed sites:
`LGraphState.lastGroupId/lastLinkId/lastRerouteId`,
`LGraphExtra.linkExtensions`, `ISerialisedGroup.id`,
`ISerialisedGraph.last_link_id`, `LinkNetwork.removeReroute`,
`INodeOutputSlot.slot_index`, `LGraphNode.{setOutputDataType,
getInputDataType, getOutputPos}` slot params,
`ExecutableNodeDTO.inputs[].linkId` + execution-id locals, and
`RenderLink/MovingLinkBase.fromSlotIndex` (plus subclasses that
redeclare).
- **FE-476** — adds `SubgraphId = UUID` in `LGraph.ts`; adopted at
`_subgraphs` Map, `findUsedSubgraphIds`, `getDirectSubgraphIds`, and
`ExportedSubgraphInstance.type`. Re-exported from the litegraph barrel.
- **FE-477** — adds app-domain entity aliases at their closest
schema/types files: `WorkflowId`, `AssetId`, `PromptId` (propagated as
existing `JobId`), `TaskId`, `UserId`, `WorkspaceId`,
`WorkspaceInviteId`, `NodePackId`. Adopted at primary use sites (entity
id fields, store state, service signatures).
## Entity reference
### ID aliases at a glance
| Alias | Underlying | Defined in | Identifies |
| --- | --- | --- | --- |
| `NodeId` | `number \| string` | `litegraph/LGraphNode.ts` | A node
within a graph |
| `LinkId` | `number` | `litegraph/LLink.ts` | A connection between two
slots |
| `RerouteId` | `number` | `litegraph/Reroute.ts` | A reroute waypoint
on a link |
| `GroupId` *(new)* | `number` | `litegraph/LGraphGroup.ts` | A visual
group of nodes |
| `SlotIndex` *(new)* | `number` | `litegraph/interfaces.ts` | A slot's
position on a node |
| `ExecutionId` | `string` | `litegraph/types/serialisation.ts` | A node
within a subgraph instance |
| `SubgraphId` *(new)* | `UUID` | `litegraph/LGraph.ts` | A subgraph
definition |
| `WorkflowId` *(new)* | `string` |
`platform/workflow/validation/schemas/workflowSchema.ts` | A saved
workflow document |
| `AssetId` *(new)* | `string` |
`platform/assets/schemas/assetSchema.ts` | A binary asset (model, image,
etc.) |
| `JobId` *(reused as `PromptId`)* | `string` | `schemas/apiSchema.ts` |
A queued prompt execution |
| `TaskId` *(new)* | `string` | `platform/tasks/services/taskService.ts`
| A backend background task |
| `UserId` *(new)* | `string` | `types/authTypes.ts` | An authenticated
user |
| `WorkspaceId` *(new)* | `string` |
`platform/workspace/workspaceTypes.ts` | A workspace |
| `WorkspaceInviteId` *(new)* | `string` |
`platform/workspace/workspaceTypes.ts` | A pending workspace invite |
| `NodePackId` *(new)* | `string` |
`workbench/extensions/manager/types/comfyManagerTypes.ts` | A Comfy
Registry / Manager node pack |
### How the entities relate
```mermaid
flowchart TB
subgraph LG["🎨 Litegraph entities"]
direction TB
Graph["LGraph<br/>id: SubgraphId"]
Subgraph["Subgraph<br/>id: SubgraphId"]
Node["LGraphNode<br/>id: NodeId"]
Link["LLink<br/>id: LinkId"]
Reroute["Reroute<br/>id: RerouteId"]
Group["LGraphGroup<br/>id: GroupId"]
Slot["INodeSlot<br/>slot_index: SlotIndex"]
Exec["ExecutableNodeDTO<br/>id: ExecutionId"]
end
subgraph AP["🌐 App-domain entities"]
direction TB
Workflow["ComfyWorkflow<br/>id: WorkflowId"]
Job["Prompt / Job<br/>id: JobId ≡ PromptId"]
Task["Task<br/>id: TaskId"]
Asset["Asset<br/>id: AssetId"]
Workspace["Workspace<br/>id: WorkspaceId"]
Invite["WorkspaceInvite<br/>id: WorkspaceInviteId"]
User["User<br/>id: UserId"]
Pack["NodePack<br/>id: NodePackId"]
end
Subgraph -. extends .-> Graph
Graph --> Node
Graph --> Link
Graph --> Group
Node --> Slot
Link --> Reroute
Link --> Slot
Node -. instantiates .-> Subgraph
Exec -. wraps .-> Node
Workflow -. serializes .-> Graph
Job --> Workflow
Job --> Task
Task --> Asset
Workspace --> Workflow
User --> Workspace
Workspace --> Invite
Pack -. provides .-> Node
```
Solid arrows are containment / direct references; dashed arrows are *“is
a kind of”* (`extends`) or cross-layer relationships (e.g. a
`ComfyWorkflow` *serializes* an `LGraph`; a `NodePack` *provides* node
definitions).
## Explicit non-goals
- `LGraphState.lastNodeId` is intentionally kept as bare `number`
(auto-increment counter; would widen if aliased to `NodeId = number |
string`).
- No new `SubgraphSlotId` alias — verified subsidiary (subgraph IO slots
are addressed via `SUBGRAPH_INPUT_ID/OUTPUT_ID` sentinel + numeric array
index, not by UUID alone).
- No `WidgetName`, `SlotName`, `WorkspaceMemberId` — verified subsidiary
(only meaningful inside a parent or as a relationship).
- No re-typing of `LGraph.id` / `Subgraph.id` — references adopt
`SubgraphId`, but the inherited UUID typing is left intact (minimal
diff).
## Type-only
All changes are structural-equivalent type aliases. No runtime behavior
changes. No new exports beyond the aliases themselves. No generated code
modified.
## Verification
- `pnpm typecheck` ✅
- `pnpm knip` ✅
- Scoped `npx eslint` on changed files ✅
- Lint-staged hooks (oxfmt, oxlint, eslint, typecheck) passed on every
commit
## Notes for reviewers
This branch was rebased onto `main` after FE-165 (`a441364a5`, PR
#11698) merged independently — the auto-skipped FE-165 commit is no
longer part of this PR. Six commits remain (oldest → newest):
| Commit | Maps to | Summary |
| --- | --- | --- |
| `e8e7ff795` | FE-166 | Add `GroupId` and `SlotIndex` aliases + barrel
re-exports |
| `e0bcb75a0` | FE-476 | Add `SubgraphId = UUID` alias |
| `2c136afb9` | FE-477 + FE-475 bulk | Add app-domain aliases;
mechanical adoption of
`NodeId`/`LinkId`/`RerouteId`/`GroupId`/`SlotIndex`/`ExecutionId`/`SubgraphId`
at audit-listed litegraph sites |
| `06d6e6a8b` | FE-477 | Adopt `TaskId` in asset stores |
| `f943e1c2b` | FE-476 | Adopt `SubgraphId` at remaining UUID reference
sites (`LGraphCanvas` clipboard map + paste, `SubgraphNode.type`) |
| `1739d5241` | review feedback | Tighten alias usage:
`linkExtensions.parentId: RerouteId`, drop redundant `String()` wraps in
`executionStore`, type `assetExportStore` map as `Map<TaskId,
AssetExport>` |
FE-475's mechanical adoption is bundled into `2c136afb9` rather than a
dedicated commit (parallel-agent execution on a shared working tree);
the substitutions themselves are complete — see the diff under
`src/lib/litegraph/src/`. PR will be squash-merged, so commit
granularity is informational.
Fixes FE-166
Fixes FE-475
Fixes FE-476
Fixes FE-477
---------
Co-authored-by: Amp <amp@ampcode.com>
|
||
|
DrJKL
|
36a9b6467c |
refactor(world): delete bridge, revert BaseWidget/useUpstreamValue
Phase 2b of temp/plans/world-consolidation.md (completes Strategy A bridge elimination begun in Phase 2a). Now that useWidgetValueStore is a World facade (Phase 2a), the bridge is redundant. Deletions: - src/world/widgetWorldBridge.ts + widgetWorldBridge.test.ts deleted. Their register/getNodeWidgets coverage rolled into widgetValueStore facade tests in Phase 2a; widgetParent tests already moved to src/stores/widgetComponents.test.ts in Phase 1. Reverts (back to pre-slice-1 baseline): - BaseWidget.setNodeId no longer double-writes via registerWidgetInWorld; the store IS the World writer now. Drops three @/world/* imports. - useUpstreamValue reads via useWidgetValueStore().getNodeWidgets(). Drops three @/world/* imports. Test updates: - useUpstreamValue.test.ts setup uses useWidgetValueStore().registerWidget instead of registerWidgetInWorld(getWorld(), ...). Hoists setActivePinia(createTestingPinia) + resetWorldInstance into beforeEach. - widgetComponents.test.ts setup inlines a 4-line widget registration to replace the deleted bridge import. - entityIds.ts: GraphId type un-exported (no external consumer; YAGNI; re-export when slice 2 needs it). End state: - src/world/ is pure substrate (5 source files: brand, entityIds, componentKey, world, worldInstance). No bridge, no components dir. - BaseWidget.ts byte-identical with pre-slice-1 form at the setNodeId seam. - useWidgetValueStore is the sole owner of widget value state; Vue tracking flows naturally through reactive(Map) inside the World. Verification: - pnpm typecheck, format:check, knip clean. - 52 tests pass across src/world, src/stores/widgetValueStore, src/stores/widgetComponents, src/composables/useUpstreamValue, src/lib/litegraph/src/widgets/BaseWidget. - rg "@/world" src/lib/litegraph/src/widgets/BaseWidget.ts returns 0. - rg "@/world" src/composables/useUpstreamValue.ts returns 0. - rg "registerWidgetInWorld|getNodeWidgetsThroughWorld|widgetWorldBridge" src/ returns 0. Combined Phase 2 non-test diff -53 LOC (under 150 LOC budget). BaseWidget._state shared reactive identity contract preserved end-to-end. Amp-Thread-ID: https://ampcode.com/threads/T-019dd146-a3ad-734d-9825-0ab356454dd5 Co-authored-by: Amp <amp@ampcode.com> |
||
|
Alexander Brown
|
95e9a9405b |
test(subgraph): pin #10849 promoted-widget-value corruption with it.fails (#11697)
*PR Created by the Glary-Bot Agent* --- ## Summary #11579 restored *categorical* test coverage for subgraph serialization but didn't reproduce the specific Z-Image-Turbo regression introduced by #10849 — pre-#10849 templates whose `widgets_values` is leftover noise get corrupted on load because the new code applies that array positionally to promoted widget views. This PR adds two **vitest** cases that pin the user-visible symptom directly: after loading a misaligned legacy payload, the promoted widget value should reflect the source default, not the legacy `widgets_values[i]`. Both use `it.fails` so the suite stays green while the bug is present and flips to failing the moment the fix on `fix/subgraph-promoted-widget-inline-state` lands. ## Tests 1. `falls back to source widget value when proxyWidgets is in legacy 2-tuple shape` — configure() with `proxyWidgets: [['-1', 'widget']]` + `widgets_values: [999]` should leave the widget at the source default (42), not 999. 2. `does not corrupt unbound promoted widgets when widgets_values length mismatches view count` — same shape with longer/wrong-length array. ## Verification - All 8 cases in the file pass under `it.fails` (CI green). - Removing `.fails` locally produces the expected failures: `expected 42, received 999` and `expected 42, received 111` — confirming both tests catch the regression. - `pnpm typecheck`, `pnpm exec eslint`, `pnpm exec oxlint` all clean. ## Why `it.fails` and not plain failing tests The actual fix (`fix/subgraph-promoted-widget-inline-state`) is unmerged. Landing genuinely-failing tests on main would break CI for everyone. `it.fails` documents the bug runnably, keeps CI green, and signals when the fix lands so the marker can be dropped. ## Why these assertions, not "widgets_values must be undefined" A first draft asserted that `serialize()` should not write `widgets_values` at all. That conflicts with existing coverage in the same file (`preserves per-instance widget values after configure`, `round-trips per-instance widget values`) which deliberately uses `widgets_values` for round-trip persistence. These rewritten assertions target the load-time corruption symptom directly without contradicting the per-instance contract — feedback from Oracle review. ## Coordination Mirrors the failing tests already on commit `6a982675e` of the unmerged `fix/subgraph-promoted-widget-inline-state` branch, with the addition of `.fails` markers and a clarifying comment so they can land on main first. Follow-up to #11579. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11697-test-subgraph-pin-10849-promoted-widget-value-corruption-with-it-fails-34f6d73d365081d7a04dcf48ebeceafe) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
DrJKL
|
b35d1f3b58 |
feat(world): slice 1 - World substrate + WidgetValue bridge
Adds minimal ECS substrate (src/world/) per ADR 0008 and bridges into
useWidgetValueStore via widgetWorldBridge. setNodeId now writes the same
reactive _state into both the Pinia store and World, preserving shared
reactive identity for the 40+ extension ecosystem.
Subsystems added:
- src/world/{world,worldInstance,componentKey,brand,entityIds,worldIndex}
- src/world/components/{WidgetValue,WidgetContainer,WidgetIdentity,WidgetDisplayState,WidgetSchema}
- src/world/widgetWorldBridge
Consumers updated:
- BaseWidget.setNodeId: registers in World after store registration
- useUpstreamValue: reads via getNodeWidgetsThroughWorld
Cross-subgraph identity: widgetEntityId(rootGraphId, nodeId, name).
Amp-Thread-ID: https://ampcode.com/threads/T-019dd146-a3ad-734d-9825-0ab356454dd5
Co-authored-by: Amp <amp@ampcode.com>
|
||
|
Alexander Brown
|
a441364a55 |
refactor(litegraph): centralize _version counter via incrementVersion() (#11698)
## Summary
Centralize all `LGraph._version` increments behind a single
`incrementVersion()` method to create the seam for a future
`VersionSystem` (ECS Migration Phase 0a).
## Changes
- **What**: Added `LGraph.incrementVersion()` and replaced all 19 direct
`graph._version++` writes across 7 files. Existing null guards at call
sites are preserved. Zero behavioral change — the counter is still only
read by `LGraphCanvas.renderInfo()` for debug display.
## Review Focus
- The new method is mechanical: `incrementVersion(): void {
this._version++ }`. Look for any sites I missed or null-guard
regressions.
- Files updated: `LGraph.ts`, `LGraphNode.ts`, `LGraphCanvas.ts`,
`widgets/BaseWidget.ts`, `subgraph/SubgraphInput.ts`,
`subgraph/SubgraphInputNode.ts`, `subgraph/SubgraphOutput.ts`.
Part of the [ECS Migration
Plan](https://github.com/Comfy-Org/ComfyUI_frontend/blob/main/docs/architecture/ecs-migration-plan.md).
Linear: FE-165.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11698-refactor-litegraph-centralize-_version-counter-via-incrementVersion-34f6d73d3650810992f8fa3adbae3f38)
by [Unito](https://www.unito.io)
Co-authored-by: Amp <amp@ampcode.com>
|
||
|
pythongosssss
|
4c892341e4 |
feat: Node search UX updates (#9714)
## Summary Addresses feedback from the initial v2 node search implementation for improved UI and UX ## Changes - **What**: - add root filter buttons - remove all extra tree categories leaving only "Most relevant" - replace input/output selection with popover - replace price badge with one from node header - add chevrons and additional styling to category tree - hide empty categories - fix bug with hovering selecting item under mouse automatically - fix tailwind merge with custom sizes removing them - keyboard navigation - general tidy/refactor/test ## Screenshots (if applicable) https://github.com/user-attachments/assets/db798dfa-e248-4b48-bb56-2fa7b6c5f65f ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9714-feat-Node-search-UX-updates-31f6d73d365081cebd96c4253ad1ca53) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
jaeone94
|
b157182a20 |
refactor: inline node footer layout to fix selection bounding box (#10741)
## Summary Refactor node footer from absolute overlay to inline flow layout, fixing the selection bounding box not encompassing footer buttons and collapsed node dimensions. ## Background The node footer (Enter Subgraph, Advanced, Error buttons) was rendered as an absolute overlay (`absolute top-full`) outside the node body. This caused: 1. **Selection bounding box** did not include footer height — the dashed multi-select border cut through footer buttons 2. **Footer offset compensation** required 3 hardcoded computed classes (`footerStateOutlineBottomClass`, `footerRootBorderBottomClass`, `footerResizeHandleBottomClass`) with magic pixel values (31px, 35px, etc.) that had to stay in sync with CSS ## Solution: Inline Footer with `isolate -z-1` The footer is moved into normal document flow (no longer `absolute top-full`). The key challenge was keeping the footer visually behind the body's rounded bottom edge (the "tuck under" effect) without adding `z-index` to the body — because adding `z-index` to the body creates a stacking context that traps slot connection dots, making them appear behind overlay borders. The solution uses CSS `isolation: isolate` combined with `-z-1` on the footer wrapper: - **`isolate`** creates an independent stacking context for the footer, so internal z-index (Error button `z-10` above Enter button) does not leak to the parent - **`-z-1`** places the entire footer behind the body (`z-index: auto`), achieving the visual overlap without touching the body's stacking behavior - **Slot dots remain free** — the body has no explicit z-index, so slots participate in the root stacking context and are never trapped behind overlay borders This eliminates all 3 footer offset computed classes and their hardcoded pixel values. ## Selection Box: `min-height` on root + unified size path Moving `min-h-(--node-height)` from the body (`node-inner-wrapper`) to the root element makes the footer height naturally included in `node.size` via ResizeObserver → layoutStore → litegraph sync. This means `boundingRect` is automatically correct for expanded nodes — no callbacks or overrides needed. For collapsed nodes, a pre-existing issue (since v1.40) caused `_collapsed_width` to fall back to `NODE_COLLAPSED_WIDTH = 80px` because Vue nodes lack a canvas context for text measurement. The fix lets collapsed dimensions flow through the **same** `batchUpdateNodeBounds` path as expanded nodes — no parallel data structure, no separate accessor, no cache: 1. ResizeObserver writes the collapsed DOM dimensions to `layoutStore.size` via `batchUpdateNodeBounds` 2. `useLayoutSync` syncs `layoutStore.size` → `liteNode.size` as it does for any other size change 3. The expanded size survives the collapse→expand round trip via CSS custom properties — the `isCollapsed` watcher in `LGraphNode.vue` swaps `--node-width` to `--node-width-x` on collapse and restores it on expand 4. `measure()` reads `this.size` directly for Vue collapsed nodes via a one-line gate: `if (!this.flags?.collapsed || LiteGraph.vueNodesMode)`. Legacy behavior is unchanged. ## Changes - **NodeFooter.vue**: `absolute top-full` overlay → inline flow with `isolate -z-1` wrappers, Error/Enter button layering via `-mr-5` + DOM order, reactive props destructuring, static `RADIUS_CLASS` lookup for Tailwind scanning, Vue 3.3+ `defineEmits` property syntax - **LGraphNode.vue**: Move `min-h-(--node-height)` from body to root; remove `footerStateOutlineBottomClass`, `footerRootBorderBottomClass`, `footerResizeHandleBottomClass`, `hasFooter` computed; replace dynamic `beforeShapeClass` interpolation with static `bypassOverlayClass`/`mutedOverlayClass` computeds for Tailwind scanning - **LGraphNode.ts**: `measure()` collapsed branch gated by `|| LiteGraph.vueNodesMode` — Vue mode defers to `this.size`; legacy path unchanged - **useVueNodeResizeTracking.ts**: Collapsed and expanded nodes both flow through `batchUpdateNodeBounds`; narrowed `useVueElementTracking` parameter from `MaybeRefOrGetter<string>` to `string`; `deferredElements.delete(element)` on unmount to prevent memory retention - **selectionBorder.ts**: Unchanged — `createBounds` just works because `boundingRect` is now correct - **12 parameterized E2E tests**: Vue mode (subgraph/regular × expanded/collapsed × bottom-left/bottom-right) + legacy mode (expanded/collapsed × bottom-left/bottom-right), driven by `keyboard.collapse()` (Alt+C) - **Unit tests**: `measure()` branching (legacy fallback, Vue `this.size` usage, expanded parity) - **Shared test helpers**: `repositionNodes`, `KeyboardHelper.collapse`, `measureSelectionBounds`, `assertSelectionEncompassesNodes` ## Review Focus - `isolate -z-1` CSS layering pattern — is this acceptable long-term? - `measure()` collapsed branch gated on `LiteGraph.vueNodesMode` — one-line gate to avoid the canvas-ctx-less fallback in Vue mode - Footer button overlap design (`-mr-5` with DOM order for painting) ## Screenshots <img width="1392" height="800" alt="image" src="https://github.com/user-attachments/assets/abaebff5-bb8c-4b5b-8734-8d44fdee4cb9" /> <img width="1493" height="872" alt="image" src="https://github.com/user-attachments/assets/6b9c77f9-e3ae-4d4e-81dc-acfa9a24c768" /> <img width="813" height="515" alt="image" src="https://github.com/user-attachments/assets/ce15bafb-e157-408c-971b-a650088f316a" /> <img width="1031" height="669" alt="image" src="https://github.com/user-attachments/assets/20fdc336-4bc2-4d47-ab7e-c0cbcee0d150" /> <img width="753" height="525" alt="image" src="https://github.com/user-attachments/assets/2dccbe31-7d18-49bc-9ed4-158b1659fddf" /> <img width="730" height="370" alt="image" src="https://github.com/user-attachments/assets/ab87edfa-a4b4-46f7-86ae-4965a4509b42" /> <img width="1132" height="465" alt="image" src="https://github.com/user-attachments/assets/54643f5b-4a31-4c3d-9475-c433f87aedb0" /> <img width="1102" height="449" alt="image" src="https://github.com/user-attachments/assets/9c045df3-e1f5-481e-b1cb-ead1db1626f5" /> --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
AustinMroz
|
e28c1e7e43 |
Show multitype slices of shared color (#11250)
A tiny update requested by the backend team. Previously, multitype slot indicators would have inputs that resolve to the same connection color display be combined into a single slice. For example, both `INT` and `FLOAT` have the same color, so an `INT,FLOAT` slot displays as a solid color instead of 2 semi-circles. This was a conscientious decision to improve readability on slots that allow many types, but meant that the more common cases (like `INT,FLOAT`) would have no indicator at all. Since priority is given to types based on order of listing, node authors can still control which types are elided on a slot accepting many types. <img width="430" height="320" alt="image" src="https://github.com/user-attachments/assets/1fc7fb1c-a634-487c-bc03-711637aeef13" /> - I do not believe there are any core nodes affected by this change. - The vue implementation of merging slot colors never functioned properly, but is still removed. - Vue was bugged to incorrectly pass slot types for widgets. This is also fixed. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11250-Show-multitype-slices-of-shared-color-3436d73d365081b6b484ea74423435a1) 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> |
||
|
Dante
|
e729e5edb8 |
fix: place cloned node above original in Vue renderer (#10361)
## Summary Cloned/pasted nodes in Node 2.0 (Vue renderer) mode now appear above the original node instead of behind it. ## Root Cause The legacy LiteGraph canvas renderer uses array ordering for z-ordering: nodes are stored in `graph._nodes` and drawn sequentially, so newly added nodes (appended to the end) are automatically drawn on top. There is no explicit z-index. The Vue renderer (Node 2.0) uses explicit CSS `z-index` for node ordering. New nodes default to `zIndex: 0` in `layoutMutations.ts`. When a node has been interacted with, `bringNodeToFront` raises its z-index. A cloned node at z-index 0 therefore appears behind any previously interacted node. The alt-click clone path in `LGraphNode.vue` already handles this correctly by calling `bringNodeToFront()` after cloning. However, the menu clone and keyboard paste paths go through `_deserializeItems` in `LGraphCanvas.ts`, which does not set z-index for new nodes. | Clone method | Legacy renderer | Vue renderer (before fix) | Vue renderer (after fix) | |---|---|---|---| | Alt-click drag | On top (array order) | On top (`bringNodeToFront` called) | On top | | Right-click menu Clone | On top (array order) | Behind original (z-index 0) | On top | | Ctrl+C / Ctrl+V | On top (array order) | Behind original (z-index 0) | On top | ## Steps to Reproduce 1. Enable Node 2.0 mode (Vue renderer) in settings 2. Add any node to the canvas 3. Click or drag the node (raises its z-index via `bringNodeToFront`) 4. Right-click the node and select "Clone" 5. **Expected**: Cloned node appears above the original, immediately draggable 6. **Actual**: Cloned node appears behind the original; user must move the original to access the clone ## Changes After `batchUpdateNodeBounds` in `_deserializeItems`, calls `bringNodeToFront` for each newly created node so they receive a z-index above all existing nodes. ## Side Effect Analysis Checked all call sites of `_deserializeItems`: 1. **Initial graph load / workflow open**: `loadGraphData` in `app.ts` does NOT call `_deserializeItems`. Workflow loading goes through `LGraph.configure()` which directly adds nodes and links. The layout store is initialized separately via `initializeFromLiteGraph`. No side effect. 2. **Paste from clipboard (Ctrl+V)**: Both `usePaste.ts` (line 52) and `pasteFromClipboard` (line 4080) call `_deserializeItems`. Pasted nodes appearing on top is the correct and desired behavior. No issue. 3. **Undo/Redo**: `ChangeTracker.updateState()` calls `app.loadGraphData()`, which does a full graph reconfigure -- it does NOT go through `_deserializeItems`. No side effect. 4. **Subgraph blueprint addition**: `litegraphService.ts` (line 906) calls `_deserializeItems` when adding subgraph blueprints from the node library. These are freshly placed nodes that should appear on top. Desired behavior. 5. **Alt-click clone in LGraphNode.vue**: This path calls `LGraphCanvas.cloneNodes()` -> `_deserializeItems()`, then separately calls `bringNodeToFront()` again on line 433 of `LGraphNode.vue`. The second call is now redundant (the node is already at max z-index), but harmless -- `bringNodeToFront` finds the current max, adds 1, and sets. The z-index will increment from N to N+1 on the second call. This is a minor redundancy, not a bug. 6. **Performance**: `bringNodeToFront` iterates all nodes in the layout store once per call (O(m)) to find max z-index. For n new nodes, the total cost is O(n*m). In practice, clone/paste operations involve a small number of nodes (typically 1-10), so this is negligible. For extremely large pastes (100+ nodes), each call also increments the max by 1, so z-indices will be sequential (which is actually a reasonable stacking order). 7. **layoutStore availability**: `layoutStore` is a module-level singleton (`new LayoutStoreImpl()`) -- not a Pinia store -- so it is always available. The `useLayoutMutations()` composable is a plain function returning an object of closures over `layoutStore`. It does not require Vue component context. No risk of runtime errors. 8. **Legacy renderer (non-Vue mode)**: When Node 2.0 mode is disabled, the layout store still exists but is not used for rendering. Calling `bringNodeToFront` will update z-index values in the Yjs document that are never read. This is harmless. ## Red-Green Verification | Commit | Result | Description | |---|---|---| | `6894b99` `test:` | RED | Test asserts cloned node z-index > original. Fails with `expected 0 to be greater than 5`. | | `3567469` `fix:` | GREEN | Calls `bringNodeToFront` for each new node in `_deserializeItems`. Test passes. | Fixes #10307 --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Terry Jia
|
63eab15c4f |
Range editor (#10936)
BE change https://github.com/Comfy-Org/ComfyUI/pull/13322 ## Summary Add RANGE widget for image levels adjustment - Add RangeEditor widget with three display modes: plain, gradient, and histogram - Support optional midpoint (gamma) control for non-linear midtone adjustment - Integrate histogram display from upstream node outputs ## Screenshots (if applicable) <img width="1450" height="715" alt="image" src="https://github.com/user-attachments/assets/864976af-9eb7-4dd0-9ce1-2f5d7f003117" /> <img width="1431" height="701" alt="image" src="https://github.com/user-attachments/assets/7ee2af65-f87a-407b-8bf2-6ec59a1dff59" /> <img width="705" height="822" alt="image" src="https://github.com/user-attachments/assets/7bcb8f17-795f-498a-9f8a-076ed6c05a98" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10936-Range-editor-33b6d73d365081089e8be040b40f6c8a) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
25d1ac7456 |
test: reorganize subgraph test suite into composable domain specs (#10759)
## Summary Reorganize the subgraph test suite so browser tests are thin representative user journeys while lower-level Vitest suites own combinatorics, migration edge cases, and data-shape semantics. ## Changes - **What**: Migrate 17 flat subgraph browser specs into 10 domain-organized specs under `browser_tests/tests/subgraph/`, move redundant semantic coverage down to 8 Vitest owner suites, delete all legacy flat files - **Browser specs** (54 tests): `subgraphSlots`, `subgraphPromotion`, `subgraphPromotionDom`, `subgraphSerialization`, `subgraphNavigation`, `subgraphNested`, `subgraphLifecycle`, `subgraphCrud`, `subgraphSearch`, `subgraphOperations` - **Vitest owners** (230 tests): `SubgraphNode.test.ts` (rename/label propagation), `subgraphNodePromotion.test.ts`, `promotedWidgetView.test.ts`, `SubgraphSerialization.test.ts` (duplicate-ID remap), `SubgraphWidgetPromotion.test.ts` (legacy hydration), `subgraphNavigationStore*.test.ts` (viewport cache, workflow-switch), `subgraphStore.test.ts` (search aliases, description) - **Net effect**: browser suite shrinks from ~96 scattered tests to 54 focused journeys ## Review Focus - Coverage ownership split: each browser test has a unique UI-only failure mode; semantic coverage lives in Vitest - `subgraphPromotionDom.spec.ts` forces LiteGraph mode and uses `canvas.openSubgraph()` instead of `navigateIntoSubgraph()` to avoid a wrapper-specific DOM overlay duplication issue — entry-affordance coverage lives in `subgraphNavigation.spec.ts` - No product code changes — test-only migration ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10759-test-reorganize-subgraph-test-suite-into-composable-domain-specs-3336d73d365081b0a56bcbf809b1f584) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
pythongosssss
|
0b83926c3e |
fix: Ensure zero uuid root graphs get assigned a valid id (#10825)
## Summary Fixes an issue where handlers would be leaked causing Vue node rendering to be corrupted (Vue nodes would not render) due to the 00000000-0000-0000-0000-000000000000 ID being used on the root graph. ## Changes - **What**: - LGraph clear() skips store cleanup for the zero uuid, leaking handlers that cause the node manager/handlers to be overwritten during operations such as undo due to stale onNodeAdded hooks - Ensures that graph configuration assigns a valid ID for root graphs ## Screenshots (if applicable) Before fix, after doing ctrl+z after entering subgraph <img width="1011" height="574" alt="image" src="https://github.com/user-attachments/assets/1ff4692b-b961-4777-bf2d-9b981e311f91" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10825-fix-Ensure-zero-uuid-root-graphs-get-assigned-a-valid-id-3366d73d3650817d8603c71ffb5e5742) by [Unito](https://www.unito.io) --------- Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Alexander Brown
|
661e3d7949 |
test: migrate as unknown as to @total-typescript/shoehorn (#10761)
*PR Created by the Glary-Bot Agent* --- ## Summary - Replace all `as unknown as Type` assertions in 59 unit test files with type-safe `@total-typescript/shoehorn` functions - Use `fromPartial<Type>()` for partial mock objects where deep-partial type-checks (21 files) - Use `fromAny<Type>()` for fundamentally incompatible types: null, undefined, primitives, variables, class expressions, and mocks with test-specific extra properties that `PartialDeepObject` rejects (remaining files) - All explicit type parameters preserved so TypeScript return types are correct - Browser test `.spec.ts` files excluded (shoehorn unavailable in `page.evaluate` browser context) ## Verification - `pnpm typecheck` ✅ - `pnpm lint` ✅ - `pnpm format` ✅ - Pre-commit hooks passed (format + oxlint + eslint + typecheck) - Migrated test files verified passing (ran representative subset) - No test behavior changes — only type assertion syntax changed - No UI changes — screenshots not applicable ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10761-test-migrate-as-unknown-as-to-total-typescript-shoehorn-3336d73d365081f6b8adc44db5dcc380) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: Amp <amp@ampcode.com> |
||
|
Christian Byrne
|
dc09eb60e4 |
fix: add deprecation warning for widget.inputEl on STRING multiline widgets (#9808)
## Summary Add a deprecation warning when custom nodes access `widget.inputEl` on STRING multiline widgets, directing them to use `widget.element` instead. ## Changes - **What**: Add a reusable `defineDeprecatedProperty` helper in `feedback.ts` that creates an ODP getter/setter proxy from a deprecated property to its replacement, logging via the existing `warnDeprecated` utility (deduplicates: warns once per unique message per session). Use it to deprecate `widget.inputEl` → `widget.element`. ## Review Focus - `defineDeprecatedProperty` is generic and can be reused for future property deprecations across the codebase. - `warnDeprecated` already handles deduplication via a `Set`, so heavy access patterns (e.g. custom nodes reading `widget.inputEl` in tight loops) won't spam. - `enumerable: false` keeps the deprecated alias out of `Object.keys()` / `for...in` / `JSON.stringify`. Fixes Comfy-Org/ComfyUI#12893 <!-- Pipeline-Ticket: 6b291ba2-694c-42d6-ac0c-fcbdcba9373a --> --------- Co-authored-by: Dante <bunggl@naver.com> |
||
|
Alexander Brown
|
5b4ebf4d99 |
test: audit skipped tests — prune stale, re-enable stable, remove dead code (#10312)
## Summary Audit all skipped/fixme tests: delete stale tests whose underlying features were removed, re-enable tests that pass with minimal fixes, and remove orphaned production code that only the deleted tests exercised. Net result: **−2,350 lines** across 50 files. ## Changes - **Pruned stale skipped tests** (entire files deleted): - `LGraph.configure.test.ts`, `LGraph.constructor.test.ts` — tested removed LGraph constructor paths - `LGraphCanvas.ghostAutoPan.test.ts`, `LGraphCanvas.linkDragAutoPan.test.ts`, `useAutoPan.test.ts`, `useSlotLinkInteraction.autoPan.test.ts` — tested removed auto-pan feature - `useNodePointerInteractions.test.ts` — single skipped test for removed callback - `ImageLightbox.test.ts` — component replaced by `MediaLightbox` - `appModeWidgetRename.spec.ts` (E2E) — feature removed; helper `AppModeHelper.ts` also deleted - `domWidget.spec.ts`, `widget.spec.ts` (E2E) — tested removed widget behavior - **Removed orphaned production code** surfaced by test pruning: - `useAutoPan.ts` — composable + 93 lines of auto-pan logic in `LGraphCanvas.ts` - `ImageLightbox.vue` — replaced by `MediaLightbox` - Auto-pan integration in `useSlotLinkInteraction.ts` and `useNodeDrag.ts` - Dead settings (`LinkSnapping.AutoPanSpeed`, `LinkSnapping.AutoPanMargin`) in `coreSettings.ts` and `useLitegraphSettings.ts` - Unused subgraph methods (`SubgraphNode.getExposedInput`, `SubgraphInput.getParentInput`) - Dead i18n key, dead API schema field, dead fixture exports (`dirtyTest`, `basicSerialisableGraph`) - Dead test utility `litegraphTestUtils.ts` - **Re-enabled skipped tests with minimal fixes**: - `useBrowserTabTitle.test.ts` — removed skip, test passes as-is - `eventUtils.test.ts` — replaced MSW dependency with direct `fetch` mock - `SubscriptionPanel.test.ts` — stabilized button selectors, timezone-safe date assertion - `LinkConnector.test.ts` — removed stale describe blocks, kept passing suite - `widgetUtil.test.ts` — removed skipped tests for deleted functionality - `comfyManagerStore.test.ts` — removed skipped `isPackInstalling` / `action buttons` / `loading states` blocks - **Re-enabled then re-skipped 3 flaky E2E tests** (fail in CI for pre-existing reasons): - `browserTabTitle.spec.ts` — canvas click timeout (element not visible) - `groupNode.spec.ts` — screenshot diff (stale golden image) - `nodeSearchBox.spec.ts` — `p-dialog-mask` intercepts pointer events - **Simplified production code** alongside test cleanup: - `useNodeDrag.ts` — removed auto-pan integration, simplified from 170→100 lines - `DropZone.vue` — refactored URL-drop handling, removed unused code path - `ToInputFromIoNodeLink.ts`, `SubgraphInputEventMap.ts` — removed dead subgraph wiring - **Dependencies**: none - **Breaking**: none (all removed code was internal/unused) ## Review Focus - Confirm deleted production code (`useAutoPan`, `ImageLightbox`, subgraph methods) has no remaining callers - Validate that simplified `useNodeDrag.ts` preserves drag behavior without auto-pan - Check that re-skipped E2E tests have clear skip reasons for future triage ## Screenshots (if applicable) N/A --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: github-actions <github-actions@github.com> |
||
|
Alexander Brown
|
68d47af075 |
fix: normalize legacy prefixed proxyWidget entries on configure (#10573)
## Summary Normalize legacy prefixed proxyWidget entries during subgraph configure so nested subgraph widgets resolve correctly. ## Changes - **What**: Extract `normalizeLegacyProxyWidgetEntry` to strip legacy `nodeId: innerNodeId: widgetName` prefixes from serialized proxyWidgets and resolve the correct `disambiguatingSourceNodeId`. Write-back comparison now checks serialized content (not just array length) so stale formats are cleaned up even when the entry count is unchanged. ## Review Focus - The iterative prefix-stripping loop in `resolveLegacyPrefixedEntry` — it peels one `N: ` prefix per iteration and tries all disambiguator candidates at each level. - The write-back condition change from length comparison to `JSON.stringify` equality. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10573-fix-normalize-legacy-prefixed-proxyWidget-entries-on-configure-32f6d73d365081e886e1c9b3939e3b9f) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
Alexander Brown
|
d940ea76ee |
fix: repoint ancestor promoted widget bindings when packing nested subgraphs (#10532)
## Summary Packing nodes inside a subgraph into a nested subgraph no longer blanks the parent subgraph node's promoted widget values. ## Changes - **What**: After `convertToSubgraph` moves interior nodes into a nested subgraph, `_repointAncestorPromotions` rewrites the promotion store entries on all host SubgraphNodes so they chain through the new nested node. `rebuildInputWidgetBindings()` then clears the stale `input._widget` PromotedWidgetView cache and re-resolves bindings from current connections. - The root cause was two separate sets of PromotedWidgetView references: `node.widgets` (rebuilt from the store — correct) vs `input._widget` (cached at promotion time — stale). `SubgraphNode.serialize()` reads `input._widget.value`, which resolved against removed node IDs → `missing-node` → blank values on the next `checkState` cycle. ## Review Focus - `_repointAncestorPromotions` iterates all graphs to find host nodes of the current subgraph type — verify this covers all cases (multiple instances of the same subgraph type). - `rebuildInputWidgetBindings()` clears `_promotedViewManager` and re-resolves — confirm no side effects on event listeners or pending promotions. - The nested node gets duplicate promotion entries (from both `_repointAncestorPromotions` and `promoteRecommendedWidgets` via the `subgraph-converted` event). `store.promote()` deduplicates via `isPromoted`, but worth verifying. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10532-fix-repoint-ancestor-promoted-widget-bindings-when-packing-nested-subgraphs-32e6d73d365081109d5aea0660434082) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Benjamin Lu <benjaminlu1107@gmail.com> Co-authored-by: Christian Byrne <cbyrne@comfy.org> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Comfy Org PR Bot <snomiao+comfy-pr@gmail.com> Co-authored-by: christian-byrne <72887196+christian-byrne@users.noreply.github.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Co-authored-by: Yourz <crazilou@vip.qq.com> |
||
|
Christian Byrne
|
f56abb3ecf |
test: add regression tests for subgraph slot label propagation (#10013)
## Summary Add regression tests for subgraph slot label propagation. The OutputSlot.vue fix (adding `slotData.label` to the display template) was already merged via another PR — this adds tests to prevent future regressions. ## Changes - **What**: Two new test files covering the label/localized_name fallback chain in OutputSlot.vue and SubgraphNode label propagation through configure() and rename event paths. ## Review Focus Tests only — no production code changes. Verifies that renamed subgraph inputs/outputs display correctly in Nodes 2.0 mode. Fixes #9998 <!-- Pipeline-Ticket: 7d887122-eea5-45f1-b6eb-aed94f708555 --> |
||
|
Alexander Brown
|
08ea013c51 |
fix: prune stale proxyWidgets referencing nodes removed by nested subgraph packing (#10390)
## Summary Prune stale proxyWidgets entries that reference grandchild nodes no longer present in the outer subgraph after nested packing. ## Changes - **What**: Filter out proxyWidgets entries during hydration when the source node doesn't exist in the subgraph. Also skip missing-node entries in `_pruneStaleAliasFallbackEntries` as defense-in-depth. Write back cleaned entries so stale data doesn't persist. ## Review Focus The fix touches two codepaths in `SubgraphNode.ts`: 1. **Hydration** (`_internalConfigureAfterSlots`): Added `getNodeById` guard before accepting a proxyWidget entry, and broadened the write-back condition from legacy-only to any filtered entries. 2. **Runtime pruning** (`_pruneStaleAliasFallbackEntries`): Added early-exit for entries whose source node no longer exists — previously these survived because failed resolution returned `undefined` which bypassed the concrete-key comparison. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10390-fix-prune-stale-proxyWidgets-referencing-nodes-removed-by-nested-subgraph-packing-32b6d73d365081e69eedcb2b67d7043d) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
Christian Byrne
|
5f14276159 |
[feat] Improve group title layout (#9839)
Rebased and adopted from #5774 by @felixturner. ## Changes - Remove unused font-size properties (`NODE_TEXT_SIZE`, `NODE_SUBTEXT_SIZE`, `DEFAULT_GROUP_FONT`) from theme palettes and color palette schema - Replace `DEFAULT_GROUP_FONT`/`DEFAULT_GROUP_FONT_SIZE` with a single `GROUP_TEXT_SIZE = 20` constant (reduced from 24px) - Use `NODE_TITLE_HEIGHT` for group header height instead of `font_size * 1.4` - Vertically center group title text using `textBaseline = 'middle'` - Use `GROUP_TEXT_SIZE` directly in TitleEditor instead of per-group `font_size` - Remove `font_size` from group serialization (no longer per-group configurable) ## Original PR Closes #5774 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9839-feat-Improve-group-title-layout-3216d73d36508112a0edc2a370af20ba) by [Unito](https://www.unito.io) --------- Co-authored-by: Felix Turner <felixturner@gmail.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
Arthur R Longbottom
|
657ae6a6c3 |
fix: subgraph promoted widget input label rename (#10195)
## Summary Promoted primitive subgraph inputs (String, Int) render their link anchor at the header position instead of the widget row. Renaming subgraph input labels breaks the match entirely, causing connections to detach from their widgets visually. ## Changes - **What**: Fix widget-input slot positioning for promoted subgraph inputs in both LiteGraph and Vue (Nodes 2.0) rendering modes - `_arrangeWidgetInputSlots`: Removed Vue mode branch that skipped setting `input.pos`. Promoted widget inputs aren't rendered as `<InputSlot>` Vue components (NodeSlots filters them out), so `input.pos` is the only position fallback - `drawConnections`: Added pre-pass to arrange nodes with unpositioned widget-input slots before link rendering. The background canvas renders before the foreground canvas calls `arrange()`, so positions weren't set on the first frame - `SubgraphNode`: Sync `input.widget.name` with the display name on label rename and initial setup. The `IWidgetLocator` name diverged from `PromotedWidgetView.name` after rename, breaking all name-based slot↔widget matching (`_arrangeWidgetInputSlots`, `getWidgetFromSlot`, `getSlotFromWidget`) ## Review Focus - The `_arrangeWidgetInputSlots` rewrite iterates `_concreteInputs` directly instead of building a spread-copy map — simpler and avoids the stale index issue - `input.widget.name` is now kept in sync with the display name (`input.label ?? subgraphInput.name`). This is a semantic shift from using the raw internal name, but it's required for all name-based matching to work after renames. The value is overwritten on deserialize by `_setWidget` anyway - The `_widget` fallback in `_arrangeWidgetInputSlots` is a safety net for edge cases where the name still doesn't match (e.g., stale cache) Fixes #9998 ## Screenshots <img width="847" height="476" alt="Screenshot 2026-03-17 at 3 05 32 PM" src="https://github.com/user-attachments/assets/38f10563-f0bc-44dd-a1a5-f4a7832575d0" /> <img width="804" height="471" alt="Screenshot 2026-03-17 at 3 05 23 PM" src="https://github.com/user-attachments/assets/3237a7ee-f3e5-4084-b330-371def3415bd" /> <img width="974" height="571" alt="Screenshot 2026-03-17 at 3 05 16 PM" src="https://github.com/user-attachments/assets/cafdca46-8d9b-40e1-8561-02cbb25ee8f2" /> <img width="967" height="558" alt="Screenshot 2026-03-17 at 3 05 06 PM" src="https://github.com/user-attachments/assets/fc03ce43-906c-474d-b3bc-ddf08eb37c75" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10195-fix-subgraph-promoted-widget-input-slot-positions-after-label-rename-3266d73d365081dfa623dd94dd87c718) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: jaeone94 <jaeone.prt@gmail.com> |
||
|
AustinMroz
|
860d049487 |
Fix snap offset for reroutes and subgraph IO (#10229)
When snapping to grid, reroutes and subgraph IO would snap at an awful y offset. | Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/285cfd52-2242-4242-b031-926677575542" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/4920ef9b-8eb1-4330-8eea-9992ad662018" />| Regretfully, the PR leaves more magic constants than I would like. There's a hardcoded `0.7` multiplier on `NODE_SLOT_HEIGHT` that isn't defined as a constant, and it seems circular imports prevent constants being used to declare the subgraphIO `roundedRadius` See #8838 (Sorry for the delay. This change wound up being real simple) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10229-Fix-snap-offset-for-reroutes-and-subgraph-IO-3276d73d365081c2866dc388898a1be4) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
cc0ba2d471 | refactor: extract helpers from _removeDuplicateLinks and add integration tests (#10332) | ||
|
Alexander Brown
|
4d57c41fdb |
test: subgraph integration contracts and expanded Playwright coverage (#10123)
## Summary Add integration contract tests (unit) and expanded Playwright coverage for subgraph promotion, hydration, navigation, and lifecycle edge behaviors. ## Changes - **What**: 22 unit/integration tests across 9 files covering promotion store sync, widget view lifecycle, input link resolution, pseudo-widget cache, navigation viewport restore, and subgraph operations. 13 Playwright E2E tests covering proxyWidgets hydration stability, promoted source removal cleanup, pseudo-preview unpack/remove, multi-link representative round-trip, nested promotion retarget, and navigation state on workflow switch. - **Helpers**: Added `isPseudoPreviewEntry`, `getPseudoPreviewWidgets`, `getNonPreviewPromotedWidgets` to promotedWidgets helper. Added `SubgraphHelper.getNodeCount()`. ## Review Focus - Test-only PR — no production code changes - Validates existing subgraph behaviors are covered by regression tests before further feature work - Phase 4 (unit/integration contracts) and Phase 5 (Playwright expansion) of the subgraph test coverage plan ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10123-test-subgraph-integration-contracts-and-expanded-Playwright-coverage-3256d73d365081258023e3a763859e00) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Alexander Brown
|
28a91fa8b6 |
fix: configure nested subgraph definitions in dependency order (#10314)
## Summary Fix workflow loading for nested subgraphs with duplicate node IDs by configuring subgraph definitions in topological (leaf-first) order. ## Changes - **What**: Three pre-existing bugs that surface when loading nested subgraphs with colliding node IDs: 1. Subgraph definitions configured in serialization order — a parent subgraph's `SubgraphNode.configure` would run before its referenced child subgraph was populated, causing link/widget resolution failures. 2. `_resolveLegacyEntry` returned `undefined` when `input._widget` wasn't set yet, instead of falling back to `resolveSubgraphInputTarget`. 3. `_removeDuplicateLinks` removed duplicate links without updating `SubgraphOutput.linkIds`, leaving stale references that broke prompt execution. - **What (housekeeping)**: Moved `subgraphDeduplication.ts` from `utils/` to `subgraph/` directory where it belongs. ## Review Focus - Topological sort correctness: Kahn's algorithm with edges from dependency→dependent ensures leaves configure first. Cycle fallback returns original order. - IO slot link repair: `_repairIOSlotLinkIds` runs after `_configureSubgraph` creates IO slots, patching any `linkIds` that point to links removed by `_removeDuplicateLinks` during `super.configure()`. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10314-fix-configure-nested-subgraph-definitions-in-dependency-order-3286d73d36508171b149e238b8de84c2) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
pythongosssss
|
be6c64c75b |
feat: Add edge panning to ghost node placement (#10308)
## Summary Enables edge panning when placing a ghost-node from the search ## Changes - **What**: - registers document level listeners so dragging over UI elements isnt blocked ## Screenshots (if applicable) https://github.com/user-attachments/assets/c3bda3c5-8255-4dda-a6be-6ef306af2244 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10308-feat-Add-edge-panning-to-ghost-node-placement-3286d73d36508151b645ec3e860bc017) by [Unito](https://www.unito.io) |
||
|
AustinMroz
|
157f0598d4 |
Display optional indicator on subgraphNode inputs (#8772)
Display an optional input on subgraphNodes when all links to the subgraph input are optional. <img width="1132" height="625" alt="image" src="https://github.com/user-attachments/assets/e87d9d33-4b95-4ec9-afb8-bc1fb8cc0638" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8772-Display-optional-indicator-on-subgraphNode-inputs-3036d73d365081cc83e8d6034302665f) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
pythongosssss
|
3ed88fbe68 |
Autopan canvas when dragging nodes/links to edges (#8773)
## Summary Adds autopanning so the canvas moves when you drag a node/link to the side of the canvas ## Changes - **What**: - adds autopan controller that runs on animation frame timer to check autopan speed - extracts updateNodePositions for reuse - specific handling for vue vs litegraph modes - adds max speed setting, allowing user to set 0 for disabling ## Screenshots (if applicable) https://github.com/user-attachments/assets/1290ae6d-b2f0-4d63-8955-39b933526874 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8773-Autopan-canvas-when-dragging-nodes-links-to-edges-3036d73d365081869a58ca5978f15f80) by [Unito](https://www.unito.io) --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> |
||
|
jaeone94
|
3591579141 |
fix: _removeDuplicateLinks incorrectly removes valid link when slot indices shift (#10289)
## Summary Fixes a regression introduced in v1.41.21 where `_removeDuplicateLinks()` (added by #9120 / backport #10045) incorrectly removes valid links during workflow loading when the target node has widget-to-input conversions that shift slot indices. - Fixes https://github.com/Comfy-Org/workflow_templates/issues/715 ## Root Cause The `_removeDuplicateLinks()` method added in #9120 uses `node.inputs[link.target_slot]` to determine which duplicate link to keep. However, `target_slot` is the slot index recorded at serialization time. During `LGraphNode.configure()`, the `onConnectionsChange` callback triggers widget-to-input conversions (e.g., KSamplerAdvanced converting `steps`, `cfg`, `start_at_step`, etc.), which inserts new entries into the `inputs` array. This shifts indices so that `node.inputs[target_slot]` no longer points to the expected input. **Concrete example with `video_wan2_2_14B_i2v.json`:** The Wan2.2 Image-to-Video subgraph contains a Switch node (id=120) connected to KSamplerAdvanced (id=85) cfg input. The serialized data has two links with the same connection tuple `(origin_id=120, origin_slot=0, target_id=85, target_slot=5)`: | Link ID | Connection | Status | |---------|-----------|--------| | 257 | 120:0 → 85:5 (FLOAT) | Orphaned duplicate (not referenced by any input.link) | | 276 | 120:0 → 85:5 (FLOAT) | Valid (referenced by node 85 input.link=276) | When `_removeDuplicateLinks()` runs after all nodes are configured: 1. KSamplerAdvanced is created with 4 default inputs, but after `configure()` with widget conversions, it has **13 inputs** (shifted indices) 2. The method checks `node.inputs[5].link` (target_slot=5 from the LLink), but index 5 is now a different input due to the shift 3. `node.inputs[5].link === null` → the method incorrectly decides link 276 is not referenced 4. **Link 276 (valid) is removed, link 257 (orphan) is kept** → connection lost This worked correctly in v1.41.20 because `_removeDuplicateLinks()` did not exist. ## Changes Replace the `target_slot`-based positional lookup with a full scan of the target node's inputs to find which duplicate link ID is actually referenced by `input.link`. Also repair `input.link` if it still points to a removed duplicate after cleanup. ## Test Plan - [x] Added regression test: shifted slot index scenario (widget-to-input conversion) - [x] Added regression test: `input.link` repair when pointing to removed duplicate - [x] Existing `_removeDuplicateLinks` tests pass (45/45) - [x] Full unit test suite passes (6885/6885) - [x] `pnpm typecheck` passes - [x] `pnpm lint` passes (0 errors) - [x] Manual verification: loaded `video_wan2_2_14B_i2v.json` in clean state — Switch→KSamplerAdvanced cfg link is now preserved - [ ] E2E testing is difficult for this fix since it requires a workflow with duplicate links in a subgraph containing nodes with widget-to-input conversions (e.g., KSamplerAdvanced). The specific conditions — duplicate LLink entries + slot index shift from widget conversion — are hard to set up in Playwright without a pre-crafted fixture workflow and backend node type registration. The unit tests cover the core logic directly. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10289-fix-_removeDuplicateLinks-incorrectly-removes-valid-link-when-slot-indices-shift-3286d73d36508140b053fd538163e383) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
f9b0f277bf |
fix: track nodePreviewImages in usePromotedPreviews (#10165)
## Summary The `computed` in `usePromotedPreviews` only tracked `nodeOutputs` as a reactive dependency. GLSL live previews (and other preview-only sources) write to `nodePreviewImages` instead of `nodeOutputs`, so promoted preview widgets on SubgraphNodes never re-evaluated when live previews updated. ## Changes **Production** (`usePromotedPreviews.ts` — 3-line fix): - Add `nodePreviewImages[locatorId]` as a second reactive dependency alongside `nodeOutputs[locatorId]` - Guard now passes when *either* source has data, not just `nodeOutputs` **Tests** (`usePromotedPreviews.test.ts`): - Add `nodePreviewImages` to mock store type and factory - Add `seedPreviewImages()` helper - Add `getNodeImageUrls.mockReset()` in `beforeEach` for proper test isolation - Two new test cases: - `returns preview when only nodePreviewImages exist (e.g. GLSL live preview)` - `recomputes when preview images are populated after first evaluation` - Clean up existing tests to use hoisted `getNodeImageUrls` mock directly instead of `vi.mocked(useNodeOutputStore().getNodeImageUrls)` ## What this supersedes This is a minimal re-implementation of #9461. That PR also modified `promotionStore.ts` with a `_version`/`_touch()` monotonic counter to manually force reactivity — that approach is dropped here as it is an anti-pattern (manually managing reactivity counters instead of using Vue's built-in reactivity system). The promotionStore changes were not needed for this fix. ## Related - Supersedes #9461 - Prerequisite work: #9198 (add GLSLShader to canvas image preview node types) - Upstream feature: #9201 (useGLSLPreview composable) - Adjacent: #9435 (centralize node image rendering state in NodeImageStore) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10165-fix-track-nodePreviewImages-in-usePromotedPreviews-for-GLSL-live-preview-propagation-3266d73d365081cd87d0d12c4c041907) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Alexander Brown
|
34a77e5016 |
test: harden subgraph test coverage and remove low-value tests (#9967)
## Summary Harden subgraph test coverage: remove low-value change-detector tests, consolidate fixtures, add behavioral coverage, and fix test infrastructure issues. Includes minor production code corrections discovered during test hardening. ## Changes - **What**: Comprehensive subgraph test suite overhaul across 6 phases - Removed change-detector tests and redundant assertions - Consolidated fixture helpers into `subgraphHelpers.ts` / `subgraphFixtures.ts` - Added Pinia initialization and fixture reset to all test files - Fixed barrel import violations (circular dependency prevention) - Added behavioral coverage for slot connections, events, edge cases - Added E2E helper and smoke test for subgraph promotion - Exported `SubgraphSlotBase` from litegraph barrel for test access - **Production code changes** (minor correctness fixes found during testing): - `resolveSubgraphInputLink.ts`: iterate forward (first-connected-wins) to match `_resolveLinkedPromotionBySubgraphInput` - `promotionSchema.ts`: return `[]` instead of throwing on invalid `proxyWidgets`; console.warn always (not DEV-only) - `LGraph.ts`: disconnect-after-veto ordering fix - `litegraph.ts`: barrel export swap for `SubgraphSlotBase` - **Stats**: 349 tests passing, 0 skipped across 26 test files ## Review Focus - Tests that merely asserted default property values were deleted (change detectors) - Fixture state is now reset via `resetSubgraphFixtureState()` in `beforeEach` - All imports use `@/lib/litegraph/src/litegraph` barrel to avoid circular deps - Production changes are small and directly motivated by test findings --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: bymyself <cbyrne@comfy.org> |
||
|
jaeone94
|
15442e7ff8 |
fix: prevent nested SubgraphNode input slots from doubling on reload (#10187)
## Summary - Fix nested SubgraphNode input slots doubling on each page reload - Root cause: during configure, `_configureSubgraph` recreates `SubgraphInput` objects with new references, and the `input-added` event handler used `===` identity check which failed for these new objects, causing `addInput()` to duplicate inputs - Add `id`-based fallback matching in the `input-added` handler and rebind `_subgraphSlot` with re-registered listeners ## Changes **`SubgraphNode.ts:614-622`**: Add UUID `id` fallback to the `===` reference check in the `input-added` event handler. When a stale reference is matched by id, call `_addSubgraphInputListeners()` to update `_subgraphSlot` and re-register listeners on the new `SubgraphInput` object. **`SubgraphNode.test.ts`**: 2 regression tests for nested subgraph reconfigure scenarios. ## Test plan - [x] Existing SubgraphNode tests pass (6 passed, 34 skipped) - [x] New tests verify inputs don't duplicate after single and repeated reconfigure cycles - [x] Manual: create a subgraph containing another subgraph node, save, reload — input slots should remain unchanged ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10187-fix-prevent-nested-SubgraphNode-input-slots-from-doubling-on-reload-3266d73d3650817286abea52365a626e) by [Unito](https://www.unito.io) |
||
|
Jukka Seppänen
|
0030cadba3 |
feat: resolveVirtualOutput for cross-subgraph virtual nodes (eg. Set/Get) (#10111)
## Summary Enable virtual nodes (e.g. Set/Get) to resolve their output source directly when the source lives in a different subgraph. ## Changes - **What**: Added optional resolveVirtualOutput method on LGraphNode and a new resolution path in ExecutableNodeDTO.resolveOutput that checks it before falling through to the existing getInputLink path. Includes unit tests for the three code paths (happy path, missing DTO, fallthrough). ## Review Focus - Fully backwards compatible — no existing node implements resolveVirtualOutput, so the new path is always skipped for current virtual nodes (Reroute, PrimitiveNode, etc.). <!-- If this PR fixes an issue, uncomment and update the line below --> <!-- Fixes #ISSUE_NUMBER --> ## Screenshots Simple example in actual use, combined with new changes in KJNodes allows using Get nodes inside subgraphs: <img width="2242" height="1434" alt="image" src="https://github.com/user-attachments/assets/cc940a95-e0bb-4adf-91b6-9adc43a74aa2" /> <img width="1436" height="440" alt="image" src="https://github.com/user-attachments/assets/62044af5-0d6e-4c4e-b34c-d33e85f2b969" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10111-feat-resolveVirtualOutput-for-cross-subgraph-virtual-nodes-eg-Set-Get-3256d73d3650816a9f20e28029561c58) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
918095f197 |
fix: prune orphaned SubgraphNode inputs after configure (#10020)
## Summary Prune orphaned inputs in `_internalConfigureAfterSlots()` to fix duplicate SubgraphNode inputs that accumulate on serialize-load cycles. ## Changes - **What**: After `_rebindInputSubgraphSlots()`, filter out inputs with no matching `_subgraphSlot`. This prevents `LGraphNode.configure()` `cloneObject` expansion from persisting stale duplicates. - Added 3 regression tests covering: corrupted serialized data, reconfigure round-trips, and serialization output. ## Review Focus The fix is a single `filter()` call. The existing `console.warn` guard at line ~976 (for inputs without `_subgraphSlot`) becomes dead code after this fix but is retained as defense-in-depth. Fixes #9977 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10020-fix-prune-orphaned-SubgraphNode-inputs-after-configure-3256d73d3650812e8cecf4a3c86f2c33) by [Unito](https://www.unito.io) |
||
|
Terry Jia
|
46d8567f10 |
feat: add linear interpolation type to CURVE widget (#10118)
## Summary
Change the CURVE widget value from CurvePoint[] to CurveData ({ points,
interpolation }) to support multiple interpolation types. Add a Select
dropdown in the widget UI for switching between Smooth (monotone cubic)
and Linear interpolation, with the SVG preview updating accordingly.
- Add CurveData type with CURVE_INTERPOLATIONS const enum
- Add createLinearInterpolator with piecewise linear + binary search
- Add createInterpolator factory dispatching by interpolation type
- Add isCurveData type guard in curveUtils
- Update ICurveWidget value type to CurveData
- Add interpolation prop to CurveEditor and useCurveEditor composable
- Linear mode generates direct M...L... SVG path (no sampling)
- Add i18n entries for interpolation labels
- Add unit tests for createLinearInterpolator
BE change is https://github.com/Comfy-Org/ComfyUI/pull/12757
## Screenshots (if applicable)
<img width="1437" height="670" alt="image"
src="https://github.com/user-attachments/assets/550aedec-e5da-425b-8233-86a4f28067fa"
/>
<img width="1445" height="648" alt="image"
src="https://github.com/user-attachments/assets/0a8dc654-3f92-4ca2-9fa2-c1fef3be6d66"
/>
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10118-feat-add-linear-interpolation-type-to-CURVE-widget-3256d73d36508185a86edf73bb555c51)
by [Unito](https://www.unito.io)
|
||
|
Alexander Brown
|
f0b91bdcfa |
fix: resolve all lint warnings (#9972)
Resolve all lint warnings (3 oxlint + 1 eslint). ## Changes - Replace `it.todo` with `it.skip` in subgraph tests (`warn-todo`) - Move `vi.mock` to top-level in `firebaseAuthStore.test.ts` (`hoisted-apis-on-top`) - Rename `DOMPurify` default import to `dompurify` (`no-named-as-default`) --- ### The Villager Who Ignored the Warnings Once there lived a villager whose compiler whispered of lint. "They are only *warnings*," she said, and went about her day. One warning became three. Three became thirty. The yellow text grew like ivy across the terminal, until no one could tell the warnings from the errors. One morning a real error appeared — a misplaced mock, a shadowed import — but nobody noticed, for the village had long since learned to stop reading. The build shipped. The users wept. And the warning, faithful to the last, sat quietly in the log where it had always been. *Moral: Today's warning is tomorrow's incident report.* ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9972-fix-resolve-all-lint-warnings-3246d73d3650810a89cde5d05e79d948) by [Unito](https://www.unito.io) Co-authored-by: Amp <amp@ampcode.com> |
||
|
Jukka Seppänen
|
a96c61d2c2 |
fix: LGraphGroup paste position (#9962)
## Summary Fix group paste position: groups now paste at the cursor location instead of on top of the original. ## Changes - **What**: Added LGraphGroup offset handling in _deserializeItems() position adjustment loop, matching existing LGraphNode and Reroute behavior. ## Screenshots Before: https://github.com/user-attachments/assets/e317af10-8009-4092-9d14-de79316cd853 After: https://github.com/user-attachments/assets/f4ffefd5-519a-4592-812c-c88e3b5940fd ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9962-fix-LGraphGroup-paste-position-3246d73d365081eea5b2e2507da861de) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
74a48ab2aa |
fix: stabilize subgraph promoted widget identity and rendering (#9896)
## Summary Fix subgraph promoted widget identity/rendering so on-node widgets stay correct through configure/hydration churn, duplicate names, and linked+independent coexistence. ## Changes - **Subgraph promotion reconciliation**: stabilize linked-entry identity by subgraph slot id, preserve deterministic linked representative selection, and prune stale alias/fallback entries without dropping legitimate independent promotions. - **Promoted view resolution**: bind slot mapping by promoted view object identity (`getSlotFromWidget` / `getWidgetFromSlot`) to avoid same-name collisions. - **On-node widget rendering**: harden `NodeWidgets` identity and dedup to avoid visual aliasing, prefer visible duplicates over hidden stale entries, include type/source execution identity, and avoid collapsing transient unresolved entries. - **Mapping correctness**: update `useGraphNodeManager` promoted source mapping to resolve by input target only when the promoted view is actually bound to that input. - **Subgraph input uniqueness**: ensure empty-slot promotion creates unique input names (`seed`, `seed_1`, etc.) for same-name multi-source promotions. - **Safety fix**: guard against undefined canvas in slot-link interaction. - **Tests/fixtures**: add focused regressions for fixture path `subgraph_complex_promotion_1`, linked+independent same-name cases, duplicate-name identity mapping, dedup behavior, and input-name uniqueness. ## Review Focus Validate behavior around transient configure/hydration states (`-1` id to concrete id), duplicate-name promotions, linked representative recovery, and that dedup never hides legitimate widgets while still removing true duplicates. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9896-fix-stabilize-subgraph-promoted-widget-identity-and-rendering-3226d73d365081c8a1e8d0a5a22e826d) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
woctordho
|
82556f02a9 |
fix: respect 'always snap to grid' when auto-scale layout from nodes 1.0 to 2.0 (#9332)
## Summary Previously when I switch from nodes 1.0 to 2.0, positions and sizes of nodes do not follow 'always snap to grid'. You can guess what a mess it is for people relying on snap to grid to retain sanity. This PR fixes it. ## Changes In `ensureCorrectLayoutScale`, we call `snapPoint` after the position and the size are updated. We also need to ensure that the snapped size is larger than the minimal size required by the content, so I've added 'ceil' mode to `snapPoint`, and the patch is larger than I thought first. I'd happily try out nodes 2.0 once this is addressed :) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9332-fix-respect-always-snap-to-grid-when-auto-scale-layout-from-nodes-1-0-to-2-0-3176d73d365081f5b6bcc035a8ffa648) by [Unito](https://www.unito.io) |
||
|
Arthur R Longbottom
|
5640eb7d92 |
fix: subgraph output slot labels not updating in v2 renderer (#9266)
## Summary Custom names set on subgraph output nodes are ignored in the v2 renderer — it always shows the data type name (e.g. "texts") instead of the user-defined label. Works correctly in v1. ## Changes - **What**: Made `outputs` in `extractVueNodeData` reactive via `shallowReactive` + `defineProperty` (matching the existing `inputs` pattern). Added a `node:slot-label:changed` graph trigger that `SubgraphNode` fires when input/output labels are renamed, so the Vue layer picks up the change. ## Review Focus - The `outputs` reactivity mirrors `inputs` exactly — same `shallowReactive` + setter pattern. The new trigger event forces `shallowReactive` to detect the deep property change by re-assigning the array. - Also handles input label renames for consistency, even though the current bug report is output-specific. ## Screenshots **v1 — output correctly shows custom label "output_text":** <img width="1076" height="628" alt="Screenshot 2026-02-26 at 4 43 00 PM" src="https://github.com/user-attachments/assets/b4d6ae4c-9970-4d99-a872-4ce1b28522f2" /> **v2 before fix — output shows type name "texts" instead of custom label:** <img width="808" height="298" alt="Screenshot 2026-02-26 at 4 43 30 PM" src="https://github.com/user-attachments/assets/cf06aa6c-6d4d-4be9-9bcd-dcc072ed1907" /> **v2 after fix — output correctly shows "output_text":** <img width="1013" height="292" alt="Screenshot 2026-02-26 at 5 14 44 PM" src="https://github.com/user-attachments/assets/3c43fa9b-0615-4758-bee6-be3481168675" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9266-fix-subgraph-output-slot-labels-not-updating-in-v2-renderer-3146d73d365081979327fd775a6ef62b) by [Unito](https://www.unito.io) |
||
|
Alexander Brown
|
1280d4110d |
fix: simplify ensureCorrectLayoutScale and fix link sync during Vue node drag (#9680)
## Summary Fix node layout drift from repeated `ensureCorrectLayoutScale` scaling, simplify it to a pure one-time normalizer, and fix links not following Vue nodes during drag. ## Changes - **What**: - `ensureCorrectLayoutScale` simplified to a one-time normalizer: unprojects legacy Vue-scaled coordinates back to canonical LiteGraph coordinates, marks the graph as corrected, and does nothing else. No longer touches the layout store, syncs reroutes, or changes canvas scale. - Removed no-op calls from `useVueNodeLifecycle.ts` (a renderer version string was passed where an `LGraph` was expected). - `layoutStore.finalizeOperation` now calls `notifyChange` synchronously instead of via `setTimeout`. This ensures `useLayoutSync`'s `onChange` callback pushes positions to LiteGraph `node.pos` and calls `canvas.setDirty()` within the same RAF frame as a drag update, fixing links not following Vue nodes during drag. - **Tests**: Added tests for `ensureCorrectLayoutScale` (idempotency, round-trip, unknown-renderer no-op) and `graphRenderTransform` (project/unproject round-trips, anchor caching). ## Review Focus - The `setTimeout(() => this.notifyChange(change), 0)` → `this.notifyChange(change)` change in `layoutStore.ts` is the key fix for the drag-link-sync bug. The listener (`useLayoutSync`) only writes to LiteGraph, not back to the layout store, so synchronous notification is safe. - `ensureCorrectLayoutScale` no longer has any side effects beyond normalizing coordinates and setting `workflowRendererVersion` metadata. --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Christian Byrne <cbyrne@comfy.org> Co-authored-by: jaeone94 <89377375+jaeone94@users.noreply.github.com> Co-authored-by: AustinMroz <austin@comfy.org> Co-authored-by: Hunter <huntcsg@users.noreply.github.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Christian Byrne
|
871715113c |
fix: detect and remove duplicate links in subgraph unpacking (#9120)
## Summary Fix duplicate LLink objects created during subgraph unpacking, where output.links contains multiple link IDs for the same connection but input.link only references one, leaving orphaned links. ## Changes - **What**: Three layers of defense against duplicate links: 1. **Serialization fix** (`slotUtils.ts`): Clone `output.links` array in `outputAsSerialisable` to prevent shared-reference mutation during serialization round-trips 2. **Self-healing** (`LGraph.ts`): `_removeDuplicateLinks()` sanitizes corrupted data during `configure()`, keeping the link referenced by `input.link` and removing orphaned duplicates from `output.links` and `_links` 3. **Unpack dedup** (`LGraph.ts`): Subgraph unpacking filters `newLinks` via a `seenLinks` Set before creating connections Runtime diagnostic logging via `graph.events` (no Sentry import in litegraph): - `_dupLinkIndex` Map for O(1) duplicate detection, only allocated when enabled - `_checkDuplicateLink()` called at the 3 link-creation sites (`connectSlots`, `SubgraphInput.connect`, `SubgraphOutput.connect`) - App layer listens for `diagnostic:duplicate-link` events and forwards to Sentry with rate-limiting (1 per key per 60s) ## Review Focus - The `_removeDuplicateLinks` strategy of keeping the link referenced by `input.link` and removing others from `output.links` + `_links` - The diagnostic index lifecycle: built on enable, updated on link create/remove, cleared on disable - Sentry integration in `app.ts` using the existing `graph.events` system to avoid coupling litegraph to Sentry ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9120-fix-detect-and-remove-duplicate-links-in-subgraph-unpacking-3106d73d3650815b995ddf8f41da67ae) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
a9f9afd062 |
fix: avoid forced layout in renderInfo by using canvas.height (#9304)
## What Replace `canvas.offsetHeight` with `canvas.height / devicePixelRatio` in `renderInfo` to avoid forced synchronous layout. ## Why `renderInfo` is called ~2,631 times in a typical session. Each call reads `this.canvas.offsetHeight`, which forces the browser to flush pending style/layout changes synchronously. With PrimeVue injecting styles dynamically and Vue patching the DOM, there are almost always pending mutations — converting every canvas-only `renderInfo` call into a forced layout. ## How `canvas.height` is the DPR-scaled internal resolution (set in `resizeCanvas` as `cssHeight * devicePixelRatio`). Dividing by `devicePixelRatio` yields the same CSS pixel value as `offsetHeight` without triggering layout. ## Verification - [x] Unit test: verifies `offsetHeight` is not accessed when y is provided - [x] Unit test: verifies fallback uses `canvas.height / devicePixelRatio` - [x] `pnpm typecheck` passes - [x] `pnpm lint` passes - [x] All litegraph tests pass (538 passed) ## Perf Impact Eliminates ~2,631 forced synchronous layouts per session from the canvas info panel. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9304-fix-avoid-forced-layout-in-renderInfo-by-using-canvas-height-3156d73d36508171973dda289b30d5ee) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Christian Byrne
|
79d0e6dc69 |
fix: cache ctx.measureText results to avoid redundant calls in draw loop (#9404)
## What Add a per-frame text measurement cache for all hot-path ctx.measureText() calls. ## Why drawTruncatingText() in BaseWidget calls ctx.measureText() per widget per frame with zero caching. For a 50-node workflow at 60fps: ~78,000-243,000 measureText calls/sec. Text labels rarely change between frames. ## How Global Map<string, number> cache keyed by font+text, cleared once per frame at the start of drawFrontCanvas(). Replaces direct ctx.measureText() calls in BaseWidget.drawTruncatingText, draw.ts truncateTextToWidth/drawTextInArea, LGraphBadge.getWidth, LGraphButton.getWidth, and textUtils.truncateText. ## Perf Impact Expected: ~95% reduction in measureText calls (only cache misses on first frame and value changes). Firefox has slower measureText than Chrome, so this disproportionately benefits Firefox. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9404-fix-cache-ctx-measureText-results-to-avoid-redundant-calls-in-draw-loop-31a6d73d3650814e9cdac16949c55cb7) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Christian Byrne
|
e119383072 |
feat: select group children on click (#9149)
## Summary Add a setting to select all children (nodes, reroutes, nested groups) when clicking a group on the canvas. ## Changes - **What**: New `LiteGraph.Group.SelectChildrenOnClick` boolean setting (default: `false`). When enabled, selecting a group cascades `select()` to all its `_children`, and deselecting cascades `deselect()`. Recursion handles nested groups naturally. No double-move risk — the drag handler already uses `skipChildren=true`. The setting is wired via `onChange` to `canvas.groupSelectChildren`, keeping litegraph free of platform imports. ## Review Focus - The select/deselect cascading in `LGraphCanvas.select()` / `deselect()` — verify no infinite recursion risk with deeply nested groups. - The `groupSelectChildren` property is set via the setting's `onChange` callback on `LGraphCanvas.active_canvas` — confirm this covers canvas re-creation scenarios. ## Screenshots (if applicable) N/A — behavioral change behind a setting toggle. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9149-feat-select-group-children-on-click-3116d73d365081a1a7b8c82dea95b242) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |