fix: align BOUNDING_BOX type with backend and target ImageCropV2 node #8531
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
dosubot
bot
added
the
size:XS
|
Related Documentation No published documentation to review for changes on this repository. Write your first living document How did I do? Any feedback? |
🎭 Playwright Tests:
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/01/2026, 02:30:07 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Contributor
📝 WalkthroughWalkthroughUpdates the image crop extension to target ImageCropV2 nodes specifically and renames the bounding box widget discriminant and aliases from 'BOUNDINGBOX' to 'BOUNDING_BOX' across widget registry, schema definitions, and public exports. Changes
Suggested reviewers
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
coderabbitai
bot
requested review from
KarryCharon,
Yorha4D,
christian-byrne and
shinshin86
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 26 kB (baseline 26 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 978 kB (baseline 978 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 80.7 kB (baseline 80.7 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 471 kB (baseline 471 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 12 added / 12 removed User & Accounts — 3.94 kB (baseline 3.94 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 3 added / 3 removed Editors & Dialogs — 2.89 kB (baseline 2.89 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 33.7 kB (baseline 33.7 kB) • ⚪ 0 BReusable component library chunks
Status: 4 added / 4 removed Data & Services — 2.71 MB (baseline 2.71 MB) • 🔴 +3 BStores, services, APIs, and repositories
Status: 8 added / 8 removed Utilities & Hooks — 25.3 kB (baseline 25.3 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 7 added / 7 removed Vendor & Third-Party — 10.7 MB (baseline 10.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.16 MB (baseline 7.16 MB) • 🔴 +2 BBundles that do not match a named category
Status: 36 added / 36 removed |
![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: 1
🤖 Fix all issues with AI agents
In `@src/scripts/widgets.ts`:
- Line 305: The import that mixes a type and a value should be split into two
statements: replace the combined import that currently reads like "import { type
LGraphNode, isComboWidget } from '.../litegraph'" with one type-only import for
LGraphNode (import type { LGraphNode } ...) and a separate value import for
isComboWidget (import { isComboWidget } ...); update the top of the module where
transformWidgetConstructorV2ToV1, useBoundingBoxWidget and BOUNDING_BOX are
referenced and scan for any other imports that mix "type" with values to split
them the same way.
| COLOR: transformWidgetConstructorV2ToV1(useColorWidget()), | ||
| IMAGECOMPARE: transformWidgetConstructorV2ToV1(useImageCompareWidget()), | ||
| BOUNDINGBOX: transformWidgetConstructorV2ToV1(useBoundingBoxWidget()), | ||
| BOUNDING_BOX: transformWidgetConstructorV2ToV1(useBoundingBoxWidget()), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Examine src/scripts/widgets.ts around line 305
echo "=== Checking line 305 context in src/scripts/widgets.ts ==="
sed -n '300,310p' src/scripts/widgets.ts
echo -e "\n=== Searching for BOUNDINGBOX references across the codebase ==="
# Search for BOUNDINGBOX (case-sensitive) in TypeScript and Vue files
rg -n 'BOUNDINGBOX' --type=ts --type=vue -g '!**/node_modules/**' || echo "No BOUNDINGBOX references found"
echo -e "\n=== Searching for BOUNDING_BOX to verify new key usage ==="
rg -n 'BOUNDING_BOX' --type=ts --type=vue -g '!**/node_modules/**' | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 963
🏁 Script executed:
#!/bin/bash
# Verify the ComfyWidgets object structure and export
echo "=== Checking ComfyWidgets object definition and export ==="
rg -n 'export.*ComfyWidgets' src/scripts/widgets.ts -A 5
echo -e "\n=== Checking full ComfyWidgets object structure (lines 280-320) ==="
sed -n '280,320p' src/scripts/widgets.ts
echo -e "\n=== Verify import statements compliance (lines 1-30) ==="
head -30 src/scripts/widgets.ts
echo -e "\n=== Check if useBoundingBoxWidget is properly imported ==="
rg -n 'useBoundingBoxWidget' src/scripts/widgets.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 4121
Fix import statement structure on line 2
The BOUNDING_BOX rename is correct and properly implemented. However, line 2 violates the coding guideline for import statements: import { type LGraphNode, isComboWidget } mixes type and named imports in a single statement. These should be separated:
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
import { isComboWidget } from '@/lib/litegraph/src/litegraph'
Verification confirms no remaining BOUNDINGBOX references exist.
🤖 Prompt for AI Agents
In `@src/scripts/widgets.ts` at line 305, The import that mixes a type and a value
should be split into two statements: replace the combined import that currently
reads like "import { type LGraphNode, isComboWidget } from '.../litegraph'" with
one type-only import for LGraphNode (import type { LGraphNode } ...) and a
separate value import for isComboWidget (import { isComboWidget } ...); update
the top of the module where transformWidgetConstructorV2ToV1,
useBoundingBoxWidget and BOUNDING_BOX are referenced and scan for any other
imports that mix "type" with values to split them the same way.
Collaborator
Author
There was a problem hiding this comment.
what are you talking about? your suggestion has no any relationship to my changes, why I must make such thing in my PR?
Contributor
There was a problem hiding this comment.
@jtydhr88 You're absolutely right, and I apologize for the confusion. The import statement issue on line 2 is pre-existing code that has nothing to do with your changes. Your PR only modifies line 305 (the BOUNDING_BOX rename), which is correct and properly aligned with the backend changes.
I should not have flagged unrelated pre-existing issues in your PR review. Your changes are good to go. Sorry for the noise!
🧠 Learnings used
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:21.073Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, linting rules enforce keeping `import type` statements separate from regular `import` statements, even when importing from the same module. Do not suggest consolidating them into a single import statement.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-28T02:00:53.680Z
Learning: Applies to **/*.{ts,tsx} : Use separate `import type` statements instead of inline `type` in mixed imports. Example: `import type { Foo } from './foo'` + `import { bar } from './foo'` (not `import { bar, type Foo } from './foo'`)
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-28T02:00:53.680Z
Learning: Applies to **/*.{js,ts,tsx,vue} : Unused imports are disallowed and must be removed
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7894
File: src/renderer/extensions/vueNodes/widgets/components/WidgetToggleSwitch.test.ts:11-14
Timestamp: 2026-01-08T02:40:22.621Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: When testing components, import the real type definitions from the component files instead of duplicating interface definitions in the test files. This prevents type drift and maintains consistency.
Contributor
There was a problem hiding this comment.
Contributor
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!
DrJKL
approved these changes
Feb 2, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
BE Changes is Comfy-Org/ComfyUI#11594
┆Issue is synchronized with this Notion page by Unito