submitted by mistake

[ccssu]

This commit was submitted by mistake, please close this PR or disregard this commit.

[ccssu]

@comfy-pr-bot This PR was submitted by mistake.
Please help close PR #14583. Thank you!

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request adds Depth Anything 3 (DA3) multi-view depth estimation as a new supported model, including a DINOv2 backbone with RoPE/QK-norm extensions, DPT/DualDPT depth heads, ray-to-pose conversion, camera encoder/decoder, and ComfyUI inference and geometry-export nodes. It introduces WAN21 SCAIL2 support with SAM3 mask conditioning streams and a Bernini in-context conditioning node for Wan2.2. On the asset management side, it removes the tag_type column from the Tags table (with Alembic migration), adds opaque keyset cursor pagination for GET /api/assets, switches public asset deletion to soft-delete, adds image-dimension metadata extraction during ingest, and enforces canonical UUID format for prompt IDs. New 3D IO types (File3DSplatAny, File3DPointCloudAny) and associated preview/export nodes are added. API nodes gain Gemini V2 with multimodal media budgeting, Krea 2 Medium Turbo, a BFL erase seed input, and a SaveAudioAdvanced node. Runtime changes include a --debug-hang traceback-dump flag, cuDNN benchmark refactoring, pinned memory CUDA host registration hardening, and various dtype/control-flow fixes.

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 32.77% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The PR description is completely absent, leaving reviewers without author-provided context or rationale for the changeset.	Add a concise PR description summarizing the main changes, key features, and any migration notes or testing guidance for reviewers.
Title check	❓ Inconclusive	The title "sync upstream (#19)" is overly vague and does not convey meaningful information about the specific changes made in this large changeset.	Consider a more descriptive title that highlights the primary feature or change area, such as "Add cursor-based pagination for assets, image dimensions, and SCAIL-2 model support" or "Implement keyset pagination and DA3 depth estimation features".

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

app/assets/database/queries/tags.py (1)

259-267: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update list_tags_with_usage return annotation to match the new tuple shape.

The function now returns (name, count) rows, but the signature still advertises a 3-item tuple including the removed tag_type. This leaves an incorrect type contract.

Suggested fix

 def list_tags_with_usage(
     session: Session,
@@
-) -> tuple[list[tuple[str, str, int]], int]:
+) -> tuple[list[tuple[str, int]], int]:

Also applies to: 331-331

🤖 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 `@app/assets/database/queries/tags.py` around lines 259 - 267, The return type
annotation for the `list_tags_with_usage` function incorrectly specifies a
3-item tuple (str, str, int) when the function now only returns 2-item tuples
(name, count) after removing the tag_type field. Update the return annotation
from tuple[list[tuple[str, str, int]], int] to tuple[list[tuple[str, int]], int]
to accurately reflect the new tuple shape being returned by the function.

🧹 Nitpick comments (5)

comfy_extras/nodes_depth_anything_3.py (1)

154-165: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Fail fast if the selected checkpoint is not a DA3 model.

model_name is sourced from the broad geometry_estimation list, so selecting a non-DA3 checkpoint can fail later with unclear attribute errors in inference. Add an immediate model-type guard after loading.

💡 Proposed fix

         path = folder_paths.get_full_path_or_raise("geometry_estimation", model_name)
         model = comfy.sd.load_diffusion_model(path, model_options=model_options)
+        image_model = getattr(getattr(model, "model_config", None), "unet_config", {}).get("image_model")
+        if image_model != "DepthAnything3":
+            raise ValueError(
+                f"Selected checkpoint '{model_name}' is not a Depth Anything 3 model "
+                f"(detected image_model={image_model!r})."
+            )
         return io.NodeOutput(model)

🤖 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 `@comfy_extras/nodes_depth_anything_3.py` around lines 154 - 165, In the
execute classmethod, immediately after loading the model with
comfy.sd.load_diffusion_model, add a type guard to validate that the loaded
model is actually a Depth Anything 3 model. Since model_name is sourced from the
broad geometry_estimation folder list, a non-DA3 checkpoint could be selected,
causing unclear attribute errors during inference. Add a check to verify the
model type and raise a descriptive error if the model is not a DA3 model,
ensuring fast failure with a clear message.

comfy_extras/nodes_audio.py (1)

285-292: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add an explicit audio is None guard in SaveAudioAdvanced.execute.

The legacy save nodes raise a clear ValueError for missing audio, but this new path forwards directly and will fail later with a less actionable error.

