From d0846626cee772fd8922a0c85ec5768fd62a45a9 Mon Sep 17 00:00:00 2001 From: pbanddev <202373785+PBandDev@users.noreply.github.com> Date: Tue, 13 Jan 2026 17:00:30 -0600 Subject: [PATCH] Merge branch 'feature/organize-selection' --- CLAUDE.md | 24 +- src/consants.ts | 5 + src/debug.ts | 20 +- src/index.ts | 116 +++++- src/layout/index.ts | 5 +- src/layout/selected-groups.ts | 469 ++++++++++++++++++++++ src/layout/types.ts | 10 + tests/integration/selected-groups.test.ts | 409 +++++++++++++++++++ 8 files changed, 1038 insertions(+), 20 deletions(-) create mode 100644 src/consants.ts create mode 100644 src/layout/selected-groups.ts create mode 100644 tests/integration/selected-groups.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 216d50f..9a0b96a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,6 +56,7 @@ Use docs-mcp server for ComfyUI API reference. 7. `positioning.ts` - Assign coordinates, place disconnected nodes, restore reroutes 8. `groups.ts` - Resize groups to fit members 9. `layout-utils.ts` - Shared functions for internal edge detection and member layer assignment +10. `selected-groups.ts` - Organize only selected groups independently **Configuration** in `types.ts`: - `maxColumns`: 0=auto, 1=vertical stack, 2+=fixed columns @@ -70,6 +71,26 @@ Use docs-mcp server for ComfyUI API reference. - `assignNodeYPositions` tracks reserved Y regions to prevent node/group overlap within layers - **Idempotency**: Layout is a pure function of topology + sizes; overlap resolution decoupled from node.pos writes +## Selected Groups Layout + +`layoutSelectedGroups(graph, selectedGroupIds, config?)` in `src/layout/selected-groups.ts`: + +**Context menu items** (in `src/index.ts`): +- "Organize Group" - when single group selected +- "Organize N Groups" - when multiple groups selected + +**Behavior**: +- Uses internal connections only (ignores external edges) +- Disconnected nodes within group stacked vertically +- Nested groups auto-included when parent selected +- External overlaps acceptable (no resolution with non-selected nodes) +- Groups resized to fit organized contents + +**Selection detection**: +- Uses `app.canvas.selectedItems` (Set of Positionable) +- Duck-typing via `isLGraphGroup()` checks for pos/size/title/id +- Handles Float64Array (ComfyUI runtime uses typed arrays for pos/size) + ## Testing **Vitest-based regression testing** in `tests/`: @@ -84,10 +105,11 @@ Use docs-mcp server for ComfyUI API reference. - `assertIdempotent` - stable after multiple runs - `assertGroupMembershipPreserved` - nodes originally in groups remain in groups - `tests/integration/layout.test.ts` - Main regression tests +- `tests/integration/selected-groups.test.ts` - Selected groups layout tests (17 cases) **Coverage**: 70% line threshold, excludes `src/index.ts` (ComfyUI runtime) and `src/layout/reroute-collapse.ts` (no fixtures) -All 4 fixtures pass all invariants including idempotency. +All fixtures pass all invariants including idempotency. 50 total tests. ## CI/CD diff --git a/src/consants.ts b/src/consants.ts new file mode 100644 index 0000000..ce2f405 --- /dev/null +++ b/src/consants.ts @@ -0,0 +1,5 @@ +export const SETTINGS_PREFIX = "Node Organizer"; +export const SETTINGS_IDS = { + VERSION: `${SETTINGS_PREFIX}. ${SETTINGS_PREFIX}`, + DEBUG_LOGGING: `${SETTINGS_PREFIX}.Debug Logging`, +}; diff --git a/src/debug.ts b/src/debug.ts index 32255a8..624f106 100644 --- a/src/debug.ts +++ b/src/debug.ts @@ -1,16 +1,22 @@ -/** Setting ID for debug logging */ -const SETTINGS_PREFIX = "Node Organizer"; -const SETTINGS_IDS = { - DEBUG_LOGGING: `${SETTINGS_PREFIX}.Debug Logging`, -}; +import { SETTINGS_IDS } from "./consants"; /** Check if debug logging is enabled */ export function isDebugEnabled(): boolean { try { // Access app only if it exists (not in test environment) if (typeof globalThis !== "undefined" && "app" in globalThis) { - const app = (globalThis as { app?: { extensionManager?: { setting?: { get: (id: string) => T } } } }).app; - return app?.extensionManager?.setting?.get(SETTINGS_IDS.DEBUG_LOGGING) ?? false; + const app = ( + globalThis as { + app?: { + extensionManager?: { setting?: { get: (id: string) => T } }; + }; + } + ).app; + return ( + app?.extensionManager?.setting?.get( + SETTINGS_IDS.DEBUG_LOGGING + ) ?? false + ); } return false; } catch { diff --git a/src/index.ts b/src/index.ts index 7196677..b4bd3a3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,7 @@ import type { ComfyApp } from "@comfyorg/comfyui-frontend-types"; -import type { LGraph } from "./layout/types"; -import { layoutGraph } from "./layout"; +import type { LGraph, LGraphGroup } from "./layout/types"; +import { layoutGraph, layoutSelectedGroups } from "./layout"; +import { SETTINGS_IDS } from "./consants"; /** Context menu option */ interface ContextMenuOption { @@ -16,7 +17,11 @@ interface LGraphCanvasType { } declare global { - const app: ComfyApp; + const app: ComfyApp & { + canvas?: { + selectedItems?: Set; + }; + }; const LGraphCanvas: LGraphCanvasType; interface Window { @@ -24,12 +29,49 @@ declare global { } } -/** Setting ID for debug logging */ -const SETTINGS_PREFIX = "Node Organizer"; -const SETTINGS_IDS = { - VERSION: `${SETTINGS_PREFIX}. ${SETTINGS_PREFIX}`, - DEBUG_LOGGING: `${SETTINGS_PREFIX}.Debug Logging`, -}; +/** + * Check if value is array-like (regular array or typed array like Float64Array) + */ +function isArrayLike(value: unknown): boolean { + if (Array.isArray(value)) return true; + if (!value || typeof value !== "object") return false; + // Check for typed arrays (Float64Array, etc.) which have numeric length + const obj = value as { length?: unknown }; + return typeof obj.length === "number" && obj.length >= 0; +} + +/** + * Check if an item is a group (duck typing) + * Groups have pos/size (can be Float64Array), title, id, but no .type like nodes + */ +function isLGraphGroup(item: unknown): item is LGraphGroup { + if (!item || typeof item !== "object") return false; + const obj = item as Record; + return ( + isArrayLike(obj.pos) && + isArrayLike(obj.size) && + typeof obj.title === "string" && + typeof obj.id === "number" && + typeof obj.type !== "string" // Nodes have .type, groups don't + ); +} + +/** + * Get selected groups from global app.canvas.selectedItems + * Per ComfyUI docs: use app.canvas.selectedItems for selection access + */ +function getSelectedGroups(): LGraphGroup[] { + const groups: LGraphGroup[] = []; + const selectedItems = app.canvas?.selectedItems; + if (!selectedItems) return groups; + + for (const item of selectedItems) { + if (isLGraphGroup(item)) { + groups.push(item); + } + } + return groups; +} app.registerExtension({ name: "ComfyUI Node Organizer", @@ -58,7 +100,7 @@ app.registerExtension({ }, ], getCanvasMenuItems(canvas) { - return [ + const items: (ContextMenuOption | null)[] = [ null, // separator { content: "Organize Workflow", @@ -81,7 +123,59 @@ app.registerExtension({ } }, }, - null, // separator ]; + + // Check for selected groups via app.canvas.selectedItems + const selectedGroups = getSelectedGroups(); + + if (selectedGroups.length === 1) { + items.push({ + content: "Organize Group", + callback: () => { + const graph = canvas.getCurrentGraph() as LGraph | null; + if (!graph) { + console.warn("[node-organizer] No active graph"); + return; + } + + try { + const groupIds = new Set([selectedGroups[0].id]); + const result = layoutSelectedGroups(graph, groupIds); + console.log( + `[node-organizer] Group organized: ${result.nodeCount} nodes, ` + + `${result.groupCount} groups, ${result.executionMs.toFixed(1)}ms` + ); + } catch (err) { + console.error("[node-organizer] Group layout failed:", err); + } + }, + }); + } else if (selectedGroups.length > 1) { + items.push({ + content: `Organize ${selectedGroups.length} Groups`, + callback: () => { + const graph = canvas.getCurrentGraph() as LGraph | null; + if (!graph) { + console.warn("[node-organizer] No active graph"); + return; + } + + try { + const groupIds = new Set(selectedGroups.map((g) => g.id)); + const result = layoutSelectedGroups(graph, groupIds); + console.log( + `[node-organizer] Groups organized: ${result.nodeCount} nodes, ` + + `${result.groupCount} groups, ${result.executionMs.toFixed(1)}ms` + ); + } catch (err) { + console.error("[node-organizer] Groups layout failed:", err); + } + }, + }); + } + + items.push(null); // separator + + return items; }, }); diff --git a/src/layout/index.ts b/src/layout/index.ts index 92d7aaa..9c58c69 100644 --- a/src/layout/index.ts +++ b/src/layout/index.ts @@ -99,5 +99,8 @@ export function layoutGraph( } // Re-export types and config -export type { LayoutConfig, LayoutResult } from "./types"; +export type { LayoutConfig, LayoutResult, SelectedGroupLayoutResult } from "./types"; export { DEFAULT_CONFIG } from "./types"; + +// Export selected groups layout +export { layoutSelectedGroups } from "./selected-groups"; diff --git a/src/layout/selected-groups.ts b/src/layout/selected-groups.ts new file mode 100644 index 0000000..efe268d --- /dev/null +++ b/src/layout/selected-groups.ts @@ -0,0 +1,469 @@ +import type { + LGraph, + LGraphNode, + LGraphGroup, + LayoutConfig, + SelectedGroupLayoutResult, +} from "./types"; +import { DEFAULT_CONFIG } from "./types"; +import { getInternalEdges, assignMemberLayers } from "./layout-utils"; +import { packNodesIntoRows } from "./bin-pack"; +import { debugLog } from "../debug"; + +/** Title bar height for groups */ +const GROUP_TITLE_HEIGHT = 50; + +/** + * Check if outer group's bounds fully contain inner group's bounds + */ +function groupContainsGroup(outer: LGraphGroup, inner: LGraphGroup): boolean { + const ox = outer.pos[0]; + const oy = outer.pos[1]; + const ow = outer.size[0]; + const oh = outer.size[1]; + + const ix = inner.pos[0]; + const iy = inner.pos[1]; + const iw = inner.size[0]; + const ih = inner.size[1]; + + return ix >= ox && iy >= oy && ix + iw <= ox + ow && iy + ih <= oy + oh; +} + +/** + * Check if a node's origin is inside a group's content area + */ +function nodeInsideGroup(node: LGraphNode, group: LGraphGroup): boolean { + const gx = group.pos[0]; + const gy = group.pos[1] + GROUP_TITLE_HEIGHT; + const gw = group.size[0]; + const gh = group.size[1] - GROUP_TITLE_HEIGHT; + + const nx = node.pos?.[0] ?? 0; + const ny = node.pos?.[1] ?? 0; + + return nx >= gx && nx < gx + gw && ny >= gy && ny < gy + gh; +} + +/** + * Collect all nested child groups within a parent group + */ +function collectNestedGroups( + parent: LGraphGroup, + allGroups: LGraphGroup[] +): LGraphGroup[] { + const nested: LGraphGroup[] = []; + const queue = [parent]; + + while (queue.length > 0) { + const current = queue.shift()!; + for (const group of allGroups) { + if (group === current) continue; + if (groupContainsGroup(current, group) && !nested.includes(group)) { + nested.push(group); + queue.push(group); + } + } + } + + return nested; +} + +/** + * Collect all member nodes within a group (excluding nodes in nested child groups) + */ +function collectDirectMembers( + group: LGraphGroup, + allNodes: Map, + nestedGroups: LGraphGroup[] +): Set { + const members = new Set(); + + for (const [nodeId, node] of allNodes) { + if (!nodeInsideGroup(node, group)) continue; + + // Check if node is inside a nested group (exclude it from direct members) + let inNestedGroup = false; + for (const nested of nestedGroups) { + if (nodeInsideGroup(node, nested)) { + inNestedGroup = true; + break; + } + } + + if (!inNestedGroup) { + members.add(nodeId); + } + } + + return members; +} + +/** + * Update group bounds using multiple methods for compatibility + */ +function updateGroupBounds( + group: LGraphGroup, + x: number, + y: number, + width: number, + height: number +): void { + // Method 1: Direct pos/size array mutation + if (Array.isArray(group.pos)) { + group.pos[0] = x; + group.pos[1] = y; + } + if (Array.isArray(group.size)) { + group.size[0] = width; + group.size[1] = height; + } + + // Method 2: Try _pos/_size internal properties + const g = group as LGraphGroup & { + _pos?: [number, number]; + _size?: [number, number]; + }; + if (Array.isArray(g._pos)) { + g._pos[0] = x; + g._pos[1] = y; + } + if (Array.isArray(g._size)) { + g._size[0] = width; + g._size[1] = height; + } + + // Method 3: Update _bounding Rectangle if present + if (group._bounding) { + const b = group._bounding; + b.x = x; + b.y = y; + b.width = width; + b.height = height; + } + + // Method 4: Try bounding array (older format) + const gAny = group as LGraphGroup & { bounding?: number[] }; + if (Array.isArray(gAny.bounding)) { + gAny.bounding[0] = x; + gAny.bounding[1] = y; + gAny.bounding[2] = width; + gAny.bounding[3] = height; + } +} + +/** + * Layout contents of a single group and resize it to fit + * Returns the bounding box of the group's contents + */ +function layoutGroupContents( + group: LGraphGroup, + memberIds: Set, + nestedGroups: LGraphGroup[], + allNodes: Map, + config: LayoutConfig, + processedGroups: Set +): { minX: number; minY: number; maxX: number; maxY: number } | null { + // Skip if already processed (handles nested groups selected individually) + if (processedGroups.has(group)) { + return null; + } + processedGroups.add(group); + + // Collect non-locked members + const members: LGraphNode[] = []; + for (const nodeId of memberIds) { + const node = allNodes.get(nodeId); + if (!node) continue; + if (node.flags?.pinned || node.locked) continue; + members.push(node); + } + + // Handle empty group with no nested groups + if (members.length === 0 && nestedGroups.length === 0) { + debugLog(`Skipping empty group "${group.title}"`); + return null; + } + + debugLog( + `layoutGroupContents "${group.title}": members=${members.length}, nestedGroups=${nestedGroups.length}` + ); + + // Get the group's current content area start position + const startX = group.pos[0] + config.groupPadding; + const startY = group.pos[1] + config.groupPadding + GROUP_TITLE_HEIGHT; + + // Track bounds of all positioned content + let minX = Infinity; + let minY = Infinity; + let maxX = -Infinity; + let maxY = -Infinity; + + // Get internal edges between members + const edges = getInternalEdges(memberIds, allNodes); + + let contentEndY = startY; + + if (members.length > 0) { + if (edges.length > 0) { + // Has internal connections - use layer-based layout + const layers = assignMemberLayers(memberIds, edges, allNodes); + + let x = startX; + for (let layerIdx = 0; layerIdx < layers.length; layerIdx++) { + const layer = layers[layerIdx]; + if (layer.length === 0) continue; + + // Sort layer by height (tallest first for stable layout) + layer.sort((a, b) => (b.size?.[1] ?? 100) - (a.size?.[1] ?? 100)); + + let y = startY; + let layerMaxWidth = 0; + + for (const node of layer) { + if (node.pos) { + node.pos[0] = x; + node.pos[1] = y; + debugLog( + ` Member ${node.id} (${node.type}): layer ${layerIdx}, pos [${x}, ${y}]` + ); + } + + const nw = node.size?.[0] ?? 200; + const nh = node.size?.[1] ?? 100; + + minX = Math.min(minX, x); + minY = Math.min(minY, y); + maxX = Math.max(maxX, x + nw); + maxY = Math.max(maxY, y + nh); + + layerMaxWidth = Math.max(layerMaxWidth, nw); + y += nh + config.verticalGap; + } + + contentEndY = Math.max(contentEndY, y - config.verticalGap); + x += layerMaxWidth + config.horizontalGap; + } + } else { + // No internal connections - use vertical stacking + members.sort((a, b) => (b.size?.[1] ?? 100) - (a.size?.[1] ?? 100)); + + // Build sizes map + const sizes = new Map(); + for (const node of members) { + sizes.set(node.id, { + width: node.size?.[0] ?? 200, + height: node.size?.[1] ?? 100, + }); + } + + // Pack into rows (vertical stack with maxColumns=1) + const nodeIds = members.map((n) => n.id); + const rows = packNodesIntoRows(nodeIds, sizes, { + ...config, + maxColumns: 1, + }); + + for (const row of rows) { + const x = startX; + const y = startY + row.yOffset; + + for (const nodeId of row.nodes) { + const node = allNodes.get(nodeId); + if (node?.pos) { + node.pos[0] = x; + node.pos[1] = y; + debugLog(` Member ${node.id} (${node.type}): stacked pos [${x}, ${y}]`); + + const nw = node.size?.[0] ?? 200; + const nh = node.size?.[1] ?? 100; + + minX = Math.min(minX, x); + minY = Math.min(minY, y); + maxX = Math.max(maxX, x + nw); + maxY = Math.max(maxY, y + nh); + } + } + contentEndY = Math.max(contentEndY, startY + row.yOffset + row.height); + } + } + } + + // Process nested groups (bottom-up: deepest first) + // Sort by containment depth + const sortedNested = [...nestedGroups].sort((a, b) => { + // Count how many other nested groups contain each + let depthA = 0; + let depthB = 0; + for (const other of nestedGroups) { + if (other !== a && groupContainsGroup(other, a)) depthA++; + if (other !== b && groupContainsGroup(other, b)) depthB++; + } + return depthB - depthA; // Deepest first + }); + + for (const nestedGroup of sortedNested) { + // Get direct members of nested group + const nestedChildren = collectNestedGroups(nestedGroup, nestedGroups); + const nestedMembers = collectDirectMembers(nestedGroup, allNodes, nestedChildren); + + // Recursively layout nested group + const nestedBounds = layoutGroupContents( + nestedGroup, + nestedMembers, + nestedChildren, + allNodes, + config, + processedGroups + ); + + if (nestedBounds) { + // Update bounds to include nested group + minX = Math.min(minX, nestedBounds.minX - config.groupPadding); + minY = Math.min(minY, nestedBounds.minY - config.groupPadding - GROUP_TITLE_HEIGHT); + maxX = Math.max(maxX, nestedBounds.maxX + config.groupPadding); + maxY = Math.max(maxY, nestedBounds.maxY + config.groupPadding); + } else { + // Include nested group's current bounds + const nx = nestedGroup.pos[0]; + const ny = nestedGroup.pos[1]; + const nw = nestedGroup.size[0]; + const nh = nestedGroup.size[1]; + + minX = Math.min(minX, nx); + minY = Math.min(minY, ny); + maxX = Math.max(maxX, nx + nw); + maxY = Math.max(maxY, ny + nh); + } + } + + // Resize group to fit contents + if (minX !== Infinity) { + const newX = minX - config.groupPadding; + const newY = minY - config.groupPadding - GROUP_TITLE_HEIGHT; + const newWidth = maxX - minX + config.groupPadding * 2; + const newHeight = maxY - minY + config.groupPadding * 2 + GROUP_TITLE_HEIGHT; + + debugLog( + `Resizing group "${group.title}" to [${newX}, ${newY}, ${newWidth}, ${newHeight}]` + ); + + updateGroupBounds(group, newX, newY, newWidth, newHeight); + + return { minX, minY, maxX, maxY }; + } + + return null; +} + +/** + * Layout only the contents of selected groups + * Ignores external connections - uses only internal edges + * Auto-includes nested groups when parent is selected + * + * @param graph - The full graph (for node/link access) + * @param selectedGroupIds - Set of group IDs to organize + * @param config - Layout configuration + */ +export function layoutSelectedGroups( + graph: LGraph, + selectedGroupIds: Set, + config?: Partial +): SelectedGroupLayoutResult { + const fullConfig = { ...DEFAULT_CONFIG, ...config }; + const startTime = performance.now(); + + // Build allNodes map + const allNodes = new Map(); + for (const node of graph._nodes ?? []) { + if (!node || node.id === undefined) continue; + allNodes.set(node.id, node); + } + + // Get all groups + const allGroups = graph._groups ?? []; + + // Find selected groups + const selectedGroups: LGraphGroup[] = []; + for (const group of allGroups) { + if (selectedGroupIds.has(group.id)) { + selectedGroups.push(group); + } + } + + if (selectedGroups.length === 0) { + debugLog("No selected groups found"); + return { + nodeCount: 0, + groupCount: 0, + executionMs: performance.now() - startTime, + }; + } + + // Filter to only top-level selected groups + // (nested groups selected individually will be handled, but if both parent and child + // are selected, we only process from the parent) + const topLevelSelected: LGraphGroup[] = []; + for (const group of selectedGroups) { + let hasSelectedParent = false; + for (const other of selectedGroups) { + if (other !== group && groupContainsGroup(other, group)) { + hasSelectedParent = true; + break; + } + } + if (!hasSelectedParent) { + topLevelSelected.push(group); + } + } + + // Track processed groups to avoid double-processing + const processedGroups = new Set(); + let totalNodes = 0; + + // Process each top-level selected group + for (const group of topLevelSelected) { + // Collect nested groups within this group + const nestedGroups = collectNestedGroups(group, allGroups); + + // Collect direct members (excluding those in nested groups) + const memberIds = collectDirectMembers(group, allNodes, nestedGroups); + + debugLog( + `Processing group "${group.title}": ${memberIds.size} direct members, ${nestedGroups.length} nested groups` + ); + + // Layout group contents + layoutGroupContents( + group, + memberIds, + nestedGroups, + allNodes, + fullConfig, + processedGroups + ); + + totalNodes += memberIds.size; + // Count nested group members too + for (const nested of nestedGroups) { + const nestedChildren = collectNestedGroups(nested, nestedGroups); + const nestedMembers = collectDirectMembers(nested, allNodes, nestedChildren); + totalNodes += nestedMembers.size; + } + } + + // Trigger canvas redraw + graph.setDirtyCanvas?.(true, true); + + const result: SelectedGroupLayoutResult = { + nodeCount: totalNodes, + groupCount: processedGroups.size, + executionMs: performance.now() - startTime, + }; + + debugLog( + `layoutSelectedGroups complete: ${result.nodeCount} nodes, ${result.groupCount} groups, ${result.executionMs.toFixed(1)}ms` + ); + + return result; +} diff --git a/src/layout/types.ts b/src/layout/types.ts index 7e33b83..32622e2 100644 --- a/src/layout/types.ts +++ b/src/layout/types.ts @@ -262,3 +262,13 @@ export interface LayoutResult { /** Execution time in ms */ executionMs: number; } + +/** Result from selected group layout */ +export interface SelectedGroupLayoutResult { + /** Number of nodes organized */ + nodeCount: number; + /** Number of groups organized */ + groupCount: number; + /** Execution time in ms */ + executionMs: number; +} diff --git a/tests/integration/selected-groups.test.ts b/tests/integration/selected-groups.test.ts new file mode 100644 index 0000000..8f7f39f --- /dev/null +++ b/tests/integration/selected-groups.test.ts @@ -0,0 +1,409 @@ +import { describe, it, beforeEach, expect } from "vitest"; +import { loadFixture, capturePositions } from "../helpers"; +import { layoutSelectedGroups } from "../../src/layout/selected-groups"; +import { + assertNodesInsideGroups, + assertFiniteCoordinates, +} from "../invariants"; +import type { LGraph } from "../../src/layout/types"; + +/** + * Tests for layoutSelectedGroups function + * Note: External overlaps are acceptable per requirements + */ +describe("layoutSelectedGroups", () => { + describe("single group with internal connections", () => { + let graph: LGraph; + + beforeEach(() => { + // nested-groups.json has groups with internal connections + // Group 1 "Step 1 - Load model" contains node 4 (CheckpointLoaderSimple) + graph = loadFixture("nested-groups.json"); + }); + + it("organizes nodes within selected group", () => { + // Select group 1 which contains node 4 + const result = layoutSelectedGroups(graph, new Set([1])); + + expect(result.nodeCount).toBeGreaterThanOrEqual(0); + expect(result.groupCount).toBe(1); + assertFiniteCoordinates(graph); + }); + + it("resizes group to fit contents", () => { + layoutSelectedGroups(graph, new Set([1])); + + const groupAfter = graph._groups.find((g) => g.id === 1); + // Group size should be finite and positive + expect(Number.isFinite(groupAfter!.size[0])).toBe(true); + expect(Number.isFinite(groupAfter!.size[1])).toBe(true); + expect(groupAfter!.size[0]).toBeGreaterThan(0); + expect(groupAfter!.size[1]).toBeGreaterThan(0); + }); + + it("preserves external node positions", () => { + // Capture positions of nodes NOT in group 1 + const nodesBefore = new Map(); + for (const node of graph._nodes) { + // Node 4 is in group 1, all others should be unchanged + if (node.id !== 4) { + nodesBefore.set(node.id, [...node.pos]); + } + } + + layoutSelectedGroups(graph, new Set([1])); + + // Verify external nodes haven't moved + for (const [nodeId, posBefore] of nodesBefore) { + const node = graph._nodes.find((n) => n.id === nodeId); + expect(node).toBeDefined(); + expect(node!.pos[0]).toBe(posBefore[0]); + expect(node!.pos[1]).toBe(posBefore[1]); + } + }); + + it("is idempotent", () => { + layoutSelectedGroups(graph, new Set([1])); + const positions1 = capturePositions(graph); + + layoutSelectedGroups(graph, new Set([1])); + const positions2 = capturePositions(graph); + + // Compare positions with small tolerance + for (const [id, pos1] of positions1) { + const pos2 = positions2.get(id); + if (pos2) { + expect(Math.abs(pos1.x - pos2.x)).toBeLessThan(5); + expect(Math.abs(pos1.y - pos2.y)).toBeLessThan(5); + } + } + }); + }); + + describe("single group with disconnected nodes", () => { + let graph: LGraph; + + beforeEach(() => { + // Create a graph with a group containing disconnected nodes + graph = { + _nodes: [ + { + id: 1, + type: "Note", + title: "Note 1", + pos: [100, 100], + size: [200, 100], + inputs: [], + outputs: [], + }, + { + id: 2, + type: "Note", + title: "Note 2", + pos: [150, 250], + size: [200, 100], + inputs: [], + outputs: [], + }, + { + id: 3, + type: "Note", + title: "External Node", + pos: [500, 500], + size: [200, 100], + inputs: [], + outputs: [], + }, + ], + _groups: [ + { + id: 1, + title: "Test Group", + pos: [50, 50], + size: [350, 350], + }, + ], + links: new Map(), + }; + }); + + it("stacks disconnected nodes vertically", () => { + const result = layoutSelectedGroups(graph, new Set([1])); + + expect(result.nodeCount).toBe(2); + expect(result.groupCount).toBe(1); + + // Nodes should have same X (stacked vertically) + const node1 = graph._nodes.find((n) => n.id === 1)!; + const node2 = graph._nodes.find((n) => n.id === 2)!; + expect(node1.pos[0]).toBe(node2.pos[0]); + + assertFiniteCoordinates(graph); + }); + + it("resizes group to fit stacked nodes", () => { + layoutSelectedGroups(graph, new Set([1])); + + const group = graph._groups.find((g) => g.id === 1)!; + expect(Number.isFinite(group.size[0])).toBe(true); + expect(Number.isFinite(group.size[1])).toBe(true); + expect(group.size[0]).toBeGreaterThan(0); + expect(group.size[1]).toBeGreaterThan(0); + }); + }); + + describe("multiple groups", () => { + let graph: LGraph; + + beforeEach(() => { + graph = loadFixture("nested-groups.json"); + }); + + it("organizes each group independently", () => { + // Select groups 1 and 2 (not nested) + const result = layoutSelectedGroups(graph, new Set([1, 2])); + + expect(result.groupCount).toBeGreaterThanOrEqual(2); + assertFiniteCoordinates(graph); + }); + + it("handles non-overlapping groups", () => { + layoutSelectedGroups(graph, new Set([1, 2])); + + // Both groups should have valid coordinates + const group1 = graph._groups.find((g) => g.id === 1)!; + const group2 = graph._groups.find((g) => g.id === 2)!; + + expect(Number.isFinite(group1.pos[0])).toBe(true); + expect(Number.isFinite(group2.pos[0])).toBe(true); + }); + }); + + describe("nested groups", () => { + let graph: LGraph; + + beforeEach(() => { + // nested-groups.json has Group 3 containing Group 4 (nested) + graph = loadFixture("nested-groups.json"); + }); + + it("auto-includes nested groups when parent selected", () => { + // Select group 3 which contains group 4 + const result = layoutSelectedGroups(graph, new Set([3])); + + // Should include the nested group + expect(result.groupCount).toBeGreaterThanOrEqual(1); + assertFiniteCoordinates(graph); + }); + + it("positions nested groups correctly", () => { + layoutSelectedGroups(graph, new Set([3])); + + // Group 4 should still be inside Group 3 + const group3 = graph._groups.find((g) => g.id === 3)!; + const group4 = graph._groups.find((g) => g.id === 4)!; + + // Check group 4 is within group 3 bounds + expect(group4.pos[0]).toBeGreaterThanOrEqual(group3.pos[0]); + expect(group4.pos[1]).toBeGreaterThanOrEqual(group3.pos[1]); + expect(group4.pos[0] + group4.size[0]).toBeLessThanOrEqual( + group3.pos[0] + group3.size[0] + ); + expect(group4.pos[1] + group4.size[1]).toBeLessThanOrEqual( + group3.pos[1] + group3.size[1] + ); + }); + + it("keeps nodes inside selected groups", () => { + layoutSelectedGroups(graph, new Set([3])); + + // Nodes inside group 3 should remain inside it + assertNodesInsideGroups(graph); + }); + }); + + describe("edge cases", () => { + it("handles empty graph", () => { + const graph: LGraph = { + _nodes: [], + _groups: [], + links: new Map(), + }; + + const result = layoutSelectedGroups(graph, new Set([1])); + + expect(result.nodeCount).toBe(0); + expect(result.groupCount).toBe(0); + }); + + it("handles non-existent group IDs", () => { + const graph = loadFixture("nested-groups.json"); + + const result = layoutSelectedGroups(graph, new Set([999])); + + expect(result.groupCount).toBe(0); + }); + + it("handles empty groups", () => { + const graph: LGraph = { + _nodes: [ + { + id: 1, + type: "Note", + title: "External", + pos: [500, 500], + size: [200, 100], + inputs: [], + outputs: [], + }, + ], + _groups: [ + { + id: 1, + title: "Empty Group", + pos: [50, 50], + size: [200, 200], + }, + ], + links: new Map(), + }; + + // Should not crash + const result = layoutSelectedGroups(graph, new Set([1])); + + // Empty group is still processed, just has no content + expect(result.nodeCount).toBe(0); + assertFiniteCoordinates(graph); + }); + + it("handles locked nodes", () => { + const graph: LGraph = { + _nodes: [ + { + id: 1, + type: "Note", + title: "Locked Node", + pos: [100, 100], + size: [200, 100], + inputs: [], + outputs: [], + locked: true, + }, + { + id: 2, + type: "Note", + title: "Normal Node", + pos: [100, 250], + size: [200, 100], + inputs: [], + outputs: [], + }, + ], + _groups: [ + { + id: 1, + title: "Test Group", + pos: [50, 50], + size: [350, 400], + }, + ], + links: new Map(), + }; + + const lockedPosBefore = [...graph._nodes[0].pos]; + + layoutSelectedGroups(graph, new Set([1])); + + // Locked node should not move + expect(graph._nodes[0].pos[0]).toBe(lockedPosBefore[0]); + expect(graph._nodes[0].pos[1]).toBe(lockedPosBefore[1]); + }); + + it("handles group with only locked nodes", () => { + const graph: LGraph = { + _nodes: [ + { + id: 1, + type: "Note", + title: "Locked Node", + pos: [100, 100], + size: [200, 100], + inputs: [], + outputs: [], + locked: true, + }, + ], + _groups: [ + { + id: 1, + title: "Test Group", + pos: [50, 50], + size: [350, 250], + }, + ], + links: new Map(), + }; + + // Should not crash + const result = layoutSelectedGroups(graph, new Set([1])); + expect(result).toBeDefined(); + }); + }); + + describe("groups with internal connections", () => { + it("uses layer-based layout for connected nodes", () => { + const graph: LGraph = { + _nodes: [ + { + id: 1, + type: "NodeA", + title: "Node A", + pos: [100, 100], + size: [200, 100], + inputs: [], + outputs: [{ name: "out", type: "FLOAT", links: [1] }], + }, + { + id: 2, + type: "NodeB", + title: "Node B", + pos: [100, 300], + size: [200, 100], + inputs: [{ name: "in", type: "FLOAT", link: 1 }], + outputs: [], + }, + ], + _groups: [ + { + id: 1, + title: "Test Group", + pos: [50, 50], + size: [400, 400], + }, + ], + links: new Map([ + [ + 1, + { + id: 1, + origin_id: 1, + origin_slot: 0, + target_id: 2, + target_slot: 0, + type: "FLOAT", + }, + ], + ]), + }; + + layoutSelectedGroups(graph, new Set([1])); + + // With layer-based layout, source (node 1) should be left of target (node 2) + const node1 = graph._nodes.find((n) => n.id === 1)!; + const node2 = graph._nodes.find((n) => n.id === 2)!; + expect(node1.pos[0]).toBeLessThan(node2.pos[0]); + + assertFiniteCoordinates(graph); + }); + }); +});