Chore: Typescript cleanup (1 / N) #7817
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughRefactors typings, parameter naming, and small APIs across litegraph and core extensions: widens GroupNodeConfig.nodeDef, replaces an unused group output flag, removes unused renderer/getControls fields, requires 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/06/2026, 09:31:20 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 01/06/2026, 09:36:36 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.24 MB (baseline 3.24 MB) • 🟢 -755 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 1.01 MB (baseline 1.01 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 — 300 kB (baseline 300 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 196 kB (baseline 196 kB) • ⚪ 0 BReusable component library chunks
Status: 9 added / 9 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 — 3.5 MB (baseline 3.5 MB) • ⚪ 0 BBundles that do not match a named category
Status: 20 added / 20 removed |
🔧 Auto-fixes AppliedThis PR has been automatically updated to fix linting and formatting issues.
Changes made:
|
dosubot
bot
added
the
size:XL
DrJKL
changed the title
DrJKL
requested review from
Myestery,
christian-byrne,
comfyui-wiki,
jtydhr88,
ltdrdata,
pythongosssss and
viva-jinyi
as code owners
dosubot
bot
added
size:M
…phCanvas.ts Amp-Thread-ID: https://ampcode.com/threads/T-019b7305-1473-7360-8484-e9373fd28b33 Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019b7305-1473-7360-8484-e9373fd28b33 Co-authored-by: Amp <amp@ampcode.com>
- Remove unused vendorString variable in flac.ts - Export postSurveyStatus function in auth.ts (kept for future use) - Remove unused ref_window variables and ContextMenu 3rd arg in LGraphCanvas.ts - Remove unused args from disconnectOutput() call in LGraphNode.ts - Remove unused private fields in load3d managers (CameraManager, SceneManager, ViewHelperManager) - Remove dead code output_is_hidden property in groupNode.ts Amp-Thread-ID: https://ampcode.com/threads/T-019b75e3-7af8-71a8-affe-231a258d37dc Co-authored-by: Amp <amp@ampcode.com>
… should be fixed properly at some point.
…ions Amp-Thread-ID: https://ampcode.com/threads/T-019b7694-a207-73ee-ba94-f0fb3ee69353 Co-authored-by: Amp <amp@ampcode.com>
DrJKL
force-pushed
the
drjkl/the-spanish-inquisition
branch
from
b2d8e4e to
a41cbab
Compare
dosubot
bot
added
size:XL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/extensions/core/groupNode.ts (1)
228-245: Address the "lie" comment indicating a type mismatch.Line 233 sets
output_node: falsewith a comment stating "This is a lie (to satisfy the interface)". This indicates a mismatch between the interface requirements and the actual semantics of group nodes. Such mismatches can lead to bugs when other code relies on this property.Consider one of the following approaches:
- Make
output_nodeoptional in the interface if group nodes don't have this concept- Document why group nodes set this to
falseand ensure consuming code handles this case- Use a more precise type (e.g., discriminated union) to distinguish between regular nodes and group nodes
Based on learnings, type assertions are a last resort and should be avoided in favor of proper types.
src/extensions/core/load3d/ViewHelperManager.ts (1)
18-27: Remove the unused_rendererparameter from the constructor signature.The
_rendererparameter is accepted but never used in the class implementation. This pattern is also present inCameraManager, confirming a consistent pattern across load3d managers. The parameter can be safely removed from the constructor, with a corresponding update to the instantiation inLoad3d.ts(line 108).src/lib/litegraph/src/LGraphCanvas.ts (2)
3540-3547: Fix potential runtime error inprocessMouseUpwhennode_overis undefinedThe new use of optional chaining:
this.node_over?.onMouseUp?.(e, [ x - this.node_over.pos[0], y - this.node_over.pos[1] ])still dereferences
this.node_over.pos[...]unguarded. Ifnode_overisundefined(for example after hover state is cleared before mouseup), this will throw even though the call is short‑circuited. The same pattern already exists fornode_capturing_input.Recommend capturing the node references into locals and guarding both the dereference and the call:
Proposed fix for safe mouse‑up handling
- this.node_over?.onMouseUp?.(e, [ - x - this.node_over.pos[0], - y - this.node_over.pos[1] - ]) - this.node_capturing_input?.onMouseUp?.(e, [ - x - this.node_capturing_input.pos[0], - y - this.node_capturing_input.pos[1] - ]) + const nodeOver = this.node_over + if (nodeOver?.onMouseUp) { + nodeOver.onMouseUp(e, [x - nodeOver.pos[0], y - nodeOver.pos[1]]) + } + + const capturingNode = this.node_capturing_input + if (capturingNode?.onMouseUp) { + capturingNode.onMouseUp(e, [ + x - capturingNode.pos[0], + y - capturingNode.pos[1] + ]) + }This keeps the same semantics while avoiding a potential null/undefined dereference.
4544-4553: Preserveout-parameter semantics inconvertOffsetToCanvaswrapper
convertOffsetToCanvasnow accepts an unused_outparameter but no longer forwards it:convertOffsetToCanvas(pos: Point, _out?: Point): Point { return this.ds.convertOffsetToCanvas(pos) }If any callers relied on the optional
outparameter to reuse a buffer and avoid allocations (or to mutate a providedPoint), this is now silently ignored. To keep the previous contract and performance characteristics, the wrapper should forward_out:Proposed fix for `convertOffsetToCanvas`
- convertOffsetToCanvas(pos: Point, _out?: Point): Point { - return this.ds.convertOffsetToCanvas(pos) - } + convertOffsetToCanvas(pos: Point, _out?: Point): Point { + return this.ds.convertOffsetToCanvas(pos, _out) + }
🤖 Fix all issues with AI Agents
In @src/extensions/core/load3d/SceneManager.ts:
- Around line 31-41: The constructor for SceneManager currently accepts an
unused parameter _getControls; remove this unused parameter from the
SceneManager constructor signature and from every call site (notably where
Load3d constructs SceneManager) and clean up any related type annotations, or if
you intend to keep it, store it as a private property (e.g., private
getControls: () => OrbitControls) and add a short JSDoc comment on the
constructor explaining its planned purpose so it’s not treated as dead code;
update any interfaces or imports referencing the old signature accordingly (look
for the SceneManager constructor and the Load3d.ts call that passes
_getControls).
In @src/lib/litegraph/src/LGraphCanvas.titleButtons.test.ts:
- Around line 70-82: The variable nodeWithMocks is created and assigned mock
implementations for drawTitleBarText, drawToggles, drawNodeShape, drawContent
and isSelectable but then never used, which is confusing; either remove
nodeWithMocks and attach the mocks directly to node (using inline type
assertions where needed) or replace subsequent references to node with
nodeWithMocks throughout the test so the mocks are clearly applied — update
assignments to target node (or consistently use nodeWithMocks) and ensure the
mock methods (drawTitleBarText, drawToggles, drawNodeShape, drawContent,
isSelectable) are the ones exercised by the tests.
In @src/lib/litegraph/src/node/NodeSlot.test.ts:
- Around line 25-26: Clarify that the TypeScript error is intentional by
replacing the vague comment above the call to outputAsSerialisable(slot) with a
descriptive one such as "@ts-expect-error intentional: passing an OutputSlot to
outputAsSerialisable to test its serialization behavior" (or similar), so
reviewers know the type mismatch for variable slot is deliberate and part of the
test; keep the @ts-expect-error directive and ensure it directly mentions
outputAsSerialisable and OutputSlot to make purpose explicit.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (26)
src/extensions/core/groupNode.tssrc/extensions/core/load3d/CameraManager.tssrc/extensions/core/load3d/SceneManager.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphCanvas.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/subgraph/SubgraphEvents.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/subgraph/SubgraphWidgetPromotion.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/widgets/ComboWidget.test.tssrc/platform/cloud/onboarding/auth.ts
💤 Files with no reviewable changes (3)
- src/lib/litegraph/src/subgraph/SubgraphEvents.test.ts
- src/lib/litegraph/src/subgraph/SubgraphWidgetPromotion.test.ts
- src/platform/cloud/onboarding/auth.ts
🧰 Additional context used
📓 Path-based instructions (12)
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/widgets/ComboWidget.test.tssrc/extensions/core/load3d/SceneManager.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/extensions/core/load3d/CameraManager.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/extensions/core/groupNode.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.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/widgets/ComboWidget.test.tssrc/extensions/core/load3d/SceneManager.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/extensions/core/load3d/CameraManager.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/extensions/core/groupNode.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.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/widgets/ComboWidget.test.tssrc/extensions/core/load3d/SceneManager.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/extensions/core/load3d/CameraManager.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/extensions/core/groupNode.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/lib/litegraph/src/widgets/ComboWidget.test.tssrc/extensions/core/load3d/SceneManager.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/extensions/core/load3d/CameraManager.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/extensions/core/groupNode.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.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/widgets/ComboWidget.test.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.ts
src/lib/litegraph/**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
When adding features, always write vitest unit tests using cursor rules in @.cursor
Files:
src/lib/litegraph/src/widgets/ComboWidget.test.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.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/widgets/ComboWidget.test.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.ts
src/lib/litegraph/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
src/lib/litegraph/**/*.{test,spec}.{ts,tsx}: When writing tests for subgraph-related code, always import from the barrel export at@/lib/litegraph/src/litegraphto avoid circular dependency issues
Use provided test helperscreateTestSubgraphandcreateTestSubgraphNodefrom./fixtures/subgraphHelpersfor consistent subgraph test setup
Files:
src/lib/litegraph/src/widgets/ComboWidget.test.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
src/lib/litegraph/src/widgets/ComboWidget.test.tssrc/extensions/core/load3d/SceneManager.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/extensions/core/load3d/CameraManager.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/extensions/core/groupNode.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.ts
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
src/lib/litegraph/src/widgets/ComboWidget.test.tssrc/extensions/core/load3d/SceneManager.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/extensions/core/load3d/CameraManager.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/extensions/core/groupNode.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Use unit/component tests intests-ui/orsrc/**/*.test.tswith Vitest framework
For mocking in tests, leverage Vitest utilities; keep module mocks contained and avoid global mutable state within test files
Do not write change detector tests or tests dependent on non-behavioral features like utility classes or styles
Aim for behavioral coverage of critical and new features in unit tests
Files:
src/lib/litegraph/src/widgets/ComboWidget.test.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
src/lib/litegraph/src/widgets/ComboWidget.test.tssrc/extensions/core/load3d/SceneManager.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/extensions/core/load3d/CameraManager.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/extensions/core/groupNode.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.ts
🧠 Learnings (31)
📓 Common learnings
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/**/*.{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/**/*.{js,ts,jsx,tsx} : Take advantage of `TypedArray` `subarray` when appropriate
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
📚 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/widgets/ComboWidget.test.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.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}.{js,ts,jsx,tsx} : When adding features, always write vitest unit tests using cursor rules in @.cursor
Applied to files:
src/lib/litegraph/src/widgets/ComboWidget.test.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.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/widgets/ComboWidget.test.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.test.ts : For mocking in tests, leverage Vitest utilities; keep module mocks contained and avoid global mutable state within test files
Applied to files:
src/lib/litegraph/src/widgets/ComboWidget.test.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.ts
📚 Learning: 2025-12-22T21:36:46.909Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: tests-ui/tests/platform/cloud/subscription/components/SubscriptionPanel.test.ts:189-194
Timestamp: 2025-12-22T21:36:46.909Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: Do not stub primitive UI components or customized primitive components (e.g., Button). Instead, import and register the real components in test setup. This ensures tests accurately reflect production behavior and component API usage.
Applied to files:
src/lib/litegraph/src/widgets/ComboWidget.test.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/lib/litegraph/src/widgets/ComboWidget.test.tssrc/extensions/core/load3d/SceneManager.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/extensions/core/load3d/CameraManager.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/extensions/core/groupNode.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.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/lib/litegraph/src/widgets/ComboWidget.test.tssrc/extensions/core/load3d/SceneManager.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/extensions/core/load3d/CameraManager.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/extensions/core/groupNode.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.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/lib/litegraph/src/widgets/ComboWidget.test.tssrc/extensions/core/load3d/SceneManager.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/extensions/core/load3d/CameraManager.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/extensions/core/groupNode.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.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/lib/litegraph/src/widgets/ComboWidget.test.tssrc/extensions/core/load3d/SceneManager.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/extensions/core/load3d/CameraManager.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/extensions/core/groupNode.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.ts
📚 Learning: 2025-12-10T03:09:13.807Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:13.807Z
Learning: In test files, prefer selecting or asserting on accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests validate actual user-facing behavior and accessibility, reducing reliance on implementation details like test IDs.
Applied to files:
src/lib/litegraph/src/widgets/ComboWidget.test.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.ts
📚 Learning: 2025-12-30T01:31:04.927Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7797
File: tests-ui/tests/lib/litegraph/src/widgets/ComboWidget.test.ts:648-648
Timestamp: 2025-12-30T01:31:04.927Z
Learning: In Vitest v4, when mocking functions that may be called as constructors (using new), the mock implementation must use function() or class syntax rather than an arrow function. Arrow mocks can cause '<anonymous> is not a constructor' errors. This is a breaking change from Vitest v3 where mocks could use an arrow function. Apply this guideline to test files that mock constructor-like calls (e.g., in tests under tests-ui, such as ComboWidget.test.ts) and ensure mock implementations are defined with function() { ... } or class { ... } to preserve constructor behavior.
Applied to files:
src/lib/litegraph/src/widgets/ComboWidget.test.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.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/lib/litegraph/src/widgets/ComboWidget.test.tssrc/extensions/core/load3d/SceneManager.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/extensions/core/load3d/CameraManager.tssrc/lib/litegraph/src/types/serialisation.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/node/NodeSlot.test.tssrc/extensions/core/load3d/ViewHelperManager.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/extensions/core/groupNode.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.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/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.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/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphNode.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.test.ts : Do not write change detector tests or tests dependent on non-behavioral features like utility classes or styles
Applied to files:
src/lib/litegraph/src/subgraph/ExecutableNodeDTO.test.tssrc/lib/litegraph/src/subgraph/SubgraphIO.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphButton.test.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.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/SubgraphIO.test.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.tssrc/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.tssrc/lib/litegraph/src/LGraphCanvas.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/SubgraphIO.test.tssrc/lib/litegraph/src/subgraph/Subgraph.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.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/lib/litegraph/src/subgraph/SubgraphIO.test.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/node/NodeSlot.test.tssrc/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/LGraphCanvas.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/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.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/lib/litegraph/src/LGraphNode.test.tssrc/lib/litegraph/src/__fixtures__/assets/testGraphs.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Use `vitest` for unit testing in this project
Applied to files:
src/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Use `test` instead of `it` for defining test cases in vitest
Applied to files:
src/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Prefer the use of `test.extend` over loose variables; import `test as baseTest` from `vitest`
Applied to files:
src/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.test.ts : Use unit/component tests in `tests-ui/` or `src/**/*.test.ts` with Vitest framework
Applied to files:
src/lib/litegraph/src/LGraphCanvas.titleButtons.test.tssrc/lib/litegraph/src/subgraph/SubgraphMemory.test.tssrc/lib/litegraph/src/subgraph/SubgraphNode.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test across multiple viewports
Applied to files:
src/lib/litegraph/src/LGraphCanvas.titleButtons.test.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.test.ts : Aim for behavioral coverage of critical and new features in unit tests
Applied to files:
src/lib/litegraph/src/__fixtures__/assets/testGraphs.tssrc/lib/litegraph/src/subgraph/SubgraphEdgeCases.test.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Mocks should be cleanly written and easy to understand, with reusable mocks where possible
Applied to files:
src/lib/litegraph/src/subgraph/SubgraphMemory.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Test user workflows in browser tests
Applied to files:
src/lib/litegraph/src/subgraph/SubgraphMemory.test.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Use Playwright fixtures for browser tests
Applied to files:
src/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.ts
🧬 Code graph analysis (6)
src/lib/litegraph/src/widgets/ComboWidget.test.ts (1)
src/lib/litegraph/src/litegraph.ts (1)
LiteGraph(17-17)
src/lib/litegraph/src/types/serialisation.ts (2)
src/lib/litegraph/src/interfaces.ts (1)
Dictionary(14-14)src/lib/litegraph/src/LGraphNode.ts (1)
NodeProperty(96-96)
src/lib/litegraph/src/node/NodeSlot.test.ts (1)
src/lib/litegraph/src/interfaces.ts (3)
ReadOnlyRect(240-242)INodeOutputSlot(359-363)INodeInputSlot(344-353)
src/extensions/core/groupNode.ts (2)
src/types/index.ts (1)
ComfyNodeDef(30-30)src/utils/executableGroupNodeDto.ts (1)
GROUP(11-11)
src/lib/litegraph/src/LGraphCanvas.titleButtons.test.ts (1)
src/lib/litegraph/src/LGraphCanvas.ts (1)
canvas(1844-1854)
src/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.ts (2)
src/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers.ts (3)
EventCapture(59-66)createTestSubgraph(86-172)createEventCapture(397-432)src/lib/litegraph/src/infrastructure/SubgraphEventMap.ts (1)
SubgraphEventMap(8-55)
🪛 Biome (2.1.2)
src/lib/litegraph/src/subgraph/__fixtures__/subgraphFixtures.ts
[error] 64-64: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 75-75: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 86-86: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 104-104: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 115-115: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 138-138: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
⏰ 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 (5, 8)
- GitHub Check: playwright-tests-chromium-sharded (7, 8)
- GitHub Check: playwright-tests-chromium-sharded (1, 8)
- GitHub Check: playwright-tests-chromium-sharded (3, 8)
- GitHub Check: playwright-tests-chromium-sharded (2, 8)
- GitHub Check: playwright-tests-chromium-sharded (8, 8)
- GitHub Check: playwright-tests-chromium-sharded (6, 8)
- GitHub Check: playwright-tests (chromium-0.5x)
- GitHub Check: playwright-tests (mobile-chrome)
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
| } | ||
| // @ts-expect-error fixme ts strict error | ||
| this.#convertedToProcess = null | ||
| if (!this.nodeDef) return |
Contributor
There was a problem hiding this comment.
Silent failure: early return without notification.
The early return when nodeDef is undefined causes registerType to fail silently. This could make debugging difficult if something goes wrong during node definition construction.
🔎 Consider adding error logging or throwing an error
if (!this.nodeDef) return
+if (!this.nodeDef) {
+ console.error(`Failed to register group node type: nodeDef is undefined for ${this.name}`)
+ return
+}Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
31
to
41
| constructor( | ||
| renderer: THREE.WebGLRenderer, | ||
| getActiveCamera: () => THREE.Camera, | ||
| getControls: () => OrbitControls, | ||
| _getControls: () => OrbitControls, | ||
| eventManager: EventManagerInterface | ||
| ) { | ||
| this.renderer = renderer | ||
| this.eventManager = eventManager | ||
| this.scene = new THREE.Scene() | ||
|
|
||
| this.getActiveCamera = getActiveCamera |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -C3 'new\s+SceneManager\s*\(' --type=tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 531
🏁 Script executed:
# Get full context of SceneManager constructor call in Load3d.ts
rg -n -A 5 'this.sceneManager = new SceneManager' src/extensions/core/load3d/Load3d.ts
# Check if _getControls is referenced anywhere in SceneManager
rg -n '_getControls\|getControls\|controls' src/extensions/core/load3d/SceneManager.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 254
🏁 Script executed:
# Check entire SceneManager.ts file structure
cat src/extensions/core/load3d/SceneManager.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 13165
Remove the unused _getControls parameter or document its intended use.
The _getControls parameter is passed by the caller (Load3d.ts) but never stored, assigned, or used anywhere in the SceneManager class. Unlike renderer and getActiveCamera which serve active purposes, this parameter adds unnecessary surface area to the module. Either remove it from both the constructor signature and the caller in Load3d.ts, or document why it needs to be accepted if there's a planned use case.
🤖 Prompt for AI Agents
In @src/extensions/core/load3d/SceneManager.ts around lines 31 - 41, The
constructor for SceneManager currently accepts an unused parameter _getControls;
remove this unused parameter from the SceneManager constructor signature and
from every call site (notably where Load3d constructs SceneManager) and clean up
any related type annotations, or if you intend to keep it, store it as a private
property (e.g., private getControls: () => OrbitControls) and add a short JSDoc
comment on the constructor explaining its planned purpose so it’s not treated as
dead code; update any interfaces or imports referencing the old signature
accordingly (look for the SceneManager constructor and the Load3d.ts call that
passes _getControls).
| id: 1 | ||
| } | ||
| ], | ||
| nodes: [{ id: 1 } as Partial<ISerialisedNode> as ISerialisedNode], |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace double type assertions with properly typed test fixtures.
The double type assertion pattern as Partial<ISerialisedNode> as ISerialisedNode circumvents TypeScript's type safety and is arguably worse than the removed @ts-expect-error suppressions. This approach lies to the type system about incomplete objects being complete.
Consider these alternatives:
- Create a test helper function that provides sensible defaults:
createTestNode({ id: 1, ...overrides }) - Type the fixture as
Partial<ISerialisedNode>and update consuming test code to handle partial types - Complete the fixture objects with all required
ISerialisedNodeproperties
Based on learnings, type assertions should be a last resort in litegraph code.
🔎 Example: Test helper approach
+function createMinimalTestNode(overrides: Partial<ISerialisedNode>): ISerialisedNode {
+ return {
+ id: 0,
+ type: 'test',
+ pos: [0, 0],
+ size: [100, 100],
+ flags: {},
+ mode: 0,
+ inputs: [],
+ outputs: [],
+ properties: {},
+ ...overrides
+ }
+}
+
export const oldSchemaGraph: ISerialisedGraph = {
// ...
- nodes: [{ id: 1 } as Partial<ISerialisedNode> as ISerialisedNode],
+ nodes: [createMinimalTestNode({ id: 1 })],Also applies to: 64-64
Comment on lines
+70
to
+82
|
|
||
| const nodeWithMocks = node as LGraphNode & { | ||
| drawTitleBarText: ReturnType<typeof vi.fn> | ||
| drawToggles: ReturnType<typeof vi.fn> | ||
| drawNodeShape: ReturnType<typeof vi.fn> | ||
| drawContent: ReturnType<typeof vi.fn> | ||
| isSelectable: ReturnType<typeof vi.fn> | ||
| } | ||
| nodeWithMocks.drawTitleBarText = vi.fn() | ||
| nodeWithMocks.drawToggles = vi.fn() | ||
| nodeWithMocks.drawNodeShape = vi.fn() | ||
| nodeWithMocks.drawContent = vi.fn() | ||
| nodeWithMocks.isSelectable = vi.fn().mockReturnValue(true) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
nodeWithMocks is unused after assignment.
The nodeWithMocks variable is created and has mock methods assigned to it, but all subsequent test code continues to use the original node variable. Since nodeWithMocks is just a type-cast reference to the same object, the mock assignments do work, but the explicit variable declaration adds confusion without benefit.
Consider simplifying by assigning mocks directly to node with inline type assertions only where needed, or use nodeWithMocks consistently throughout the tests.
🔎 Suggested simplification
- const nodeWithMocks = node as LGraphNode & {
- drawTitleBarText: ReturnType<typeof vi.fn>
- drawToggles: ReturnType<typeof vi.fn>
- drawNodeShape: ReturnType<typeof vi.fn>
- drawContent: ReturnType<typeof vi.fn>
- isSelectable: ReturnType<typeof vi.fn>
- }
- nodeWithMocks.drawTitleBarText = vi.fn()
- nodeWithMocks.drawToggles = vi.fn()
- nodeWithMocks.drawNodeShape = vi.fn()
- nodeWithMocks.drawContent = vi.fn()
- nodeWithMocks.isSelectable = vi.fn().mockReturnValue(true)
+ // Assign additional mocks needed by drawNode
+ ;(node as LGraphNode & { drawTitleBarText: unknown }).drawTitleBarText = vi.fn()
+ ;(node as LGraphNode & { drawToggles: unknown }).drawToggles = vi.fn()
+ ;(node as LGraphNode & { drawNodeShape: unknown }).drawNodeShape = vi.fn()
+ ;(node as LGraphNode & { drawContent: unknown }).drawContent = vi.fn()
+ ;(node as LGraphNode & { isSelectable: unknown }).isSelectable = vi.fn().mockReturnValue(true)Or alternatively, use nodeWithMocks throughout the test file instead of node.
🤖 Prompt for AI Agents
In @src/lib/litegraph/src/LGraphCanvas.titleButtons.test.ts around lines 70 -
82, The variable nodeWithMocks is created and assigned mock implementations for
drawTitleBarText, drawToggles, drawNodeShape, drawContent and isSelectable but
then never used, which is confusing; either remove nodeWithMocks and attach the
mocks directly to node (using inline type assertions where needed) or replace
subsequent references to node with nodeWithMocks throughout the test so the
mocks are clearly applied — update assignments to target node (or consistently
use nodeWithMocks) and ensure the mock methods (drawTitleBarText, drawToggles,
drawNodeShape, drawContent, isSelectable) are the ones exercised by the tests.
Comment on lines
25
to
26
| // @ts-expect-error Argument type mismatch for test | ||
| const serialized = outputAsSerialisable(slot) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify the intentional type mismatch in this test.
The @ts-expect-error comment should be more explicit about what is being tested. The current comment "Argument type mismatch for test" doesn't clearly indicate this is intentional testing of the serialization function's behavior with an output slot.
🔎 Suggested improvement
- // @ts-expect-error Argument type mismatch for test
+ // @ts-expect-error Testing that outputAsSerialisable removes _data property (intentionally passing INodeOutputSlot)
const serialized = outputAsSerialisable(slot)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // @ts-expect-error Argument type mismatch for test | |
| const serialized = outputAsSerialisable(slot) | |
| // @ts-expect-error Testing that outputAsSerialisable removes _data property (intentionally passing INodeOutputSlot) | |
| const serialized = outputAsSerialisable(slot) |
🤖 Prompt for AI Agents
In @src/lib/litegraph/src/node/NodeSlot.test.ts around lines 25 - 26, Clarify
that the TypeScript error is intentional by replacing the vague comment above
the call to outputAsSerialisable(slot) with a descriptive one such as
"@ts-expect-error intentional: passing an OutputSlot to outputAsSerialisable to
test its serialization behavior" (or similar), so reviewers know the type
mismatch for variable slot is deliberate and part of the test; keep the
@ts-expect-error directive and ensure it directly mentions outputAsSerialisable
and OutputSlot to make purpose explicit.
AustinMroz
approved these changes
Jan 6, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
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/LGraphCanvas.ts (1)
1281-1334: Fixthistype ininner_clickedcallback to match the interface contractThe
inner_clickedcallback at line 1321 has an incorrectthistype annotation. PerIContextMenuValue.callbackin interfaces.ts, menu item callbacks receivethis: ContextMenuDivElement<TValue>, notContextMenu<TValue>. At runtime,thisis the DOM element, sothis.rootdoes not exist andthis.root.getBoundingClientRect()will throw an error.Change line 1321-1322 from:
function inner_clicked( this: ContextMenu<string>, v?: string | IContextMenuValue<string> ) {to:
function inner_clicked( this: ContextMenuDivElement<string>, v?: string | IContextMenuValue<string> ) {And update line 1327 to call
getBoundingClientRect()directly onthisinstead ofthis.root:const rect = this.getBoundingClientRect()This matches the pattern already used correctly in the
onShowMenuNodeSlotcallback at line 1237.
🤖 Fix all issues with AI Agents
In @src/lib/litegraph/src/LGraphCanvas.ts:
- Around line 4549-4551: The wrapper method convertOffsetToCanvas currently
ignores the optional _out parameter causing callers that expect the
out-parameter to be written to break; update convertOffsetToCanvas to forward
the _out argument to this.ds.convertOffsetToCanvas so the underlying
implementation can write into the provided out Point (keep the signature
convertOffsetToCanvas(pos: Point, _out?: Point): Point and simply pass _out
through to this.ds.convertOffsetToCanvas).
In @src/lib/litegraph/src/LGraphNode.ts:
- Line 2777: The call that previously passed an object instead of a node to
disconnectOutput must be changed to call this.disconnectOutput(slot) so the
method receives the expected LGraphNode context and disconnects all connections
for that output before creating a new one; locate the code path guarded by
LiteGraph.allow_multi_output_for_events and replace the erroneous argument with
the slot-only call to disconnectOutput, ensuring the method signature for
disconnectOutput(nodeOrSlot) is respected and no suppressed types are
reintroduced.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/lib/litegraph/src/LGraphCanvas.tssrc/lib/litegraph/src/LGraphNode.ts
🧰 Additional context used
📓 Path-based instructions (9)
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/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.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/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.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/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.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/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.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/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
src/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.ts
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
src/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
src/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.ts
🧠 Learnings (13)
📓 Common learnings
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/**/*.{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/**/*.{js,ts,jsx,tsx} : Take advantage of `TypedArray` `subarray` when appropriate
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/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
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: 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} : 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} : Run ESLint instead of manually figuring out whitespace fixes or other trivial style concerns using the `pnpm lint:fix` command
📚 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}.{js,ts,jsx,tsx} : When adding features, always write vitest unit tests using cursor rules in @.cursor
Applied to files:
src/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/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/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.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/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.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/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.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/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.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/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.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/lib/litegraph/src/LGraphNode.tssrc/lib/litegraph/src/LGraphCanvas.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/LGraphCanvas.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/LGraphCanvas.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/LGraphCanvas.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/LGraphCanvas.ts
🧬 Code graph analysis (2)
src/lib/litegraph/src/LGraphNode.ts (3)
src/lib/litegraph/src/types/events.ts (1)
CanvasPointerEvent(45-45)src/renderer/core/layout/types.ts (1)
Point(18-21)src/renderer/core/canvas/pathRenderer.ts (1)
Point(10-13)
src/lib/litegraph/src/LGraphCanvas.ts (2)
src/lib/litegraph/src/interfaces.ts (3)
IContextMenuValue(409-430)IContextMenuOptions(384-407)Point(229-229)src/lib/litegraph/src/ContextMenu.ts (2)
ContextMenu(9-13)ContextMenu(19-411)
⏰ 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)
- GitHub Check: deploy-and-comment
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (6)
src/lib/litegraph/src/LGraphNode.ts (2)
682-687: LGTM: Canvas parameter added to onMouseUp callbackThe addition of the
canvasparameter properly exposes the canvas context to node-level mouse event handlers, enabling more context-aware event handling. The signature change aligns with the broader API evolution described in the PR objectives.
690-695: LGTM: Canvas parameter and explicit return type improve API clarityThe addition of the
canvasparameter and explicitbooleanreturn type makes the contract clearer for implementers. The return type was previously implicit, and making it explicit improves type safety.src/lib/litegraph/src/LGraphCanvas.ts (4)
971-1192: Static context-menu handlers: TS hygiene changes look correctThe changes here (underscore-prefixed unused params, explicit generics on
LiteGraph.ContextMenucallbacks, and simplified constructor calls without extra window args) are consistent with theIContextMenuOptions/IContextMenuValuesignatures and the currentContextMenu(values, options)API. I don’t see behavioural changes; they should only reduce TS noise around unused parameters and mismatched callback shapes.Also applies to: 1343-1370, 1503-1613, 1667-1717, 6184-6215, 8123-8127
1446-1461: Dialog/search/prompt close timers: scoped handles and guard logic look solidThe introduction of scoped
dialogCloseTimer/timeout_close/timeoutvariables, with appropriate null/undefined checks beforeclearTimeout/clearInterval, keeps the existing close-on-mouse-leave behaviour while avoiding stray timers and TS “timer handle” confusion. No functional issues spotted.Also applies to: 6707-6722, 6901-6939, 7582-7597
3541-3553: Passing canvas intoonMouseUpfornode_capturing_inputrestores callback shapeExtending the
onMouseUpinvocation to:this.node_capturing_input?.onMouseUp?.( e, [ x - this.node_capturing_input.pos[0], y - this.node_capturing_input.pos[1] ], this )matches the
(event, localPos, canvas)pattern used elsewhere and restores the canvas parameter for nodes that capture input. Existing callbacks that only expect(event, localPos)remain compatible since JS ignores extra arguments. This should address the earlier concern about the missing canvas parameter without introducing breaking changes.
7255-7294: Search-box and enum widget helpers: type tightening without behavioural changeThe added
string[]annotations forfiltered_extraand the explicitLiteGraph.ContextMenuconstruction increatePanel.addWidget’s enum/combo widget (with adarkclass and arrow callback to keepthisbound to the value span) are all consistent with existing behaviour. They just make the intent clearer and typings stricter, without altering runtime behaviour.Also applies to: 7798-7802
Comment on lines
+4549
to
4551
| convertOffsetToCanvas(pos: Point, _out?: Point): Point { | ||
| return this.ds.convertOffsetToCanvas(pos) | ||
| } |
Contributor
There was a problem hiding this comment.
convertOffsetToCanvas should still honour the optional out-parameter
The wrapper now ignores the _out parameter:
convertOffsetToCanvas(pos: Point, _out?: Point): Point {
return this.ds.convertOffsetToCanvas(pos)
}If any callers rely on the “write into out-param and ignore the return value” convention (which is typical for hot-path geometry helpers), this is a silent behavioural break: the passed _out array will no longer be updated.
You can keep the public signature cleanup (making the second arg optional and underscore-prefixed) while preserving behaviour by forwarding _out:
Proposed fix
- convertOffsetToCanvas(pos: Point, _out?: Point): Point {
- return this.ds.convertOffsetToCanvas(pos)
- }
+ convertOffsetToCanvas(pos: Point, _out?: Point): Point {
+ return this.ds.convertOffsetToCanvas(pos, _out)
+ }🤖 Prompt for AI Agents
In @src/lib/litegraph/src/LGraphCanvas.ts around lines 4549 - 4551, The wrapper
method convertOffsetToCanvas currently ignores the optional _out parameter
causing callers that expect the out-parameter to be written to break; update
convertOffsetToCanvas to forward the _out argument to
this.ds.convertOffsetToCanvas so the underlying implementation can write into
the provided out Point (keep the signature convertOffsetToCanvas(pos: Point,
_out?: Point): Point and simply pass _out through to
this.ds.convertOffsetToCanvas).
| graph.beforeChange() | ||
| // @ts-expect-error Unused param | ||
| this.disconnectOutput(slot, false, { doProcessChange: false }) | ||
| this.disconnectOutput(slot) |
Contributor
There was a problem hiding this comment.
Critical bug fix: Corrected disconnectOutput call
The old code passed an object { doProcessChange: false } where a LGraphNode was expected (see method signature at line 2996). This was a type mismatch that likely went unnoticed due to suppressed type checking. The corrected call properly disconnects all existing connections on the slot before establishing the new one, which is the intended behavior when !LiteGraph.allow_multi_output_for_events.
This exemplifies the value of this TypeScript cleanup PR—removing suppressions surfaces real bugs.
🤖 Prompt for AI Agents
In @src/lib/litegraph/src/LGraphNode.ts at line 2777, The call that previously
passed an object instead of a node to disconnectOutput must be changed to call
this.disconnectOutput(slot) so the method receives the expected LGraphNode
context and disconnects all connections for that output before creating a new
one; locate the code path guarded by LiteGraph.allow_multi_output_for_events and
replace the erroneous argument with the slot-only call to disconnectOutput,
ensuring the method signature for disconnectOutput(nodeOrSlot) is respected and
no suppressed types are reintroduced.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Remove 178
@ts-expect-errorsuppressions (935 → 757, 19% reduction) by fixing underlying type issues instead of suppressing errors.Changes
src/lib/litegraph/and related test files_instead of suppressingPartial<T>for incomplete test objects instead ofas unknown.toBeDefined()checks in testsReview Focus
LGraphCanvas.tshas the largest changes (232 lines) — all mechanical unused parameter fixesnode as LGraphNode & { mockFn: ... }src/platform/cloud/onboarding/auth.ts(47 lines, unused)Key Files
LGraphCanvas.tssubgraph/__fixtures__/**.test.tsfiles┆Issue is synchronized with this Notion page by Unito