Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/composables/useCoreCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import { useBottomPanelStore } from '@/stores/workspace/bottomPanelStore'
import { useColorPaletteStore } from '@/stores/workspace/colorPaletteStore'
import { useSearchBoxStore } from '@/stores/workspace/searchBoxStore'
import { useWorkspaceStore } from '@/stores/workspaceStore'
import { useEagerExecutionStore } from '@/stores/eagerExecutionStore'
import {
getAllNonIoNodesInSubgraph,
getExecutionIdsForSelectedNodes
Expand Down Expand Up @@ -1225,6 +1226,25 @@ export function useCoreCommands(): ComfyCommand[] {
icon: 'pi pi-database',
label: 'toggle linear mode',
function: () => (canvasStore.linearMode = !canvasStore.linearMode)
},
{
id: 'Comfy.ToggleEagerExecution',
icon: 'pi pi-bolt',
label: 'Toggle Eager Execution',
function: () => {
const eagerExecutionStore = useEagerExecutionStore()
eagerExecutionStore.toggle()
toastStore.add({
severity: 'info',
summary: eagerExecutionStore.enabled
? t('Eager execution enabled')
: t('Eager execution disabled'),
detail: eagerExecutionStore.enabled
? t('Nodes will auto-execute when ancestors change')
: t('Auto-execution disabled'),
life: 3000
})
}
}
Comment on lines +1230 to 1248
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix i18n key usage in toast messages.

Lines 1240-1244 pass plain English strings to the t() function, but they should be i18n keys. Other commands in this file follow the pattern of using keys like t('g.interrupted') or t('toastMessages.interrupted').

Apply this diff to use proper i18n keys:

       function: () => {
         const eagerExecutionStore = useEagerExecutionStore()
         eagerExecutionStore.toggle()
         toastStore.add({
           severity: 'info',
           summary: eagerExecutionStore.enabled
-            ? t('Eager execution enabled')
-            : t('Eager execution disabled'),
+            ? t('eagerExecution.enabled')
+            : t('eagerExecution.disabled'),
           detail: eagerExecutionStore.enabled
-            ? t('Nodes will auto-execute when ancestors change')
-            : t('Auto-execution disabled'),
+            ? t('eagerExecution.enabledDetail')
+            : t('eagerExecution.disabledDetail'),
           life: 3000
         })
       }

Note: You'll also need to add these keys to your i18n locale files.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{
id: 'Comfy.ToggleEagerExecution',
icon: 'pi pi-bolt',
label: 'Toggle Eager Execution',
function: () => {
const eagerExecutionStore = useEagerExecutionStore()
eagerExecutionStore.toggle()
toastStore.add({
severity: 'info',
summary: eagerExecutionStore.enabled
? t('Eager execution enabled')
: t('Eager execution disabled'),
detail: eagerExecutionStore.enabled
? t('Nodes will auto-execute when ancestors change')
: t('Auto-execution disabled'),
life: 3000
})
}
}
{
id: 'Comfy.ToggleEagerExecution',
icon: 'pi pi-bolt',
label: 'Toggle Eager Execution',
function: () => {
const eagerExecutionStore = useEagerExecutionStore()
eagerExecutionStore.toggle()
toastStore.add({
severity: 'info',
summary: eagerExecutionStore.enabled
? t('eagerExecution.enabled')
: t('eagerExecution.disabled'),
detail: eagerExecutionStore.enabled
? t('eagerExecution.enabledDetail')
: t('eagerExecution.disabledDetail'),
life: 3000
})
}
}
🤖 Prompt for AI Agents
In src/composables/useCoreCommands.ts around lines 1230 to 1248, the toast
summary/detail currently pass plain English strings into t() instead of i18n
keys; replace those literal strings with descriptive i18n keys (e.g.
t('toastMessages.eagerEnabled') / t('toastMessages.eagerDisabled') for summary
and t('toastMessages.eagerAutoExecuteEnabled') /
t('toastMessages.eagerAutoExecuteDisabled') for detail) and ensure the
enabled/disabled ternary logic remains the same; finally add matching entries
for these new keys to the project's locale files.

]

