feat: add WidgetValueStore for centralized widget value management

browser_tests/tests/subgraph-duplicate-ids.spec.ts

@@ -0,0 +1,100 @@
+import { expect } from '@playwright/test'
+
+import { comfyPageFixture as test } from '../fixtures/ComfyPage'
+
+test.describe('Subgraph duplicate ID remapping', { tag: ['@subgraph'] }, () => {
+  const WORKFLOW = 'subgraphs/subgraph-nested-duplicate-ids'
+
+  test('All node IDs are globally unique after loading', async ({
+    comfyPage
+  }) => {
+    await comfyPage.workflow.loadWorkflow(WORKFLOW)
+
+    const result = await comfyPage.page.evaluate(() => {
+      const graph = window.app!.canvas.graph!
+      // TODO: Extract allGraphs accessor (root + subgraphs) into LGraph
+      // TODO: Extract allNodeIds accessor into LGraph
+      const allGraphs = [graph, ...graph.subgraphs.values()]
+      const allIds = allGraphs
+        .flatMap((g) => g._nodes)
+        .map((n) => n.id)
+        .filter((id): id is number => typeof id === 'number')
+
+      return { allIds, uniqueCount: new Set(allIds).size }
+    })
+
+    expect(result.uniqueCount).toBe(result.allIds.length)
+    expect(result.allIds.length).toBeGreaterThanOrEqual(10)
+  })
+
+  test('Root graph node IDs are preserved as canonical', async ({
+    comfyPage
+  }) => {
+    await comfyPage.workflow.loadWorkflow(WORKFLOW)
+
+    const rootIds = await comfyPage.page.evaluate(() => {
+      const graph = window.app!.canvas.graph!
+      return graph._nodes
+        .map((n) => n.id)
+        .filter((id): id is number => typeof id === 'number')
+        .sort((a, b) => a - b)
+    })
+
+    expect(rootIds).toEqual([1, 2, 5])
+  })
+
+  test('All links reference valid nodes in their graph', async ({
+    comfyPage
+  }) => {
+    await comfyPage.workflow.loadWorkflow(WORKFLOW)
+
+    const invalidLinks = await comfyPage.page.evaluate(() => {
+      const graph = window.app!.canvas.graph!
+      const labeledGraphs: [string, typeof graph][] = [
+        ['root', graph],
+        ...[...graph.subgraphs.entries()].map(
+          ([id, sg]) => [`subgraph:${id}`, sg] as [string, typeof graph]
+        )
+      ]
+
+      const isNonNegative = (id: number | string) =>
+        typeof id === 'number' && id >= 0
+
+      return labeledGraphs.flatMap(([label, g]) =>
+        [...g._links.values()].flatMap((link) =>
+          [
+            isNonNegative(link.origin_id) &&
+              !g._nodes_by_id[link.origin_id] &&
+              `${label}: origin_id ${link.origin_id} not found`,
+            isNonNegative(link.target_id) &&
+              !g._nodes_by_id[link.target_id] &&
+              `${label}: target_id ${link.target_id} not found`
+          ].filter(Boolean)
+        )
+      )
+    })
+
+    expect(invalidLinks).toEqual([])
+  })
+
+  test('Subgraph navigation works after ID remapping', async ({
+    comfyPage
+  }) => {
+    await comfyPage.workflow.loadWorkflow(WORKFLOW)
+
+    const subgraphNode = await comfyPage.nodeOps.getNodeRefById('5')
+    await subgraphNode.navigateIntoSubgraph()
+
+    const isInSubgraph = () =>
+      comfyPage.page.evaluate(
+        () => window.app!.canvas.graph?.isRootGraph === false
+      )
+
+    expect(await isInSubgraph()).toBe(true)
+
+    await comfyPage.page.keyboard.press('Escape')
+    await comfyPage.nextFrame()
+
+    expect(await isInSubgraph()).toBe(false)
+  })
+})

browser_tests/tests/subgraph.spec.ts

