fix(#1528): two comments name RetakePipeline as the caller that sets force_uncond_pass, and none of upstream's seven construction sites sets it - #1795
Merged
Conversation
…s `force_uncond_pass`, and none of upstream's seven construction sites sets it `include/vllm/model_executor/models/ltx2_denoisers.h:144-145` and `src/vllm/model_executor/models/ltx2_denoisers.cpp:111-113` both told the reader that `RetakePipeline` is the one upstream caller that enables `force_uncond_pass`, citing `retake.py:305-311`. Both landed in `daeff67f2` (row `LTX25-GUIDED-VIDEO`, #1092, PR #1102), and both are false at the pin. Re-derived at `Lightricks/LTX-2` `fd4ded7f2d88d3da713abcdd4ad41ecc4a9314ca`, the revision `.agents/specs/ltx25-guided-video.md:6` and `.agents/specs/ltx25-retake.md:12` both pin, read at the local checkout `/home/mudler/_git/LTX-2` whose `git rev-parse HEAD` is that SHA and whose working tree is clean. `git grep -n force_uncond_pass` over the whole upstream tree, not only `*.py`, returns 11 lines and not one is an assignment by a caller. The filing counted 10 because it restricted the grep to `*.py`, so the two counts agree and name different populations. `utils/denoisers.py:74` declares the `_guided_denoise` keyword parameter, `:102-103` are the two reads, `:267,273,297` are `GuidedDenoiser` storing and forwarding it, and `:313,319,357` are `FactoryGuidedDenoiser` doing the same. `utils/samplers.py:692` is the substring `force_uncond_pass=True` inside a raised error message. `packages/ltx-pipelines/CLAUDE.md:76` documents it. The filing checked `retake.py` alone. `git grep -n 'GuidedDenoiser('` finds seven construction sites, three `GuidedDenoiser` and four `FactoryGuidedDenoiser`: `a2vid_two_stage.py:230`, `keyframe_interpolation.py:232`, `retake.py:305`, `t2a_one_stage.py:155`, `ti2vid_one_stage.py:221`, `ti2vid_two_stages.py:248` and `ti2vid_two_stages_hq.py:271`. None passes the keyword, so all seven take the `force_uncond_pass: bool = False` default. The corrected comments state that stronger fact rather than the retake-only one. The flag is a CFG++ affordance, per `CLAUDE.md:76`: run the uncond pass even when `cfg_scale` is 1.0, because CFG++ still needs the uncond prediction for the ordinary differential equation derivative. Upstream ships the plumbing and turns it on nowhere. Two smaller errors rode along and are corrected in the same text. The header cited the declaration as `:76`, which is a docstring line; the declaration is `:74`. Both comments cited `retake.py:305-311`, whose last line is blank; the construction expression is `:305-310`. BEHAVIOR DOES NOT CHANGE, AND THE CODE WAS NOT THE DEFECT. `Ltx2GuidedDenoiseInputs::force_uncond_pass` already defaults to `false`, `grep -rn force_uncond_pass src/ include/ tests/ examples/` finds only reads and no assignment anywhere, and the condition mirrors `denoisers.py:102-103` operand for operand. That is exactly upstream, where nothing enables it either. The scope stayed at comment text, and no wider repair was taken. A comment-only change cannot be red-first tested, and this change does not pretend otherwise. Two things stand in for that. The upstream re-derivation above is cited by `file:line` at the pin, and a fresh reviewer re-derived every anchor from the upstream checkout rather than reading it from this message. And the comment-only claim is not asserted, it is measured: stripping comments from each file at the base and at the head with `gcc -fpreprocessed -dD -E -P` gives byte-identical output, `e59efade7344a9bc...` for the header and `c6188d0f6682b3ad...` for the translation unit, so the token stream the compiler sees did not move. THE ANCHOR WAS LOAD-BEARING, which is why this is a bug and not a typo. A reader who trusts it concludes that our retake path is missing a flag upstream sets, and sets it. `force_uncond_pass` adds an uncond pass for every present modality, so that repair would double retake's DiT forward count from 8 to 16 and change its output. The comment invited the exact divergence it appeared to describe. Two specs were checked and deliberately left alone. `.agents/specs/ltx25-retake.md` never mentions the field. `.agents/specs/ltx25-guided-video.md:198` states the condition, `uncond` when either guider asks or `force_uncond_pass`, without claiming a caller, which is true at the pin. Closes #1528. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
localai-bot
pushed a commit
that referenced
this pull request
Aug 23, 2026
#1795 landed `edaa8c810` and appended a row to `.agents/issue-index.md`, which this branch also appends to, so GitHub reported the pull request CONFLICTING while `git merge-tree` reported clean. Both are right: `.gitattributes:7` declares the path `merge=union`, git honours the driver locally and the forge ignores it. The form that predicts the forge is git -c merge.union.driver=false -c core.attributesFile=/dev/null \ merge-tree origin/main <head> and it names `.agents/issue-index.md` as the one conflicting path. RESOLVED BY THE KEYED-RECORD RULE, not by hand and not by the driver. The driver's own output is discarded here even though it merged without a conflict: it placed this branch's row in the interior of the table rather than at the end, and a driver that silently relocates a row is not a resolution anyone can check. The file is `git show origin/main:.agents/issue-index.md` in full, with this branch's `#1791` row re-appended as the last line. Verified rather than asserted. The merged file minus its last line is byte-identical to main's complete file, sha256 321d799f8a87c899d328faddb2cb30ab0bc048e34ea4b1c72bb691c061d224e9 on both sides, so no existing row moved or changed. The appended line is byte-identical to the one authored at `98517dd7b`. Counts go 646 to 647 rows, 647 distinct issue ids, 0 duplicates. `.agents/issue-index.md` is the only `merge=union` path in `.gitattributes` and the only file this merge had to resolve; the row's own five files are untouched, each hashing identically to `98517dd7b`. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
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.
include/vllm/model_executor/models/ltx2_denoisers.h:144-145andsrc/vllm/model_executor/models/ltx2_denoisers.cpp:111-113both told the readerthat
RetakePipelineis the one upstream caller that enablesforce_uncond_pass, citingretake.py:305-311. Both landed indaeff67f2(row
LTX25-GUIDED-VIDEO, #1092, PR #1102), and both are false at the pin.Re-derived at
Lightricks/LTX-2fd4ded7f2d88d3da713abcdd4ad41ecc4a9314ca, therevision
.agents/specs/ltx25-guided-video.md:6and.agents/specs/ltx25-retake.md:12both pin, read at the local checkout/home/mudler/_git/LTX-2whosegit rev-parse HEADis that SHA and whoseworking tree is clean.
git grep -n force_uncond_passover the whole upstream tree, not only*.py,returns 11 lines and not one is an assignment by a caller. The filing counted 10
because it restricted the grep to
*.py, so the two counts agree and namedifferent populations.
utils/denoisers.py:74declares the_guided_denoisekeyword parameter,:102-103are the two reads,:267,273,297areGuidedDenoiserstoring andforwarding it, and
:313,319,357areFactoryGuidedDenoiserdoing the same.utils/samplers.py:692is the substringforce_uncond_pass=Trueinside araised error message.
packages/ltx-pipelines/CLAUDE.md:76documents it.The filing checked
retake.pyalone.git grep -n 'GuidedDenoiser('findsseven construction sites, three
GuidedDenoiserand fourFactoryGuidedDenoiser:a2vid_two_stage.py:230,keyframe_interpolation.py:232,retake.py:305,t2a_one_stage.py:155,ti2vid_one_stage.py:221,ti2vid_two_stages.py:248andti2vid_two_stages_hq.py:271. None passes the keyword, so all seven take theforce_uncond_pass: bool = Falsedefault. The corrected comments state thatstronger fact rather than the retake-only one.
The flag is a CFG++ affordance, per
CLAUDE.md:76: run the uncond pass evenwhen
cfg_scaleis 1.0, because CFG++ still needs the uncond prediction for theordinary differential equation derivative. Upstream ships the plumbing and turns
it on nowhere.
Two smaller errors rode along and are corrected in the same text. The header
cited the declaration as
:76, which is a docstring line; the declaration is:74. Both comments citedretake.py:305-311, whose last line is blank; theconstruction expression is
:305-310.BEHAVIOR DOES NOT CHANGE, AND THE CODE WAS NOT THE DEFECT.
Ltx2GuidedDenoiseInputs::force_uncond_passalready defaults tofalse,grep -rn force_uncond_pass src/ include/ tests/ examples/returns five lines,the two comments, the two reads and that default member initializer, and no
assignment anywhere, and the condition mirrors
denoisers.py:102-103operand foroperand. The struct has one construction site in the tree,
src/vllm/multimodal/ltx2_video.cpp:4306, which default-initializes and thenassigns ten fields by name, none of them this one, so no positional or designated
initializer can reach the field either.
include/vllm.hnames neither the fieldnor the struct, so no external caller can set it across the application binary
interface. That is exactly upstream, where nothing enables it. The scope stayed
at comment text, and no wider repair was taken.
A comment-only change cannot be red-first tested, and this change does not
pretend otherwise. Two things stand in for that. The upstream re-derivation
above is cited by
file:lineat the pin, and a fresh reviewer re-derived everyanchor from the upstream checkout rather than reading it from this message. And
the comment-only claim is not asserted, it is measured: stripping comments from
each file at the base and at the head with
gcc -fpreprocessed -dD -E -Pgivesbyte-identical output,
e59efade7344a9bc...for the header andc6188d0f6682b3ad...for the translation unit, so the token stream the compilersees did not move.
THE ANCHOR WAS LOAD-BEARING, which is why this is a bug and not a typo. A reader
who trusts it concludes that our retake path is missing a flag upstream sets,
and sets it.
force_uncond_passadds an uncond pass for every present modality,so that repair would double retake's DiT forward count from 8 to 16 and change
its output. The comment invited the exact divergence it appeared to describe.
Two specs were checked and deliberately left alone.
.agents/specs/ltx25-retake.mdnever mentions the field..agents/specs/ltx25-guided-video.md:198states the condition,uncondwheneither guider asks or
force_uncond_pass, without claiming a caller, which istrue at the pin.
WHY THIS TOOK TWO ATTEMPTS, because the issue thread still says it is blocked.
The first attempt wrote the same correction on
row/LTX25-GUIDED-VIDEO-1528atdafde0dc4and deliberately opened no pull request, becausescripts/check-doc-checkpoint.pyclassified any byte changed underinclude/vllm/as a user-usage change and demanded adocs/USAGE.mdedit that acomment fix had nothing true to write. That checker no longer exists.
1db7e59cf(
docs: retire shared status and split benchmark details, #1714) deleted italong with
docs/STATUS.md, so the obstacle the thread records is gone ratherthan waived, and the full preflight is green here with no skip. The earlier
branch is superseded by this one.
Closes #1528.
FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [claude-code]