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
…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>
📝 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
| parts = layer_key.split(".") | ||
| for i in range(1, len(parts)): | ||
| candidate = ".".join(parts[i:]) | ||
| if "{}.weight".format(candidate) in state_dict and "{}.comfy_quant".format(candidate) in state_dict: | ||
| return candidate |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve unique weight matches before requiring an existing marker.
When model_prefix="", metadata still contains the full prefix, and the state dict is already stripped, this fallback sees proj_in.weight but rejects it because proj_in.comfy_quant does not yet exist. Conversion then returns the original prefixed key and writes the marker under the wrong prefix. Prefer an existing marker when available, but accept a unique .weight candidate on the first conversion.
🔧 Proposed fix
parts = layer_key.split(".")
+ weight_candidates = []
for i in range(1, len(parts)):
candidate = ".".join(parts[i:])
- if "{}.weight".format(candidate) in state_dict and "{}.comfy_quant".format(candidate) in state_dict:
- return candidate
+ if "{}.weight".format(candidate) in state_dict:
+ if "{}.comfy_quant".format(candidate) in state_dict:
+ return candidate
+ weight_candidates.append(candidate)
+ if len(weight_candidates) == 1:
+ return weight_candidates[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.
| parts = layer_key.split(".") | |
| for i in range(1, len(parts)): | |
| candidate = ".".join(parts[i:]) | |
| if "{}.weight".format(candidate) in state_dict and "{}.comfy_quant".format(candidate) in state_dict: | |
| return candidate | |
| parts = layer_key.split(".") | |
| weight_candidates = [] | |
| for i in range(1, len(parts)): | |
| candidate = ".".join(parts[i:]) | |
| if "{}.weight".format(candidate) in state_dict: | |
| if "{}.comfy_quant".format(candidate) in state_dict: | |
| return candidate | |
| weight_candidates.append(candidate) | |
| if len(weight_candidates) == 1: | |
| return weight_candidates[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/utils.py` around lines 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.
1 participant
Summary
_quantization_metadatalayer keys against the state dict using either the full diffusion-model prefix or the already-stripped form..comfy_quantmarker only when its bytes match the current metadata; replace conflicting markers.comfy/sd.pycall order and model-loading API unchanged.Test plan
scaled_fp8, andQuantizedTensorregression coverage intests-unit/comfy_quant/test_convert_old_quants_prefix.pytests-unit/comfy_quant/+comfy_test/+utils/— 73 passed🤖 Generated with Claude Code and Codex