Expand Down
10 changes: 10 additions & 0 deletions src/renderer/extensions/vueNodes/components/NodeWidgets.vue
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ import {
getComponent,
shouldRenderAsVue
} from '@/renderer/extensions/vueNodes/widgets/registry/widgetRegistry'
import { app } from '@/scripts/app'
import { eagerExecutionService } from '@/services/eagerExecutionService'
import type { SimplifiedWidget, WidgetValue } from '@/types/simplifiedWidget'
import { cn } from '@/utils/tailwindUtil'

Expand Down Expand Up @@ -170,6 +172,14 @@ const processedWidgets = computed((): ProcessedWidget[] => {
if (widget.type !== 'asset') {
widget.callback?.(value)
}

// Trigger eager execution for this widget change in Vue Nodes mode
if (nodeData?.id) {
const node = app.rootGraph?.getNodeById(Number(nodeData.id))
if (node) {
eagerExecutionService.onNodeChanged(node)
}
}
}

const tooltipText = getWidgetTooltip(widget)
Expand Down
296 changes: 296 additions & 0 deletions src/services/eagerExecutionService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,296 @@
import log from 'loglevel'

import { LGraph } from '@/lib/litegraph/src/litegraph'
import type { LGraphNode, NodeId } from '@/lib/litegraph/src/litegraph'
import { api } from '@/scripts/api'
import { app } from '@/scripts/app'
import { useQueuePendingTaskCountStore } from '@/stores/queueStore'

const logger = log.getLogger('EagerExecutionService')
logger.setLevel('debug')
Comment on lines +9 to +10
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Make logger level configurable.

The logger is hard-coded to 'debug' level, which will spam console logs in production. Since this is a prototype, consider making the log level configurable or conditional based on environment.

 const logger = log.getLogger('EagerExecutionService')
-logger.setLevel('debug')
+logger.setLevel(
+  import.meta.env.DEV || import.meta.env.MODE === 'development' 
+    ? 'debug' 
+    : 'warn'
+)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const logger = log.getLogger('EagerExecutionService')
logger.setLevel('debug')
const logger = log.getLogger('EagerExecutionService')
logger.setLevel(
import.meta.env.DEV || import.meta.env.MODE === 'development'
? 'debug'
: 'warn'
)
🤖 Prompt for AI Agents
In src/services/eagerExecutionService.ts around lines 9 to 10, the logger level
is hard-coded to 'debug'; change this to read a configurable value (e.g., from
process.env.LOG_LEVEL or an existing config object) and apply it only if
provided, with a safe default such as 'info' or 'warn' for production; ensure
the code validates/normalizes the level before calling logger.setLevel and
document the default behavior so production doesn't get debug noise.


class EagerExecutionService {
private enabled: boolean = false
private executionPending: boolean = false
private isExecuting: boolean = false
private changedNodes: Set<string> = new Set()
private graphChangedListenerAttached: boolean = false
private isSettingUpListeners: boolean = false
private lastChangeTimestamp: Map<string, number> = new Map()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Address memory leak in timestamp tracking.

The lastChangeTimestamp Map accumulates entries indefinitely and never removes them when nodes are deleted. This will cause a memory leak over time, especially in long-running sessions with many node create/delete operations.

Consider cleaning up stale entries periodically or on node deletion:

  onNodeChanged(node: LGraphNode) {
    if (!this.enabled) return

    const nodeId = String(node.id)

    const now = Date.now()
    const lastChange = this.lastChangeTimestamp.get(nodeId) || 0
    const timeSinceLastChange = now - lastChange

    if (timeSinceLastChange < 50) {
      return
    }

    this.lastChangeTimestamp.set(nodeId, now)
+   
+   // Clean up old entries periodically to prevent memory leak
+   if (this.lastChangeTimestamp.size > 1000) {
+     const cutoff = now - 60000 // Remove entries older than 1 minute
+     for (const [id, timestamp] of this.lastChangeTimestamp.entries()) {
+       if (timestamp < cutoff) {
+         this.lastChangeTimestamp.delete(id)
+       }
+     }
+   }

    // ... rest of method
  }

Also applies to: 142-147

🤖 Prompt for AI Agents
In src/services/eagerExecutionService.ts around line 19 (and also referencing
logic at lines 142-147), the lastChangeTimestamp Map accumulates node IDs
indefinitely causing a memory leak; modify the code to remove entries when nodes
are deleted by calling lastChangeTimestamp.delete(nodeId) in the node-deletion
path(s) and/or when nodes are garbage-collected, and add an optional periodic
cleanup (setInterval) that scans the map and deletes entries older than a
configured TTL (e.g., 1h), making sure to clear that interval on service
shutdown to avoid timers running indefinitely.


private wrappedNodes: Set<NodeId> = new Set()
private wrappedWidgets: WeakSet<object> = new WeakSet()
private graphPatched: boolean = false

enable() {
if (this.enabled) {
return
}
this.enabled = true
this.setupEventListeners()
}

disable() {
if (!this.enabled) {
return
}
this.enabled = false
}
Comment on lines +33 to +38
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Disable method lacks cleanup.

The disable() method only sets the enabled flag but doesn't clean up event listeners or state. If eager execution is toggled on/off multiple times, event listeners will accumulate.

Consider adding cleanup logic:

  disable() {
    if (!this.enabled) {
      return
    }
    this.enabled = false
+   // Clear state tracking
+   this.changedNodes.clear()
+   this.lastChangeTimestamp.clear()
+   this.executionPending = false
  }

Note: Event listeners and prototype patches cannot easily be removed without additional tracking. Document this limitation if keeping the current approach.

Committable suggestion skipped: line range outside the PR's diff.


private setupEventListeners() {
this.setupWidgetListeners()

if (!this.graphChangedListenerAttached) {
logger.debug('Attaching graphChanged event listener')
api.addEventListener('graphChanged', () => {
if (this.enabled && !this.isExecuting) {
logger.debug('Graph changed, re-setting up widget listeners')
this.setupWidgetListeners()
}
})
this.graphChangedListenerAttached = true
}
}

private setupWidgetListeners() {
if (this.isSettingUpListeners) {
logger.debug('Already setting up listeners, skipping duplicate call')
return
}

const graph = app.rootGraph
if (!graph) {
setTimeout(() => this.setupWidgetListeners(), 100)
return
}

this.isSettingUpListeners = true
logger.debug('Setting up widget listeners for all nodes')

try {
this.wrappedNodes.clear()

graph.nodes.forEach((node) => {
this.attachWidgetCallbacks(node)
})

if (!this.graphPatched) {
const originalAdd = LGraph.prototype.add
LGraph.prototype.add = function (node: LGraphNode) {
const result = originalAdd.call(this, node)
if (eagerExecutionService.enabled) {
eagerExecutionService.attachWidgetCallbacks(node)
}
return result
}
this.graphPatched = true
}

logger.debug('Finished setting up widget listeners')
} finally {
this.isSettingUpListeners = false
}
}

private attachWidgetCallbacks(node: LGraphNode) {
if (!node.widgets) return

if (this.wrappedNodes.has(node.id)) {
logger.debug(
`Skipping callback attachment for ${node.title || node.type} (${node.id}) - already attached`
)
return
}
this.wrappedNodes.add(node.id)

logger.debug(
`Attaching callbacks to ${node.title || node.type} (${node.id}) - ${node.widgets.length} widget(s)`
)

node.widgets.forEach((widget, index) => {
if (this.wrappedWidgets.has(widget)) {
logger.debug(
` Widget ${index} (${widget.name}) already wrapped, skipping`
)
return
}
this.wrappedWidgets.add(widget)

const originalCallback = widget.callback

widget.callback = (value?: any) => {
if (originalCallback) {
originalCallback.call(widget, value)
}

if (this.enabled) {
this.onNodeChanged(node)
}
}
})
}

onNodeChanged(node: LGraphNode) {
if (!this.enabled) return

const nodeId = String(node.id)

const now = Date.now()
const lastChange = this.lastChangeTimestamp.get(nodeId) || 0
const timeSinceLastChange = now - lastChange

if (timeSinceLastChange < 50) {
return
}

this.lastChangeTimestamp.set(nodeId, now)

if (this.isExecuting) {
return
}

const queueStore = useQueuePendingTaskCountStore()
if (queueStore.count > 0) {
return
}

this.changedNodes.add(nodeId)

if (!this.executionPending) {
this.scheduleExecution()
}
}

private scheduleExecution() {
this.executionPending = true

setTimeout(() => {
void this.executeEager()
this.executionPending = false
this.changedNodes.clear()
}, 300)
}

private async executeEager() {
if (!app.rootGraph || this.changedNodes.size === 0) return

const queueStore = useQueuePendingTaskCountStore()

if (queueStore.count > 0 || this.isExecuting) {
logger.info('Execution already in progress, skipping eager execution')
return
}

this.isExecuting = true

try {
logger.info('===== EAGER EXECUTION TRIGGERED =====')

const changedNodesInfo = Array.from(this.changedNodes).map((nodeId) => {
const node = app.rootGraph?.getNodeById(Number(nodeId))
const title = node?.title || node?.type || `Node ${nodeId}`
return `${title} (${nodeId})`
})
logger.info(`Changed nodes: [${changedNodesInfo.join(', ')}]`)

const affectedNodes = this.getAffectedNodes(
app.rootGraph,
this.changedNodes
)

if (affectedNodes.size === 0) {
logger.info('No downstream nodes to execute')
logger.info('===== EAGER EXECUTION COMPLETE =====')
return
}

const affectedNodesInfo = Array.from(affectedNodes).map((nodeId) => {
const node = app.rootGraph?.getNodeById(Number(nodeId))
const title = node?.title || node?.type || `Node ${nodeId}`
return `${title} (${nodeId})`
})
logger.info(`Nodes to execute: [${affectedNodesInfo.join(', ')}]`)
logger.info(`Total: ${affectedNodes.size} node(s) will execute`)

const allNodesInfo = app.rootGraph?.nodes.map((node) => {
const nodeId = String(node.id)
const title = node.title || node.type || `Node ${nodeId}`
const willExecute = affectedNodes.has(nodeId)

if (willExecute) return `WILL EXECUTE - ${title} (${nodeId})`
return `SKIPPED - ${title} (${nodeId})`
})
logger.info('Full execution plan:')
allNodesInfo?.forEach((info) => logger.info(` ${info}`))

logger.info(
`Triggering partial execution with ${affectedNodes.size} targets...`
)

await app.queuePrompt(0, 1, Array.from(affectedNodes))
logger.info('===== EAGER EXECUTION COMPLETE =====')
} catch (error) {
logger.error('Failed to execute eagerly:', error)
} finally {
this.isExecuting = false
}
}

private getAffectedNodes(
graph: LGraph,
changedNodeIds: Set<string>
): Set<string> {
const affected = new Set<string>()
const visited = new Set<string>()

logger.info('Analyzing downstream dependencies...')

const findDownstream = (nodeId: string, depth: number = 0) => {
if (visited.has(nodeId)) return
visited.add(nodeId)

const node = graph.getNodeById(Number(nodeId))
if (!node) return

const indent = ' '.repeat(depth)

if (node.outputs) {
node.outputs.forEach((output) => {
if (!output.links || output.links.length === 0) return

output.links.forEach((linkId) => {
const link = graph.links.get(linkId)
if (!link) return

const targetNodeId = String(link.target_id)
const targetNode = graph.getNodeById(link.target_id)
const targetTitle =
targetNode?.title || targetNode?.type || `Node ${targetNodeId}`

logger.info(
`${indent} └─> ${targetTitle} (${targetNodeId}) - will execute`
)
affected.add(targetNodeId)
findDownstream(targetNodeId, depth + 1)
})
})
}
}

changedNodeIds.forEach((nodeId) => {
const node = graph.getNodeById(Number(nodeId))
const nodeTitle = node?.title || node?.type || `Node ${nodeId}`

logger.info(`Starting from: ${nodeTitle} (${nodeId})`)
affected.add(nodeId)

findDownstream(nodeId, 1)
})

logger.info(`Analysis complete: ${affected.size} node(s) will execute`)

return affected
}
}

export const eagerExecutionService = new EagerExecutionService()
Loading