Re-register the backbone pre-hook on every apply - #2
Closed
jjdejong wants to merge 1 commit into
Closed
Conversation
Registration was gated on a sentinel attribute living on the backbone, which model.clone() shares by reference. From the second apply onward in a single ComfyUI process the gate was already set, so the new pre-hook was never attached and the first apply's closure stayed live, writing into a state dict nothing else could read. The effect was that captured_latent_shape and current_sigma stayed None for the rest of the process: latent_frame_0 silently returned unmodified output from the second prompt onward, and the skip_when_sigma_above gate stopped working in every mode including guide. Only the first prompt after a restart ever worked, which makes it easy to mistake for a working node. Track the handle, remove any prior registration, and re-register unconditionally -- the same lifecycle the attn1 hooks already use, and that LTXLatentAnchorAware already applies to both its hook types. The bypass path now detaches the pre-hook too instead of only dropping the sentinel, which previously left it attached and firing. Verified by applying three times against a shared module: each apply's state dict is the one populated, and exactly one pre-hook stays live.
Owner
Author
|
Superseded by TenStrip#15, which carries both commits upstream. Branch retained. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The backbone pre-hook registration is gated on a sentinel attribute:
That sentinel lives on the backbone module, which
model.clone()shares by reference. So from the second apply onward in a single ComfyUI process the gate is already set: the new pre-hook is never attached, and the first apply's closure stays live — writingcaptured_latent_shapeandcurrent_sigmainto a state dict that nothing else can read.Consequences for every apply after the first, for the lifetime of the process:
latent_frame_0silently returns unmodified output.captured_latent_shapestaysNone, so the hook bails at every block.skip_when_sigma_abovestops working in all modes, includingguide—current_sigmacomes from the same dead closure.Only the first prompt after a restart ever worked, which makes this easy to mistake for a functioning node: you see it work once, then never notice it stop.
The attn1 hooks in this same node already remove-and-re-register correctly, and
LTXLatentAnchorAwaredoes so for both its hook types — this path was the outlier.Fix
Track the handle, remove any prior registration, re-register unconditionally. The bypass path now detaches the pre-hook too, rather than only dropping the sentinel — which previously left the hook attached and firing.
Verification
Applying three times against a shared module:
Each apply's own state dict is the one populated, and hooks don't accumulate.
Confirmed on a live LTX-AV run: before the fix a second apply logged
video latent shape was never captured; after it, the same second apply reportsHOOK ACTIVE.