feat: add RemoveBackgroundAndResizeNode — standalone PIL-only backgro… - #13
Conversation
…und removal with resize - Single-file node combining background removal + resize (no external background_remover.py dependency) - PIL-only implementation: k-means colour clustering, dominant-bg estimation from edges, alpha binarisation - ScalingMixin from grabcut_nodes.py: power-of-8 resize, 6 preset sizes + ORIGINAL - ComfyUI contract: INPUT_TYPES, RETURN_TYPES (IMAGE + MASK), FUNCTION, CATEGORY - Registered in __init__.py alongside existing TransparencyBackgroundRemover and GrabCut nodes - structlog logging, torch.no_grad() guard, full validation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 37 minutes and 18 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughA new ComfyUI node for background removal and image resizing is introduced via a standalone module. It uses PIL-based image processing with k-means color clustering to detect background, applies binarized alpha masking, and supports multiple interpolation methods for resizing, integrated into the ComfyUI node registry through updated init mappings. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review Roast 🔥Verdict: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: The overall architecture is solid — combining background removal with resize in a single PIL-only node is a clean approach. The batch processing loop is well-structured and the ComfyUI contract is properly followed. 💀 Worst part: The unused 📊 Overall: Like a mostly-complete IKEA bookshelf — solid frame, correct screws used, but there's that one drawer that doesn't quite close because you grabbed the wrong cam bolt. Fix the unused parameter and this is good to merge. Files Reviewed (2 files)
Review conducted in READ-ONLY mode — these are advisory observations only. Reviewed by minimax-m2.5-20260211 · 170,491 tokens |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the RemoveBackgroundAndResizeNode, which provides integrated background removal and resizing for ComfyUI workflows using PIL and NumPy. The review identifies several critical areas for improvement: addressing a potential NaN propagation bug in the k-means clustering algorithm, ensuring the IMAGE output follows the standard 3-channel RGB format for downstream compatibility, and optimizing memory efficiency for high-resolution images. Additionally, the feedback recommends enforcing the repository's 64x64 minimum dimension requirement, adhering to PEP 8 import standards, and utilizing more performant PIL methods for alpha channel extraction.
| for _ in range(max_iter): | ||
| dists = np.linalg.norm(pixels[:, None] - centres[None], axis=-1) | ||
| labels = dists.argmin(axis=1) | ||
| new_centres = np.stack([pixels[labels == i].mean(axis=0) for i in range(n_clusters)]) |
There was a problem hiding this comment.
The k-means implementation does not handle cases where a cluster becomes empty (i.e., no pixels are assigned to it). In such cases, pixels[labels == i].mean(axis=0) will return NaN, which propagates through the remaining iterations and corrupts the cluster centers. This can lead to incorrect background removal results.
| new_centres = np.stack([pixels[labels == i].mean(axis=0) for i in range(n_clusters)]) | |
| new_centres = np.array([pixels[labels == i].mean(axis=0) if np.any(labels == i) else centres[i] for i in range(n_clusters)]) |
| # ------------------------------------------------------------------ | ||
| # stack and return in ComfyUI tensor format | ||
| # ------------------------------------------------------------------ | ||
| stacked_image: torch.Tensor = torch.stack(rgba_tensors).to(dtype=torch.float32) |
There was a problem hiding this comment.
The node returns a 4-channel RGBA tensor as the IMAGE output. However, the standard ComfyUI IMAGE format is a 3-channel RGB tensor. Providing a 4-channel tensor can cause compatibility issues with many downstream nodes that expect exactly 3 channels. Since the transparency information is already provided via the MASK output, the IMAGE output should be sliced to RGB.
| stacked_image: torch.Tensor = torch.stack(rgba_tensors).to(dtype=torch.float32) | |
| stacked_image: torch.Tensor = torch.stack(rgba_tensors).to(dtype=torch.float32)[..., :3] |
| centres = pixels[indices].copy() | ||
|
|
||
| for _ in range(max_iter): | ||
| dists = np.linalg.norm(pixels[:, None] - centres[None], axis=-1) |
There was a problem hiding this comment.
Calculating distances using np.linalg.norm(pixels[:, None] - centres[None], axis=-1) creates a large intermediate 3D array of shape (N_pixels, N_clusters, 3). For high-resolution images (e.g., 2048x2048), this can consume several gigabytes of memory and potentially lead to a MemoryError on systems with limited RAM. Consider downsampling the pixel array before fitting the k-means model or using a more memory-efficient distance calculation.
|
|
||
| # 4. edge refinement | ||
| if dither_handling: | ||
| from PIL import ImageFilter |
There was a problem hiding this comment.
The ImageFilter import is placed inside the function. According to PEP 8, imports should be at the top of the file. Moving it to the top level also avoids repeated import overhead during batch processing.
References
- Imports should be at the top of the file, after any module comments and docstrings, and before module globals and constants. (link)
| if image is None or image.numel() == 0: | ||
| raise ValueError("RemoveBackgroundAndResizeNode: received empty image tensor.") |
There was a problem hiding this comment.
The node does not validate the minimum image dimensions. The repository style guide (line 69) specifies a minimum of 64x64 pixels. Without this check, very small images could cause the k-means clustering to fail if the number of pixels is less than color_clusters.
if image is None or image.numel() == 0:
raise ValueError("RemoveBackgroundAndResizeNode: received empty image tensor.")
if image.shape[1] < 64 or image.shape[2] < 64:
raise ValueError(f"RemoveBackgroundAndResizeNode: image must be at least 64x64, got {image.shape[2]}x{image.shape[1]}")References
- Input Validation: Minimum 64x64 pixels, 4D tensor format
| rgba_tensors.append(torch.from_numpy(arr)) | ||
|
|
||
| # extract alpha channel as mask | ||
| mask_np = (np.array(pil_rgba.split()[3], dtype=np.float32) / 255.0) |
There was a problem hiding this comment.
Using pil_rgba.split()[3] is inefficient because it creates four new Image objects (one for each channel) just to retrieve the alpha channel. Using getchannel is more direct and performant.
| mask_np = (np.array(pil_rgba.split()[3], dtype=np.float32) / 255.0) | |
| mask_np = (np.array(pil_rgba.getchannel('A'), dtype=np.float32) / 255.0) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__init__.py`:
- Around line 21-34: The import block for RemoveBackgroundAndResizeNode
currently catches all Exceptions which hides implementation bugs; update the
try/except around the import of RemoveBackgroundAndResizeNode so it only
swallows import/dependency errors (e.g., catch ImportError and
ModuleNotFoundError) and sets REMBG_MAPPINGS, REMBG_DISPLAY_MAPPINGS, and
REMBG_AVAILABLE as before; for any other exception type re-raise it so real
implementation/runtime errors in RemoveBackgroundAndResizeNode are not silently
marked unavailable. Ensure you reference the same symbols
(RemoveBackgroundAndResizeNode, REMBG_MAPPINGS, REMBG_DISPLAY_MAPPINGS,
REMBG_AVAILABLE) when making the change.
In `@RemoveBackgroundAndResizeNode.py`:
- Around line 155-163: The function parameter edge_sensitivity is declared in
remove_background_pil but never wired through the node inputs or used; either
remove it or propagate it: add edge_sensitivity to the node's INPUT_TYPES (range
0.0–1.0), accept it in process and pass it into remove_background_pil, then
forward it into _dominant_bg (or wherever edge detection is implemented) so it
actually affects processing; also ensure tolerance (0–255) and foreground_bias
(0.0–1.0) are exposed in INPUT_TYPES with those ranges; if you opt to remove
edge_sensitivity, delete the param from remove_background_pil and any docs to
avoid dead parameters.
- Around line 118-127: The K-means step can fail on tiny or low-colour images
because indices = np.random.RandomState(42).choice(..., replace=False) will
raise when len(pixels) < n_clusters and new_centres = np.stack([... .mean(...)])
produces NaNs when a cluster is empty; change initialization to cap clusters:
actual_k = min(n_clusters, len(pixels)) and sample indices with replace=True
only when len(pixels) < n_clusters (or simply use replace=True guarded by the
min), and when computing new_centres handle empty clusters by detecting labels
== i with no members and replacing that centre by either the previous centre
(centres[i]) or a random existing pixel from pixels (e.g.,
pixels[np.random.randint(len(pixels))]) instead of taking mean to avoid NaNs;
update any references to centres, indices, labels, new_centres, n_clusters, and
max_iter accordingly so the loop still uses actual_k for distances/labels.
- Around line 180-181: The mask logic contradicts the documented UI contract:
the parameter tolerance is documented as "Higher = more pixels accepted as
foreground" but the mask check uses (fg_dist + effective_tol) < bg_dist which
makes larger tolerance stricter; fix by changing the mask condition to use
subtraction i.e. (fg_dist - effective_tol) < bg_dist so increasing tolerance
accepts more foreground (or alternatively invert the docstring/tooltip to state
higher tolerance rejects more); update the docstring for tolerance and the UI
tooltip text to clearly match the chosen behavior (referencing the variables
tolerance/effective_tol, fg_dist, bg_dist and the mask evaluation logic).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2f899199-04e1-4290-8d22-97790cfb5972
📒 Files selected for processing (2)
RemoveBackgroundAndResizeNode.py__init__.py
| pixels = arr.reshape(-1, arr.shape[-1]).astype(np.float32) | ||
| indices = np.random.RandomState(42).choice( | ||
| len(pixels), size=n_clusters, replace=False | ||
| ) | ||
| centres = pixels[indices].copy() | ||
|
|
||
| for _ in range(max_iter): | ||
| dists = np.linalg.norm(pixels[:, None] - centres[None], axis=-1) | ||
| labels = dists.argmin(axis=1) | ||
| new_centres = np.stack([pixels[labels == i].mean(axis=0) for i in range(n_clusters)]) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import numpy as np
pixels = np.zeros((1, 3), dtype=np.float32)
try:
np.random.RandomState(42).choice(len(pixels), size=2, replace=False)
except ValueError as exc:
print("reproduced:", exc)
PYRepository: Limbicnation/ComfyUI-TransparencyBackgroundRemover
Length of output: 170
🏁 Script executed:
fd -t f "RemoveBackgroundAndResizeNode.py"Repository: Limbicnation/ComfyUI-TransparencyBackgroundRemover
Length of output: 126
🏁 Script executed:
# Once we find the file, let's read the _kmeans_colors function
rg -n "def _kmeans_colors" -A 30Repository: Limbicnation/ComfyUI-TransparencyBackgroundRemover
Length of output: 2323
🏁 Script executed:
# Verify the NaN center issue
python - <<'PY'
import numpy as np
# Simulate the NaN center problem
arr = np.array([[[100, 100, 100], [101, 101, 101]]], dtype=np.uint8)
pixels = arr.reshape(-1, arr.shape[-1]).astype(np.float32)
print(f"pixels shape: {pixels.shape}, unique colors: {np.unique(pixels, axis=0).shape[0]}")
# With 3 clusters but only 2 unique colors, one cluster will be empty
n_clusters = 3
indices = np.arange(n_clusters) # Simplified index selection
indices = indices[indices < len(pixels)] # Clamp to available pixels
centres = pixels[np.random.choice(len(pixels), size=min(n_clusters, len(pixels)), replace=False)].copy()
print(f"Initial centers shape: {centres.shape}")
# Simulate one iteration
dists = np.linalg.norm(pixels[:, None] - centres[None], axis=-1)
labels = dists.argmin(axis=1)
print(f"Label distribution: {np.bincount(labels, minlength=n_clusters)}")
# Original code - computes mean on potentially empty clusters
new_centres_original = np.stack([pixels[labels == i].mean(axis=0) for i in range(n_clusters)])
print(f"\nOriginal approach (line 127):")
print(f"new_centres:\n{new_centres_original}")
print(f"Contains NaN: {np.any(np.isnan(new_centres_original))}")
PYRepository: Limbicnation/ComfyUI-TransparencyBackgroundRemover
Length of output: 518
Guard k-means against tiny or low-colour images.
Line 119 samples n_clusters pixels without replacement, so images with fewer pixels than clusters raise ValueError. Line 127 can create NaN centres when a cluster receives no pixels, which is common for flat-colour inputs.
🐛 Proposed fix
def _kmeans_colors(
arr: np.ndarray, n_clusters: int, tol: float = 0.01, max_iter: int = 20
) -> np.ndarray:
"""
Simple k-means on pixel array [H*W, channels].
Returns ``(n_clusters, channels)`` cluster centres.
Fallback when scikit-learn is unavailable.
"""
pixels = arr.reshape(-1, arr.shape[-1]).astype(np.float32)
+ n_clusters = min(n_clusters, len(pixels))
indices = np.random.RandomState(42).choice(
len(pixels), size=n_clusters, replace=False
)
centres = pixels[indices].copy()
for _ in range(max_iter):
dists = np.linalg.norm(pixels[:, None] - centres[None], axis=-1)
labels = dists.argmin(axis=1)
- new_centres = np.stack([pixels[labels == i].mean(axis=0) for i in range(n_clusters)])
+ new_centres = centres.copy()
+ for i in range(n_clusters):
+ cluster_pixels = pixels[labels == i]
+ if len(cluster_pixels) > 0:
+ new_centres[i] = cluster_pixels.mean(axis=0)
if np.abs(new_centres - centres).max() < tol:
break📝 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.
| pixels = arr.reshape(-1, arr.shape[-1]).astype(np.float32) | |
| indices = np.random.RandomState(42).choice( | |
| len(pixels), size=n_clusters, replace=False | |
| ) | |
| centres = pixels[indices].copy() | |
| for _ in range(max_iter): | |
| dists = np.linalg.norm(pixels[:, None] - centres[None], axis=-1) | |
| labels = dists.argmin(axis=1) | |
| new_centres = np.stack([pixels[labels == i].mean(axis=0) for i in range(n_clusters)]) | |
| def _kmeans_colors( | |
| arr: np.ndarray, n_clusters: int, tol: float = 0.01, max_iter: int = 20 | |
| ) -> np.ndarray: | |
| """ | |
| Simple k-means on pixel array [H*W, channels]. | |
| Returns ``(n_clusters, channels)`` cluster centres. | |
| Fallback when scikit-learn is unavailable. | |
| """ | |
| pixels = arr.reshape(-1, arr.shape[-1]).astype(np.float32) | |
| n_clusters = min(n_clusters, len(pixels)) | |
| indices = np.random.RandomState(42).choice( | |
| len(pixels), size=n_clusters, replace=False | |
| ) | |
| centres = pixels[indices].copy() | |
| for _ in range(max_iter): | |
| dists = np.linalg.norm(pixels[:, None] - centres[None], axis=-1) | |
| labels = dists.argmin(axis=1) | |
| new_centres = centres.copy() | |
| for i in range(n_clusters): | |
| cluster_pixels = pixels[labels == i] | |
| if len(cluster_pixels) > 0: | |
| new_centres[i] = cluster_pixels.mean(axis=0) | |
| if np.abs(new_centres - centres).max() < tol: | |
| break | |
| centres = new_centres | |
| return centres |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@RemoveBackgroundAndResizeNode.py` around lines 118 - 127, The K-means step
can fail on tiny or low-colour images because indices =
np.random.RandomState(42).choice(..., replace=False) will raise when len(pixels)
< n_clusters and new_centres = np.stack([... .mean(...)]) produces NaNs when a
cluster is empty; change initialization to cap clusters: actual_k =
min(n_clusters, len(pixels)) and sample indices with replace=True only when
len(pixels) < n_clusters (or simply use replace=True guarded by the min), and
when computing new_centres handle empty clusters by detecting labels == i with
no members and replacing that centre by either the previous centre (centres[i])
or a random existing pixel from pixels (e.g.,
pixels[np.random.randint(len(pixels))]) instead of taking mean to avoid NaNs;
update any references to centres, indices, labels, new_centres, n_clusters, and
max_iter accordingly so the loop still uses actual_k for distances/labels.
| def remove_background_pil( | ||
| image: Image.Image, | ||
| tolerance: int = 30, | ||
| edge_sensitivity: float = 0.8, | ||
| color_clusters: int = 8, | ||
| foreground_bias: float = 0.7, | ||
| binary_threshold: int = 128, | ||
| dither_handling: bool = True, | ||
| ) -> Image.Image: |
There was a problem hiding this comment.
Wire edge_sensitivity through or remove it.
edge_sensitivity is documented and accepted by remove_background_pil, but it is not exposed in INPUT_TYPES, not passed from process, and not used by _dominant_bg, so changing it is impossible and it has no effect.
🔧 Proposed wiring
-def _dominant_bg(arr: np.ndarray, samples: int = 5000) -> np.ndarray:
+def _dominant_bg(
+ arr: np.ndarray,
+ samples: int = 5000,
+ edge_sensitivity: float = 0.8,
+) -> np.ndarray:
@@
- bw = max(1, min(h, w) // 10)
+ edge_sensitivity = max(0.0, min(1.0, edge_sensitivity))
+ bw = max(1, int(min(h, w) * 0.1 * edge_sensitivity))
@@
- bg_colour = _dominant_bg(arr)
+ bg_colour = _dominant_bg(arr, edge_sensitivity=edge_sensitivity) "tolerance": (
"INT",
{
@@
},
),
+ "edge_sensitivity": (
+ "FLOAT",
+ {
+ "default": 0.8,
+ "min": 0.0,
+ "max": 1.0,
+ "step": 0.01,
+ "display": "slider",
+ "tooltip": "How much of the image edge region to sample for background estimation.",
+ },
+ ),
"foreground_bias": ( image: torch.Tensor,
tolerance: int,
+ edge_sensitivity: float,
foreground_bias: float,
@@
pil_in,
tolerance=tolerance,
+ edge_sensitivity=edge_sensitivity,
foreground_bias=foreground_bias,Based on learnings, all processing parameters should be configurable via ComfyUI interface with specified ranges: tolerance 0-255, edge_sensitivity 0.0-1.0, foreground_bias 0.0-1.0.
Also applies to: 182-183, 207-207, 267-339, 414-421
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@RemoveBackgroundAndResizeNode.py` around lines 155 - 163, The function
parameter edge_sensitivity is declared in remove_background_pil but never wired
through the node inputs or used; either remove it or propagate it: add
edge_sensitivity to the node's INPUT_TYPES (range 0.0–1.0), accept it in process
and pass it into remove_background_pil, then forward it into _dominant_bg (or
wherever edge detection is implemented) so it actually affects processing; also
ensure tolerance (0–255) and foreground_bias (0.0–1.0) are exposed in
INPUT_TYPES with those ranges; if you opt to remove edge_sensitivity, delete the
param from remove_background_pil and any docs to avoid dead parameters.
| tolerance : int | ||
| Colour distance threshold (0-255). Higher = more pixels accepted as foreground. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -type f -name "RemoveBackgroundAndResizeNode.py"Repository: Limbicnation/ComfyUI-TransparencyBackgroundRemover
Length of output: 264
🏁 Script executed:
fd -t f "RemoveBackgroundAndResizeNode.py"Repository: Limbicnation/ComfyUI-TransparencyBackgroundRemover
Length of output: 126
🏁 Script executed:
wc -l RemoveBackgroundAndResizeNode.pyRepository: Limbicnation/ComfyUI-TransparencyBackgroundRemover
Length of output: 130
🏁 Script executed:
cat -n RemoveBackgroundAndResizeNode.py | sed -n '175,185p'Repository: Limbicnation/ComfyUI-TransparencyBackgroundRemover
Length of output: 575
🏁 Script executed:
cat -n RemoveBackgroundAndResizeNode.py | sed -n '210,230p'Repository: Limbicnation/ComfyUI-TransparencyBackgroundRemover
Length of output: 951
🏁 Script executed:
cat -n RemoveBackgroundAndResizeNode.py | sed -n '275,290p'Repository: Limbicnation/ComfyUI-TransparencyBackgroundRemover
Length of output: 808
🏁 Script executed:
cat -n RemoveBackgroundAndResizeNode.py | sed -n '1,50p'Repository: Limbicnation/ComfyUI-TransparencyBackgroundRemover
Length of output: 1946
🏁 Script executed:
cat -n RemoveBackgroundAndResizeNode.py | sed -n '160,200p'Repository: Limbicnation/ComfyUI-TransparencyBackgroundRemover
Length of output: 1872
Tolerance logic contradicts the documented UI contract.
The docstring (line 181) states "Higher = more pixels accepted as foreground," but the implementation at lines 218–220 adds tolerance to fg_dist in the mask condition (fg_dist + effective_tol) < bg_dist. This makes the condition harder to satisfy as tolerance increases, rejecting more pixels rather than accepting more. Either invert the wording to match the implementation or adjust the logic to (fg_dist - effective_tol) < bg_dist to align with the documented behavior.
The UI tooltip (lines 280–283) uses similarly ambiguous phrasing; clarify whether higher tolerance means "accept more foreground" or "remove background more aggressively."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@RemoveBackgroundAndResizeNode.py` around lines 180 - 181, The mask logic
contradicts the documented UI contract: the parameter tolerance is documented as
"Higher = more pixels accepted as foreground" but the mask check uses (fg_dist +
effective_tol) < bg_dist which makes larger tolerance stricter; fix by changing
the mask condition to use subtraction i.e. (fg_dist - effective_tol) < bg_dist
so increasing tolerance accepts more foreground (or alternatively invert the
docstring/tooltip to state higher tolerance rejects more); update the docstring
for tolerance and the UI tooltip text to clearly match the chosen behavior
(referencing the variables tolerance/effective_tol, fg_dist, bg_dist and the
mask evaluation logic).
Hermes Agent Code ReviewVerdict: Request Changes — 2 bugs, 3 warnings, 3 suggestions. The node logic is clean but there are a dead parameter, a performance issue on large images, and a structlog import that will crash in standalone mode. Summary
Bugs (2)
Warnings (3)
Suggestions (3)
Looks Good
Reviewed by Hermes Agent |
Limbicnation
left a comment
Limbicnation
left a comment
There was a problem hiding this comment.
Hermes Agent review - 2 bugs, 3 warnings, 2 suggestions. See inline comments.
| binary_threshold: int = 128, | ||
| dither_handling: bool = True, | ||
| ) -> Image.Image: | ||
| """ |
There was a problem hiding this comment.
Bug: edge_sensitivity is declared in the function signature and documented in the docstring, but never used in the function body. It's also NOT passed from process() (line ~430). This is a dead parameter — either wire it to _dominant_bg() or remove it entirely.
| import numpy as np | ||
| import torch | ||
| from PIL import Image | ||
|
|
There was a problem hiding this comment.
Bug: structlog is imported unconditionally. When running standalone (outside ComfyUI), structlog may not be installed, causing an immediate ImportError. Use a try/except fallback to stdlib logging.
| ) | ||
| centres = pixels[indices].copy() | ||
|
|
||
| for _ in range(max_iter): |
There was a problem hiding this comment.
Warning: _kmeans_colors() runs on the FULL pixel array. For a 4K image (8.3M pixels), this creates an (8.3M, n_clusters) distance matrix per iteration. This will OOM on large images. Add subsampling with _MAX_SAMPLES = 50_000.
| def INPUT_TYPES(cls): | ||
| return { | ||
| "required": { | ||
| "image": ("IMAGE", {"tooltip": "Input image tensor from upstream node"}), |
There was a problem hiding this comment.
Warning: OUTPUT_NODE = False — this node produces IMAGE + MASK output that users will preview or save. Should be OUTPUT_NODE = True.
| from .RemoveBackgroundAndResizeNode import ( | ||
| NODE_CLASS_MAPPINGS as REMBG_MAPPINGS, | ||
| NODE_DISPLAY_NAME_MAPPINGS as REMBG_DISPLAY_MAPPINGS, | ||
| ) |
There was a problem hiding this comment.
Warning: except Exception catches ALL exceptions including SyntaxError, NameError — silently hiding real bugs. Should be except ImportError.
Bugs: - Remove unused edge_sensitivity param from remove_background_pil() - Add try/except fallback for structlog import (was ImportError outside ComfyUI) Warnings: - Subsample pixels in _kmeans_colors() to prevent OOM on 4K images - Set OUTPUT_NODE = True (node produces terminal IMAGE+MASK output) - Narrow except Exception to except ImportError in __init__.py Suggestions: - Extract ScalingMixin to shared scaling.py (used by both nodes) - Remove unused comfy.utils import and _COMFY_AVAILABLE flag
Code Review Roast 🔥Verdict: 2 Issues (Unchanged) | Recommendation: Address remaining issues before merge Overview
Issue Details (click to expand)
✅ Fixed issues from previous review:
🏆 Best part: Most of the previous issues were addressed. The refactoring to extract ScalingMixin to a shared module is clean, and the OOM protection for k-means is a solid performance improvement. 💀 Worst part: The mask extraction at line 384 still creates an unnecessary numpy array. You already have 📊 Overall: Like a restaurant that fixed most of the issues from the health inspector visit - they removed the expired ingredients, fixed the broken fridge, and cleaned the kitchen. But that one corner with the mysterious smell? Still there. Two small items left to address before you get the gold plate. Files Reviewed (4 files)
Review conducted in READ-ONLY mode — advisory only. Reviewed by minimax-m2.5-20260211 · 609,142 tokens |
1 participant
…und removal with resize
Summary by CodeRabbit