refactor: address test code review feedback

- Rename loadWithPositions to repositionNodes, decompose into
  getSerializedGraph + applyNodePositions (pure) + loadGraph
- Extract measureSelectionBounds to shared boundsUtils helper
- Add JSDoc to setCollapsed
- Move test helpers out of spec file into fixture utils

browser_tests/fixtures/helpers/NodeOperationsHelper.ts

@@ -114,25 +114,25 @@ export class NodeOperationsHelper {
     }
   }
 
-  async loadWithPositions(
+  async getSerializedGraph(): Promise<ComfyWorkflowJSON> {
+    return this.page.evaluate(
+      () => window.app!.graph.serialize() as ComfyWorkflowJSON
+    )
+  }
+
+  async loadGraph(data: ComfyWorkflowJSON): Promise<void> {
+    await this.page.evaluate(
+      (d) => window.app!.loadGraphData(d, true, true, null),
+      data
+    )
+  }
+
+  async repositionNodes(
     positions: Record<string, [number, number]>
   ): Promise<void> {
-    await this.page.evaluate(
-      async ({ positions }) => {
-        const data = window.app!.graph.serialize()
-        for (const node of data.nodes) {
-          const pos = positions[String(node.id)]
-          if (pos) node.pos = pos
-        }
-        await window.app!.loadGraphData(
-          data as ComfyWorkflowJSON,
-          true,
-          true,
-          null
-        )
-      },
-      { positions }
-    )
+    const data = await this.getSerializedGraph()
+    applyNodePositions(data, positions)
+    await this.loadGraph(data)
   }
 
   async resizeNode(
@@ -209,3 +209,13 @@ export class NodeOperationsHelper {
     await this.comfyPage.nextFrame()
   }
 }
+
+function applyNodePositions(
+  data: ComfyWorkflowJSON,
+  positions: Record<string, [number, number]>
+): void {
+  for (const node of data.nodes) {
+    const pos = positions[String(node.id)]
+    if (pos) node.pos = pos
+  }
+}

browser_tests/fixtures/utils/boundsUtils.ts

@@ -0,0 +1,83 @@
+import type { Page } from '@playwright/test'
+
+export interface CanvasRect {
+  x: number
+  y: number
+  w: number
+  h: number
+}
+
+export interface MeasureResult {
+  selectionBounds: CanvasRect | null
+  nodeVisualBounds: Record<string, CanvasRect>
+}
+
+// Must match the padding value passed to createBounds() in selectionBorder.ts
+const SELECTION_PADDING = 10
+
+export async function measureSelectionBounds(
+  page: Page,
+  nodeIds: string[]
+): Promise<MeasureResult> {
+  return page.evaluate(
+    ({ ids, padding }) => {
+      const canvas = window.app!.canvas
+      const ds = canvas.ds
+
+      const selectedItems = canvas.selectedItems
+      let minX = Infinity
+      let minY = Infinity
+      let maxX = -Infinity
+      let maxY = -Infinity
+      for (const item of selectedItems) {
+        const rect = item.boundingRect
+        minX = Math.min(minX, rect[0])
+        minY = Math.min(minY, rect[1])
+        maxX = Math.max(maxX, rect[0] + rect[2])
+        maxY = Math.max(maxY, rect[1] + rect[3])
+      }
+      const selectionBounds =
+        selectedItems.size > 0
+          ? {
+              x: minX - padding,
+              y: minY - padding,
+              w: maxX - minX + 2 * padding,
+              h: maxY - minY + 2 * padding
+            }
+          : null
+
+      const canvasEl = canvas.canvas as HTMLCanvasElement
+      const canvasRect = canvasEl.getBoundingClientRect()
+      const nodeVisualBounds: Record<
+        string,
+        { x: number; y: number; w: number; h: number }
+      > = {}
+
+      for (const id of ids) {
+        const nodeEl = document.querySelector(
+          `[data-node-id="${id}"]`
+        ) as HTMLElement | null
+        if (!nodeEl) continue
+
+        const domRect = nodeEl.getBoundingClientRect()
+        const footerEls = nodeEl.querySelectorAll(
+          '[data-testid="subgraph-enter-button"], [data-testid="node-footer"]'
+        )
+        let bottom = domRect.bottom
+        for (const footerEl of footerEls) {
+          bottom = Math.max(bottom, footerEl.getBoundingClientRect().bottom)
+        }
+
+        nodeVisualBounds[id] = {
+          x: (domRect.left - canvasRect.left) / ds.scale - ds.offset[0],
+          y: (domRect.top - canvasRect.top) / ds.scale - ds.offset[1],
+          w: domRect.width / ds.scale,
+          h: (bottom - domRect.top) / ds.scale
+        }
+      }
+
+      return { selectionBounds, nodeVisualBounds }
+    },
+    { ids: nodeIds, padding: SELECTION_PADDING }
+  ) as Promise<MeasureResult>
+}

browser_tests/fixtures/utils/litegraphUtils.ts

@@ -332,6 +332,7 @@ export class NodeReference {
   async isCollapsed() {
     return !!(await this.getFlags()).collapsed
   }
+  /** Deterministic setter using node.collapse() API (not a toggle). */
   async setCollapsed(collapsed: boolean) {
     await this.comfyPage.page.evaluate(
       ([id, collapsed]) => {

browser_tests/tests/selectionBoundingBox.spec.ts

@@ -1,86 +1,7 @@
 import { expect } from '@playwright/test'
-import type { Page } from '@playwright/test'
 
 import { comfyPageFixture as test } from '../fixtures/ComfyPage'
-
-interface CanvasRect {
-  x: number
-  y: number
-  w: number
-  h: number
-}
-
-interface MeasureResult {
-  selectionBounds: CanvasRect | null
-  nodeVisualBounds: Record<string, CanvasRect>
-}
-
-// Must match the padding value passed to createBounds() in selectionBorder.ts
-const SELECTION_PADDING = 10
-
-async function measureBounds(
-  page: Page,
-  nodeIds: string[]
-): Promise<MeasureResult> {
-  return page.evaluate(
-    ({ ids, padding }) => {
-      const canvas = window.app!.canvas
-      const ds = canvas.ds
-
-      const selectedItems = canvas.selectedItems
-      let minX = Infinity
-      let minY = Infinity
-      let maxX = -Infinity
-      let maxY = -Infinity
-      for (const item of selectedItems) {
-        const rect = item.boundingRect
-        minX = Math.min(minX, rect[0])
-        minY = Math.min(minY, rect[1])
-        maxX = Math.max(maxX, rect[0] + rect[2])
-        maxY = Math.max(maxY, rect[1] + rect[3])
-      }
-      const selectionBounds =
-        selectedItems.size > 0
-          ? {
-              x: minX - padding,
-              y: minY - padding,
-              w: maxX - minX + 2 * padding,
-              h: maxY - minY + 2 * padding
-            }
-          : null
-
-      const canvasEl = canvas.canvas as HTMLCanvasElement
-      const canvasRect = canvasEl.getBoundingClientRect()
-      const nodeVisualBounds: Record<string, CanvasRect> = {}
-
-      for (const id of ids) {
-        const nodeEl = document.querySelector(
-          `[data-node-id="${id}"]`
-        ) as HTMLElement | null
-        if (!nodeEl) continue
-
-        const domRect = nodeEl.getBoundingClientRect()
-        const footerEls = nodeEl.querySelectorAll(
-          '[data-testid="subgraph-enter-button"], [data-testid="node-footer"]'
-        )
-        let bottom = domRect.bottom
-        for (const footerEl of footerEls) {
-          bottom = Math.max(bottom, footerEl.getBoundingClientRect().bottom)
-        }
-
-        nodeVisualBounds[id] = {
-          x: (domRect.left - canvasRect.left) / ds.scale - ds.offset[0],
-          y: (domRect.top - canvasRect.top) / ds.scale - ds.offset[1],
-          w: domRect.width / ds.scale,
-          h: (bottom - domRect.top) / ds.scale
-        }
-      }
-
-      return { selectionBounds, nodeVisualBounds }
-    },
-    { ids: nodeIds, padding: SELECTION_PADDING }
-  ) as Promise<MeasureResult>
-}
+import { measureSelectionBounds } from '../fixtures/utils/boundsUtils'
 
 const SUBGRAPH_ID = '2'
 const REGULAR_ID = '3'
@@ -129,7 +50,7 @@ test.describe('Selection bounding box', { tag: ['@canvas', '@node'] }, () => {
           const targetId = getTargetId(type)
           const refId = getRefId(type)
 
-          await comfyPage.nodeOps.loadWithPositions({
+          await comfyPage.nodeOps.repositionNodes({
             [refId]: REF_POS,
             [targetId]: TARGET_POSITIONS[pos]
           })
@@ -148,7 +69,7 @@ test.describe('Selection bounding box', { tag: ['@canvas', '@node'] }, () => {
             .toBe(2)
           await comfyPage.nextFrame()
 
-          const result = await measureBounds(page, [refId, targetId])
+          const result = await measureSelectionBounds(page, [refId, targetId])
           expect(result.selectionBounds).not.toBeNull()
 
           const sel = result.selectionBounds!