Make convert_old_quants prefix-aware for _quantization_metadata layer keys - #15085
Make convert_old_quants prefix-aware for _quantization_metadata layer keys#15085Alan5168 wants to merge 3 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates quantization conversion to resolve metadata layer keys against state-dict prefix variants before writing 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 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 `@comfy/utils.py`:
- Around line 1404-1412: Trim the comment above the quantization metadata
matching helper to briefly state only that layer keys may use either the full
diffusion-model prefix or the already-stripped form. Remove issue references,
call-history details, and extended rationale while preserving the implementation
unchanged.
- Around line 1482-1483: Update the marker-write guard in convert_old_quants so
an existing marker_key is skipped only when its stored tensor bytes match the
newly encoded JSON metadata. Overwrite the marker when the key exists but
contents differ, while preserving idempotent behavior for matching markers.
In `@tests-unit/comfy_quant/test_convert_old_quants_prefix.py`:
- Around line 220-274: Remove the out-of-scope TestKnownResidualGap class and
its expectedFailure regression test from this PR; track the pre-existing issue
separately instead of retaining the long narrative docstring. If the test must
remain, replace unittest.expectedFailure with unittest.skip using the associated
issue number so future fixes do not create an unexpected-success CI failure.
- Around line 190-193: Update the test around the two convert_old_quants calls
to snapshot the first result’s keys and proj_in tensor before invoking
convert_old_quants again, then compare the snapshot against the second result.
Ensure assertions validate preserved values independently rather than comparing
out_sd2 with the potentially mutated out_sd1 object.
🪄 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: 30dd96ec-5536-406b-b035-95c2ac49a100
📒 Files selected for processing (2)
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
comfy/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
comfy/**/*.py: Treat dtype, device placement, VRAM use, offloading, and memory behavior as correctness concerns; use existing ComfyUI quantization, memory, offload, and optimized-operation helpers.
Prefer shared optimized kernels, backend dispatchers, and documented interfaces over duplicate handwritten operations; treat selected backend callables as opaque.
Do not duplicate operations with custom float32-upcasting inference kernels; use generic ComfyUI operations or native PyTorch operations.
If a model constructor has anoperationsparameter, assume it is neverNone; do not add fallback torch operations.
Avoid unnecessary parameters in model, block, and operation constructors or forwards; reuse existing model classes, blocks, operations, and helpers.
Model detectors must inspect only the first dimension of linear weights, guard every dereferenced state-dict key, and order specific signatures before broad fallbacks.
Avoideinopsin core inference code; use native tensor operations such asreshape,view,permute,transpose,flatten, and related methods.
Keep metadata, counters, shape calculations, indices, split boundaries, and control-flow values as Python values rather than tensors.
Avoid unnecessary casts and transfers; preserve intended compute and storage dtypes, tensor shapes, and backend result contracts.
Keep model-native latent layout handling inside the model or latent-format owner rather than reshaping it in nodes or caller-side adapters.
DiT models must pad every patchified target or reference input withcomfy.ldm.common_dit.pad_to_patch_sizeand crop only the target output back to its original dimensions.
Do not add defensive shape, configuration, or dtype casts that merely obscure a clear tensor-operation failure; validate only at meaningful boundaries.
Raw parameters not owned by an operation should be cast at use withcomfy.ops.cast_to_inputorcomfy.model_management.cast_to; model constructors should not contain dtype workaro...
Files:
comfy/utils.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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
comfy/**
⚙️ CodeRabbit configuration file
comfy/**: Core ML/diffusion engine. Focus on:
- Backward compatibility (breaking changes affect all custom nodes)
- Memory management and GPU resource handling
- Performance implications in hot paths
- Thread safety for concurrent execution
Files:
comfy/utils.py
🧠 Learnings (2)
📚 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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
📚 Learning: 2026-05-13T12:31:45.069Z
Learnt from: rattus128
Repo: Comfy-Org/ComfyUI PR: 13802
File: comfy/pinned_memory.py:19-30
Timestamp: 2026-05-13T12:31:45.069Z
Learning: When reviewing code that uses comfy/pinned_memory.py’s `HostBuffer.extend(size=..., reallocate=...)`: by default (`reallocate` is not True / False), `extend(size=...)` is a *relative increment* that grows the buffer by `size` bytes—so slicing like `[offset:offset+size]` after `hostbuf.extend(size=size)` is correct and the argument should not be rewritten to `offset + size`. Only in the single-segment reallocation mode (`reallocate=True`, e.g., as used by `resize_pin_buffer()` in `comfy/model_management.py`) should `size` be treated as an *absolute target* and the call/arguments should be checked accordingly.
Applied to files:
comfy/utils.py
🪛 ast-grep (0.44.1)
comfy/utils.py
[info] 1482-1482: use jsonify instead of json.dumps for JSON output
Context: json.dumps(v)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
tests-unit/comfy_quant/test_convert_old_quants_prefix.py
[info] 57-57: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"layers": layer_quant_config})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 74-74: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"layers": layer_quant_config})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 94-94: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"layers": layer_quant_config})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 113-113: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"layers": layer_quant_config})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 156-158: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{"layers": {layer_key: {"format": "float8_e4m3fn"}}}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 184-184: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"layers": layer_quant_config})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 209-209: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"layers": layer_quant_config})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 262-262: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"layers": layers_meta})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🔇 Additional comments (4)
tests-unit/comfy_quant/test_convert_old_quants_prefix.py (4)
48-142: LGTM!
145-180: LGTM!
196-217: LGTM!
8-15: 📐 Maintainability & Code QualityNo change needed for the CPU bootstrap.
tests-unit/comfy_quant/test_mixed_precision.pyalready uses the sameargs.cpu = Truebootstrap in this file, so this follows the existing pattern.
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)
tests-unit/comfy_quant/test_convert_old_quants_prefix.py (1)
143-180: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAssert that the second pass does not leave a duplicate marker.
In the prefixed-metadata case, the first marker is transformed to
proj_in.comfy_quantduring prefix stripping, but the second call withmodel_prefix=""can addmodel.diffusion_model.proj_in.comfy_quantagain. The current assertions only verify the desired key, so this regression remains green with an extra marker.🧪 Add the missing assertion
sd, metadata = comfy.utils.convert_old_quants(sd, "", metadata=metadata) self.assertIn("proj_in.comfy_quant", sd, f"{convention} metadata convention did not resolve after the two-call dance") + self.assertNotIn("model.diffusion_model.proj_in.comfy_quant", sd) self.assertEqual(marker_json(sd, "proj_in.comfy_quant")["format"], "float8_e4m3fn")🤖 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/comfy_quant/test_convert_old_quants_prefix.py` around lines 143 - 180, Extend test_two_call_pattern_mirrors_load_diffusion_model_state_dict to assert that the second convert_old_quants pass does not leave the duplicate marker key model.diffusion_model.proj_in.comfy_quant. Keep the existing assertion for proj_in.comfy_quant and validate the duplicate is absent for both metadata conventions.
🤖 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 `@tests-unit/comfy_quant/test_convert_old_quants_prefix.py`:
- Around line 143-180: Extend
test_two_call_pattern_mirrors_load_diffusion_model_state_dict to assert that the
second convert_old_quants pass does not leave the duplicate marker key
model.diffusion_model.proj_in.comfy_quant. Keep the existing assertion for
proj_in.comfy_quant and validate the duplicate is absent for both metadata
conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6008c5f6-4c3c-40fb-a2f3-f18f00ce0ce8
📒 Files selected for processing (2)
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
comfy/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
comfy/**/*.py: Treat dtype, device placement, VRAM use, offloading, and memory behavior as correctness concerns; use existing ComfyUI quantization, memory, offload, and optimized-operation helpers.
Prefer shared optimized kernels, backend dispatchers, and documented interfaces over duplicate handwritten operations; treat selected backend callables as opaque.
Do not duplicate operations with custom float32-upcasting inference kernels; use generic ComfyUI operations or native PyTorch operations.
If a model constructor has anoperationsparameter, assume it is neverNone; do not add fallback torch operations.
Avoid unnecessary parameters in model, block, and operation constructors or forwards; reuse existing model classes, blocks, operations, and helpers.
Model detectors must inspect only the first dimension of linear weights, guard every dereferenced state-dict key, and order specific signatures before broad fallbacks.
Avoideinopsin core inference code; use native tensor operations such asreshape,view,permute,transpose,flatten, and related methods.
Keep metadata, counters, shape calculations, indices, split boundaries, and control-flow values as Python values rather than tensors.
Avoid unnecessary casts and transfers; preserve intended compute and storage dtypes, tensor shapes, and backend result contracts.
Keep model-native latent layout handling inside the model or latent-format owner rather than reshaping it in nodes or caller-side adapters.
DiT models must pad every patchified target or reference input withcomfy.ldm.common_dit.pad_to_patch_sizeand crop only the target output back to its original dimensions.
Do not add defensive shape, configuration, or dtype casts that merely obscure a clear tensor-operation failure; validate only at meaningful boundaries.
Raw parameters not owned by an operation should be cast at use withcomfy.ops.cast_to_inputorcomfy.model_management.cast_to; model constructors should not contain dtype workaro...
Files:
comfy/utils.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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
comfy/**
⚙️ CodeRabbit configuration file
comfy/**: Core ML/diffusion engine. Focus on:
- Backward compatibility (breaking changes affect all custom nodes)
- Memory management and GPU resource handling
- Performance implications in hot paths
- Thread safety for concurrent execution
Files:
comfy/utils.py
🧠 Learnings (2)
📚 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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
📚 Learning: 2026-05-13T12:31:45.069Z
Learnt from: rattus128
Repo: Comfy-Org/ComfyUI PR: 13802
File: comfy/pinned_memory.py:19-30
Timestamp: 2026-05-13T12:31:45.069Z
Learning: When reviewing code that uses comfy/pinned_memory.py’s `HostBuffer.extend(size=..., reallocate=...)`: by default (`reallocate` is not True / False), `extend(size=...)` is a *relative increment* that grows the buffer by `size` bytes—so slicing like `[offset:offset+size]` after `hostbuf.extend(size=size)` is correct and the argument should not be rewritten to `offset + size`. Only in the single-segment reallocation mode (`reallocate=True`, e.g., as used by `resize_pin_buffer()` in `comfy/model_management.py`) should `size` be treated as an *absolute target* and the call/arguments should be checked accordingly.
Applied to files:
comfy/utils.py
🪛 ast-grep (0.44.1)
tests-unit/comfy_quant/test_convert_old_quants_prefix.py
[info] 199-199: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"format": "nvfp4"})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 208-208: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"layers": layer_quant_config})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🔇 Additional comments (2)
comfy/utils.py (1)
1403-1415: LGTM!Also applies to: 1473-1477
tests-unit/comfy_quant/test_convert_old_quants_prefix.py (1)
1-142: LGTM!Also applies to: 181-197, 198-213, 214-237, 239-240
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@comfy/utils.py`:
- Around line 1415-1419: Update the layer-key fallback loop to collect or
resolve candidates based on existing “.weight” entries independently of
“.comfy_quant” markers. Prefer a candidate with an existing marker when
available, but accept a unique weight-only candidate so first conversion uses
the stripped key; preserve the current marker-based behavior when multiple
weight matches remain ambiguous.
🪄 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: fc410a9d-ea2f-4828-881f-9d1d05a5e382
📒 Files selected for processing (2)
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
comfy/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
comfy/**/*.py: Treat dtype, device placement, VRAM use, offloading, and memory behavior as correctness concerns; use existing ComfyUI quantization, memory, offload, and optimized-operation helpers.
Prefer shared optimized kernels, backend dispatchers, and documented interfaces over duplicate handwritten operations; treat selected backend callables as opaque.
Do not duplicate operations with custom float32-upcasting inference kernels; use generic ComfyUI operations or native PyTorch operations.
If a model constructor has anoperationsparameter, assume it is neverNone; do not add fallback torch operations.
Avoid unnecessary parameters in model, block, and operation constructors or forwards; reuse existing model classes, blocks, operations, and helpers.
Model detectors must inspect only the first dimension of linear weights, guard every dereferenced state-dict key, and order specific signatures before broad fallbacks.
Avoideinopsin core inference code; use native tensor operations such asreshape,view,permute,transpose,flatten, and related methods.
Keep metadata, counters, shape calculations, indices, split boundaries, and control-flow values as Python values rather than tensors.
Avoid unnecessary casts and transfers; preserve intended compute and storage dtypes, tensor shapes, and backend result contracts.
Keep model-native latent layout handling inside the model or latent-format owner rather than reshaping it in nodes or caller-side adapters.
DiT models must pad every patchified target or reference input withcomfy.ldm.common_dit.pad_to_patch_sizeand crop only the target output back to its original dimensions.
Do not add defensive shape, configuration, or dtype casts that merely obscure a clear tensor-operation failure; validate only at meaningful boundaries.
Raw parameters not owned by an operation should be cast at use withcomfy.ops.cast_to_inputorcomfy.model_management.cast_to; model constructors should not contain dtype workaro...
Files:
comfy/utils.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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
comfy/**
⚙️ CodeRabbit configuration file
comfy/**: Core ML/diffusion engine. Focus on:
- Backward compatibility (breaking changes affect all custom nodes)
- Memory management and GPU resource handling
- Performance implications in hot paths
- Thread safety for concurrent execution
Files:
comfy/utils.py
🧠 Learnings (2)
📚 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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
📚 Learning: 2026-05-13T12:31:45.069Z
Learnt from: rattus128
Repo: Comfy-Org/ComfyUI PR: 13802
File: comfy/pinned_memory.py:19-30
Timestamp: 2026-05-13T12:31:45.069Z
Learning: When reviewing code that uses comfy/pinned_memory.py’s `HostBuffer.extend(size=..., reallocate=...)`: by default (`reallocate` is not True / False), `extend(size=...)` is a *relative increment* that grows the buffer by `size` bytes—so slicing like `[offset:offset+size]` after `hostbuf.extend(size=size)` is correct and the argument should not be rewritten to `offset + size`. Only in the single-segment reallocation mode (`reallocate=True`, e.g., as used by `resize_pin_buffer()` in `comfy/model_management.py`) should `size` be treated as an *absolute target* and the call/arguments should be checked accordingly.
Applied to files:
comfy/utils.py
🔇 Additional comments (2)
comfy/utils.py (1)
1403-1414: LGTM!Also applies to: 1475-1482
tests-unit/comfy_quant/test_convert_old_quants_prefix.py (1)
1-20: LGTM!Also applies to: 22-25, 27-36, 38-142, 143-180, 183-198, 199-214, 215-238, 240-241
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests-unit/comfy_quant/test_convert_old_quants_prefix.py`:
- Around line 241-455: Optionally reduce repeated metadata construction in
TestResidualPrefixPoisoning by adding a small make_metadata(layers) helper that
returns the existing _quantization_metadata JSON structure, then use it across
the affected tests without changing test behavior.
🪄 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: a7c3f6b8-1b6e-4824-bc32-029a1bbde845
📒 Files selected for processing (2)
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep changes and file scope as small and direct as possible; prefer practical fixes, minimal dependencies, existing patterns, and removal of obsolete code.
Preserve existing APIs, node names, model-loading behavior, file layout, and workflow compatibility unless replacement is intentional.
Do not add core ComfyUI code that makes outbound internet requests, including telemetry, analytics, tracking, reporting, update checks, remote configuration, licensing checks, or background network activity. User-authorized model downloads are permitted only for the requested artifact and without telemetry.
Keep state and capability flags on the object that owns the behavior; prefer explicit parent-owned attributes over probing child objects withgetattrfor parent control flow.
Preserve shared method signatures, argument conventions, return types, side effects, and error behavior unless the shared contract and all affected callers are intentionally updated.
Files:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Do not addtorch.no_grad,torch.inference_mode, inference-mode wrappers, or explicit model freeze/trainability toggles; only disable globally enabled inference mode when a training path needs gradients.
Remove inference-only training behavior such as dropout while preserving checkpoint and state-dict compatibility; usenn.Identitywhen removing a module would alter keys or ordering.
Keep imports at module scope except for established optional-backend probes or import-cycle avoidance; avoid unnecessary exception handling and use specific exception types with useful fallbacks.
Do not add code for unsupported pinned library versions or obsolete PyTorch workarounds; unsupported formats, invalid quantization metadata, and bad states should fail clearly rather than silently degrading output.
Match local Python style, keep comments sparse and useful, and remove comments that merely restate obvious code.
Treat dtype, device placement, VRAM usage, and offloading as correctness concerns; use existing ComfyUI cast, offload, cleanup, quantization, and memory helpers.
Model implementations must use an existing optimized Comfy Kitchen, ComfyUI, quantization, or backend operation when it supports the required math, layout, dtype, device, memory, and interface contracts; inspect available operations before writing local kernels.
Retain local or differentiable fallbacks only when no optimized operation satisfies the required math or patch/autograd contract; adapt inputs to shared operation layouts while preserving exact model behavior.
Treat optimized attention and similar backend-selected callables as opaque; callers must rely on documented interfaces and result contracts rather than function identity, names, modules, or implementation details.
Do not duplicate existing inference operations with custom float32-upcasting implementations, such as custom RMSNorm variants; use generic ComfyUI or native torch operations.
If a model constructor has anoperationsparame...
Files:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
comfy/**
⚙️ CodeRabbit configuration file
comfy/**: Core ML/diffusion engine. Focus on:
- Backward compatibility (breaking changes affect all custom nodes)
- Memory management and GPU resource handling
- Performance implications in hot paths
- Thread safety for concurrent execution
Files:
comfy/utils.py
🧠 Learnings (2)
📚 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:
comfy/utils.pytests-unit/comfy_quant/test_convert_old_quants_prefix.py
📚 Learning: 2026-05-13T12:31:45.069Z
Learnt from: rattus128
Repo: Comfy-Org/ComfyUI PR: 13802
File: comfy/pinned_memory.py:19-30
Timestamp: 2026-05-13T12:31:45.069Z
Learning: When reviewing code that uses comfy/pinned_memory.py’s `HostBuffer.extend(size=..., reallocate=...)`: by default (`reallocate` is not True / False), `extend(size=...)` is a *relative increment* that grows the buffer by `size` bytes—so slicing like `[offset:offset+size]` after `hostbuf.extend(size=size)` is correct and the argument should not be rewritten to `offset + size`. Only in the single-segment reallocation mode (`reallocate=True`, e.g., as used by `resize_pin_buffer()` in `comfy/model_management.py`) should `size` be treated as an *absolute target* and the call/arguments should be checked accordingly.
Applied to files:
comfy/utils.py
🪛 ast-grep (0.45.0)
tests-unit/comfy_quant/test_convert_old_quants_prefix.py
[info] 247-249: use jsonify instead of json.dumps for JSON output
Context: json.dumps({
"layers": {layer: {"format": "nvfp4"}}
})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 266-266: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"layers": layers})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 283-285: use jsonify instead of json.dumps for JSON output
Context: json.dumps({
"layers": {"model.diffusion_model.block": {"format": "nvfp4"}}
})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 295-295: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"format": "nvfp4"})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 304-306: use jsonify instead of json.dumps for JSON output
Context: json.dumps({
"layers": {"model.foo.bar": {"format": "float8_e4m3fn"}}
})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 318-323: use jsonify instead of json.dumps for JSON output
Context: json.dumps({
"layers": {
"model.a.block": {"format": "nvfp4"},
"model.b.block": {"format": "float8_e4m3fn"},
}
})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 336-341: use jsonify instead of json.dumps for JSON output
Context: json.dumps({
"layers": {
"direct": {"format": "nvfp4"},
"model.diffusion_model.wrapped": {"format": "float8_e4m3fn"},
}
})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 352-357: use jsonify instead of json.dumps for JSON output
Context: json.dumps({
"layers": {
"block": {"format": "nvfp4"},
"model.diffusion_model.block": {"format": "float8_e4m3fn"},
}
})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 370-372: use jsonify instead of json.dumps for JSON output
Context: json.dumps({
"layers": {"block": {"format": "nvfp4"}}
})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 388-393: use jsonify instead of json.dumps for JSON output
Context: json.dumps({
"layers": {
"block": layer_config,
f"{prefix}block": layer_config,
}
})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 403-405: use jsonify instead of json.dumps for JSON output
Context: json.dumps({
"layers": {"model.diffusion_model.missing": {"format": "nvfp4"}}
})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 421-421: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"layers": layers})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 429-429: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"format": "nvfp4"})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🔇 Additional comments (2)
comfy/utils.py (1)
1403-1443: LGTM!Also applies to: 1500-1508
tests-unit/comfy_quant/test_convert_old_quants_prefix.py (1)
19-19: LGTM!Also applies to: 184-198, 200-214
|
Rewrote the description — it had grown into a wall of text that buried the part that actually matters for review. Two substantive changes, not just trimming:
The code is unchanged. |
|
Extracted the Went back through the earlier review comments while I was in here. The two from 07-26 on the test file are already addressed by later commits: the idempotency test now snapshots and asserts
|
4f5eed7 to
1be447e
Compare
…a branch (Comfy-Org#11864) The new-format branch of convert_old_quants() writes "{layer_key}.comfy_quant" markers straight from the checkpoint's _quantization_metadata JSON, ignoring model_prefix entirely. Checkpoint metadata stores layer keys either with the full diffusion-model prefix or already stripped of it, and convert_old_quants() is invoked before and/or after that prefix is stripped from state_dict (comfy/sd.py calls it up to twice around the strip). A fixed assumption about which convention is in play silently mismatches the other, so affected layers never get wrapped in a QuantizedTensor and fall back to plain-dtype storage (observed as VRAM blowup / manual-cast warnings on NVFP4 checkpoints such as LTX-Video 2.3). PR Comfy-Org#13328 tried to fix this by reordering comfy/sd.py's calls around the prefix strip, but that only swaps which convention works and was closed by the maintainer for risking regressions on checkpoints using the other convention. This instead makes convert_old_quants() match each layer key against the state_dict's real key first (zero behavior change when metadata is already aligned with state_dict), then try adding/stripping model_prefix, and only fall back to today's blind write when neither matches. No call sites in comfy/sd.py are touched or reordered. The write is now also idempotent, guarding against the double-call case rewriting a resolved key. Adds tests-unit/comfy_quant/test_convert_old_quants_prefix.py covering both metadata conventions, the legacy scaled_fp8 branch (unaffected), the exact two-call dance from load_diffusion_model_state_dict, and a documented (xfail) residual gap: when a checkpoint's real weight keys carry no prefix at all but its metadata keys do, comfy/sd.py's model_prefix="" call-site argument still lets model_detection.unet_prefix_from_state_dict get poisoned by the resulting spurious marker keys. That is pre-existing (reproduces identically without this patch) and out of scope here since fixing it needs a comfy/sd.py or model_detection.py change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1be447e to
48454d1
Compare
1 participant
convert_old_quants()writes.comfy_quantmarkers using the layer keys from_quantization_metadataverbatim. When a checkpoint's metadata prefixes don't line up with its state dict keys, every marker lands on a key that has no weight behind it,MixedPrecisionOpsnever engages, and the model silently loads as plain float16 — no error, just the wrong dtype and the VRAM to match.@insecure-erasure identified this failure mode in #11864 (comment) back in March, and pointed at conversion tooling as the place to fix it. This takes the other half: making the loader resilient to checkpoints that are already out there.
The change resolves each metadata layer against the real state dict keys before writing a marker, and skips any layer it can't map to an actual weight.
On the breakage risk that closed #13328: that PR reordered conversion around the prefix strip in
sd.pyand was closed with "This is going to break some models." This one doesn't touchsd.pyor any call site — resolution happens insideconvert_old_quants(). To check it doesn't change behaviour on checkpoints people actually use, I read the safetensors headers (headers only, no weight bodies) of 125 files across 47 HF repos — Wan, FLUX, LTX, Qwen, Z-Image; NVFP4, MXFP8 and FP8:model.diffusion_model.× samemodel.× samemodel.diffusion_model.122 of 125 resolve exactly and behave identically. The 3 that differ are a partial LTX vocoder payload carrying stale whole-model metadata (0 of 1176 metadata layers match that file's 194 tensors) — the old code would have attached whole-model metadata to unrelated component weights there.
Ambiguity fails closed rather than guessing. That matters more than it sounds: an earlier revision of this PR did guess, and while testing it I found the guessed marker could poison
unet_prefix_from_state_dict()'s prefix vote badly enough that a laterstate_dict_prefix_replace(..., filter_keys=True)discarded every real weight tensor — strictly worse than the bug being fixed. That path is closed now, and the regression tests for it fail hard against the pre-fix resolver.Testing
tests-unit/comfy_quantandtests-unit/comfy_testpass. Coverage: prefix mismatch, the two-call loader flow, idempotency, marker collision, legacyscaled_fp8,QuantizedTensorloading, and the orphan-marker cases (known wrappers at various layer counts, mixed direct/wrapper metadata, many-to-one and direct/wrapper conflicts, alias coalescing, unmatched metadata).Not verified: I don't have a prefixed NVFP4 checkpoint to run end-to-end on GPU. The evidence above is header analysis plus unit tests, not a real load of an affected model.
API Node PR Checklist
Scope
Pricing & Billing
If Need pricing update:
QA
Comms