Chore: TypeScript cleanup - remove 254 @ts-expect-error suppressions #7884
Conversation
📝 WalkthroughWalkthroughReplaced the flat group-node workflow model with explicit map-based types for links/externals/IO, updated public GroupNode APIs/serialisation, and tightened TypeScript signatures across group-node logic, graph typings, app surface, and PNG/EXIF importer code. Changes
Possibly related PRs
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/10/2026, 09:09:52 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results✅ All tests passed! ⏰ Completed at: 01/10/2026, 09:15:13 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.25 MB (baseline 3.25 MB) • 🔴 +4.9 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 1.05 MB (baseline 1.05 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.63 kB (baseline 6.63 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 337 kB (baseline 337 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 199 kB (baseline 199 kB) • ⚪ 0 BReusable component library chunks
Status: 8 added / 8 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 1.41 kB (baseline 1.41 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 9.19 MB (baseline 9.19 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 4.73 MB (baseline 4.73 MB) • ⚪ 0 BBundles that do not match a named category
Status: 16 added / 16 removed |
…ication Amp-Thread-ID: https://ampcode.com/threads/T-019ba620-f16c-74b2-aff7-890551ba6be9 Co-authored-by: Amp <[email protected]>
dosubot
bot
added
the
size:XXL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
src/scripts/app.ts (3)
490-512: Avoid unsafe casts when treatingvalueas a file-like object.
const valueObj = value as Record<string, unknown> | undefined+ latervalue as ResultItemcan still mis-handle non-object widget values. Prefer a narrow type guard for{ filename: string }before building thesubfolder/filenamestring.Proposed refactor
- const valueObj = value as Record<string, unknown> | undefined + const valueObj = + typeof value === 'object' && value !== null + ? (value as Record<string, unknown>) + : undefined if ( prop.type != 'image' && typeof prop.value == 'string' && - valueObj?.filename + typeof valueObj?.filename === 'string' ) { - const resultItem = value as ResultItem + const resultItem = valueObj as unknown as ResultItem prop.value = (resultItem.subfolder ? resultItem.subfolder + '/' : '') + resultItem.filename + (resultItem.type ? ` [${resultItem.type}]` : '') } else {
972-1014: Fix localStorage clipboard restore on parse/insert failures.If
JSON.parse(template.data)ordeserialiseAndCreate()throws,litegrapheditor_clipboardmay stay modified and break subsequent paste behavior. Wrap per-template work intryand always restoreoldinfinally.Proposed fix
loadTemplateData(templateData: { templates?: { name?: string; data?: string }[] }): void { if (!templateData?.templates) { return } const old = localStorage.getItem('litegrapheditor_clipboard') + try { for (const template of templateData.templates) { if (!template?.data) { continue } // Check for old clipboard format - const data = JSON.parse(template.data) - if (!data.reroutes) { - deserialiseAndCreate(template.data, app.canvas) - } else { - localStorage.setItem('litegrapheditor_clipboard', template.data) - app.canvas.pasteFromClipboard() - } + try { + const data = JSON.parse(template.data) + if (!data.reroutes) { + deserialiseAndCreate(template.data, app.canvas) + } else { + localStorage.setItem('litegrapheditor_clipboard', template.data) + app.canvas.pasteFromClipboard() + } + } catch (e) { + console.warn('Failed to load template data, skipping template', e) + continue + } // Move mouse position down to paste the next template below let maxY: number | undefined for (const i in app.canvas.selected_nodes) { const node = app.canvas.selected_nodes[i] const nodeBottom = node.pos[1] + node.size[1] if (maxY === undefined || nodeBottom > maxY) { maxY = nodeBottom } } if (maxY !== undefined) { app.canvas.graph_mouse[1] = maxY + 50 } } - if (old !== null) { - localStorage.setItem('litegrapheditor_clipboard', old) - } + } finally { + if (old !== null) localStorage.setItem('litegrapheditor_clipboard', old) + } }
153-320: Add defensive getters or optional chaining guards for definite-assignment fields.
canvas!,ctx!,extensionManager!,_nodeOutputs!are declared with non-null assertions but not initialized untilsetup()runs. While the codebase already uses optional chaining extensively (e.g.,app.canvas?.setDirty()), consider adding explicit defensive getters that throw with a clear error message (e.g., "canvas accessed before app.setup()") to catch pre-setup access paths more explicitly.src/extensions/core/groupNodeManage.ts (2)
121-128: Remove the new@ts-expect-errorby wideningGroupNodeHandler.getGroupDatainput type.
GroupNodeHandler.getGroupData(this.groupNodeType)works with the implementation (it checksnode.nodeData ?? node.constructor?.nodeData), but the signature only acceptsLGraphNode. Add an overload to acceptLGraphNodeConstructor<LGraphNode>and drop the suppression. As per coding guidelines/learnings: avoid@ts-expect-errorwhen a proper type is feasible.
485-515: Typetypesand eliminate index-based suppressions by normalizing indices.This block is doing data-shape surgery; it’s worth making it typesafe:
- type
typesasRecord<string, GroupNodeWorkflowData>- normalize
l[0]/l[2]/idthroughNumber(...)and guardNumber.isFinite(...)before indexing.Also applies to: 538-539
src/extensions/core/nodeTemplates.ts (1)
370-410: Handle missing group-node dependencies when applying templates.You call
GroupNodeConfig.registerFromWorkflow(data.groupNodes ?? {}, [])but discard the missing-node output, so failures become silent (group types won’t register, paste may degrade). Consider capturingmissingNodeTypesand surfacing a toast + abort paste if anything is missing.Example adjustment
- await GroupNodeConfig.registerFromWorkflow( - data.groupNodes ?? {}, - [] - ) + const missing: (string | { type: string; hint?: string; action?: unknown })[] = [] + await GroupNodeConfig.registerFromWorkflow(data.groupNodes ?? {}, missing) + if (missing.length) { + useToastStore().addAlert(t('toastMessages.missingNodesInTemplate')) + return + }src/scripts/pnginfo.ts (2)
81-135: Fix WebP EXIF parsing: incorrect offset adjustment + missing bounds checks.Current logic (
offset += 6) changes the chunk header offset and then slices using the originalchunk_length, which can overrun the EXIF chunk and/or misparse data. Also,getUint32(offset + 4)needsoffset + 8 <= lengthguards to avoid OOB reads on malformed files.Proposed fix
reader.onload = (event) => { - const webp = new Uint8Array(event.target?.result as ArrayBuffer) + const buf = event.target?.result + if (!(buf instanceof ArrayBuffer)) { + r({}) + return + } + const webp = new Uint8Array(buf) const dataView = new DataView(webp.buffer) // Start searching for chunks after the WEBP signature let offset = 12 const txt_chunks: Record<string, string> = {} // Loop through the chunks in the WEBP file - while (offset < webp.length) { + while (offset + 8 <= webp.length) { const chunk_length = dataView.getUint32(offset + 4, true) const chunk_type = String.fromCharCode( ...webp.slice(offset, offset + 4) ) if (chunk_type === 'EXIF') { - if ( - String.fromCharCode(...webp.slice(offset + 8, offset + 8 + 6)) == - 'Exif\0\0' - ) { - offset += 6 - } - let data = parseExifData( - webp.slice(offset + 8, offset + 8 + chunk_length) - ) + const start = offset + 8 + const end = Math.min(start + chunk_length, webp.length) + const hasHeader = + end - start >= 6 && + String.fromCharCode(...webp.slice(start, start + 6)) === 'Exif\0\0' + const exifStart = start + (hasHeader ? 6 : 0) + const data = parseExifData(webp.slice(exifStart, end)) for (const key in data) { const value = data[Number(key)] if (typeof value === 'string') { const index = value.indexOf(':') txt_chunks[value.slice(0, index)] = value.slice(index + 1) } } break } offset += 8 + chunk_length }
259-299: Bug:createLoraNodeshardcodes output slots; breaks when no Loras are present.
prevClip.node.connect(1, clipNode, 0)ignoresprevClip.index. When there are zero Lora tags,prevClip.indexis0(clipSkipNode output), so connecting output slot1is likely wrong. Same issue for model (prevModel.index).Proposed fix
- prevClip.node.connect(1, clipNode, 0) - prevModel.node.connect(0, targetSamplerNode, 0) + prevClip.node.connect(prevClip.index, clipNode, 0) + prevModel.node.connect(prevModel.index, targetSamplerNode, 0) if (hrSamplerNode) { - prevModel.node.connect(0, hrSamplerNode, 0) + prevModel.node.connect(prevModel.index, hrSamplerNode, 0) }src/extensions/core/groupNode.ts (2)
302-352: Validate numeric coercions ingetLinks()to avoidNaNkeys / broken maps.
Number(sourceNodeId)etc. can becomeNaNif the serialized data has strings or unexpected nullables; that will createthis.linksFrom[NaN]and silently break routing. GuardNumber.isFinite(...)andcontinueon invalid IDs/slots.Proposed fix
const srcId = Number(sourceNodeId) const srcSlot = Number(sourceNodeSlot) const tgtId = Number(targetNodeId) const tgtSlot = Number(targetNodeSlot) + if ( + !Number.isFinite(srcId) || + !Number.isFinite(srcSlot) || + !Number.isFinite(tgtId) || + !Number.isFinite(tgtSlot) + ) + continue
1300-1330: Removeasyncfrom menu callbacks; invoke underlying synchronous functions directly.The context menu callbacks in litegraph are invoked synchronously (see ContextMenu.ts line 312:
value.callback.call(...)) and not awaited. Marking callbacks asasynccreates unhandled promise rejections if they throw, and return values (Promises) are ignored by the caller. SinceconvertToNodes()andconvertSelectedNodesToGroupNode()are synchronous, remove theasyncwrapper and call them directly.This affects three locations: lines 1300–1330, 1916–1921, and 1942–1949.
🤖 Fix all issues with AI agents
In @src/extensions/core/groupNode.ts:
- Around line 1765-1780: Add an overload for getGroupData to accept a node
constructor: declare getGroupData(node: LGraphNodeConstructor<LGraphNode>):
GroupNodeConfig | undefined, and update the implementation signature to accept
node: LGraphNode | LGraphNodeConstructor<LGraphNode>; inside the function use a
simple type guard (e.g. check if the argument is a function/constructor) or
narrow with "in" checks so you can read either node.nodeData (for constructors)
or node.nodeData / node.constructor?.nodeData (for instances) without
@ts-expect-error; adjust callers in groupNodeManage.ts to pass the constructor
overload and remove the downstream suppressions referencing GROUP/ nodeData
where applicable.
In @src/extensions/core/groupNodeManage.ts:
- Around line 485-510: The external remap currently assigns a node object to
ext[0] (ext[0] = type.nodes[ext[0]]), corrupting the expected [nodeIndex,
slotIndex, type] shape; change the assignment to store the numeric index instead
(ext[0] = type.nodes[ext[0]].index) and add a null/undefined guard like the link
rewrite does (check ext[0] != null) so you only replace valid node indices.
Ensure this change is applied in the loop handling type.external (variables:
type.external, ext, type.nodes).
In @src/lib/litegraph/src/LGraph.ts:
- Around line 105-123: The GroupNodeWorkflowData.links type is too loose and
should be the tuple array type used by the serializer; update the links property
on the GroupNodeWorkflowData interface to use SerialisedLLinkArray[] instead of
(number | string | null)[][], import the SerialisedLLinkArray type from LLink
(e.g. import type { SerialisedLLinkArray } from '...LLink') and adjust any
consumers (e.g. code in groupNode.ts that reads index 5 for type) to rely on the
stronger tuple shape.
In @src/scripts/app.ts:
- Around line 1549-1580: The code in processNodeInputs uses convertWidgetToInput
and then assumes the new input is at index (node.inputs?.length ?? 1) - 1 which
is brittle; update the logic so after calling convertWidgetToInput on the widget
you re-find the target slot by name (node.inputs?.findIndex(inp => inp.name ===
input)) and use that index if found, only falling back to the last input if the
inputs array length actually increased compared to a captured pre-conversion
length; if the slot still isn’t found, skip connecting (don’t assume an index).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
src/extensions/core/groupNode.tssrc/extensions/core/groupNodeManage.tssrc/extensions/core/nodeTemplates.tssrc/lib/litegraph/src/LGraph.tssrc/platform/workflow/validation/schemas/workflowSchema.tssrc/scripts/app.tssrc/scripts/pnginfo.ts
💤 Files with no reviewable changes (1)
- src/platform/workflow/validation/schemas/workflowSchema.ts
🧰 Additional context used
📓 Path-based instructions (8)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/extensions/core/groupNodeManage.tssrc/extensions/core/nodeTemplates.tssrc/lib/litegraph/src/LGraph.tssrc/scripts/app.tssrc/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
src/**/*.ts: Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Use es-toolkit for utility functions
Minimize the surface area (exported values) of each module and composable
Favor pure functions, especially testable ones
Files:
src/extensions/core/groupNodeManage.tssrc/extensions/core/nodeTemplates.tssrc/lib/litegraph/src/LGraph.tssrc/scripts/app.tssrc/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
src/**/*.{ts,tsx,vue}: Use separateimport typestatements instead of inlinetypein mixed imports
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, 80-character width
Sort and group imports by plugin, runpnpm formatbefore committing
Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Write code that is expressive and self-documenting - avoid unnecessary comments
Do not add or retain redundant comments - clean as you go
Avoid mutable state - prefer immutability and assignment at point of declaration
Watch out for Code Smells and refactor to avoid them
Files:
src/extensions/core/groupNodeManage.tssrc/extensions/core/nodeTemplates.tssrc/lib/litegraph/src/LGraph.tssrc/scripts/app.tssrc/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/extensions/core/groupNodeManage.tssrc/extensions/core/nodeTemplates.tssrc/lib/litegraph/src/LGraph.tssrc/scripts/app.tssrc/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
src/**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,vue}: Usereffor reactive state,computed()for derived values, andwatch/watchEffectfor side effects in Composition API
Avoid usingrefwithwatchif acomputedwould suffice - minimize refs and derived state
Useprovide/injectfor dependency injection only when simpler alternatives (Store or shared composable) won't work
Leverage VueUse functions for performance-enhancing composables
Use VueUse function for useI18n in composition API for string literals
Files:
src/extensions/core/groupNodeManage.tssrc/extensions/core/nodeTemplates.tssrc/lib/litegraph/src/LGraph.tssrc/scripts/app.tssrc/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional
Minimize nesting (if statements, for loops, etc.)
Use function declarations instead of function expressions when possible
Files:
src/extensions/core/groupNodeManage.tssrc/extensions/core/nodeTemplates.tssrc/lib/litegraph/src/LGraph.tssrc/scripts/app.tssrc/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
src/lib/litegraph/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
src/lib/litegraph/**/*.{js,ts,jsx,tsx}: Run ESLint instead of manually figuring out whitespace fixes or other trivial style concerns using thepnpm lint:fixcommand
Take advantage ofTypedArraysubarraywhen appropriate
Thesizeandposproperties ofRectangleshare the same array buffer (subarray); they may be used to set the rectangle's size and position
Prefer single lineifsyntax over adding curly braces, when the statement has a very concise expression and concise, single line statement
Do not replace&&=or||=with=when there is no reason to do so. If you do find a reason to remove either&&=or||=, leave a comment explaining why the removal occurred
When writing methods, prefer returning idiomatic JavaScriptundefinedovernull
Files:
src/lib/litegraph/src/LGraph.ts
src/lib/litegraph/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
Type assertions are an absolute last resort. In almost all cases, they are a crutch that leads to brittle code
Files:
src/lib/litegraph/src/LGraph.ts
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Implement proper TypeScript types throughout the codebase
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7894
File: src/renderer/extensions/vueNodes/widgets/components/WidgetToggleSwitch.test.ts:11-14
Timestamp: 2026-01-08T02:40:15.482Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: When testing components, import the real type definitions from the component files instead of duplicating interface definitions in the test files. This prevents type drift and maintains consistency.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.ts : Use TypeScript for type safety
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Do not replace `&&=` or `||=` with `=` when there is no reason to do so. If you do find a reason to remove either `&&=` or `||=`, leave a comment explaining why the removal occurred
Applied to files:
src/extensions/core/groupNodeManage.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/extensions/core/groupNodeManage.tssrc/extensions/core/nodeTemplates.tssrc/lib/litegraph/src/LGraph.tssrc/scripts/app.tssrc/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/extensions/core/groupNodeManage.tssrc/extensions/core/nodeTemplates.tssrc/lib/litegraph/src/LGraph.tssrc/scripts/app.tssrc/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/extensions/core/groupNodeManage.tssrc/extensions/core/nodeTemplates.tssrc/lib/litegraph/src/LGraph.tssrc/scripts/app.tssrc/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/extensions/core/groupNodeManage.tssrc/extensions/core/nodeTemplates.tssrc/lib/litegraph/src/LGraph.tssrc/scripts/app.tssrc/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/extensions/core/groupNodeManage.tssrc/extensions/core/nodeTemplates.tssrc/lib/litegraph/src/LGraph.tssrc/scripts/app.tssrc/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
src/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Implement proper TypeScript types throughout the codebase
Applied to files:
src/scripts/app.ts
📚 Learning: 2026-01-08T02:40:15.482Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7894
File: src/renderer/extensions/vueNodes/widgets/components/WidgetToggleSwitch.test.ts:11-14
Timestamp: 2026-01-08T02:40:15.482Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: When testing components, import the real type definitions from the component files instead of duplicating interface definitions in the test files. This prevents type drift and maintains consistency.
Applied to files:
src/scripts/app.tssrc/extensions/core/groupNode.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.ts : Use TypeScript for type safety
Applied to files:
src/scripts/app.ts
📚 Learning: 2026-01-10T00:24:17.662Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.662Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Never use `as any` type assertions - fix the underlying type issue
Applied to files:
src/scripts/app.ts
📚 Learning: 2026-01-10T00:24:17.662Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.662Z
Learning: Applies to src/**/*.ts : Minimize the surface area (exported values) of each module and composable
Applied to files:
src/scripts/app.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/stores/**/*.{ts,tsx} : Maintain clear public interfaces and restrict extension access in stores
Applied to files:
src/scripts/app.ts
📚 Learning: 2026-01-10T00:24:17.662Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.662Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Use separate `import type` statements instead of inline `type` in mixed imports
Applied to files:
src/scripts/app.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid using ts-expect-error; use proper TypeScript types instead
Applied to files:
src/scripts/app.ts
📚 Learning: 2026-01-10T00:24:17.662Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.662Z
Learning: Applies to src/**/*.{ts,tsx} : Minimize nesting (if statements, for loops, etc.)
Applied to files:
src/scripts/app.ts
📚 Learning: 2026-01-09T02:07:54.558Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7898
File: src/composables/usePaste.test.ts:248-248
Timestamp: 2026-01-09T02:07:54.558Z
Learning: In test files at src/**/*.test.ts, when creating mock objects that partially implement an interface (e.g., LGraphNode), use `as Partial<InterfaceType> as InterfaceType` instead of `as any` or `as unknown as InterfaceType` to explicitly acknowledge the incomplete implementation while maintaining type safety.
Applied to files:
src/scripts/app.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : When writing tests for subgraph-related code, always import from the barrel export at `@/lib/litegraph/src/litegraph` to avoid circular dependency issues
Applied to files:
src/extensions/core/groupNode.tssrc/scripts/pnginfo.ts
📚 Learning: 2026-01-09T07:29:27.929Z
Learnt from: LittleSound
Repo: Comfy-Org/ComfyUI_frontend PR: 7812
File: src/components/rightSidePanel/RightSidePanel.vue:100-132
Timestamp: 2026-01-09T07:29:27.929Z
Learning: The `findParentGroupInGraph` function in `src/components/rightSidePanel/RightSidePanel.vue` is a temporary workaround for a bug where group sub-items were not updating correctly after a page refresh. It can be removed once that underlying bug is fixed.
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Take advantage of `TypedArray` `subarray` when appropriate
Applied to files:
src/scripts/pnginfo.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{ts,tsx} : Type assertions are an absolute last resort. In almost all cases, they are a crutch that leads to brittle code
Applied to files:
src/scripts/pnginfo.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Prefer single line `if` syntax over adding curly braces, when the statement has a very concise expression and concise, single line statement
Applied to files:
src/scripts/pnginfo.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : When writing methods, prefer returning idiomatic JavaScript `undefined` over `null`
Applied to files:
src/scripts/pnginfo.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Run ESLint instead of manually figuring out whitespace fixes or other trivial style concerns using the `pnpm lint:fix` command
Applied to files:
src/scripts/pnginfo.ts
🧬 Code graph analysis (4)
src/extensions/core/groupNodeManage.ts (1)
src/extensions/core/groupNode.ts (1)
GroupNodeConfig(225-934)
src/extensions/core/nodeTemplates.ts (1)
src/extensions/core/groupNode.ts (2)
GroupNodeHandler(936-1810)GroupNodeConfig(225-934)
src/lib/litegraph/src/LGraph.ts (2)
src/lib/litegraph/src/interfaces.ts (1)
Dictionary(14-14)src/lib/litegraph/src/types/serialisation.ts (1)
SerialisableReroute(193-199)
src/extensions/core/groupNode.ts (1)
src/lib/litegraph/src/LGraph.ts (2)
GroupNodeWorkflowData(105-115)nodes(400-402)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: playwright-tests-chromium-sharded (8, 8)
- GitHub Check: playwright-tests-chromium-sharded (1, 8)
- GitHub Check: playwright-tests-chromium-sharded (3, 8)
- GitHub Check: playwright-tests-chromium-sharded (4, 8)
- GitHub Check: playwright-tests-chromium-sharded (5, 8)
- GitHub Check: playwright-tests-chromium-sharded (2, 8)
- GitHub Check: playwright-tests-chromium-sharded (7, 8)
- GitHub Check: playwright-tests-chromium-sharded (6, 8)
- GitHub Check: playwright-tests (chromium-2x)
- GitHub Check: playwright-tests (chromium-0.5x)
- GitHub Check: playwright-tests (mobile-chrome)
- GitHub Check: test
| const processNodeInputs = (id: string) => { | ||
| const data = apiData[id] | ||
| const node = app.rootGraph.getNodeById(id) | ||
| for (const input in data.inputs ?? {}) { | ||
| const value = data.inputs[input] | ||
| if (value instanceof Array) { | ||
| const [fromId, fromSlot] = value | ||
| const fromNode = app.rootGraph.getNodeById(fromId) | ||
| // @ts-expect-error fixme ts strict error | ||
| let toSlot = node.inputs?.findIndex((inp) => inp.name === input) | ||
| if (toSlot == null || toSlot === -1) { | ||
| try { | ||
| // Target has no matching input, most likely a converted widget | ||
| // @ts-expect-error fixme ts strict error | ||
| const widget = node.widgets?.find((w) => w.name === input) | ||
| // @ts-expect-error | ||
| if (widget && node.convertWidgetToInput?.(widget)) { | ||
| // @ts-expect-error fixme ts strict error | ||
| toSlot = node.inputs?.length - 1 | ||
| } | ||
| } catch (error) {} | ||
| } | ||
| if (toSlot != null || toSlot !== -1) { | ||
| // @ts-expect-error fixme ts strict error | ||
| fromNode.connect(fromSlot, node, toSlot) | ||
| } | ||
| } else { | ||
| // @ts-expect-error fixme ts strict error | ||
| const widget = node.widgets?.find((w) => w.name === input) | ||
| if (widget) { | ||
| widget.value = value | ||
| widget.callback?.(value) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| app.rootGraph.arrange() | ||
| if (!node) return | ||
|
|
||
| for (const id of ids) { | ||
| const data = apiData[id] | ||
| const node = app.rootGraph.getNodeById(id) | ||
| for (const input in data.inputs ?? {}) { | ||
| const value = data.inputs[input] | ||
| if (value instanceof Array) { | ||
| const [fromId, fromSlot] = value | ||
| const fromNode = app.rootGraph.getNodeById(fromId) | ||
| // @ts-expect-error fixme ts strict error | ||
| let toSlot = node.inputs?.findIndex((inp) => inp.name === input) | ||
| if (toSlot == null || toSlot === -1) { | ||
| if (!fromNode) continue | ||
|
|
||
| let toSlot = node.inputs?.findIndex((inp) => inp.name === input) ?? -1 | ||
| if (toSlot === -1) { | ||
| try { | ||
| // Target has no matching input, most likely a converted widget | ||
| // @ts-expect-error fixme ts strict error | ||
| const widget = node.widgets?.find((w) => w.name === input) | ||
| // @ts-expect-error | ||
| if (widget && node.convertWidgetToInput?.(widget)) { | ||
| // @ts-expect-error fixme ts strict error | ||
| toSlot = node.inputs?.length - 1 | ||
| const convertFn = ( | ||
| node as LGraphNode & { | ||
| convertWidgetToInput?: (w: IBaseWidget) => boolean | ||
| } | ||
| ).convertWidgetToInput | ||
| if (widget && convertFn?.(widget)) { | ||
| toSlot = (node.inputs?.length ?? 1) - 1 | ||
| } | ||
| } catch (error) {} | ||
| } catch (_error) { | ||
| // Ignore conversion errors | ||
| } | ||
| } | ||
| if (toSlot != null || toSlot !== -1) { | ||
| // @ts-expect-error fixme ts strict error | ||
| if (toSlot !== -1) { | ||
| fromNode.connect(fromSlot, node, toSlot) | ||
| } | ||
| } else { |
There was a problem hiding this comment.
convertWidgetToInput slot selection is brittle; recompute after conversion.
toSlot = (node.inputs?.length ?? 1) - 1 assumes conversion appends exactly one input at the end. If conversion inserts elsewhere (or doesn’t create an input), connections may go to the wrong slot. Prefer re-finding by name after conversion, and only fall back to “last input” if the input list actually grew.
Proposed fix
if (toSlot === -1) {
try {
const widget = node.widgets?.find((w) => w.name === input)
const convertFn = (
node as LGraphNode & {
convertWidgetToInput?: (w: IBaseWidget) => boolean
}
).convertWidgetToInput
- if (widget && convertFn?.(widget)) {
- toSlot = (node.inputs?.length ?? 1) - 1
- }
+ const beforeLen = node.inputs?.length ?? 0
+ if (widget && convertFn?.(widget)) {
+ toSlot = node.inputs?.findIndex((inp) => inp.name === input) ?? -1
+ const afterLen = node.inputs?.length ?? 0
+ if (toSlot === -1 && afterLen === beforeLen + 1) toSlot = afterLen - 1
+ }
} catch (_error) {
// Ignore conversion errors
}
}🤖 Prompt for AI Agents
In @src/scripts/app.ts around lines 1549 - 1580, The code in processNodeInputs
uses convertWidgetToInput and then assumes the new input is at index
(node.inputs?.length ?? 1) - 1 which is brittle; update the logic so after
calling convertWidgetToInput on the widget you re-find the target slot by name
(node.inputs?.findIndex(inp => inp.name === input)) and use that index if found,
only falling back to the last input if the inputs array length actually
increased compared to a captured pre-conversion length; if the slot still isn’t
found, skip connecting (don’t assume an index).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/extensions/core/groupNode.ts:
- Line 3: Replace the inline type import syntax with a dedicated type-only
import for GroupNodeWorkflowData: change the current import that uses "type"
inline to an "import type { GroupNodeWorkflowData }" form from the same module
so the compiler/tsserver treats it as a purely erased type import; update the
import statement where GroupNodeWorkflowData is referenced (the import at the
top of groupNode.ts) accordingly.
- Line 1860: The module-level mutable variable globalDefs is populated by
addCustomNodeDefs and later mutated by refreshComboInNodes, which creates an
implicit initialization dependency; add a clear documentation comment above the
globalDefs declaration stating that globalDefs is initialized via the
addCustomNodeDefs callback before use, that refreshComboInNodes mutates it, and
any functions reading it must ensure addCustomNodeDefs has run (include expected
lifecycle order and thread-safety/usage notes for maintainers).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/extensions/core/groupNode.ts
🧰 Additional context used
📓 Path-based instructions (6)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/extensions/core/groupNode.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
src/**/*.ts: Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Use es-toolkit for utility functions
Minimize the surface area (exported values) of each module and composable
Favor pure functions, especially testable ones
Files:
src/extensions/core/groupNode.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
src/**/*.{ts,tsx,vue}: Use separateimport typestatements instead of inlinetypein mixed imports
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, 80-character width
Sort and group imports by plugin, runpnpm formatbefore committing
Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Write code that is expressive and self-documenting - avoid unnecessary comments
Do not add or retain redundant comments - clean as you go
Avoid mutable state - prefer immutability and assignment at point of declaration
Watch out for Code Smells and refactor to avoid them
Files:
src/extensions/core/groupNode.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/extensions/core/groupNode.ts
src/**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,vue}: Usereffor reactive state,computed()for derived values, andwatch/watchEffectfor side effects in Composition API
Avoid usingrefwithwatchif acomputedwould suffice - minimize refs and derived state
Useprovide/injectfor dependency injection only when simpler alternatives (Store or shared composable) won't work
Leverage VueUse functions for performance-enhancing composables
Use VueUse function for useI18n in composition API for string literals
Files:
src/extensions/core/groupNode.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional
Minimize nesting (if statements, for loops, etc.)
Use function declarations instead of function expressions when possible
Files:
src/extensions/core/groupNode.ts
🧠 Learnings (10)
📚 Learning: 2026-01-09T02:07:54.558Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7898
File: src/composables/usePaste.test.ts:248-248
Timestamp: 2026-01-09T02:07:54.558Z
Learning: In test files at src/**/*.test.ts, when creating mock objects that partially implement an interface (e.g., LGraphNode), use `as Partial<InterfaceType> as InterfaceType` instead of `as any` or `as unknown as InterfaceType` to explicitly acknowledge the incomplete implementation while maintaining type safety.
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2026-01-08T02:40:15.482Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7894
File: src/renderer/extensions/vueNodes/widgets/components/WidgetToggleSwitch.test.ts:11-14
Timestamp: 2026-01-08T02:40:15.482Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: When testing components, import the real type definitions from the component files instead of duplicating interface definitions in the test files. This prevents type drift and maintains consistency.
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : When writing tests for subgraph-related code, always import from the barrel export at `@/lib/litegraph/src/litegraph` to avoid circular dependency issues
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2026-01-09T07:29:27.929Z
Learnt from: LittleSound
Repo: Comfy-Org/ComfyUI_frontend PR: 7812
File: src/components/rightSidePanel/RightSidePanel.vue:100-132
Timestamp: 2026-01-09T07:29:27.929Z
Learning: The `findParentGroupInGraph` function in `src/components/rightSidePanel/RightSidePanel.vue` is a temporary workaround for a bug where group sub-items were not updating correctly after a page refresh. It can be removed once that underlying bug is fixed.
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/extensions/core/groupNode.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: setup
- GitHub Check: collect
🔇 Additional comments (9)
src/extensions/core/groupNode.ts (9)
33-82: Well-structured type definitions.The new interfaces (
GroupNodeLink,GroupNodeInput,GroupNodeOutput,GroupNodeData,GroupNodeDef,NodeConfigEntry,SerializedGroupConfig) and map types provide proper typing for the group node data model, significantly improving type safety.
111-223: LGTM!The
GroupNodeBuilderclass has been properly typed withGroupNodeWorkflowData, includes appropriate null checks (line 212-213), and the sort comparison (lines 165-169) handles string IDs correctly for stability.
885-927: LGTM!The
registerFromWorkflowmethod has proper typing for parameters, handles missing node types correctly with null checks (line 898), and the callback properly typesMouseEventandHTMLElement(lines 908-913).
1764-1774:getHandlerfactory pattern is well-implemented.The method correctly handles the case where the handler hasn't been created yet by the async
nodeCreatedhook, synchronously creating it if needed. The@ts-expect-errorat line 1765 is necessary for GROUP symbol indexing onLGraphNode.
1422-1455: Event handling properly typed.The
EventDetailtype andhandleEventhelper function provide good type safety while accommodating the legacy custom event API. The type assertions at lines 1446 and 1450-1453 are necessary for the dynamic event system.
1738-1757: Guard condition handles edge cases correctly.The check at line 1741 properly filters out object-typed
actualOriginIdvalues. The@ts-expect-errorat line 1751 documents the use of the deprecatedconnectinterface that accepts node ID instead of node reference.
1905-1943: Async menu callbacks require@ts-expect-error.The
@ts-expect-errorcomments at lines 1913-1914 and 1940-1941 are necessary because the legacyIContextMenuValue.callbacktype doesn't supportPromisereturn types. These are legitimate legacy API boundary exceptions per PR constraints.
1495-1500: Widget value type handling.The type assertions for
widget.options.values(line 1495, 1497) andwidget.value(lines 1499-1500) rely on runtime data structure assumptions. This is acceptable given the dynamic nature of widget configurations, but consider adding a runtime guard if widget value mismatches cause issues.
608-611: Remove@ts-expect-errorand use proper TypeScript types instead.The suppressions at lines 608–611, 714, 758, and 852 violate coding guidelines. The real issue is that Zod's type inference produces readonly types; the schema's index signature via
z.record(zInputSpec)is already sufficient. Assign mutated properties toRecord<string, unknown>or useas const satisfiesto properly narrow the type at object creation without suppressions.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/scripts/app.ts (3)
972-1014: Clipboard restore logic likely regressed when there was no previous clipboard value.If
old === null, we currently leave the last template’s clipboard content behind instead of restoring “no clipboard”. That’s a user-visible behavior change.Proposed fix
- if (old !== null) { - localStorage.setItem('litegrapheditor_clipboard', old) - } + if (old !== null) localStorage.setItem('litegrapheditor_clipboard', old) + else localStorage.removeItem('litegrapheditor_clipboard')
490-513: Narrowvaluebefore casting toResultItemto avoid unsafe property access.
const resultItem = value as ResultItemis only guarded byvalueObj?.filename; objects can havefilenamewithout matching theResultItemshape. A minimal structural guard keeps this safe.Proposed fix
- if ( + if ( prop.type != 'image' && typeof prop.value == 'string' && valueObj?.filename ) { - const resultItem = value as ResultItem + const resultItem = value as Partial<ResultItem> prop.value = - (resultItem.subfolder ? resultItem.subfolder + '/' : '') + - resultItem.filename + - (resultItem.type ? ` [${resultItem.type}]` : '') + ((resultItem.subfolder ? resultItem.subfolder + '/' : '') + + String(resultItem.filename) + + (resultItem.type ? ` [${resultItem.type}]` : '')) } else {
1549-1596: Remove redundantarrange()call and consider documenting the two-pass input processing.There are two consecutive
app.rootGraph.arrange()calls at the end; one is redundant. Also, runningprocessNodeInputstwice is non-obvious—worth a short comment if intentional (conversion side-effects).Proposed fix
for (const id of ids) processNodeInputs(id) app.rootGraph.arrange() for (const id of ids) processNodeInputs(id) app.rootGraph.arrange() - app.rootGraph.arrange()src/extensions/core/groupNodeManage.ts (1)
484-516: Null-check isn’t enough: add bounds checks when rewriting links/externals/config.
l[0] != null/ext[0] != nullstill permits out-of-range indices and will throw ontype.nodes[idx].index. This is especially risky after reorder operations.Proposed fix
for (const l of type.links) { - if (l[0] != null) l[0] = type.nodes[l[0]].index - if (l[2] != null) l[2] = type.nodes[l[2]].index + const src = l[0] + const tgt = l[2] + if (typeof src === 'number' && type.nodes[src]) l[0] = type.nodes[src].index + if (typeof tgt === 'number' && type.nodes[tgt]) l[2] = type.nodes[tgt].index } ... for (const ext of type.external) { - if (ext[0] != null) { - ext[0] = type.nodes[ext[0]].index - } + const idx = ext[0] + if (typeof idx === 'number' && type.nodes[idx]) ext[0] = type.nodes[idx].index }src/extensions/core/groupNode.ts (1)
1294-1327: Returning[]discards menu items from the originalgetExtraMenuOptionsmethod.The code calls
getExtraMenuOptions?.call(this, _canvas, options)without capturing its return value, then always returns[]. Per theLGraphCanvasinterface,getExtraMenuOptionsshould return(IContextMenuValue<string> | null)[], and the call sites at line 8060 and 8186 inLGraphCanvas.tsexpect and use these return values via concatenation. If the original node's method returned items, they will be lost. While options mutations survive, the return contract is violated. The inline comment suggests this is intentional to avoid concatenation, but it's still a behavioral change.
🤖 Fix all issues with AI agents
In @src/extensions/core/groupNode.ts:
- Around line 1521-1534: The conversion of linked.nodeId to linkedNodeId using
Number(linked.nodeId) can yield NaN and cause silent misses when indexing
this.innerNodes; update the code in the loop around linkedNodeId/linkedNode to
validate the converted index (e.g., const linkedNodeId = typeof linked.nodeId
=== 'number' ? linked.nodeId : Number(linked.nodeId); if
(!Number.isFinite(linkedNodeId) || Number.isNaN(linkedNodeId)) continue;) or use
parseInt and check Number.isInteger before using linkedNodeId to index
this.innerNodes, skipping any non-numeric ids to avoid accessing
this.innerNodes[NaN].
- Around line 1882-1899: globalDefs is declared uninitialized which can crash if
accessed before addCustomNodeDefs runs; initialize it to an empty object (e.g.,
let globalDefs: Record<string, ComfyNodeDef> = {}) and add lightweight guards in
readers like getNodeDef and checkPrimitiveConnection (use safe lookups like
globalDefs?.[node.type] or default empty object) so code remains safe during
partial/late initialization and tests; also ensure refreshComboInNodes merges
into the existing object rather than replacing the reference.
In @src/extensions/core/groupNodeManage.ts:
- Around line 539-540: The local variable passed to
GroupNodeConfig.registerFromWorkflow is untyped (const types = {}) — update it
to use the actual parameter type from registerFromWorkflow to prevent
any/implicit-typing leaks: locate the signature of
GroupNodeConfig.registerFromWorkflow and type the variable (e.g. const types:
ExpectedType = { ... }) or derive it via a utility type (e.g. const types:
Parameters<typeof GroupNodeConfig.registerFromWorkflow>[0] = { ... }); ensure
the shape matches the required fields and remove any casts/suppressions.
In @src/lib/litegraph/src/LGraph.ts:
- Line 18: The config property in LGraph is typed as Record<number, unknown> but
JSON.parse converts object keys to strings, so update the config type to
Record<string, unknown> (and any related usages) to match runtime data; locate
the config declaration on the LGraph class and replace Record<number, unknown>
with Record<string, unknown>, and adjust any accesses like config[node.index] to
use string keys (e.g., String(node.index) or ensure node.index is a string) so
property lookups work after JSON.parse.
In @src/scripts/app.ts:
- Around line 1181-1184: The code mutates ModelFile by setting an ad-hoc
directory_invalid property on m; instead create a separate side-structure (e.g.,
a Set or Map such as invalidModelFiles or invalidDirectoryMap) and add m (or a
unique id from m) to that structure when modelFolder is falsy; remove the cast
and property write to directory_invalid on m (the code around modelFolder and
the variable m / ModelFile), and update any later consumers to check the
side-structure rather than reading a synthetic directory_invalid field.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/extensions/core/groupNode.tssrc/extensions/core/groupNodeManage.tssrc/lib/litegraph/src/LGraph.tssrc/scripts/app.ts
🧰 Additional context used
📓 Path-based instructions (8)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/extensions/core/groupNodeManage.tssrc/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
src/**/*.ts: Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Use es-toolkit for utility functions
Minimize the surface area (exported values) of each module and composable
Favor pure functions, especially testable ones
Files:
src/extensions/core/groupNodeManage.tssrc/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
src/**/*.{ts,tsx,vue}: Use separateimport typestatements instead of inlinetypein mixed imports
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, 80-character width
Sort and group imports by plugin, runpnpm formatbefore committing
Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Write code that is expressive and self-documenting - avoid unnecessary comments
Do not add or retain redundant comments - clean as you go
Avoid mutable state - prefer immutability and assignment at point of declaration
Watch out for Code Smells and refactor to avoid them
Files:
src/extensions/core/groupNodeManage.tssrc/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/extensions/core/groupNodeManage.tssrc/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
src/**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,vue}: Usereffor reactive state,computed()for derived values, andwatch/watchEffectfor side effects in Composition API
Avoid usingrefwithwatchif acomputedwould suffice - minimize refs and derived state
Useprovide/injectfor dependency injection only when simpler alternatives (Store or shared composable) won't work
Leverage VueUse functions for performance-enhancing composables
Use VueUse function for useI18n in composition API for string literals
Files:
src/extensions/core/groupNodeManage.tssrc/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional
Minimize nesting (if statements, for loops, etc.)
Use function declarations instead of function expressions when possible
Files:
src/extensions/core/groupNodeManage.tssrc/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
src/lib/litegraph/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
src/lib/litegraph/**/*.{js,ts,jsx,tsx}: Run ESLint instead of manually figuring out whitespace fixes or other trivial style concerns using thepnpm lint:fixcommand
Take advantage ofTypedArraysubarraywhen appropriate
Thesizeandposproperties ofRectangleshare the same array buffer (subarray); they may be used to set the rectangle's size and position
Prefer single lineifsyntax over adding curly braces, when the statement has a very concise expression and concise, single line statement
Do not replace&&=or||=with=when there is no reason to do so. If you do find a reason to remove either&&=or||=, leave a comment explaining why the removal occurred
When writing methods, prefer returning idiomatic JavaScriptundefinedovernull
Files:
src/lib/litegraph/src/LGraph.ts
src/lib/litegraph/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
Type assertions are an absolute last resort. In almost all cases, they are a crutch that leads to brittle code
Files:
src/lib/litegraph/src/LGraph.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid using ts-expect-error; use proper TypeScript types instead
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Implement proper TypeScript types throughout the codebase
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7894
File: src/renderer/extensions/vueNodes/widgets/components/WidgetToggleSwitch.test.ts:11-14
Timestamp: 2026-01-08T02:40:15.482Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: When testing components, import the real type definitions from the component files instead of duplicating interface definitions in the test files. This prevents type drift and maintains consistency.
Learnt from: Myestery
Repo: Comfy-Org/ComfyUI_frontend PR: 7422
File: .github/workflows/pr-update-playwright-expectations.yaml:131-135
Timestamp: 2025-12-12T23:02:37.473Z
Learning: In the `.github/workflows/pr-update-playwright-expectations.yaml` workflow in the Comfy-Org/ComfyUI_frontend repository, the snapshot update process is intentionally scoped to only add and update snapshot images. Deletions of snapshot files are handled explicitly outside this workflow and should not be suggested as part of this automation.
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Do not replace `&&=` or `||=` with `=` when there is no reason to do so. If you do find a reason to remove either `&&=` or `||=`, leave a comment explaining why the removal occurred
Applied to files:
src/extensions/core/groupNodeManage.tssrc/extensions/core/groupNode.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/extensions/core/groupNodeManage.tssrc/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/extensions/core/groupNodeManage.tssrc/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/extensions/core/groupNodeManage.tssrc/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/extensions/core/groupNodeManage.tssrc/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/extensions/core/groupNodeManage.tssrc/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
src/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.ts
📚 Learning: 2026-01-09T02:07:54.558Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7898
File: src/composables/usePaste.test.ts:248-248
Timestamp: 2026-01-09T02:07:54.558Z
Learning: In test files at src/**/*.test.ts, when creating mock objects that partially implement an interface (e.g., LGraphNode), use `as Partial<InterfaceType> as InterfaceType` instead of `as any` or `as unknown as InterfaceType` to explicitly acknowledge the incomplete implementation while maintaining type safety.
Applied to files:
src/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{ts,tsx} : Type assertions are an absolute last resort. In almost all cases, they are a crutch that leads to brittle code
Applied to files:
src/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.tssrc/scripts/app.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Take advantage of `TypedArray` `subarray` when appropriate
Applied to files:
src/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : When writing tests for subgraph-related code, always import from the barrel export at `@/lib/litegraph/src/litegraph` to avoid circular dependency issues
Applied to files:
src/lib/litegraph/src/LGraph.tssrc/extensions/core/groupNode.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Run ESLint instead of manually figuring out whitespace fixes or other trivial style concerns using the `pnpm lint:fix` command
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2026-01-10T00:24:17.662Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.662Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Use separate `import type` statements instead of inline `type` in mixed imports
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Prefer single line `if` syntax over adding curly braces, when the statement has a very concise expression and concise, single line statement
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2026-01-09T07:29:27.929Z
Learnt from: LittleSound
Repo: Comfy-Org/ComfyUI_frontend PR: 7812
File: src/components/rightSidePanel/RightSidePanel.vue:100-132
Timestamp: 2026-01-09T07:29:27.929Z
Learning: The `findParentGroupInGraph` function in `src/components/rightSidePanel/RightSidePanel.vue` is a temporary workaround for a bug where group sub-items were not updating correctly after a page refresh. It can be removed once that underlying bug is fixed.
Applied to files:
src/extensions/core/groupNode.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Implement proper TypeScript types throughout the codebase
Applied to files:
src/scripts/app.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.ts : Use TypeScript for type safety
Applied to files:
src/scripts/app.ts
📚 Learning: 2026-01-10T00:24:17.662Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.662Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Write tests for all changes, especially bug fixes to catch future regressions
Applied to files:
src/scripts/app.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/stores/**/*.{ts,tsx} : Maintain clear public interfaces and restrict extension access in stores
Applied to files:
src/scripts/app.ts
📚 Learning: 2026-01-10T00:24:17.662Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.662Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Never use `as any` type assertions - fix the underlying type issue
Applied to files:
src/scripts/app.ts
🧬 Code graph analysis (3)
src/lib/litegraph/src/LGraph.ts (3)
src/lib/litegraph/src/LLink.ts (1)
SerialisedLLinkArray(28-35)src/lib/litegraph/src/interfaces.ts (1)
Dictionary(14-14)src/lib/litegraph/src/types/serialisation.ts (1)
SerialisableReroute(193-199)
src/extensions/core/groupNode.ts (6)
src/lib/litegraph/src/LLink.ts (1)
SerialisedLLinkArray(28-35)src/lib/litegraph/src/LGraph.ts (2)
GroupNodeWorkflowData(105-115)nodes(400-402)src/lib/litegraph/src/LGraphNode.ts (9)
NodeId(94-94)slot(3922-3937)slot(3959-3966)slot(3968-3970)inputs(3309-3313)LGraphNode(211-213)LGraphNode(223-4198)LGraphNode(745-753)LGraphNode(755-761)src/types/index.ts (2)
ComfyNodeDef(30-30)InputSpec(31-31)src/utils/executableGroupNodeDto.ts (1)
GROUP(11-11)src/constants/groupNodeConstants.ts (2)
PREFIX(7-7)SEPARATOR(8-8)
src/scripts/app.ts (2)
src/types/index.ts (2)
ExtensionManager(62-62)ComfyApp(29-29)src/schemas/apiSchema.ts (1)
NodeExecutionOutput(36-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (2)
src/extensions/core/groupNode.ts (2)
1761-1795:getGroupData()/getHandler()consolidation looks good.Centralizing GROUP-symbol access removes scattered assertions and should reduce future
@ts-expect-errorchurn.
35-39: Type aliasGroupNodeLink = SerialisedLLinkArrayis imprecise but runtime code is correct.The vintageClipboard serialization produces a 5-element array
[fromNodeIndex, fromSlot, toNodeIndex, toSlot, originNodeId], which is then extended to 6 elements instoreLinkTypes()by pushing the type. The current index accesses (link[0], link[1], link[2], link[3], link[4]) are correct for this format. However, the type alias is misleading since initially the array doesn't match the full SerialisedLLinkArray structure until after type extraction.Consider renaming to clarify the actual persisted format, or documenting that it's extended post-processing. No runtime fixes needed for the index accesses themselves.
| if (innerNode.type === 'PrimitiveNode') { | ||
| // @ts-expect-error primitiveValue is a custom property on PrimitiveNode | ||
| innerNode.primitiveValue = newValue | ||
| const primitiveLinked = this.groupData.primitiveToWidget[old.node.index] | ||
| const primitiveLinked = this.groupData.primitiveToWidget[nodeIdx] | ||
| for (const linked of primitiveLinked ?? []) { | ||
| const node = this.innerNodes[linked.nodeId] | ||
| // @ts-expect-error fixme ts strict error | ||
| const widget = node.widgets.find((w) => w.name === linked.inputName) | ||
| const linkedNodeId = | ||
| typeof linked.nodeId === 'number' | ||
| ? linked.nodeId | ||
| : Number(linked.nodeId) | ||
| const linkedNode = this.innerNodes[linkedNodeId] | ||
| const widget = linkedNode?.widgets?.find( | ||
| (w) => w.name === linked.inputName | ||
| ) | ||
|
|
There was a problem hiding this comment.
Guard against NaN when converting linked.nodeId to an inner-node index.
Number(linked.nodeId) can become NaN (non-numeric string ids), which will index this.innerNodes[NaN] as undefined and silently skip updates.
🤖 Prompt for AI Agents
In @src/extensions/core/groupNode.ts around lines 1521 - 1534, The conversion of
linked.nodeId to linkedNodeId using Number(linked.nodeId) can yield NaN and
cause silent misses when indexing this.innerNodes; update the code in the loop
around linkedNodeId/linkedNode to validate the converted index (e.g., const
linkedNodeId = typeof linked.nodeId === 'number' ? linked.nodeId :
Number(linked.nodeId); if (!Number.isFinite(linkedNodeId) ||
Number.isNaN(linkedNodeId)) continue;) or use parseInt and check
Number.isInteger before using linkedNodeId to index this.innerNodes, skipping
any non-numeric ids to avoid accessing this.innerNodes[NaN].
| /** | ||
| * Global node definitions cache, populated and mutated by extension callbacks. | ||
| * | ||
| * **Initialization**: Set by `addCustomNodeDefs` during extension initialization. | ||
| * This callback runs early in the app lifecycle, before any code that reads | ||
| * `globalDefs` is executed. | ||
| * | ||
| * **Mutation**: `refreshComboInNodes` merges updated definitions into this object | ||
| * when combo options are refreshed (e.g., after model files change). | ||
| * | ||
| * **Usage Notes**: | ||
| * - Functions reading `globalDefs` (e.g., `getNodeDef`, `checkPrimitiveConnection`) | ||
| * must only be called after `addCustomNodeDefs` has run. | ||
| * - Not thread-safe; assumes single-threaded JS execution model. | ||
| * - The object reference is stable after initialization; only contents are mutated. | ||
| */ | ||
| let globalDefs: Record<string, ComfyNodeDef> | ||
| const ext: ComfyExtension = { |
There was a problem hiding this comment.
Make globalDefs safe-by-default (initialize + guard) to avoid hard crashes.
let globalDefs: Record<string, ComfyNodeDef> is relied on as initialized “early”, but any lifecycle shift (tests, partial init, future refactor) will cause globalDefs[node.type] to throw.
Proposed fix
-let globalDefs: Record<string, ComfyNodeDef>
+let globalDefs: Record<string, ComfyNodeDef> = {}
...
addCustomNodeDefs(defs) {
// Store this so we can mutate it later with group nodes
- globalDefs = defs
+ globalDefs = defs ?? {}
},Also applies to: 1995-1999
🤖 Prompt for AI Agents
In @src/extensions/core/groupNode.ts around lines 1882 - 1899, globalDefs is
declared uninitialized which can crash if accessed before addCustomNodeDefs
runs; initialize it to an empty object (e.g., let globalDefs: Record<string,
ComfyNodeDef> = {}) and add lightweight guards in readers like getNodeDef and
checkPrimitiveConnection (use safe lookups like globalDefs?.[node.type] or
default empty object) so code remains safe during partial/late initialization
and tests; also ensure refreshComboInNodes merges into the existing object
rather than replacing the reference.
| await GroupNodeConfig.registerFromWorkflow(types, []) | ||
|
|
There was a problem hiding this comment.
Type types to match the new registerFromWorkflow signature.
const types = {} is currently untyped, so it’s easy to accidentally pass the wrong shape while still compiling via any leakage in this file.
Proposed fix
- const types = {}
+ const types: Record<string, import('@/lib/litegraph/src/LGraph').GroupNodeWorkflowData> = {}Based on learnings, avoid leaving type holes / suppressions where a proper type is available.
🤖 Prompt for AI Agents
In @src/extensions/core/groupNodeManage.ts around lines 539 - 540, The local
variable passed to GroupNodeConfig.registerFromWorkflow is untyped (const types
= {}) — update it to use the actual parameter type from registerFromWorkflow to
prevent any/implicit-typing leaks: locate the signature of
GroupNodeConfig.registerFromWorkflow and type the variable (e.g. const types:
ExpectedType = { ... }) or derive it via a utility type (e.g. const types:
Parameters<typeof GroupNodeConfig.registerFromWorkflow>[0] = { ... }); ensure
the shape matches the required fields and remove any casts/suppressions.
| import type { NodeId } from './LGraphNode' | ||
| import { LLink } from './LLink' | ||
| import type { LinkId } from './LLink' | ||
| import type { LinkId, SerialisedLLinkArray } from './LLink' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the GroupNodeWorkflowData interface definition in LGraph.ts
cat -n src/lib/litegraph/src/LGraph.ts | sed -n '1,150p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 5392
🏁 Script executed:
# Search for GroupNodeWorkflowData usage
rg -n "GroupNodeWorkflowData" src/lib/litegraph/src/ src/extensions/core/Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1180
🏁 Script executed:
# Look for how groupNodes.links and groupNodes.config are accessed
rg -n "groupNodes\[" src/extensions/core/groupNode.ts src/extensions/core/groupNodeManage.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 976
🏁 Script executed:
# Check for JSON serialization patterns with groupNodes
rg -n "JSON\.(stringify|parse)" src/extensions/core/groupNode.ts src/extensions/core/groupNodeManage.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 271
🏁 Script executed:
# Check the SerialisedLLinkArray definition in LLink.ts
rg -n "type SerialisedLLinkArray|interface SerialisedLLinkArray" src/lib/litegraph/src/LLink.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 109
🏁 Script executed:
# Look at the LLink.ts file to understand the structure
cat -n src/lib/litegraph/src/LLink.ts | head -60Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2505
🏁 Script executed:
# Check how links are actually used in groupNode and groupNodeManage when serializing
rg -A 3 -B 3 "\.links" src/extensions/core/groupNode.ts | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3190
🏁 Script executed:
# Check actual config usage patterns
rg -n "\.config" src/extensions/core/groupNode.ts src/extensions/core/groupNodeManage.ts | head -30Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1071
🏁 Script executed:
# Let's look at how SerializedGroupConfig is defined and what the actual structure is
rg -n "SerializedGroupConfig|interface.*GroupConfig" src/extensions/core/groupNode.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 317
🏁 Script executed:
# Check the actual structure more carefully - look at the getNodeData function
cat -n src/extensions/core/groupNode.ts | sed -n '175,230p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2386
🏁 Script executed:
# Search for where links are actually created/stored to see the real structure
rg -B 5 -A 5 "links.*push|links.*\[\]" src/extensions/core/groupNode.ts | head -60Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1530
🏁 Script executed:
# Look for GroupNodeLink definition
rg -n "type GroupNodeLink|interface GroupNodeLink" src/extensions/core/groupNode.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 114
🏁 Script executed:
# Get more context around line 80 where SerializedGroupConfig is defined
cat -n src/extensions/core/groupNode.ts | sed -n '70,95p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 875
🏁 Script executed:
# Check actual link indices being used in the getLinks function
cat -n src/extensions/core/groupNode.ts | sed -n '265,295p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1194
🏁 Script executed:
# Let me verify how config is actually keyed by checking if there's any numeric key conversion happening
rg -n "config\[" src/extensions/core/groupNode.ts src/extensions/core/groupNodeManage.ts | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 818
🏁 Script executed:
# Check if there's actual usage showing string vs number keys
cat -n src/extensions/core/groupNodeManage.ts | sed -n '462,470p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 580
🏁 Script executed:
# Check the serialise function to understand what shape SerializedGroupConfig actually has
rg -n "function serialise|const serialise" src/extensions/core/groupNode.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 125
🏁 Script executed:
# Let's trace one more time - check if config is directly JSON stringified/parsed or transformed
cat -n src/extensions/core/groupNode.ts | sed -n '213,225p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 594
🏁 Script executed:
# Double-check how config keys are used after parsing
cat -n src/extensions/core/groupNode.ts | sed -n '520,535p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 664
Change config type from Record<number, unknown> to Record<string, unknown> — JSON.parse converts all keys to strings.
After JSON.parse() at line 217, numeric keys become strings. The current Record<number, unknown> won't match the runtime data structure, causing property access like config[node.index] to fail. The links field is already correctly typed as SerialisedLLinkArray[].
Type fix
export interface GroupNodeWorkflowData {
external: (number | string)[][]
links: SerialisedLLinkArray[]
nodes: {
index?: number
type?: string
inputs?: unknown[]
outputs?: unknown[]
}[]
- config?: Record<number, unknown>
+ config?: Record<string, unknown>
}📝 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.
| import type { LinkId, SerialisedLLinkArray } from './LLink' | |
| export interface GroupNodeWorkflowData { | |
| external: (number | string)[][] | |
| links: SerialisedLLinkArray[] | |
| nodes: { | |
| index?: number | |
| type?: string | |
| inputs?: unknown[] | |
| outputs?: unknown[] | |
| }[] | |
| config?: Record<string, unknown> | |
| } |
🤖 Prompt for AI Agents
In @src/lib/litegraph/src/LGraph.ts at line 18, The config property in LGraph is
typed as Record<number, unknown> but JSON.parse converts object keys to strings,
so update the config type to Record<string, unknown> (and any related usages) to
match runtime data; locate the config declaration on the LGraph class and
replace Record<number, unknown> with Record<string, unknown>, and adjust any
accesses like config[node.index] to use string keys (e.g., String(node.index) or
ensure node.index is a string) so property lookups work after JSON.parse.
| extensionManager!: ExtensionManager | ||
| private _nodeOutputs!: Record<string, NodeExecutionOutput> | ||
| nodePreviewImages: Record<string, string[]> | ||
|
|
||
| private rootGraphInternal: LGraph | undefined |
There was a problem hiding this comment.
Avoid hard-crash non-null assertions for required DOM/canvas primitives.
extensionManager!/canvas!/ctx! and document.getElementById(...)! / getContext('2d')! will throw if the DOM layout or canvas context ever changes (including tests, embedded mounts, or partial renders). Prefer explicit guards with a clear error.
Proposed fix
async setup(canvasEl: HTMLCanvasElement) {
- this.bodyTop = document.getElementById('comfyui-body-top')!
- this.bodyLeft = document.getElementById('comfyui-body-left')!
- this.bodyRight = document.getElementById('comfyui-body-right')!
- this.bodyBottom = document.getElementById('comfyui-body-bottom')!
- this.canvasContainer = document.getElementById('graph-canvas-container')!
+ const bodyTop = document.getElementById('comfyui-body-top')
+ const bodyLeft = document.getElementById('comfyui-body-left')
+ const bodyRight = document.getElementById('comfyui-body-right')
+ const bodyBottom = document.getElementById('comfyui-body-bottom')
+ const canvasContainer = document.getElementById('graph-canvas-container')
+ if (!bodyTop || !bodyLeft || !bodyRight || !bodyBottom || !canvasContainer) {
+ throw new Error('ComfyUI layout is missing required DOM nodes')
+ }
+ this.bodyTop = bodyTop
+ this.bodyLeft = bodyLeft
+ this.bodyRight = bodyRight
+ this.bodyBottom = bodyBottom
+ this.canvasContainer = canvasContainer
...
- this.ctx = canvasEl.getContext('2d')!
+ const ctx = canvasEl.getContext('2d')
+ if (!ctx) throw new Error('Failed to acquire 2D canvas context')
+ this.ctx = ctx
}Also applies to: 175-181, 187-187, 749-755, 791-791
| if (!modelFolder) | ||
| (m as ModelFile & { directory_invalid?: boolean }).directory_invalid = | ||
| true | ||
|
|
There was a problem hiding this comment.
Don’t mutate ModelFile with ad-hoc fields.
(m as ModelFile & { directory_invalid?: boolean }).directory_invalid = true leaks an undocumented field into typed data (and can conflict with validation/serialization). Prefer a side map or a richer view-model type.
🤖 Prompt for AI Agents
In @src/scripts/app.ts around lines 1181 - 1184, The code mutates ModelFile by
setting an ad-hoc directory_invalid property on m; instead create a separate
side-structure (e.g., a Set or Map such as invalidModelFiles or
invalidDirectoryMap) and add m (or a unique id from m) to that structure when
modelFolder is falsy; remove the cast and property write to directory_invalid on
m (the code around modelFolder and the variable m / ModelFile), and update any
later consumers to check the side-structure rather than reading a synthetic
directory_invalid field.
Summary
Removes 254
@ts-expect-errorsuppressions through proper type fixes rather than type assertions.Key Changes
Type System Improvements
globalDefsandgroupNodestypes toComfyAppWindowExtensionGroupNodeHandler,InnerNodeOutput, etc.)getHandler()helper to consolidate GROUP symbol access patternFiles Fixed
Approach
Following established constraints:
anytypesas unknown as Tcasts (except legacy API boundaries)Validation
pnpm typecheckpassespnpm lintpassespnpm knippasses┆Issue is synchronized with this Notion page by Unito