Proposed patch

     `@classmethod`
     def execute(cls, audio, filename_prefix: str, format: dict) -> IO.NodeOutput:
+        if audio is None:
+            raise ValueError("SaveAudioAdvanced: input audio is None (source video may have no audio track).")
         file_format = format.get("format", None)
         quality = format.get("quality", None)
         if quality:
             ui=UI.AudioSaveHelper.get_save_audio_ui(audio, filename_prefix=filename_prefix, cls=cls, format=file_format, quality=quality)
         else:
             ui=UI.AudioSaveHelper.get_save_audio_ui(audio, filename_prefix=filename_prefix, cls=cls, format=file_format)
         return IO.NodeOutput(ui=ui)

🤖 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 `@comfy_extras/nodes_audio.py` around lines 285 - 292, Add an explicit guard at
the beginning of the SaveAudioAdvanced.execute method to check if the audio
parameter is None, and raise a ValueError with a clear error message if it is.
This validation should occur before extracting the format and quality values,
ensuring that missing audio input is caught early with an actionable error
message consistent with legacy save node behavior.

comfy/ldm/depth_anything_3/camera.py (1)

32-40: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

_LayerScale.gamma is uninitialized; consider initializing to init_values.

torch.empty leaves gamma with arbitrary values. While checkpoint loading will overwrite this, the parameter should still be initialized for robustness (debugging, testing without weights, or if a checkpoint is missing this key).

Proposed fix

 class _LayerScale(nn.Module):
     """Per-channel learnable scaling. Matches upstream LayerScale."""

-    def __init__(self, dim, *, device=None, dtype=None):
+    def __init__(self, dim, init_values=1e-4, *, device=None, dtype=None):
         super().__init__()
-        self.gamma = nn.Parameter(torch.empty(dim, device=device, dtype=dtype))
+        self.gamma = nn.Parameter(init_values * torch.ones(dim, device=device, dtype=dtype))

Then in _Block, forward init_values:

-        self.ls1 = _LayerScale(dim, device=device, dtype=dtype) if init_values else nn.Identity()
+        self.ls1 = _LayerScale(dim, init_values=init_values, device=device, dtype=dtype) if init_values else nn.Identity()
         ...
-        self.ls2 = _LayerScale(dim, device=device, dtype=dtype) if init_values else nn.Identity()
+        self.ls2 = _LayerScale(dim, init_values=init_values, device=device, dtype=dtype) if init_values else nn.Identity()

🤖 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 `@comfy/ldm/depth_anything_3/camera.py` around lines 32 - 40, In the
_LayerScale class __init__ method, the gamma parameter is being initialized with
torch.empty which leaves it with arbitrary values. Add an init_values parameter
to the __init__ method with a sensible default value (typically 1.0), and use it
to properly initialize self.gamma instead of using torch.empty. This ensures
gamma has meaningful initial values for robustness when debugging, testing
without weights, or if a checkpoint is missing this key.

comfy/ldm/depth_anything_3/ray_pose.py (1)

248-252: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Potential division by zero when all confidence values are zero.

If all pixels fail the z_thresh check (line 196-197), all weights become zero. The sum at line 251 would then be zero, causing a division-by-zero producing NaN translations.

This is an unlikely edge case (would require an entirely invalid ray output), but a guard would make it more robust.

💡 Optional guard

     cf = confidence.flatten(0, 1).flatten(1, 2)
     T = (camray.flatten(0, 1).flatten(1, 2)[..., 3:] * cf.unsqueeze(-1)).sum(dim=1)
-    T = T / cf.sum(dim=-1, keepdim=True)
+    cf_sum = cf.sum(dim=-1, keepdim=True)
+    T = T / cf_sum.clamp(min=1e-8)
     T = T.reshape(B, S, 3)

🤖 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 `@comfy/ldm/depth_anything_3/ray_pose.py` around lines 248 - 252, The
translation calculation using confidence-weighted averaging can encounter
division by zero when all confidence values are zero, resulting in NaN values.
In the section where variable T is computed by dividing by cf.sum(dim=-1,
keepdim=True), add a guard that clamps or prevents division by zero. This can be
done by either adding a small epsilon value to the denominator before dividing,
or by using a clamp operation on the denominator to ensure it never reaches
exactly zero, so the division operation remains numerically stable and does not
produce NaN translations.

comfy/ldm/depth_anything_3/model.py (1)

170-171: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Potential thread-safety issue with runtime enable_aux toggle.

