Conversation
jtydhr88
requested review from
Kosinkadink,
comfyanonymous and
guill
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds range input support: a new 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@comfy_api/latest/_input/range_types.py`:
- Around line 43-44: The to_lut() docstring states it returns float32 but the
implementation builds/returns a float64 NumPy array; update either the docstring
to float64 or (preferred) cast the final LUT to np.float32 before returning in
to_lut(), and apply the same fix to the other LUT-producing code paths noted
(the blocks referenced around lines 49-53 and line 66) so all LUT-returning
functions consistently produce float32; look for the to_lut() function and any
similarly named LUT builders in this module and add an explicit
.astype(np.float32) (or update their docstrings) to resolve the mismatch.
- Around line 34-39: The from_raw parsing in RangeInput currently calls
float(data["midpoint"]) when "midpoint" exists which crashes on payloads like
{"midpoint": None}; update the dict branch in RangeInput.from_raw to check that
data.get("midpoint") is not None before converting to float and otherwise set
midpoint to None (e.g., use a conditional: midpoint = float(value) if value is
not None else None), ensuring other fields (min_val/max_val) remain converted as
before.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 58e52ad1-06bc-4395-bf35-65151ad95f40
📒 Files selected for processing (4)
comfy_api/input/__init__.pycomfy_api/latest/_input/__init__.pycomfy_api/latest/_input/range_types.pycomfy_api/latest/_io.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
comfy_api/latest/_io.py (1)
1269-1304: LGTM! Clean implementation following established patterns.The
RangeComfyTypeIO class correctly follows the existing patterns in this file (similar toCurve,BoundingBox). Thesuper().__init__call properly maps all parameters, the default value{"min": 0.0, "max": 1.0}aligns withRangeInput.from_raw()expectations, andas_dict()correctly serializes the UI configuration fields.One minor type hint improvement:
📝 Optional: More specific type hint for gradient_stops
- gradient_stops: list=None, + gradient_stops: list[dict]=None,This matches the pattern used in
Float.Input(line 300).,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@comfy_api/latest/_io.py` around lines 1269 - 1304, Update the type hint for the Range.Input.gradient_stops attribute to be more specific (matching Float.Input's pattern) by changing its annotation from plain list to an optional list of dicts (e.g., gradient_stops: list[dict] | None or Optional[List[dict]]), and ensure any imports (typing.Optional or List) are added if needed; locate and edit the attribute declaration inside the Range.Input __init__ and the as_dict usage remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@comfy_api/latest/_io.py`:
- Around line 1269-1304: Update the type hint for the Range.Input.gradient_stops
attribute to be more specific (matching Float.Input's pattern) by changing its
annotation from plain list to an optional list of dicts (e.g., gradient_stops:
list[dict] | None or Optional[List[dict]]), and ensure any imports
(typing.Optional or List) are added if needed; locate and edit the attribute
declaration inside the Range.Input __init__ and the as_dict usage remains
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fca090c0-a28a-43b4-b1dd-3591dba54a25
📒 Files selected for processing (4)
comfy_api/input/__init__.pycomfy_api/latest/_input/__init__.pycomfy_api/latest/_input/range_types.pycomfy_api/latest/_io.py
🚧 Files skipped from review as they are similar to previous changes (3)
- comfy_api/input/init.py
- comfy_api/latest/_input/init.py
- comfy_api/latest/_input/range_types.py
BE change Comfy-Org/ComfyUI#13322 ## Summary Add RANGE widget for image levels adjustment - Add RangeEditor widget with three display modes: plain, gradient, and histogram - Support optional midpoint (gamma) control for non-linear midtone adjustment - Integrate histogram display from upstream node outputs ## Screenshots (if applicable) <img width="1450" height="715" alt="image" src="https://github.com/user-attachments/assets/864976af-9eb7-4dd0-9ce1-2f5d7f003117" /> <img width="1431" height="701" alt="image" src="https://github.com/user-attachments/assets/7ee2af65-f87a-407b-8bf2-6ec59a1dff59" /> <img width="705" height="822" alt="image" src="https://github.com/user-attachments/assets/7bcb8f17-795f-498a-9f8a-076ed6c05a98" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10936-Range-editor-33b6d73d365081089e8be040b40f6c8a) by [Unito](https://www.unito.io)
2 participants
FE change (but not required) Comfy-Org/ComfyUI_frontend#10936
Summary
Design
The RANGE type represents an input levels adjustment with three parameters: min, max, and an optional midpoint.
This follows the same mathematical model as
https://gitlab.gnome.org/GNOME/gimp/-/blob/master/app/operations/gimpoperationlevels.c
(gimp_operation_levels_map), which applies a three-step mapping:
midpoint instead of gamma
GIMP stores gamma directly but needs a logarithmic UI mapping (log10(1/gamma)) to make the slider feel linear. here it uses midpoint instead — a value in [0, 1] representing where the midtone sits within the input range. The conversion is gamma = -log2(midpoint), so midpoint=0.5 means gamma=1.0 (linear, no correction). This is more intuitive for a frontend slider widget since the handle position directly corresponds to the midpoint value, with no additional log transform needed.
Input side only
GIMP's Levels has both input mapping (black point, white point, gamma) and output mapping (output black, output white). In ComfyUI's node-based architecture, output remapping can be achieved by chaining another node downstream. Keeping the RANGE type focused on the input side (min, max, midpoint) keeps the type minimal and composable.
widget sceenshot