feat(view): serve downscaled image previews via the assets system#14976
feat(view): serve downscaled image previews via the assets system#14976jtydhr88 wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThe PR adds cached WebP preview generation with deterministic filenames, source-asset validation, preview registration, and 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/assets/services/preview.py`:
- Around line 32-41: Update the temporary-file workflow around Image.open and
preview_img.save so any exception during processing or saving removes tmp_path
before propagating the failure. Preserve the successful os.replace(tmp_path,
preview_path) flow, and only clean up the temporary file when it exists.
In `@openapi.yaml`:
- Around line 4483-4508: Update the endpoint’s 200 response content declaration
to include image/webp alongside the existing JPEG and PNG media types. Keep the
preview query parameter behavior unchanged and ensure generated clients and
validators recognize webp as a successful response format.
In `@server.py`:
- Around line 627-633: Update the preview generation flow around
get_or_create_preview_file so failures do not fall back to serving the original
full-resolution file when max_size is requested. Remove the broad Exception
fallback and either propagate an actionable error or catch only a specific,
genuinely recoverable failure whose fallback still honors the requested size.
- Around line 479-484: Update the hash-resolution failure flow in image_upload
and its caller upload_mask so a failed resolve_hash_to_path cannot be discarded
and produce HTTP 200. Resolve the hash before invoking image_upload, or
propagate a dedicated worker exception and translate it to HTTP 400 in
upload_mask, while preserving the existing public method contracts and
successful mask-writing behavior.
- Around line 528-557: Update the paint-output flow around `sibling_path` and
the direct `paint_pil.save`, `painted_pil.save`, and `painted_masked_pil.save`
calls to validate all sibling paths for uniqueness and the existing overwrite
policy before writing. Stage every generated output, including the existing
masked output as needed, in temporary files and atomically replace final paths
only after all image loading, resizing, compositing, and saving succeeds; clean
up staged files on failure so a failed request cannot leave partial outputs.
- Line 624: Update the Content-Disposition construction in the preview response
path to escape quotes and backslashes in filename before interpolating it into
the header, matching the escaping used by the existing response path around the
other Content-Disposition handling.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 25b4e601-26e4-4ebd-b957-1df2101f0847
📒 Files selected for processing (3)
app/assets/services/preview.pyopenapi.yamlserver.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: test (windows-latest)
- GitHub Check: test (windows-2022)
🧰 Additional context used
📓 Path-based instructions (3)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep changes small, direct, and limited to the narrowest code path and fewest files necessary; prefer practical fixes over broad architectural work.
Prefer fewer dependencies and do not add a ComfyUI dependency unless absolutely necessary.
Remove obsolete code, dead branches, unused options, debug prints, and unnecessary compatibility paths.
Preserve existing APIs, node names, model-loading behavior, file layout, and workflow compatibility unless replacement is explicit.
Keep architectural layers focused; do not leak UI, API, workflow, queue, persistence, telemetry, model-loading, node, or execution concerns into unrelated layers.
Shared core modules should depend only on lower-level primitives and their own domain concepts; higher-level concepts belong at callers, adapters, services, or boundaries.
Pass only the narrowest data needed across boundaries and keep identity mapping, persistence, history, telemetry, response shaping, and UI state in their owning layers.
Before touching many files, identify the smallest owner layer; use caller-side mappings, adapters, events, or narrow interfaces instead of exposing private concepts across layers.
Core ComfyUI code must not make unsolicited internet requests or add uploads, telemetry, analytics, tracking, reporting, update checks, remote configuration, licensing checks, or similar outbound paths.
Model downloading is allowed only when explicitly authorized by the user, limited to the requested artifact, and free of telemetry, tracking, unrelated metadata, or background activity.
Warning and info messages should be short and actionable; documentation and README changes should be concise, factual, and tied to changed behavior.
Use short direct commit subjects such asFix ...,Add ...,Support ...,Remove ..., orUpdate ...; keep PR descriptions short and state the problem, behavior change, and tests.
Prefer one coherent behavioral change per commit and prioritize crashes, incorrect dtype/device behavior, m...
Files:
app/assets/services/preview.pyopenapi.yamlserver.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Keep state and capability flags on the object that owns the behavior; use explicit parent-owned fields rather than probing children withgetattrfor parent control flow.
Keep public methods aligned with caller contracts; preserve arguments, parameter order, return types, side effects, and error behavior unless all affected interfaces are intentionally updated.
Do not add unused compatibility parameters, flags, attributes, constructor options, or model-specific options to shared helpers.
Normalize third-party and upstream return conventions at integration boundaries so core code receives the expected type and shape.
Avoid caller-side unwrapping such asout = out[0]unless the called interface documents that return structure.
Do not addtorch.no_grad,torch.inference_mode, or inference-mode wrappers; only disable a globally enabled inference mode when a training path requires gradients.
Do not add freeze, unfreeze, or trainability toggles to ComfyUI model classes.
Remove training-only behavior such as dropout from inference models while preserving checkpoint and state-dict compatibility, usingnn.Identitywhen necessary to retain slots.
Keep imports at module scope; use inline imports only for established optional-backend probes or import-cycle avoidance.
Usetry/exceptonly for optional dependency, platform, or backend detection with a useful fallback, and prefer specific exception types.
Do not add code for unsupported pinned library versions or obsolete PyTorch workarounds; unsupported formats, quantization metadata, and bad states should fail clearly.
Match local file style and keep comments sparse, useful, and non-obvious; remove comments that merely restate code.
Files:
app/assets/services/preview.pyserver.py
**
⚙️ CodeRabbit configuration file
**: IMPORTANT: Only comment on issues directly introduced by this PR's code changes.
Treat AGENTS.md as mandatory repository policy, not optional style guidance.
Flag PR changes that violate AGENTS.md even when the code is otherwise functional.
In particular, enforce architecture boundaries, dtype/device/memory rules,
interface contracts, import style, no unnecessary try/except blocks, no inline
imports, no outbound internet paths in core ComfyUI, and narrow scoped fixes.
Prefer direct findings over suggestions when a rule is violated. Only ignore
AGENTS.md when it clearly conflicts with a newer explicit maintainer instruction
in the PR.
Do NOT flag pre-existing issues in code that was merely moved, re-indented,
de-indented, or reformatted without logic changes. If code appears in the diff
only due to whitespace or structural reformatting (e.g., removing awith:block),
treat it as unchanged. Contributors should not feel obligated to address
pre-existing issues outside the scope of their contribution.
Files:
app/assets/services/preview.pyopenapi.yamlserver.py
🧠 Learnings (1)
📚 Learning: 2026-02-21T14:01:41.482Z
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI PR: 12555
File: comfy_extras/nodes_glsl.py:719-724
Timestamp: 2026-02-21T14:01:41.482Z
Learning: In PyOpenGL, bare Python scalars can be accepted for 1-element array parameters by NumberHandler. This means you can pass an int/float directly to OpenGL texture deletion (e.g., glDeleteTextures(tex)) without wrapping in a list. Verify function-specific expectations and ensure types match what the OpenGL call expects; use explicit lists only when the API requires an array.
Applied to files:
app/assets/services/preview.pyserver.py
🔇 Additional comments (2)
openapi.yaml (1)
3811-3828: LGTM!Also applies to: 3844-3862
server.py (1)
52-52: LGTM!Also applies to: 517-527, 559-561, 607-623, 635-653
d31aeea to
e7bef27
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/assets/services/preview.py`:
- Around line 89-90: Update the preview image registration error handling around
the existing try/except to catch only the specific, expected registration
exceptions, allowing unexpected database or SQLAlchemy failures to propagate.
Preserve the warning log and fallback behavior for those handled exceptions, and
remove the broad Exception catch.
In `@server.py`:
- Around line 627-632: The max_size contract is inconsistent between request
parsing and the OpenAPI schema. In server.py lines 627-632, parse signed integer
values, clamp every integer to the 512–8192 range, and ensure non-positive
values do not become an unbounded preview; in openapi.yaml lines 4493-4508,
remove the contradictory minimum and maximum schema bounds so out-of-range
integers are accepted and clamped.
- Around line 532-538: The new paint layer path around Image.open in server.py
must validate the source image dimensions before convert("RGBA"), rejecting any
edge of 16384 or more or total area above 67108864 pixels; preserve existing
resizing for valid images. Update openapi.yaml lines 3850-3853 to document that
paint is subject to the same image limits as the mask and original.
- Around line 623-625: Clamp or reject the preview quality parsed in server.py
so only values from 0 through 100 reach preview generation, preserving the
default quality for inputs without a digit. Update openapi.yaml at the
documented preview quality parameter to explicitly describe the accepted 0–100
range.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 79281f44-45b3-4e1f-bb61-2796887b8d93
📒 Files selected for processing (3)
app/assets/services/preview.pyopenapi.yamlserver.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: test (macos-latest)
- GitHub Check: test (ubuntu-latest)
- GitHub Check: test (windows-latest)
- GitHub Check: test (windows-2022)
🧰 Additional context used
📓 Path-based instructions (3)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep changes small, direct, and limited to the narrowest code path and fewest files necessary; prefer practical fixes over broad architectural work.
Prefer fewer dependencies and do not add a ComfyUI dependency unless absolutely necessary.
Remove obsolete code, dead branches, unused options, debug prints, and unnecessary compatibility paths.
Preserve existing APIs, node names, model-loading behavior, file layout, and workflow compatibility unless replacement is explicit.
Keep architectural layers focused; do not leak UI, API, workflow, queue, persistence, telemetry, model-loading, node, or execution concerns into unrelated layers.
Shared core modules should depend only on lower-level primitives and their own domain concepts; higher-level concepts belong at callers, adapters, services, or boundaries.
Pass only the narrowest data needed across boundaries and keep identity mapping, persistence, history, telemetry, response shaping, and UI state in their owning layers.
Before touching many files, identify the smallest owner layer; use caller-side mappings, adapters, events, or narrow interfaces instead of exposing private concepts across layers.
Core ComfyUI code must not make unsolicited internet requests or add uploads, telemetry, analytics, tracking, reporting, update checks, remote configuration, licensing checks, or similar outbound paths.
Model downloading is allowed only when explicitly authorized by the user, limited to the requested artifact, and free of telemetry, tracking, unrelated metadata, or background activity.
Warning and info messages should be short and actionable; documentation and README changes should be concise, factual, and tied to changed behavior.
Use short direct commit subjects such asFix ...,Add ...,Support ...,Remove ..., orUpdate ...; keep PR descriptions short and state the problem, behavior change, and tests.
Prefer one coherent behavioral change per commit and prioritize crashes, incorrect dtype/device behavior, m...
Files:
openapi.yamlapp/assets/services/preview.pyserver.py
**
⚙️ CodeRabbit configuration file
**: IMPORTANT: Only comment on issues directly introduced by this PR's code changes.
Treat AGENTS.md as mandatory repository policy, not optional style guidance.
Flag PR changes that violate AGENTS.md even when the code is otherwise functional.
In particular, enforce architecture boundaries, dtype/device/memory rules,
interface contracts, import style, no unnecessary try/except blocks, no inline
imports, no outbound internet paths in core ComfyUI, and narrow scoped fixes.
Prefer direct findings over suggestions when a rule is violated. Only ignore
AGENTS.md when it clearly conflicts with a newer explicit maintainer instruction
in the PR.
Do NOT flag pre-existing issues in code that was merely moved, re-indented,
de-indented, or reformatted without logic changes. If code appears in the diff
only due to whitespace or structural reformatting (e.g., removing awith:block),
treat it as unchanged. Contributors should not feel obligated to address
pre-existing issues outside the scope of their contribution.
Files:
openapi.yamlapp/assets/services/preview.pyserver.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Keep state and capability flags on the object that owns the behavior; use explicit parent-owned fields rather than probing children withgetattrfor parent control flow.
Keep public methods aligned with caller contracts; preserve arguments, parameter order, return types, side effects, and error behavior unless all affected interfaces are intentionally updated.
Do not add unused compatibility parameters, flags, attributes, constructor options, or model-specific options to shared helpers.
Normalize third-party and upstream return conventions at integration boundaries so core code receives the expected type and shape.
Avoid caller-side unwrapping such asout = out[0]unless the called interface documents that return structure.
Do not addtorch.no_grad,torch.inference_mode, or inference-mode wrappers; only disable a globally enabled inference mode when a training path requires gradients.
Do not add freeze, unfreeze, or trainability toggles to ComfyUI model classes.
Remove training-only behavior such as dropout from inference models while preserving checkpoint and state-dict compatibility, usingnn.Identitywhen necessary to retain slots.
Keep imports at module scope; use inline imports only for established optional-backend probes or import-cycle avoidance.
Usetry/exceptonly for optional dependency, platform, or backend detection with a useful fallback, and prefer specific exception types.
Do not add code for unsupported pinned library versions or obsolete PyTorch workarounds; unsupported formats, quantization metadata, and bad states should fail clearly.
Match local file style and keep comments sparse, useful, and non-obvious; remove comments that merely restate code.
Files:
app/assets/services/preview.pyserver.py
🧠 Learnings (1)
📚 Learning: 2026-02-21T14:01:41.482Z
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI PR: 12555
File: comfy_extras/nodes_glsl.py:719-724
Timestamp: 2026-02-21T14:01:41.482Z
Learning: In PyOpenGL, bare Python scalars can be accepted for 1-element array parameters by NumberHandler. This means you can pass an int/float directly to OpenGL texture deletion (e.g., glDeleteTextures(tex)) without wrapping in a list. Verify function-specific expectations and ensure types match what the OpenGL call expects; use explicit lists only when the API requires an array.
Applied to files:
app/assets/services/preview.pyserver.py
🔇 Additional comments (5)
server.py (3)
528-560: The output transaction remains partial.Line 530 commits the main mask before paint decoding, and sibling replacements remain sequential without uniqueness or overwrite checks. A later failure can therefore leave partial or overwritten outputs. This is the same unresolved finding from the previous review.
640-646: The broad preview fallback is still present.Although the fallback now preserves sizing,
except Exceptionstill hides cache or asset-registration failures and returns success without guaranteed preview linkage. This duplicates the prior broad-catch finding.As per coding guidelines, “Use
try/exceptonly for optional dependency, platform, or backend detection with a useful fallback.”Sources: Coding guidelines, Path instructions
52-52: LGTM!Also applies to: 477-512, 513-527, 569-571, 617-622, 634-638
openapi.yaml (2)
3811-3828: LGTM!Also applies to: 3844-3846, 3854-3862
4522-4527: LGTM!
| quality = 90 | ||
| if preview_info[-1].isdigit(): | ||
| quality = int(preview_info[-1]) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the repository's declared Pillow version constraints.
fd -t f -g 'pyproject.toml' -g 'requirements*.txt' -g 'uv.lock' -g 'poetry.lock' \
-x rg -n -i '\bpillow\b' {}Repository: Comfy-Org/ComfyUI
Length of output: 372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## server.py excerpt\n'
nl -ba server.py | sed -n '590,690p'
printf '\n## openapi.yaml excerpt\n'
nl -ba openapi.yaml | sed -n '4470,4510p'
printf '\n## search preview quality/cache references\n'
rg -n "preview_info|quality|preview cache|cache key|Image\.save|save\(" server.py openapi.yamlRepository: Comfy-Org/ComfyUI
Length of output: 217
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## server.py excerpt\n'
nl -ba server.py | sed -n '600,680p'
printf '\n## openapi.yaml excerpt\n'
nl -ba openapi.yaml | sed -n '4478,4500p'
printf '\n## preview-related search\n'
rg -n "preview_info|preview.*cache|cache.*preview|quality|Image\.save|webp;90|channel containing 'a'" server.py openapi.yamlRepository: Comfy-Org/ComfyUI
Length of output: 217
🌐 Web query:
Pillow WebP quality range 0 100 documentation invalid quality behavior
💡 Result:
In the Python Pillow library, the WebP quality parameter is documented as an integer ranging from 0 to 100 [1][2]. For lossy WebP compression, this parameter controls image quality (where 0 is the smallest size/poorest quality and 100 is the largest size/best quality) [1][2]. For lossless compression, it controls the effort put into compression (where 0 is the fastest and 100 is the slowest but most efficient) [1][2]. Regarding invalid behavior: Pillow does not explicitly enforce the 0–100 range via a raised exception within the Python-level wrapper before passing the value to the underlying C-based libwebp encoder [3]. When you provide a value outside this expected range (e.g., negative numbers or values exceeding 100), the behavior depends on the underlying libwebp library [4]. In many cases, libwebp may clamp the value or treat it as invalid, which can lead to unexpected encoding results or errors [4]. Because Pillow passes the quality value as a float to the internal _webp.WebPEncode function [5][3], values outside the 0–100 range are often passed directly to the encoder, making it best practice to manually clamp your inputs to the 0–100 range in your application code to ensure consistent behavior across different versions and environments [4].
Citations:
- 1: https://pillow.readthedocs.io/en/stable/handbook/image-file-formats.html?highlight=webp
- 2: https://pillow.readthedocs.io/en/stable/_sources/handbook/image-file-formats.rst.txt
- 3: https://github.com/python-pillow/Pillow/blob/main/src/PIL/WebPImagePlugin.py
- 4: DEFAULT_QUALITY causes problems for webp. un1t/django-resized#62
- 5: https://pillow.readthedocs.io/en/latest/%5Fmodules/PIL/WebPImagePlugin.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## server.py excerpt\n'
sed -n '600,690p' server.py
printf '\n## openapi.yaml excerpt\n'
sed -n '4478,4500p' openapi.yaml
printf '\n## preview-related search\n'
rg -n "preview_info|preview.*cache|cache.*preview|quality|Image\.save|webp;90|channel containing 'a'" server.py openapi.yamlRepository: Comfy-Org/ComfyUI
Length of output: 7585
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## locate preview cache helper\n'
rg -n "def get_or_create_preview_file|preview_id|content hash|quality" server.py
printf '\n## helper excerpt\n'
sed -n '1,260p' server.py | sed -n '1,260p'Repository: Comfy-Org/ComfyUI
Length of output: 10335
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -A40 -B20 "def get_or_create_preview_file" server.pyRepository: Comfy-Org/ComfyUI
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## locate preview service file\n'
fd -a 'preview.py' app server.py . | sed -n '1,20p'
printf '\n## search preview service references\n'
rg -n "get_or_create_preview_file|preview_id|content hash|max_size|quality" app server.py
printf '\n## preview service excerpt if present\n'
sed -n '1,260p' app/assets/services/preview.pyRepository: Comfy-Org/ComfyUI
Length of output: 11616
Clamp preview quality to 0–100.
server.py passes the raw value into preview generation and app/assets/services/preview.py bakes it into the cache filename; reject or bound out-of-range values, and document the accepted range in openapi.yaml.
📍 Affects 2 files
server.py#L623-L625(this comment)openapi.yaml#L4483-L4492
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server.py` around lines 623 - 625, Clamp or reject the preview quality parsed
in server.py so only values from 0 through 100 reach preview generation,
preserving the default quality for inputs without a digit. Update openapi.yaml
at the documented preview quality parameter to explicitly describe the accepted
0–100 range.
e7bef27 to
676a6df
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/assets/services/preview.py`:
- Around line 22-23: Update the hash extraction in the preview flow around
_ensure_source_asset to use the final colon-delimited component, preserving the
full raw hash when no colon is present. Keep the existing source_hash and
filename generation behavior unchanged otherwise.
- Around line 34-40: Update the image-processing flow around Image.open in the
preview generation function to apply EXIF-based orientation transposition before
resizing or saving. Use the transposed image for the max_size check,
ImageOps.contain call, and WebP save while preserving the existing quality and
output behavior.
- Around line 34-40: Update the image handling around Image.open in the preview
generation flow to apply ImageOps.exif_transpose() before checking dimensions
and calling ImageOps.contain(), then save the correctly oriented image. Preserve
the existing resize threshold, resampling, and WebP output behavior.
In `@server.py`:
- Around line 647-652: Update the preview-generation handling around
get_or_create_preview_file to catch only the preview-service exception
representing an expected generation failure. Expose or import that specific
exception from the preview service, replace the broad Exception handler, and
preserve the existing in-memory downscaling fallback only for that exception;
allow programming and asset-registration errors to propagate.
- Around line 528-559: Update the output-saving flow around the main masked
image and sibling_outputs so every requested image is first written to unique
temporary files, validated successfully, and only then atomically replaces its
destination. Prevent caller-controlled sibling names from overwriting existing
files or colliding with each other by validating destinations before
replacement, and ensure the main output is committed only after sibling
generation and staging complete.
- Around line 518-519: Remove the Image.new-based putalpha call after
original_pil.convert('RGBA') so original_pil retains its source alpha for
painted and painted_masked compositing; leave masked_pil.putalpha(new_alpha)
unchanged.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: bde1bc34-5cf2-42d2-95ba-168397e7ab2c
📒 Files selected for processing (3)
app/assets/services/preview.pyopenapi.yamlserver.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: test (windows-2022)
- GitHub Check: test (windows-latest)
🧰 Additional context used
📓 Path-based instructions (3)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep changes small, direct, and limited to the narrowest code path and fewest files necessary; prefer practical fixes over broad architectural work.
Prefer fewer dependencies and do not add a ComfyUI dependency unless absolutely necessary.
Remove obsolete code, dead branches, unused options, debug prints, and unnecessary compatibility paths.
Preserve existing APIs, node names, model-loading behavior, file layout, and workflow compatibility unless replacement is explicit.
Keep architectural layers focused; do not leak UI, API, workflow, queue, persistence, telemetry, model-loading, node, or execution concerns into unrelated layers.
Shared core modules should depend only on lower-level primitives and their own domain concepts; higher-level concepts belong at callers, adapters, services, or boundaries.
Pass only the narrowest data needed across boundaries and keep identity mapping, persistence, history, telemetry, response shaping, and UI state in their owning layers.
Before touching many files, identify the smallest owner layer; use caller-side mappings, adapters, events, or narrow interfaces instead of exposing private concepts across layers.
Core ComfyUI code must not make unsolicited internet requests or add uploads, telemetry, analytics, tracking, reporting, update checks, remote configuration, licensing checks, or similar outbound paths.
Model downloading is allowed only when explicitly authorized by the user, limited to the requested artifact, and free of telemetry, tracking, unrelated metadata, or background activity.
Warning and info messages should be short and actionable; documentation and README changes should be concise, factual, and tied to changed behavior.
Use short direct commit subjects such asFix ...,Add ...,Support ...,Remove ..., orUpdate ...; keep PR descriptions short and state the problem, behavior change, and tests.
Prefer one coherent behavioral change per commit and prioritize crashes, incorrect dtype/device behavior, m...
Files:
openapi.yamlapp/assets/services/preview.pyserver.py
**
⚙️ CodeRabbit configuration file
**: IMPORTANT: Only comment on issues directly introduced by this PR's code changes.
Treat AGENTS.md as mandatory repository policy, not optional style guidance.
Flag PR changes that violate AGENTS.md even when the code is otherwise functional.
In particular, enforce architecture boundaries, dtype/device/memory rules,
interface contracts, import style, no unnecessary try/except blocks, no inline
imports, no outbound internet paths in core ComfyUI, and narrow scoped fixes.
Prefer direct findings over suggestions when a rule is violated. Only ignore
AGENTS.md when it clearly conflicts with a newer explicit maintainer instruction
in the PR.
Do NOT flag pre-existing issues in code that was merely moved, re-indented,
de-indented, or reformatted without logic changes. If code appears in the diff
only due to whitespace or structural reformatting (e.g., removing awith:block),
treat it as unchanged. Contributors should not feel obligated to address
pre-existing issues outside the scope of their contribution.
Files:
openapi.yamlapp/assets/services/preview.pyserver.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Keep state and capability flags on the object that owns the behavior; use explicit parent-owned fields rather than probing children withgetattrfor parent control flow.
Keep public methods aligned with caller contracts; preserve arguments, parameter order, return types, side effects, and error behavior unless all affected interfaces are intentionally updated.
Do not add unused compatibility parameters, flags, attributes, constructor options, or model-specific options to shared helpers.
Normalize third-party and upstream return conventions at integration boundaries so core code receives the expected type and shape.
Avoid caller-side unwrapping such asout = out[0]unless the called interface documents that return structure.
Do not addtorch.no_grad,torch.inference_mode, or inference-mode wrappers; only disable a globally enabled inference mode when a training path requires gradients.
Do not add freeze, unfreeze, or trainability toggles to ComfyUI model classes.
Remove training-only behavior such as dropout from inference models while preserving checkpoint and state-dict compatibility, usingnn.Identitywhen necessary to retain slots.
Keep imports at module scope; use inline imports only for established optional-backend probes or import-cycle avoidance.
Usetry/exceptonly for optional dependency, platform, or backend detection with a useful fallback, and prefer specific exception types.
Do not add code for unsupported pinned library versions or obsolete PyTorch workarounds; unsupported formats, quantization metadata, and bad states should fail clearly.
Match local file style and keep comments sparse, useful, and non-obvious; remove comments that merely restate code.
Files:
app/assets/services/preview.pyserver.py
🧠 Learnings (1)
📚 Learning: 2026-02-21T14:01:41.482Z
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI PR: 12555
File: comfy_extras/nodes_glsl.py:719-724
Timestamp: 2026-02-21T14:01:41.482Z
Learning: In PyOpenGL, bare Python scalars can be accepted for 1-element array parameters by NumberHandler. This means you can pass an int/float directly to OpenGL texture deletion (e.g., glDeleteTextures(tex)) without wrapping in a list. Verify function-specific expectations and ensure types match what the OpenGL call expects; use explicit lists only when the API requires an array.
Applied to files:
app/assets/services/preview.pyserver.py
🔇 Additional comments (3)
openapi.yaml (1)
3811-3828: LGTM!Also applies to: 3844-3862, 4483-4527
server.py (1)
52-52: LGTM!Also applies to: 477-517, 520-527, 569-571, 617-646, 654-678
app/assets/services/preview.py (1)
22-23: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRobustify hash prefix stripping.
Using
partition(":")[2]successfully strips the prefix for hashes likeblake3:abc. However, if thesource_hashever lacks a colon (e.g., an older or custom raw hash),partition(":")[2]will evaluate to an empty string. This would cause all such previews to collide on the same filename (e.g.,_512_q80.webp).Using
split(":")[-1]safely extracts the hash whether the colon prefix is present or not.🛠️ Proposed fix
- source_hash, source_ref_id = _ensure_source_asset(source_abs_path) - hash_hex = source_hash.partition(":")[2] + source_hash, source_ref_id = _ensure_source_asset(source_abs_path) + hash_hex = source_hash.split(":")[-1]> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server.py (1)
491-508: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winValidate the final original path with realpath containment.
The string checks miss Windows drive/UNC paths, while
abspathcontainment still permits symlink escapes. A craftedoriginal_refcan therefore read an image outside the selected directory. Build the final path first, then validate it withfolder_paths.is_within_directory.Proposed fix
- # validation for security: prevent accessing arbitrary path - if filename[0] == '/' or '..' in filename: - raise web.HTTPBadRequest() - if output_dir is None: type = original_ref.get("type", "output") output_dir = folder_paths.get_directory_by_type(type) if output_dir is None: raise web.HTTPBadRequest() - if original_ref.get("subfolder", "") != "": - full_output_dir = os.path.join(output_dir, original_ref["subfolder"]) - if os.path.commonpath((os.path.abspath(full_output_dir), output_dir)) != output_dir: - raise web.HTTPForbidden() - output_dir = full_output_dir - - file = os.path.join(output_dir, filename) + subfolder = original_ref.get("subfolder", "") + file = os.path.abspath( + os.path.join(output_dir, subfolder, filename) + ) + if not folder_paths.is_within_directory(output_dir, file): + raise web.HTTPForbidden()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server.py` around lines 491 - 508, Update the original image path handling around the filename and subfolder logic to build the final path first, then validate that path with folder_paths.is_within_directory using the selected output directory. Replace the current string-based filename checks and abspath/commonpath containment checks, ensuring Windows drive/UNC paths and symlink escapes are rejected before accessing the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@server.py`:
- Around line 491-508: Update the original image path handling around the
filename and subfolder logic to build the final path first, then validate that
path with folder_paths.is_within_directory using the selected output directory.
Replace the current string-based filename checks and abspath/commonpath
containment checks, ensuring Windows drive/UNC paths and symlink escapes are
rejected before accessing the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1ac2d7be-d09e-49b1-9734-df9ea67eba30
📒 Files selected for processing (2)
app/assets/services/preview.pyserver.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: test (windows-2022)
- GitHub Check: test (windows-latest)
- GitHub Check: test (ubuntu-latest)
- GitHub Check: test (macos-latest)
🧰 Additional context used
📓 Path-based instructions (3)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep changes small, direct, and limited to the narrowest code path and fewest files necessary; prefer practical fixes over broad architectural work.
Prefer fewer dependencies and do not add a ComfyUI dependency unless absolutely necessary.
Remove obsolete code, dead branches, unused options, debug prints, and unnecessary compatibility paths.
Preserve existing APIs, node names, model-loading behavior, file layout, and workflow compatibility unless replacement is explicit.
Keep architectural layers focused; do not leak UI, API, workflow, queue, persistence, telemetry, model-loading, node, or execution concerns into unrelated layers.
Shared core modules should depend only on lower-level primitives and their own domain concepts; higher-level concepts belong at callers, adapters, services, or boundaries.
Pass only the narrowest data needed across boundaries and keep identity mapping, persistence, history, telemetry, response shaping, and UI state in their owning layers.
Before touching many files, identify the smallest owner layer; use caller-side mappings, adapters, events, or narrow interfaces instead of exposing private concepts across layers.
Core ComfyUI code must not make unsolicited internet requests or add uploads, telemetry, analytics, tracking, reporting, update checks, remote configuration, licensing checks, or similar outbound paths.
Model downloading is allowed only when explicitly authorized by the user, limited to the requested artifact, and free of telemetry, tracking, unrelated metadata, or background activity.
Warning and info messages should be short and actionable; documentation and README changes should be concise, factual, and tied to changed behavior.
Use short direct commit subjects such asFix ...,Add ...,Support ...,Remove ..., orUpdate ...; keep PR descriptions short and state the problem, behavior change, and tests.
Prefer one coherent behavioral change per commit and prioritize crashes, incorrect dtype/device behavior, m...
Files:
app/assets/services/preview.pyserver.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Keep state and capability flags on the object that owns the behavior; use explicit parent-owned fields rather than probing children withgetattrfor parent control flow.
Keep public methods aligned with caller contracts; preserve arguments, parameter order, return types, side effects, and error behavior unless all affected interfaces are intentionally updated.
Do not add unused compatibility parameters, flags, attributes, constructor options, or model-specific options to shared helpers.
Normalize third-party and upstream return conventions at integration boundaries so core code receives the expected type and shape.
Avoid caller-side unwrapping such asout = out[0]unless the called interface documents that return structure.
Do not addtorch.no_grad,torch.inference_mode, or inference-mode wrappers; only disable a globally enabled inference mode when a training path requires gradients.
Do not add freeze, unfreeze, or trainability toggles to ComfyUI model classes.
Remove training-only behavior such as dropout from inference models while preserving checkpoint and state-dict compatibility, usingnn.Identitywhen necessary to retain slots.
Keep imports at module scope; use inline imports only for established optional-backend probes or import-cycle avoidance.
Usetry/exceptonly for optional dependency, platform, or backend detection with a useful fallback, and prefer specific exception types.
Do not add code for unsupported pinned library versions or obsolete PyTorch workarounds; unsupported formats, quantization metadata, and bad states should fail clearly.
Match local file style and keep comments sparse, useful, and non-obvious; remove comments that merely restate code.
Files:
app/assets/services/preview.pyserver.py
**
⚙️ CodeRabbit configuration file
**: IMPORTANT: Only comment on issues directly introduced by this PR's code changes.
Treat AGENTS.md as mandatory repository policy, not optional style guidance.
Flag PR changes that violate AGENTS.md even when the code is otherwise functional.
In particular, enforce architecture boundaries, dtype/device/memory rules,
interface contracts, import style, no unnecessary try/except blocks, no inline
imports, no outbound internet paths in core ComfyUI, and narrow scoped fixes.
Prefer direct findings over suggestions when a rule is violated. Only ignore
AGENTS.md when it clearly conflicts with a newer explicit maintainer instruction
in the PR.
Do NOT flag pre-existing issues in code that was merely moved, re-indented,
de-indented, or reformatted without logic changes. If code appears in the diff
only due to whitespace or structural reformatting (e.g., removing awith:block),
treat it as unchanged. Contributors should not feel obligated to address
pre-existing issues outside the scope of their contribution.
Files:
app/assets/services/preview.pyserver.py
🧠 Learnings (1)
📚 Learning: 2026-02-21T14:01:41.482Z
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI PR: 12555
File: comfy_extras/nodes_glsl.py:719-724
Timestamp: 2026-02-21T14:01:41.482Z
Learning: In PyOpenGL, bare Python scalars can be accepted for 1-element array parameters by NumberHandler. This means you can pass an int/float directly to OpenGL texture deletion (e.g., glDeleteTextures(tex)) without wrapping in a list. Verify function-specific expectations and ensure types match what the OpenGL call expects; use explicit lists only when the API requires an array.
Applied to files:
app/assets/services/preview.pyserver.py
🔇 Additional comments (2)
app/assets/services/preview.py (1)
23-23: LGTM!Also applies to: 35-38
server.py (1)
52-52: LGTM!Also applies to: 513-574, 620-681
1 participant
Large images made the mask editor hang in the browser while decoding the full-resolution bitmap. Generate compressed, downscaled previews on the server so the client can edit a small copy and composite back at full resolution.
Some notes:
FE PR Comfy-Org/ComfyUI_frontend#13766
Cloud PR https://github.com/Comfy-Org/cloud/pull/5184
2026-07-17.14-46-20.mp4