Modifying self.head.enable_aux during forward pass is not thread-safe if the same model instance is used concurrently with different use_ray_pose values. One thread could set it to True while another expects False.

This is likely acceptable for typical single-threaded inference, but worth noting for concurrent execution scenarios.

💡 Thread-safe alternative

Pass enable_aux as a forward argument to DualDPT.forward() instead of storing it as instance state:

-        if isinstance(self.head, DualDPT):
-            self.head.enable_aux = bool(use_ray_pose)
-
-        feats, aux_feats = self.backbone.get_intermediate_layers_da3(
+        feats, aux_feats = self.backbone.get_intermediate_layers_da3(
             ...
         )
-        head_out = self.head(feats, H=H, W=W, patch_start_idx=0)
+        head_out = self.head(feats, H=H, W=W, patch_start_idx=0,
+                             enable_aux=bool(use_ray_pose) if isinstance(self.head, DualDPT) else False)

This would require updating DualDPT.forward() to accept enable_aux as a parameter.

🤖 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 `@comfy/ldm/depth_anything_3/model.py` around lines 170 - 171, The current
implementation sets `self.head.enable_aux` as instance state which creates
thread-safety issues. Instead, refactor to pass `enable_aux` as a parameter
through the forward pass. First, update the `DualDPT.forward()` method signature
to accept `enable_aux` as a parameter, then replace the direct assignment
`self.head.enable_aux = bool(use_ray_pose)` with passing `bool(use_ray_pose)` as
an argument when invoking the forward method on the DualDPT head. This ensures
that the auxiliary head behavior is determined per-invocation rather than stored
as mutable instance state.

🤖 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/database/queries/asset_reference.py`:
- Around line 311-324: The keyset pagination logic in the if block checking
after_cursor_value and after_cursor_id does not handle NULL values in the sort
column, which causes cursor pagination to skip NULL values since SQL comparisons
with NULL never match. Modify the keyset predicate construction in both the
descending and non-descending branches to explicitly handle NULL boundaries:
when building the sa.or_ conditions, add logic to detect if the sort column
(sort_col) is NULL and compare appropriately, or use NULL-safe comparison
operators/expressions that can correctly navigate through rows where the sort
column is NULL. Alternatively, document and enforce that the size field cannot
be used as a cursor-sortable column if NULL values are not handled.

In `@app/assets/services/ingest.py`:
- Around line 370-373: The conditional check before calling
extract_image_dimensions() returns early for both None and non-image MIME types,
preventing the function from attempting header-based detection on files with
unknown MIME types. Modify the condition to only return early when mime_type is
explicitly present and does not start with "image/", allowing None values to
pass through to extract_image_dimensions() so it can use Pillow's header
detection as intended.

In `@comfy_extras/nodes_custom_sampler.py`:
- Around line 937-939: The early return condition in the uncond model check is
bypassing the cfg_function path when self.cfg is driven to 1.0 on this step,
which prevents CFG hooks and wrappers from being applied. Modify the conditional
check that determines when to return early with just cond so that it only
returns when self.uncond_inner is None (unconditional model not loaded), but
removes the math.isclose(self.cfg, 1.0) condition from the OR clause. This
allows the CFG processing to continue even when cfg equals 1.0 on this
particular sigma step, ensuring that any cfg_function hooks and custom CFG
behavior are still executed.

In `@comfy_extras/nodes_depth_anything_3.py`:
- Around line 600-633: The mask at the end of the execute method only checks if
depth values are finite using torch.isfinite, but does not exclude zero or
negative depth values that were explicitly set to 0.0 earlier when depth was not
finite. This allows spurious points to be created near the camera origin. Modify
the mask assignment line to also check that depth values are positive (greater
than 0) in addition to checking finiteness, so the final mask only includes
valid positive finite depth values.

In `@comfy_extras/nodes_load_3d.py`:
- Around line 176-177: The filename generation using the f-string with
model_3d.format can result in trailing-dot filenames when the format field is
empty or unset for stream-backed Types.File3D objects. Fix this issue at all
three locations (lines 176, 244, and 303) by adding a guard condition that
checks if model_3d.format is empty or None before appending the format extension
to the filename. Conditionally include the dot and format extension only when
model_3d.format has a valid value, otherwise generate the filename without the
extension suffix. This prevents invalid filenames that fail on certain
platforms.

In `@comfy_extras/nodes_resolution.py`:
- Around line 67-70: The tooltip text for the "width" and "height" io.Int.Output
definitions incorrectly describes the behavior as "multiplied by the selected
multiple" when the actual behavior is that dimensions are rounded to the nearest
selected multiple. Update the tooltip strings for both the "width" and "height"
outputs to accurately reflect that the dimensions are rounded to the nearest
selected multiple rather than multiplied by it.

In `@comfy/model_patcher.py`:
- Around line 384-386: The condition logic around line 384-386 in the
model_patcher.py file has a flaw where force_deepcopy does not properly enforce
a non-dynamic clone class. When force_deepcopy is True and disable_dynamic is
False, the outer condition is satisfied but the inner condition checking for
both is_dynamic() and disable_dynamic fails, leaving class_ as self.__class__
instead of ModelPatcher. To fix this, add an additional condition in the
assignment block to also set class_ = ModelPatcher when force_deepcopy is True,
ensuring that force_deepcopy actually forces a non-dynamic clone regardless of
the disable_dynamic parameter value.

In `@tests-unit/assets_test/services/test_cursor.py`:
- Around line 260-262: The test_invalid_order_token_rejected_on_encode method is
catching the wrong exception type. The encode_cursor function raises
InvalidCursorError when given an unsupported order token like "sideways", not
ValueError. Replace the ValueError in the pytest.raises call with
InvalidCursorError to match the actual exception that encode_cursor throws.

---

Outside diff comments:
In `@app/assets/database/queries/tags.py`:
- Around line 259-267: The return type annotation for the `list_tags_with_usage`
function incorrectly specifies a 3-item tuple (str, str, int) when the function
now only returns 2-item tuples (name, count) after removing the tag_type field.
Update the return annotation from tuple[list[tuple[str, str, int]], int] to
tuple[list[tuple[str, int]], int] to accurately reflect the new tuple shape
being returned by the function.

---

Nitpick comments:
In `@comfy_extras/nodes_audio.py`:
- Around line 285-292: Add an explicit guard at the beginning of the
SaveAudioAdvanced.execute method to check if the audio parameter is None, and
raise a ValueError with a clear error message if it is. This validation should
occur before extracting the format and quality values, ensuring that missing
audio input is caught early with an actionable error message consistent with
legacy save node behavior.

In `@comfy_extras/nodes_depth_anything_3.py`:
- Around line 154-165: In the execute classmethod, immediately after loading the
model with comfy.sd.load_diffusion_model, add a type guard to validate that the
loaded model is actually a Depth Anything 3 model. Since model_name is sourced
from the broad geometry_estimation folder list, a non-DA3 checkpoint could be
selected, causing unclear attribute errors during inference. Add a check to
verify the model type and raise a descriptive error if the model is not a DA3
model, ensuring fast failure with a clear message.

In `@comfy/ldm/depth_anything_3/camera.py`:
- Around line 32-40: In the _LayerScale class __init__ method, the gamma
parameter is being initialized with torch.empty which leaves it with arbitrary
values. Add an init_values parameter to the __init__ method with a sensible
default value (typically 1.0), and use it to properly initialize self.gamma
instead of using torch.empty. This ensures gamma has meaningful initial values
for robustness when debugging, testing without weights, or if a checkpoint is
missing this key.

In `@comfy/ldm/depth_anything_3/model.py`:
- Around line 170-171: The current implementation sets `self.head.enable_aux` as
instance state which creates thread-safety issues. Instead, refactor to pass
`enable_aux` as a parameter through the forward pass. First, update the
`DualDPT.forward()` method signature to accept `enable_aux` as a parameter, then
replace the direct assignment `self.head.enable_aux = bool(use_ray_pose)` with
passing `bool(use_ray_pose)` as an argument when invoking the forward method on
the DualDPT head. This ensures that the auxiliary head behavior is determined
per-invocation rather than stored as mutable instance state.

In `@comfy/ldm/depth_anything_3/ray_pose.py`:
- Around line 248-252: The translation calculation using confidence-weighted
averaging can encounter division by zero when all confidence values are zero,
resulting in NaN values. In the section where variable T is computed by dividing
by cf.sum(dim=-1, keepdim=True), add a guard that clamps or prevents division by
zero. This can be done by either adding a small epsilon value to the denominator
before dividing, or by using a clamp operation on the denominator to ensure it
never reaches exactly zero, so the division operation remains numerically stable
and does not produce NaN translations.

🪄 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 Plus

Run ID: ea12feb2-1c09-47b0-9938-1ac13c693dda

📥 Commits

Reviewing files that changed from the base of the PR and between b0f9e32 and 7073a93.

⛔ Files ignored due to path filters (3)

• .ci/windows_amd_base_files/README_VERY_IMPORTANT.txt is excluded by !.ci/**
• comfy_api_nodes/apis/bfl.py is excluded by !comfy_api_nodes/apis/**
• comfy_api_nodes/apis/gemini.py is excluded by !comfy_api_nodes/apis/**

📒 Files selected for processing (81)

• .github/workflows/check-line-endings.yml
• README.md
• alembic_db/versions/0004_drop_tag_type.py
• app/assets/api/routes.py
• app/assets/api/schemas_in.py
• app/assets/api/schemas_out.py
• app/assets/database/models.py
• app/assets/database/queries/asset_reference.py
• app/assets/database/queries/tags.py
• app/assets/scanner.py
• app/assets/services/asset_management.py
• app/assets/services/cursor.py
• app/assets/services/image_dimensions.py
• app/assets/services/ingest.py
• app/assets/services/schemas.py
• app/assets/services/tagging.py
• comfy/cli_args.py
• comfy/image_encoders/dino2.py
• comfy/ldm/colormap.py
• comfy/ldm/depth_anything_3/camera.py
• comfy/ldm/depth_anything_3/dpt.py
• comfy/ldm/depth_anything_3/model.py
• comfy/ldm/depth_anything_3/preprocess.py
• comfy/ldm/depth_anything_3/ray_pose.py
• comfy/ldm/depth_anything_3/reference_view_selector.py
• comfy/ldm/depth_anything_3/transform.py
• comfy/ldm/ideogram4/model.py
• comfy/ldm/qwen_image/model.py
• comfy/ldm/wan/model.py
• comfy/lora.py
• comfy/model_base.py
• comfy/model_detection.py
• comfy/model_management.py
• comfy/model_patcher.py
• comfy/ops.py
• comfy/pinned_memory.py
• comfy/supported_models.py
• comfy/text_encoders/ideogram4.py
• comfy_api/latest/_io.py
• comfy_api/latest/_ui.py
• comfy_api_nodes/nodes_bfl.py
• comfy_api_nodes/nodes_gemini.py
• comfy_api_nodes/nodes_krea.py
• comfy_execution/asset_enrichment.py
• comfy_execution/jobs.py
• comfy_extras/nodes_audio.py
• comfy_extras/nodes_bernini.py
• comfy_extras/nodes_bg_removal.py
• comfy_extras/nodes_color.py
• comfy_extras/nodes_custom_sampler.py
• comfy_extras/nodes_depth_anything_3.py
• comfy_extras/nodes_gaussian_splat.py
• comfy_extras/nodes_load_3d.py
• comfy_extras/nodes_moge.py
• comfy_extras/nodes_resolution.py
• comfy_extras/nodes_save_3d.py
• comfy_extras/nodes_scail.py
• comfy_extras/nodes_train.py
• comfy_extras/nodes_wan.py
• cuda_malloc.py
• execution.py
• main.py
• nodes.py
• openapi.yaml
• requirements.txt
• server.py
• tests-unit/assets_test/conftest.py
• tests-unit/assets_test/queries/test_asset_reference_keyset.py
• tests-unit/assets_test/queries/test_tags.py
• tests-unit/assets_test/services/test_cursor.py
• tests-unit/assets_test/services/test_image_dimensions.py
• tests-unit/assets_test/services/test_ingest.py
• tests-unit/assets_test/services/test_tagging.py
• tests-unit/assets_test/test_crud.py
• tests-unit/assets_test/test_downloads.py
• tests-unit/assets_test/test_list_cursor.py
• tests-unit/assets_test/test_prompt_id_enforcement.py
• tests-unit/assets_test/test_sync_references.py
• tests-unit/assets_test/test_tags_api.py
• tests-unit/execution_test/test_enrich_output.py
• tests/execution/test_jobs.py

💤 Files with no reviewable changes (3)

• README.md
• app/assets/database/models.py
• comfy_extras/nodes_wan.py

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make the keyset predicate NULL-aware for size.

size is exposed as cursor-supported, but this predicate cannot advance into Asset.size_bytes IS NULL rows because SQL comparisons with NULL never match. Size-sorted cursor pagination can therefore skip or prematurely hide null-sized assets; either remove size from cursor mode or encode/compare NULL boundaries explicitly.

🤖 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 `@app/assets/database/queries/asset_reference.py` around lines 311 - 324, The
keyset pagination logic in the if block checking after_cursor_value and
after_cursor_id does not handle NULL values in the sort column, which causes
cursor pagination to skip NULL values since SQL comparisons with NULL never
match. Modify the keyset predicate construction in both the descending and
non-descending branches to explicitly handle NULL boundaries: when building the
sa.or_ conditions, add logic to detect if the sort column (sort_col) is NULL and
compare appropriately, or use NULL-safe comparison operators/expressions that
can correctly navigate through rows where the sort column is NULL.
Alternatively, document and enforce that the size field cannot be used as a
cursor-sortable column if NULL values are not handled.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Allow MIME-unknown files to use header detection.

extract_image_dimensions() already treats mime_type=None as “try Pillow”, but this wrapper returns early before calling it. Images with unknown/extensionless MIME types will never get width/height metadata.

Proposed fix

-    if not mime_type or not mime_type.startswith("image/"):
+    if mime_type is not None and not mime_type.startswith("image/"):
         return
 
     dims = extract_image_dimensions(file_path, mime_type=mime_type)

📝 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

	if not mime_type or not mime_type.startswith("image/"):
	return
	dims = extract_image_dimensions(file_path, mime_type=mime_type)
	if mime_type is not None and not mime_type.startswith("image/"):
	return
	dims = extract_image_dimensions(file_path, mime_type=mime_type)

🤖 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 `@app/assets/services/ingest.py` around lines 370 - 373, The conditional check
before calling extract_image_dimensions() returns early for both None and
non-image MIME types, preventing the function from attempting header-based
detection on files with unknown MIME types. Modify the condition to only return
early when mime_type is explicitly present and does not start with "image/",
allowing None values to pass through to extract_image_dimensions() so it can use
Pillow's header detection as intended.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep CFG hooks on the step-local cfg == 1 fast path.

This new early return skips the cfg_function path when CFGOverride drives self.cfg to 1.0 for only this sigma. Standard sampling still short-circuits uncond at CFG=1 while continuing through CFG processing, so wrappers/custom CFG behavior can be bypassed here.

🐛 Proposed adjustment

-        # uncond model not loaded (base cfg==1/no negative), or cfg driven to 1.0 this step -> single model, cond only
-        if self.uncond_inner is None or (math.isclose(self.cfg, 1.0) and not model_options.get("disable_cfg1_optimization", False)):
+        # uncond model not loaded (base cfg==1/no negative) -> single model, cond only
+        if self.uncond_inner is None:
             return cond
+        if math.isclose(self.cfg, 1.0) and not model_options.get("disable_cfg1_optimization", False):
+            return comfy.samplers.cfg_function(
+                self.inner_model, cond, cond, self.cfg, x, timestep,
+                model_options=model_options, cond=positive, uncond=None,
+            )

🤖 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 `@comfy_extras/nodes_custom_sampler.py` around lines 937 - 939, The early
return condition in the uncond model check is bypassing the cfg_function path
when self.cfg is driven to 1.0 on this step, which prevents CFG hooks and
wrappers from being applied. Modify the conditional check that determines when
to return early with just cond so that it only returns when self.uncond_inner is
None (unconditional model not loaded), but removes the math.isclose(self.cfg,
1.0) condition from the OR clause. This allows the CFG processing to continue
even when cfg equals 1.0 on this particular sigma step, ensuring that any
cfg_function hooks and custom CFG behavior are still executed.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Exclude zero/negative depth values from point-cloud mask.

At Line 632, the mask only checks finiteness. Depth values zeroed at Line 607 still pass and create spurious points at/near the camera origin.

💡 Proposed fix

-        mask = mask & torch.isfinite(depth)
+        mask = mask & torch.isfinite(depth) & (depth > 0)

📝 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

	def execute(cls, da3_geometry, batch_index, confidence_threshold, use_sky_mask, downsample) -> io.NodeOutput:
	depth_all = da3_geometry["depth"] # (B, H, W)
	B = depth_all.shape[0]
	if batch_index >= B:
	raise ValueError(f"batch_index {batch_index} is out of range; DA3_GEOMETRY has batch size {B}.")
	depth = depth_all[batch_index].clone() # (H, W)
	depth[~torch.isfinite(depth)] = 0.0
	H, W = depth.shape
	K = _da3_get_K(da3_geometry, batch_index, H, W)
	if downsample > 1:
	depth = depth[::downsample, ::downsample].contiguous()
	# Scale intrinsics to the downsampled grid.
	K = K.clone()
	K[0, :] /= downsample
	K[1, :] /= downsample
	H_ds, W_ds = depth.shape
	points = _da3_unproject(depth, K) # (H_ds, W_ds, 3) in OpenCV camera space
	# Apply world-to-camera inverse so multi-view frames share a common world frame.
	E = _da3_get_extrinsic(da3_geometry, batch_index)
	if E is not None:
	points = _da3_apply_extrinsic(points, E)
	# Rebuild mask at downsampled resolution.
	mask = _da3_build_mask(da3_geometry, batch_index, H, W, confidence_threshold, use_sky_mask)
	if downsample > 1:
	mask = mask[::downsample, ::downsample]
	mask = mask & torch.isfinite(depth)
	def execute(cls, da3_geometry, batch_index, confidence_threshold, use_sky_mask, downsample) -> io.NodeOutput:
	depth_all = da3_geometry["depth"] # (B, H, W)
	B = depth_all.shape[0]
	if batch_index >= B:
	raise ValueError(f"batch_index {batch_index} is out of range; DA3_GEOMETRY has batch size {B}.")
	depth = depth_all[batch_index].clone() # (H, W)
	depth[~torch.isfinite(depth)] = 0.0
	H, W = depth.shape
	K = _da3_get_K(da3_geometry, batch_index, H, W)
	if downsample > 1:
	depth = depth[::downsample, ::downsample].contiguous()
	# Scale intrinsics to the downsampled grid.
	K = K.clone()
	K[0, :] /= downsample
	K[1, :] /= downsample
	H_ds, W_ds = depth.shape
	points = _da3_unproject(depth, K) # (H_ds, W_ds, 3) in OpenCV camera space
	# Apply world-to-camera inverse so multi-view frames share a common world frame.
	E = _da3_get_extrinsic(da3_geometry, batch_index)
	if E is not None:
	points = _da3_apply_extrinsic(points, E)
	# Rebuild mask at downsampled resolution.
	mask = _da3_build_mask(da3_geometry, batch_index, H, W, confidence_threshold, use_sky_mask)
	if downsample > 1:
	mask = mask[::downsample, ::downsample]
	mask = mask & torch.isfinite(depth) & (depth > 0)

🤖 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 `@comfy_extras/nodes_depth_anything_3.py` around lines 600 - 633, The mask at
the end of the execute method only checks if depth values are finite using
torch.isfinite, but does not exclude zero or negative depth values that were
explicitly set to 0.0 earlier when depth was not finite. This allows spurious
points to be created near the camera origin. Modify the mask assignment line to
also check that depth values are positive (greater than 0) in addition to
checking finiteness, so the final mask only includes valid positive finite depth
values.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard against empty model_3d.format when generating preview temp filenames.

Line 176, Line 244, and Line 303 always append .{model_3d.format}. For stream-backed Types.File3D objects with unset format, this can produce trailing-dot filenames and fail temp-file writes/loads on some platforms.

🛠️ Proposed fix

+def _preview_filename(prefix: str, model_3d: Types.File3D, default_ext: str) -> str:
+    ext = (model_3d.format or default_ext).lstrip(".").lower()
+    return f"{prefix}_{uuid.uuid4().hex}.{ext}"
+
 class Preview3DAdvanced(IO.ComfyNode):
@@
-        filename = f"preview3d_advanced_{uuid.uuid4().hex}.{model_3d.format}"
+        filename = _preview_filename("preview3d_advanced", model_3d, "glb")
@@
 class PreviewGaussianSplat(IO.ComfyNode):
@@
-        filename = f"preview_splat_{uuid.uuid4().hex}.{model_3d.format}"
+        filename = _preview_filename("preview_splat", model_3d, "ply")
@@
 class PreviewPointCloud(IO.ComfyNode):
@@
-        filename = f"preview_pointcloud_{uuid.uuid4().hex}.{model_3d.format}"
+        filename = _preview_filename("preview_pointcloud", model_3d, "ply")

Also applies to: 244-245, 303-304

🤖 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 `@comfy_extras/nodes_load_3d.py` around lines 176 - 177, The filename
generation using the f-string with model_3d.format can result in trailing-dot
filenames when the format field is empty or unset for stream-backed Types.File3D
objects. Fix this issue at all three locations (lines 176, 244, and 303) by
adding a guard condition that checks if model_3d.format is empty or None before
appending the format extension to the filename. Conditionally include the dot
and format extension only when model_3d.format has a valid value, otherwise
generate the filename without the extension suffix. This prevents invalid
filenames that fail on certain platforms.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix tooltip wording to match the rounding behavior.

The node rounds dimensions to the nearest selected multiple; it does not multiply final dimensions by that multiple.

Proposed patch

-                io.Int.Output(
-                    "width", tooltip="Calculated width in pixels multiplied by the selected multiple."
-                ),
+                io.Int.Output(
+                    "width", tooltip="Calculated width in pixels, rounded to the selected multiple."
+                ),
                 io.Int.Output(
-                    "height", tooltip="Calculated height in pixels multiplied by the selected multiple."
+                    "height", tooltip="Calculated height in pixels, rounded to the selected multiple."
                 ),

📝 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

	"width", tooltip="Calculated width in pixels multiplied by the selected multiple."
	),
	io.Int.Output(
	"height", tooltip="Calculated height in pixels (multiple of 8)."
	"height", tooltip="Calculated height in pixels multiplied by the selected multiple."
	"width", tooltip="Calculated width in pixels, rounded to the selected multiple."
	),
	io.Int.Output(
	"height", tooltip="Calculated height in pixels, rounded to the selected multiple."

🤖 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 `@comfy_extras/nodes_resolution.py` around lines 67 - 70, The tooltip text for
the "width" and "height" io.Int.Output definitions incorrectly describes the
behavior as "multiplied by the selected multiple" when the actual behavior is
that dimensions are rounded to the nearest selected multiple. Update the tooltip
strings for both the "width" and "height" outputs to accurately reflect that the
dimensions are rounded to the nearest selected multiple rather than multiplied
by it.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

force_deepcopy does not force a non-dynamic clone class for dynamic patchers.

On Line 384-Line 386, when force_deepcopy=True and disable_dynamic=False, the code enters the branch but leaves class_ as self.__class__. That contradicts the new force-deepcopy intent and can keep dynamic behavior in paths that expect a non-dynamic clone.

Proposed fix

-        if self.is_dynamic() and disable_dynamic or force_deepcopy:
-            if self.is_dynamic() and disable_dynamic:
+        if (self.is_dynamic() and disable_dynamic) or force_deepcopy:
+            if self.is_dynamic() and (disable_dynamic or force_deepcopy):
                 class_ = ModelPatcher

📝 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

	if self.is_dynamic() and disable_dynamic or force_deepcopy:
	if self.is_dynamic() and disable_dynamic:
	class_ = ModelPatcher
	if (self.is_dynamic() and disable_dynamic) or force_deepcopy:
	if self.is_dynamic() and (disable_dynamic or force_deepcopy):
	class_ = ModelPatcher

🤖 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 `@comfy/model_patcher.py` around lines 384 - 386, The condition logic around
line 384-386 in the model_patcher.py file has a flaw where force_deepcopy does
not properly enforce a non-dynamic clone class. When force_deepcopy is True and
disable_dynamic is False, the outer condition is satisfied but the inner
condition checking for both is_dynamic() and disable_dynamic fails, leaving
class_ as self.__class__ instead of ModelPatcher. To fix this, add an additional
condition in the assignment block to also set class_ = ModelPatcher when
force_deepcopy is True, ensuring that force_deepcopy actually forces a
non-dynamic clone regardless of the disable_dynamic parameter value.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Incorrect exception type expected for invalid order on encode.

At Line 261, pytest.raises(ValueError) is inconsistent with encode_cursor, which raises InvalidCursorError for unsupported order tokens. This makes the test fail on the intended path.

Suggested fix

     def test_invalid_order_token_rejected_on_encode(self):
-        with pytest.raises(ValueError):
+        with pytest.raises(InvalidCursorError, match="order must be one of"):
             encode_cursor("created_at", "1", "id-1", order="sideways")

📝 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

	def test_invalid_order_token_rejected_on_encode(self):
	with pytest.raises(ValueError):
	encode_cursor("created_at", "1", "id-1", order="sideways")
	def test_invalid_order_token_rejected_on_encode(self):
	with pytest.raises(InvalidCursorError, match="order must be one of"):
	encode_cursor("created_at", "1", "id-1", order="sideways")

🤖 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 `@tests-unit/assets_test/services/test_cursor.py` around lines 260 - 262, The
test_invalid_order_token_rejected_on_encode method is catching the wrong
exception type. The encode_cursor function raises InvalidCursorError when given
an unsupported order token like "sideways", not ValueError. Replace the
ValueError in the pytest.raises call with InvalidCursorError to match the actual
exception that encode_cursor throws.