Fix zombie linkIds on node deletion, add safety check #7153
Conversation
dosubot
bot
added
the
size:S
📝 WalkthroughWalkthroughHandles stale or missing subgraph output links during disconnection: adds a guard in Changes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (13)**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,js,vue}📄 CodeRabbit inference engine (.cursorrules)
Files:
src/**/*.{vue,ts}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/*.ts📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
src/**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
src/lib/litegraph/**/*.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
Files:
src/lib/litegraph/**/*.{ts,tsx}📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
Files:
🧠 Learnings (12)📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
📚 Learning: 2025-11-24T19:47:14.779ZApplied to files:
📚 Learning: 2025-11-24T19:47:34.324ZApplied to files:
📚 Learning: 2025-11-24T19:47:14.779ZApplied to files:
📚 Learning: 2025-11-24T19:47:56.371ZApplied to files:
🧬 Code graph analysis (1)src/lib/litegraph/src/LGraphNode.ts (1)
⏰ 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). (5)
🔇 Additional comments (2)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/04/2025, 08:03:12 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/04/2025, 08:17:03 PM 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.2 MB (baseline 3.2 MB) • 🔴 +384 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 972 kB (baseline 972 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 173 kB (baseline 173 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
Status: 17 added / 17 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/litegraph/src/subgraph/SubgraphOutput.ts (1)
156-175: LGTM! Safety check prevents errors from stale linkIds.The added guard correctly handles the case where a linkId exists but its corresponding link has already been removed (zombie linkIds). This aligns with the PR objectives and prevents potential null reference errors.
Optional improvement: Consider adding a debug log when stale linkIds are encountered to aid in troubleshooting:
override disconnect() { const { subgraph } = this.parent //should never have more than one connection for (const linkId of this.linkIds) { const link = subgraph.links[linkId] - if (!link) continue + if (!link) { + if (LiteGraph.debug) console.warn('SubgraphOutput.disconnect: stale linkId', linkId) + continue + } subgraph.removeLink(linkId)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/lib/litegraph/src/LGraphNode.ts(3 hunks)src/lib/litegraph/src/subgraph/SubgraphOutput.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.ts
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/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.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
Files:
src/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.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/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.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/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.ts
🧠 Learnings (7)
📚 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/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.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/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.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} : 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/lib/litegraph/src/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.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/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.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/lib/litegraph/src/subgraph/SubgraphOutput.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/subgraph/SubgraphOutput.tssrc/lib/litegraph/src/LGraphNode.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} : The `size` and `pos` properties of `Rectangle` share the same array buffer (`subarray`); they may be used to set the rectangle's size and position
Applied to files:
src/lib/litegraph/src/LGraphNode.ts
🧬 Code graph analysis (1)
src/lib/litegraph/src/LGraphNode.ts (2)
src/lib/litegraph/src/LGraph.ts (1)
Subgraph(2401-2672)src/lib/litegraph/src/litegraph.ts (1)
Subgraph(15-15)
🪛 ESLint
src/lib/litegraph/src/LGraphNode.ts
[error] 14-14: Unable to resolve path to module '@/lib/litegraph/src/constants'.
(import-x/no-unresolved)
[error] 49-49: Unable to resolve path to module './litegraph'.
(import-x/no-unresolved)
⏰ 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/lib/litegraph/src/LGraphNode.ts (2)
14-14: Import addition looks correct.The
SUBGRAPH_OUTPUT_IDconstant is imported to support the new subgraph output cleanup logic at lines 3066-3070.Note: The ESLint error about unresolved path is likely a false positive due to path alias configuration.
49-50: Import additions are necessary for the fix.The
SubgraphandSubgraphNodeimports support the instanceof check and type awareness for the new cleanup logic.Note: The ESLint error about unresolved path is a false positive related to module resolution configuration.
src/lib/litegraph/src/LGraphNode.ts
Outdated
| if ( | ||
| link_info.target_id === SUBGRAPH_OUTPUT_ID && | ||
| graph instanceof Subgraph | ||
| ) | ||
| graph.outputNode.slots[link_info.target_slot].linkIds.length = 0 |
There was a problem hiding this comment.
Add defensive checks to prevent potential runtime errors.
The logic correctly clears stale linkIds for subgraph outputs, but lacks validation that could cause crashes in edge cases.
Apply this diff to add defensive checks:
-if (
- link_info.target_id === SUBGRAPH_OUTPUT_ID &&
- graph instanceof Subgraph
-)
- graph.outputNode.slots[link_info.target_slot].linkIds.length = 0
+if (
+ link_info.target_id === SUBGRAPH_OUTPUT_ID &&
+ graph instanceof Subgraph
+) {
+ const slot = graph.outputNode?.slots?.[link_info.target_slot]
+ if (slot?.linkIds) {
+ slot.linkIds.length = 0
+ }
+}The optional chaining prevents crashes if:
outputNodeis undefinedslotsarray is undefinedtarget_slotindex is out of boundslinkIdsarray doesn't exist
📝 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.
| if ( | |
| link_info.target_id === SUBGRAPH_OUTPUT_ID && | |
| graph instanceof Subgraph | |
| ) | |
| graph.outputNode.slots[link_info.target_slot].linkIds.length = 0 | |
| if ( | |
| link_info.target_id === SUBGRAPH_OUTPUT_ID && | |
| graph instanceof Subgraph | |
| ) { | |
| const slot = graph.outputNode?.slots?.[link_info.target_slot] | |
| if (slot?.linkIds) { | |
| slot.linkIds.length = 0 | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/lib/litegraph/src/LGraphNode.ts around lines 3066-3070, the code clears
linkIds on a subgraph output without validating that graph.outputNode, its slots
array, the requested target_slot index, and the linkIds array exist; update the
logic to guard each access (use optional chaining / index bounds checks) so you
only set linkIds.length = 0 when graph.outputNode exists, graph.outputNode.slots
is an array, the target_slot index is within bounds, and the specific
slot.linkIds array exists — skip or no-op otherwise to avoid runtime exceptions.
There was a problem hiding this comment.
Given how often we accidentally try to dereference undefined values, I agree with the robot.
There was a problem hiding this comment.
@DrJKL, absolutely, defensive checks are essential here. Here's the improved code with proper guards:
if (
link_info.target_id === SUBGRAPH_OUTPUT_ID &&
graph instanceof Subgraph
) {
const slot = graph.outputNode?.slots?.[link_info.target_slot]
if (slot?.linkIds) {
slot.linkIds.length = 0
}
}This defensive approach protects against:
outputNodebeing undefined or nullslotsarray not existingtarget_slotindex being out of boundslinkIdsarray not being initialized
The optional chaining (?.) will short-circuit the evaluation if any property in the chain is nullish, preventing the runtime errors that could occur with direct property access. This is especially important in this cleanup code path where we're dealing with potentially stale or partially-cleaned-up state.
🧠 Learnings used
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
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
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
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
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
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`
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
There was a problem hiding this comment.
- I wasn't talking to you.
- Rabbit Season
There was a problem hiding this comment.
@DrJKL, fair enough! 🦆
Hope the defensive checks help prevent those undefined dereference issues. Let me know if you need anything else!
🧠 Learnings used
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
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
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
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
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
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
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}.{js,ts,jsx,tsx} : When adding features, always write vitest unit tests using cursor rules in @.cursor
There was a problem hiding this comment.
I'll add a check specifically for the array indexing. I note that the actual bug here was only found because there was a single code section that doesn't silently ignore broken state.
Blindly adding nullchecks for properties that are defined to never be null is actively detrimental to the health of the codebase. It allows that broken state to bleed into other areas and makes it harder to determine the root cause.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
If we didn't have so many as any and // @ts-expect-errors (and maybe more immutable objects...), I'd be much more confident about their non-nullish nature.
christian-byrne
added
needs-backport
Resolves #7152 - Adds a safety check to make sure link exists. - Actually solves the problem by making sure the linkId is removed from the subgraphOutput node when a node is deleted. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7153-Fix-zombie-linkIds-on-node-deletion-add-safety-check-2bf6d73d365081d98583e6a987431bd1) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org>
|
@AustinMroz Successfully backported to #7264 |
github-actions
bot
removed
the
needs-backport
…check (#7264) Backport of #7153 to `core/1.33` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7264-backport-core-1-33-Fix-zombie-linkIds-on-node-deletion-add-safety-check-2c46d73d365081ae9161f6b464a8e446) by [Unito](https://www.unito.io) Co-authored-by: AustinMroz <austin@comfy.org> Co-authored-by: Alexander Brown <drjkl@comfy.org>
Resolves #7152
┆Issue is synchronized with this Notion page by Unito