Comfy-Org / Comfy-Org/ComfyUI

H3 FinalLayer signature change breaks downstream custom nodes (no default for new params)

Open
#16,149 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
133k
Forks
15.7k
Avg merge
1d 7h
Merged PRs (30d)
158

Description

# ComfyUI core 反馈(给 ComfyUI 官方)

**标题**: H3 `FinalLayer` signature change breaks downstream custom nodes (no default for new params) — suggest graceful defaults or a deprecation window

**中文标题**: H3 FinalLayer 签名变更无默认值,导致第三方采样/缓存插件全部崩溃——建议给新参数默认值或 deprecation 期

---

## Summary / 概述

The recent H3 changes on `master` (post v0.34.5, around PR #16072 "Add Sparse Attention node" and related MiniMax-H3 sampler work) upgraded `comfy/ldm/minimax/model.py`:

```python
# before
def forward(self, x, t_emb, video_seg, audio_seg)
# after
def forward(self, x, t_emb, video_seg, audio_seg, sigma, sample_sigmas, shifts)
```

`FinalLayer` now **requires** `sigma`, `sample_sigmas` and `shifts` (used by the PDD heads). Core's own `_forward` passes them, so core is internally consistent — but **any third-party node that copies/patches the H3 forward** (a common pattern for MiniMax-H3 caches, dual-clock samplers, etc.) now crashes at runtime with:

```
TypeError: FinalLayer.forward() missing 3 required positional arguments: 'sigma', 'sample_sigmas', and 'shifts'
```

## Impact / 影响

- Breaks every custom H3 node that patches `MiniMaxH3Model._forward` (e.g. `ComfyUI-MiniMaxH3-Cache`, community dual-clock sampler nodes, and similar).
- The failure is **silent until a user actually queues a job** — node loads fine, crash happens deep inside sampling, which is confusing to debug.
- Multiple users in the Chinese ComfyUI community are hitting update-related breakage after pulling master (this FinalLayer change, plus the new default-on compiler from comfy-aimdo increasing VRAM pressure) — the ecosystem is currently in "hold off on updating" mode.

## Suggestion / 建议(友好,非 bug 报告)

1. **Graceful defaults**: give the new `FinalLayer.forward` params safe fallbacks when the caller doesn't provide them, e.g.

```python
def forward(self, x, t_emb, video_seg, audio_seg, sigma=None, sample_sigmas=None, shifts=None):
if sigma is None or sample_sigmas is None or shifts is None:
# fall back to the old (non-PDD) behaviour or derive from transformer_options
...
```

This keeps core's new PDD path while not instantly breaking every downstream node.

2. **Deprecate loudly**: if the signature must change, log a one-time warning when a patched forward is detected calling the old signature, so plugin authors get a clear signal instead of a runtime crash on users' machines.

3. **Document the breaking change** in the release notes / changelog entry for the H3 model internals, so custom-node authors know to update.

None of this blocks core — it just keeps the ecosystem from silently exploding on every user who updates. Happy to provide more detail.

---

Contributor guide

Open the contributing guide

Research direction

Start in comfy/ldm/minimax/model.py, comparing FinalLayer.forward with MiniMaxH3Model._forward and the pre-change signature described in the issue. Determine how the PDD arguments should interact with old callers, then verify that the current H3 path remains functional while legacy custom-node calls no longer fail; the issue does not name a regression test.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
ai
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.