@@ -61,7 +61,10 @@ test.describe('Subgraph Operations', { tag: ['@slow', '@subgraph'] }, () => {
       await subgraphNode.navigateIntoSubgraph()
 
       const initialCount = await getSubgraphSlotCount(comfyPage, 'inputs')
-      const vaeEncodeNode = await comfyPage.nodeOps.getNodeRefById('2')
+      const [vaeEncodeNode] = await comfyPage.nodeOps.getNodeRefsByType(
+        'VAEEncode',
+        true
+      )
 
       await comfyPage.subgraph.connectFromInput(vaeEncodeNode, 0)
       await comfyPage.nextFrame()
@@ -77,7 +80,10 @@ test.describe('Subgraph Operations', { tag: ['@slow', '@subgraph'] }, () => {
       await subgraphNode.navigateIntoSubgraph()
 
       const initialCount = await getSubgraphSlotCount(comfyPage, 'outputs')
-      const vaeEncodeNode = await comfyPage.nodeOps.getNodeRefById('2')
+      const [vaeEncodeNode] = await comfyPage.nodeOps.getNodeRefsByType(
+        'VAEEncode',
+        true
+      )
 
       await comfyPage.subgraph.connectToOutput(vaeEncodeNode, 0)
       await comfyPage.nextFrame()

src/components/rightSidePanel/parameters/TabNormalInputs.vue

@@ -33,10 +33,7 @@ const advancedWidgetsSectionDataList = computed((): NodeWidgetsListList => {
     .map((node) => {
       const { widgets = [] } = node
       const advancedWidgets = widgets
-        .filter(
-          (w) =>
-            !(w.options?.canvasOnly || w.options?.hidden) && w.options?.advanced
-        )
+        .filter((w) => !(w.options?.canvasOnly || w.hidden) && w.advanced)
         .map((widget) => ({ node, widget }))
       return { widgets: advancedWidgets, node }
     })

src/components/rightSidePanel/parameters/WidgetItem.vue

@@ -82,10 +82,7 @@ const favoriteNode = computed(() =>
 )
 
 const widgetValue = computed({
-  get: () => {
-    widget.vueTrack?.()
-    return widget.value
-  },
+  get: () => widget.value,
   set: (newValue: string | number | boolean | object) => {
     emit('update:widgetValue', newValue)
   }

src/components/rightSidePanel/shared.ts

@@ -265,8 +265,8 @@ export function computedSectionDataList(nodes: MaybeRefOrGetter<LGraphNode[]>) {
           (w) =>
             !(
               w.options?.canvasOnly ||
-              w.options?.hidden ||
-              (w.options?.advanced && !includesAdvanced.value)
+              w.hidden ||
+              (w.advanced && !includesAdvanced.value)
             )
         )
         .map((widget) => ({ node, widget }))

src/composables/graph/useGraphNodeManager.test.ts

@@ -1,49 +1,76 @@
 import { setActivePinia } from 'pinia'
 import { createTestingPinia } from '@pinia/testing'
-import { describe, expect, it, vi } from 'vitest'
-import { nextTick, watch } from 'vue'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { computed, nextTick, watch } from 'vue'
 
 import { useGraphNodeManager } from '@/composables/graph/useGraphNodeManager'
-import { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
+import { BaseWidget, LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
 import { NodeSlotType } from '@/lib/litegraph/src/types/globalEnums'
+import { useWidgetValueStore } from '@/stores/widgetValueStore'
 
-setActivePinia(createTestingPinia())
+describe('Node Reactivity', () => {
+  beforeEach(() => {
+    setActivePinia(createTestingPinia({ stubActions: false }))
+  })
 
-function createTestGraph() {
-  const graph = new LGraph()
-  const node = new LGraphNode('test')
-  node.addInput('input', 'INT')
-  node.addWidget('number', 'testnum', 2, () => undefined, {})
-  graph.add(node)
+  function createTestGraph() {
+    const graph = new LGraph()
+    const node = new LGraphNode('test')
+    node.addInput('input', 'INT')
+    node.addWidget('number', 'testnum', 2, () => undefined, {})
+    graph.add(node)
 
-  const { vueNodeData } = useGraphNodeManager(graph)
-  const onReactivityUpdate = vi.fn()
-  watch(vueNodeData, onReactivityUpdate)
+    const { vueNodeData } = useGraphNodeManager(graph)
 
-  return [node, graph, onReactivityUpdate] as const
-}
+    return { node, graph, vueNodeData }
+  }
 
-describe('Node Reactivity', () => {
-  it('should trigger on callback', async () => {
-    const [node, , onReactivityUpdate] = createTestGraph()
+  it('widget values are reactive through the store', async () => {
+    const { node } = createTestGraph()
+    const store = useWidgetValueStore()
+    const widget = node.widgets![0]
+
+    // Verify widget is a BaseWidget with correct value and node assignment
+    expect(widget).toBeInstanceOf(BaseWidget)
+    expect(widget.value).toBe(2)
+    expect((widget as BaseWidget).node.id).toBe(node.id)
+
+    // Initial value should be in store after setNodeId was called
+    expect(store.getWidget(node.id, 'testnum')?.value).toBe(2)
 
-    node.widgets![0].callback!(2)
+    const onValueChange = vi.fn()
+    const widgetValue = computed(
+      () => store.getWidget(node.id, 'testnum')?.value
+    )
+    watch(widgetValue, onValueChange)
+
+    widget.value = 42
     await nextTick()
-    expect(onReactivityUpdate).toHaveBeenCalledTimes(1)
+
+    expect(widgetValue.value).toBe(42)
+    expect(onValueChange).toHaveBeenCalledTimes(1)
   })
 
-  it('should remain reactive after a connection is made', async () => {
-    const [node, graph, onReactivityUpdate] = createTestGraph()
+  it('widget values remain reactive after a connection is made', async () => {
+    const { node, graph } = createTestGraph()
+    const store = useWidgetValueStore()
+    const onValueChange = vi.fn()
 
     graph.trigger('node:slot-links:changed', {
-      nodeId: '1',
+      nodeId: String(node.id),
       slotType: NodeSlotType.INPUT
     })
     await nextTick()
-    onReactivityUpdate.mockClear()
 
-    node.widgets![0].callback!(2)
+    const widgetValue = computed(
+      () => store.getWidget(node.id, 'testnum')?.value
+    )
+    watch(widgetValue, onValueChange)
+
+    node.widgets![0].value = 99
     await nextTick()
-    expect(onReactivityUpdate).toHaveBeenCalledTimes(1)
+
+    expect(onValueChange).toHaveBeenCalledTimes(1)
+    expect(widgetValue.value).toBe(99)
   })
 })