From d71378a727fffc63db6db35e84bad26bfe3a7cc4 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 15 Aug 2026 18:04:37 +0000 Subject: [PATCH 1/4] spec(LTX25-TOKEN-APPEND): one fixed-width phase loop, three blocked arms (#930) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two rows hit the same wall hours apart without talking to each other. #930 was filed from the IC-LoRA port and #920 from the generated-keyframe-slots port, and both stopped at the same sentence: this engine's phase loop is fixed at one `Ltx2VideoTokenCount(vshape, 1)`. A limitation two independent rows reach by different routes is a shared seam, so it gets its own spec. The sizing in #930 is corrected here rather than inherited. It names two blocked arms because it predates #920; there are three — reference video, the last-frame keyframe, and generated keyframe slots. Three findings the spec records before any code, because each one moves what the row is allowed to build: The appending conditioning ITEMS are already ported and gated. What is missing is `extend_keyframes_mask` (mask_utils.py:74-105), which upstream's docstring makes an obligation of appending itself, and `clear_conditioning` (tools.py:88-117), which trims back to the target count and restores an ALL-ONES mask rather than slicing the conditioned one. The attention mask is NOT the gap, so no field is added for it. Both ported video items pass a literal `attention_mask=None` (keyframe_cond.py:68-76, reference_video_cond.py:88-96) and the only route to a non-None mask is `ConditioningItemAttentionStrengthWrapper`, applied solely at iclora_utils.py:169 on the IC-LoRA path. A field here would be one no ported item could populate. The sigma schedule must keep reading the TARGET count. Upstream derives its shift from `math.prod(latent.shape[2:])` (schedulers.py:38-39), the unpatchified target, and the pipelines compute sigmas before the state exists at all. This engine's call sits AFTER the conditioning block, so it re-shifts the whole schedule the moment anything appends. Scope is the seam plus one arm lifted as its demonstration. Reference video stays refused and its message stays byte-identical: at this base PR #938 is open and unmerged, so the IC-LoRA metadata cause that refusal names is still true, and rewording it onto token-append alone would ship a refusal naming the first of two causes. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/issue-index.md | 1 + .agents/specs/ltx25-token-append.md | 302 ++++++++++++++++++++++++++++ 2 files changed, 303 insertions(+) create mode 100644 .agents/specs/ltx25-token-append.md diff --git a/.agents/issue-index.md b/.agents/issue-index.md index e3fad2048..30cce5cbe 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -242,3 +242,4 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#810](https://github.com/mudler/vllm.cpp/issues/810) | `MODEL-TEXT-nemotron-h-nemotron-hfor-causal-lm` | NemotronH is not reachable through `include/vllm.h`: `GPUModelRunner::initialize_kv_cache` rebuilds the RECURRENT half of the allocation from `config_.linear_*` instead of the `MambaSpec` the model published, so every non-Qwen3.5 hybrid is refused by Qwen3.5's name at `runner.cpp:525`, and per-layer membership comes from `config_.layer_types[l] == "linear_attention"` rather than `KVCacheGroupSpec::layer_names`. Spec [`nemotron-h-abi-e2e.md`](specs/nemotron-h-abi-e2e.md); the attention half at `runner.cpp:539-607` is already spec-driven and is the model to mirror. Note the SAFETY constraint recorded there: neutering the check alone reaches a forward that ignores `attn_kv`/`gdn_state`/`num_reqs`, which is strictly more dangerous than the refusal | bug | | [#873](https://github.com/mudler/vllm.cpp/issues/873) | `GATE-CI-CONCURRENCY` | `main` went RED on six release/registration gates after the #865 `ci.yml` rewrite: `check-release-binary-contract.py` and `check-test-registration.py` credit a checker to CI only through an UNCONDITIONAL job, and #865 gave `agent-record` an `if:`; the closed-PR skip is re-expressed through `needs:` and the byte-exact Windows PR proof schema restored (spec [`ci-concurrency.md`](specs/ci-concurrency.md)) | bug | | [#874](https://github.com/mudler/vllm.cpp/issues/874) | — | `windows-msvc-cpu`/`windows-msvc-vulkan` still start on a CLOSED pull request: `check-release-workflow.py::validate_pr_ci` compares their whole job mapping for equality, so neither an `if:` clause nor a `needs:` guard can be added. Listed under `## Owed` in [`ci-concurrency.md`](specs/ci-concurrency.md) | bug | +| [#930](https://github.com/mudler/vllm.cpp/issues/930) | `LTX25-TOKEN-APPEND` | LTX-2.5's phase loop is fixed at one `Ltx2VideoTokenCount(vshape, 1)`, and that single limitation blocks THREE conditioning arms, not the two #930 was written against: reference video, the LAST-frame keyframe, and generated keyframe slots ([#920](https://github.com/mudler/vllm.cpp/issues/920)). Row `LTX25-TOKEN-APPEND` (spec [`ltx25-token-append.md`](specs/ltx25-token-append.md)) ports the two missing halves of the append — `extend_keyframes_mask` (`mask_utils.py:74-105`), which upstream's own docstring says EVERY appending item must call, and `clear_conditioning` (`tools.py:88-117`), which trims back to the target count and restores an ALL-ONES mask rather than the conditioned one — and lifts the last-frame keyframe as the demonstration. The attention mask is NOT the gap and no field is added for it: both ported video items pass a literal `attention_mask=None` (`keyframe_cond.py:68-76`, `reference_video_cond.py:88-96`) and the only route to a non-None mask is `ConditioningItemAttentionStrengthWrapper`, applied solely at `iclora_utils.py:169`. The sigma schedule must keep reading the TARGET count — `math.prod(latent.shape[2:])` (`schedulers.py:38-39`) is the unpatchified target and cannot see an append — so the engine's `Ltx2SigmaSchedule(steps, video.tokens)` call, which sits AFTER the conditioning block (`src/vllm/multimodal/ltx2_video.cpp:1719 @ bc6433d1b`), re-shifts the whole schedule the moment anything appends. Reference video and generated slots stay refused: at `bc6433d1b` the reference refusal's LoRA-metadata cause is still true because PR [#938](https://github.com/mudler/vllm.cpp/pull/938) is open and unmerged | feature | diff --git a/.agents/specs/ltx25-token-append.md b/.agents/specs/ltx25-token-append.md new file mode 100644 index 000000000..0f74bf526 --- /dev/null +++ b/.agents/specs/ltx25-token-append.md @@ -0,0 +1,302 @@ +# LTX-2.5 — the token-APPEND seam, and the three arms one fixed-width loop blocks + +Row: `LTX25-TOKEN-APPEND` +Issue: [#930](https://github.com/mudler/vllm.cpp/issues/930) +Campaign: [#644](https://github.com/mudler/vllm.cpp/issues/644) +Pin: `Lightricks/LTX-2 @ fd4ded7fa` — verified at the working checkout +`/home/mudler/_git/LTX-2`, `git rev-parse HEAD` = +`fd4ded7f2d88d3da713abcdd4ad41ecc4a9314ca`. +Base: `origin/main` at `bc6433d1bc4da3062b846b753cca6e57c20fcb41`. + +## 0. Why this is a row and not a line inside another one + +Two rows hit the same wall hours apart without talking to each other. Row +`LTX25-IC-LORA` filed [#930](https://github.com/mudler/vllm.cpp/issues/930) from +the IC-LoRA port; row `LTX25-GENERATED-KEYFRAMES` filed +[#920](https://github.com/mudler/vllm.cpp/issues/920) from the generated-slots +port. Both stopped at the same sentence: *this engine's phase loop is fixed at +one `Ltx2VideoTokenCount(vshape, 1)`*. A limitation two independent rows reach +by different routes is a shared seam, so it gets its own spec. + +**Sizing correction, carried deliberately.** #930's own accounting names **two** +blocked arms. There are **three**, and the row that wrote that number has asked +for the correction rather than the inheritance: #930 predates #920. The three +are the reference-video / reference-image arm, the last-frame keyframe arm, and +generated keyframe slots. This spec is the record that supersedes the two. + +## 1. What upstream does, with anchors on both sides + +A conditioning item that *appends* grows the token sequence, the loop runs over +the grown sequence, and a trim on the way out restores the target grid. Three +pieces, and this tree has the first one already. + +### 1.1 The append — ported, and not the gap + +| upstream | ours | +|---|---| +| `VideoConditionByKeyframeIndex.apply_to` — `ltx-core/conditioning/types/keyframe_cond.py:36-90`, concatenation at `:79-82` | `Ltx2ConditionVideoByKeyframe` | +| `VideoConditionByReferenceLatent.apply_to` — `ltx-core/conditioning/types/reference_video_cond.py:46-108`, concatenation at `:97-100` | `Ltx2ConditionVideoByReference` | +| `AudioConditionByReferenceLatent.apply_to` — `ltx-core/conditioning/types/reference_audio_cond.py:33-65` | `Ltx2ConditionAudioByReference` | + +All three already concatenate onto `latent`, `clean`, `mask` and `positions` +through the shared `AppendTokens` helper in +`src/vllm/model_executor/models/ltx2_conditioning.cpp`, and all three are gated +against executed upstream by `scripts/gen-ltx2-vae-goldens.py` section 9. The +items are not what is missing. + +### 1.2 The per-token marker the append must carry — MISSING + +`extend_keyframes_mask` (`ltx-core/conditioning/mask_utils.py:74-105`). Every +appending item calls it, and upstream's own docstring says why: *"Every +conditioning item that appends tokens must call this, otherwise the per-token +marker goes out of sync with the token sequence."* `marked=False` for both video +items ported here (`keyframe_cond.py:85-86`, `reference_video_cond.py:103-105`); +`marked=True` has exactly one upstream caller, `VideoGeneratedKeyframeSlots` +(`keyframe_slots.py:121`), which is #920's arm and not this row's. + +`Ltx2LatentState` carries no `keyframes_mask` at all today, so the engine holds +it beside the state on `StreamState::keyframes_mask` and an append would +desynchronise the two. This row moves the field onto `Ltx2LatentState` and +extends it inside `AppendTokens`, which is the only place that can guarantee the +call upstream's docstring demands. + +### 1.3 The trim — MISSING + +`LatentTools.clear_conditioning` (`ltx-core/tools.py:88-117`), called at +`ltx-pipelines/utils/blocks.py:576` and `:579`, immediately before `unpatchify`. +It truncates `latent`, `clean_latent` and `positions` to +`self.patchifier.get_token_count(self.target_shape)`, replaces `denoise_mask` +with `torch.ones_like(...)[:, :num_tokens]` — **ones, not the original mask** — +and drops `attention_mask` and `keyframes_mask` to `None`. + +The engine's comment at the unpatchify site already says *"There are no +conditioning tokens on this path, so the clear is the identity."* This row makes +it not the identity. + +### 1.4 The attention mask — NOT the gap, and this is load-bearing + +Both ported appending video items call `update_attention_mask` with a literal +`attention_mask=None` (`keyframe_cond.py:68-76`, `reference_video_cond.py:88-96`), +and `update_attention_mask` (`mask_utils.py:110-143`) returns `None` when the +argument is `None` and `latent_state.attention_mask is None`. The only upstream +route to a non-`None` mask is `ConditioningItemAttentionStrengthWrapper`, whose +sole application site is `ltx-pipelines/iclora_utils.py:169` on the IC-LoRA +path — `combined_image_conditionings` (`ltx-pipelines/utils/helpers.py:272-308`), +which is the route this engine mirrors, never wraps. + +So `Ltx2LatentState` still grows **no** attention-mask field. Adding one here +would be a field no ported item can ever populate, which is the unpassed-parameter +shape `.agents/reachability.md` enumerates. The refusal messages that cite the +absent field stay literally true; what changes is that the absence stops being +offered as a blocker. + +### 1.5 The sigma schedule reads the TARGET count, never the grown one + +`LTX2Scheduler.execute` (`ltx-core/components/schedulers.py:21-57`) derives its +shift from `tokens = math.prod(latent.shape[2:])` — the **unpatchified** target +latent's `F*H*W`, which by construction cannot see appended tokens. And the +pipelines compute `sigmas` before the state exists at all: `ti2vid_one_stage.py:207` +calls `self._scheduler.execute(steps=num_inference_steps)` with no latent, and +`distilled.py:200-201` uses frozen `DISTILLED_SIGMAS` constants. + +This engine calls `Ltx2SigmaSchedule(steps, video.tokens)` *after* the +conditioning block. Today that is the target count because nothing appends. +The moment something appends, that line silently re-shifts the whole schedule. +It is the one behaviour-preserving-today edit this row must make anyway, and it +gets its own mutation. + +## 2. Scope + +**Build the seam. Lift one arm as the demonstration. Leave the other two +refused.** + +In scope: + +* `Ltx2LatentState::keyframes_mask`, `Ltx2ExtendKeyframesMask`, and the call to + it from `AppendTokens`. +* `Ltx2ClearConditioning`. +* The engine's phase loop: a `target_tokens` that the sigma schedule and the + trim both read, a grown `video.tokens` through the DiT, and the trim before + `Ltx2VideoUnpatchify`. +* The **last-frame keyframe** arm, lifted, driven from `VideoGenParams::last_frame_path` + / `last_frame_ppm`. + +Out of scope, and refused as today: + +* **Reference video / reference image.** At this row's base, `row/LTX25-IC-LORA` + (PR [#938](https://github.com/mudler/vllm.cpp/pull/938)) is **open and + unmerged**, so `--lora` does **not** read the IC-LoRA scale factors here and + `git log --grep '#923'` is empty. The refusal at `ltx2_video.cpp` naming + `downscale_factor` / `temporal_scale_factor` in LoRA metadata is **still the + true and current cause** on this base. #930's body describes that refusal as + already rewritten onto token-append; **that is not the state of `origin/main` + at `bc6433d1b`.** This row therefore leaves that refusal byte-identical. + Rewording it to name only token-append would ship the exact defect a sibling + row nearly shipped — a refusal naming the first of two causes. +* **Generated keyframe slots** (#920, PR + [#929](https://github.com/mudler/vllm.cpp/pull/929), also open). Needs + `marked=True`, `GeneratedKeyframeLayout`, and a standalone one-frame decode + per slot (`types.py:269-273`). The seam this row builds is its prerequisite, + not its implementation. +* **Reference audio.** Blocked on the audio VAE encoder key filter, untouched. +* Any `include/vllm.h` growth. `last_frame` / `last_frame_ppm` already exist on + the ABI; nothing new is exposed. + +## 3. Design + +### 3.1 `ltx2_conditioning.h` / `.cpp` + +``` +Ltx2LatentState += std::vector keyframes_mask; // types.py LatentState.keyframes_mask +Ltx2ExtendKeyframesMask(state, num_new_tokens, marked) <- mask_utils.py:74-105 +Ltx2ClearConditioning(state, target_tokens) <- tools.py:88-117 +``` + +`AppendTokens` calls `Ltx2ExtendKeyframesMask(..., marked=false)` before it grows +`state->tokens`, mirroring the fact that upstream passes the **pre-append** +`latent_state`. `Ltx2CreateVideoLatentState` fills `keyframes_mask` from the +existing `Ltx2FirstFrameKeyframesMask`, which is `tools.py:184`'s own +`replace(state, keyframes_mask=...)`. + +Empty vector is upstream's `None`. `extend_keyframes_mask` returns `None` when +there is no existing mask and `marked` is false, and zero-fills a fresh mask when +there is no existing mask and `marked` is true; both branches are mirrored, so +#920's arm finds the function it needs already correct. + +### 3.2 `ltx2_video.cpp` + +The engine keeps `StreamState` and converts at the conditioning boundary, as it +already does for the first-frame arm. Two local converters replace the existing +hand-rolled six-line copy so there is **one** statement of the mapping rather +than two. `StreamState::positions` is `double` and `Ltx2LatentState::positions` +is `float`; the round trip is exact because the engine builds those doubles by +widening floats (`ltx2_video.cpp`, the `video.positions[i] = temporal ? ...` +line), and that is stated at the converter. + +``` +target_tokens = Ltx2VideoTokenCount(vshape, 1) // fixed, per phase +video.tokens = target_tokens // then GROWN by any append +sigmas = Ltx2SigmaSchedule(steps, target_tokens) // §1.5 +...denoise over video.tokens... +Ltx2ClearConditioning(&state, target_tokens) // §1.3 +Ltx2VideoUnpatchify(...) +``` + +The last-frame arm mirrors `combined_image_conditionings` +(`helpers.py:272-308`): `frame_idx == 0` takes `VideoConditionByLatentIndex`, +anything else takes `VideoConditionByKeyframeIndex`. The last frame of the output +is pixel frame `frames - 1`, and `num_pixel_frames` stays at upstream's default +of `1`. It shares the first-frame arm's CRF resolution, strength polarity and +encoder-shape checks, because upstream shares them too — one loop over +`images`, one `load_image_and_preprocess`, one `video_encoder(image)`. + +## 4. Risks + +| risk | why it is real | what catches it | +|---|---|---| +| The sigma schedule silently re-shifts once something appends | the call site sits after the conditioning block and reads `video.tokens` | mutation M4: point it back at `video.tokens` and the last-frame render must move | +| `keyframes_mask` desynchronises from the token count | it lives on `StreamState`, the append lives on `Ltx2LatentState` | the engine's existing `VT_CHECK` on `keyframes_mask.size() == video.tokens` fires; mutation M2 | +| The trim is forgotten or trims to the wrong count | `Ltx2VideoUnpatchify` would read past the target grid or reshape appended tokens into pixels | mutation M3; and the unpatchify's own size arithmetic | +| A blind pixel witness reads as a weak-but-real effect | this campaign has already had exactly that: every arm identical *including* the control, which is what made it diagnosable | the witness carries a **no-op control arm** (§5) | +| The trace cannot see any of it | `Ltx2ConditioningTrace` is filled **before** denoise, so it cannot observe the loop | the witness is on **rendered artifact bytes**, never on the trace | + +## 5. Tests + +Upstream ships **no tests**: `find /home/mudler/_git/LTX-2 -name 'test_*.py'` +returns 0 across the whole repository at the pin, confirmed at this row. So there +is no suite to port and every case below is written **against upstream anchors** +instead — each one cites the `file:line` that justifies the behaviour it +asserts. The bar does not move: each fails for the intended reason first, and +the engine-level cases enter through `LoadVideoEngine` / `VideoEngine::Generate`, +which is the production entry point. + +1. **`test_ltx2_vae` — the seam, at the unit.** `Ltx2ExtendKeyframesMask` on + both `marked` polarities and on the empty-mask branch (`mask_utils.py:74-105`). + `Ltx2ClearConditioning` restores the target token count, restores an + **all-ones** mask rather than the conditioned one (`tools.py:103`), trims + positions per dimension, and drops the keyframes mask (`tools.py:112`). + `AppendTokens` keeps `keyframes_mask.size() == tokens` across an append. +2. **`test_ltx2_video` — the arm, through the entry point.** A last-frame + keyframe renders instead of refusing, and the refusal case's last-frame + subcase is replaced by a served-arm case. The reference-video and + reference-audio subcases stay exactly as they are. +3. **`test_ltx2_video` — the pixel witness, with a no-op control.** Three renders + through `Generate`, byte-compared on the artifact: + - `noop` — no image, no keyframe: nothing is appended. + - `kf_a` — a last-frame keyframe. + - `kf_b` — a *different* last-frame keyframe. + + `kf_a != noop` proves the append reached the maths. `kf_a != kf_b` proves the + appended **content** reached it rather than merely the token count. And + `noop == noop` re-rendered proves the instrument is not simply noisy. All + three arms identical *including* the control would mean the instrument is + blind, which is the reading this control exists to separate — without it a + blind instrument reads as a real-but-subtle effect, and that is the direction + that ships. +4. **Token count observed, not assumed.** The last-frame render's grown count is + `target + tokens_per_latent_frame`, and the finished latent is back at + `target` — otherwise `Ltx2VideoUnpatchify` could not have produced the frame + count the artifact carries. Asserted through the artifact's frame count. + +## 6. Gates + +``` +cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DVLLM_CPP_CUDA=OFF +cmake --build build -j6 +ctest --test-dir build -j4 --output-on-failure +``` + +Focused: `ctest --test-dir build -R 'ltx2' --output-on-failure`. + +No GPU. This row's evidence is CPU-only and needs none: the seam is host-side +sequence bookkeeping, and the fixture DiT is reduced-dimension. + +**Mutation discipline.** Every mutation records three facts — `git diff --stat` +after applying it, whether it **BUILT** with the compile-error count, and the +**exit code**. A mutation that fails to build establishes nothing and is re-run +compiling; a harness that reports exit status alone cannot tell "the guard is +load-bearing" from "the compiler rejected my edit", because a mutation that never +builds prints no test failures at all. A thrown doctest case prints `0 failed`, +so the exit code is the authority. The tree is restored byte-for-byte with a +`sha256` check after each. + +Planned mutations: + +| id | mutation | expected | +|---|---|---| +| M1 | reachability: delete the `Ltx2ConditionVideoByKeyframe` call site in the engine | RED | +| M2 | drop the `Ltx2ExtendKeyframesMask` call from `AppendTokens` | RED | +| M3 | make `Ltx2ClearConditioning` the identity | RED | +| M4 | point the sigma schedule back at `video.tokens` | RED | +| M5 | append with `marked=true` instead of `false` | RED | + +## 7. Stop conditions + +* Stop and report `NEEDS_DECISION` rather than lifting the reference-video arm: + its second cause is another row's, and PR #938 is open against it. +* Stop rather than growing `include/vllm.h`; the arm's fields already exist. +* Stop rather than editing `docs/FEATURES.md` or `docs/USAGE.md` outside the + keys this row's arm changes. + +## 8. Owed + +* **Reference video / reference image** — needs the IC-LoRA metadata scale + factors. Owned by row `LTX25-IC-LORA`, issue + [#930](https://github.com/mudler/vllm.cpp/issues/930) for the append half, + PR [#938](https://github.com/mudler/vllm.cpp/pull/938) for the metadata half. + This row removes the append half of that blocker and touches neither the + refusal nor its test. +* **Generated keyframe slots** — needs `marked=True`, `GeneratedKeyframeLayout`, + and per-slot standalone decode. Owned by row `LTX25-GENERATED-KEYFRAMES`, + issue [#920](https://github.com/mudler/vllm.cpp/issues/920). +* **Reference audio** — needs the audio VAE encoder key filter. Owned by the + campaign, [#644](https://github.com/mudler/vllm.cpp/issues/644). +* **`Ltx2ExtendKeyframesMask`'s `marked=true` branch** lands with a unit driver + and **no production caller**: the only upstream construct that passes `true` is + `VideoGeneratedKeyframeSlots`, which is #920's arm. Declared here rather than + discovered later, per `.agents/reachability.md` `## Landing a slice that is not + reached yet`. Owner: row `LTX25-GENERATED-KEYFRAMES`, issue #920. + +## 9. Now + +`ACTIVE` — spec committed, implementation to follow on the same branch. From a06f8fabb99e460891ed8d3496050048e0d0d560 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 15 Aug 2026 18:44:20 +0000 Subject: [PATCH 2/4] feat(LTX25-TOKEN-APPEND): grow the phase loop's token sequence, and trim it back (#930) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LTX-2.5 phase loop was fixed at one `Ltx2VideoTokenCount(vshape, 1)`, and that single limitation blocked three conditioning arms. Two rows walked into it hours apart without talking to each other — #930 from the IC-LoRA port and #920 from the generated-keyframe-slots port — which is why it gets a seam of its own rather than a fix inside whichever row noticed it last. The appending conditioning ITEMS were already ported and gated. What was missing were the two halves that make an append survive the loop: `Ltx2ExtendKeyframesMask` mirrors `extend_keyframes_mask` (conditioning/mask_utils.py:74-105). Upstream's docstring makes the call an obligation of appending itself — "Every conditioning item that appends tokens must call this, otherwise the per-token marker goes out of sync with the token sequence" — so it lives inside `AppendTokens` rather than at the three call sites. Both None branches are mirrored, including the zero-fill that only `marked=true` reaches, so #920's arm finds it already correct. `Ltx2ClearConditioning` mirrors `clear_conditioning` (tools.py:88-117). Two things there are not a truncation: the denoise mask comes back ALL ONES rather than sliced (`:103`), because the returned state describes a finished latent, and `keyframes_mask` is dropped entirely (`:112`). `Ltx2LatentState` grows a `keyframes_mask` field so the marker can be extended with the sequence it describes. It does NOT grow an attention-mask field, and that is a finding rather than a shortcut: both appending video items pass a literal `attention_mask=None` (keyframe_cond.py:68-76, reference_video_cond.py:88-96), `update_attention_mask` returns None for that case, and the only route to a non-None mask is `ConditioningItemAttentionStrengthWrapper`, applied solely at iclora_utils.py:169 on the IC-LoRA path. A field here would be one no ported item could populate. The refusals that cite the absent field were correct to mention it and wrong to offer it as the blocker. The sigma schedule now reads a `target_tokens` local rather than `video.tokens`. That call sits after the conditioning block, so before this change it would have re-shifted the whole trajectory the moment anything appended. Upstream fixes the count at the target twice over: the argument is `math.prod(latent.shape[2:])` of the UNPATCHIFIED target (schedulers.py:38-39), and every pipeline computes its sigmas before a state exists (ti2vid_one_stage.py:207, distilled.py:200-201). The LAST-frame keyframe arm is lifted as the demonstration, mirroring `combined_image_conditionings` (helpers.py:272-308): one preprocess-and-encode shared by both arms, and a branch on `frame_idx` that sends 0 to `VideoConditionByLatentIndex` and anything else to `VideoConditionByKeyframeIndex`. Both may be supplied at once. Reference video, reference audio and generated keyframe slots stay refused, and the reference-video message is byte-identical. At this base PR #938 is open and unmerged, so `--lora` still does not read the IC-LoRA scale factors and that refusal's stated cause is still true. #930's body describes the message as already rewritten onto token-append; it is not, on `origin/main` at `bc6433d1b`. Rewording it would have shipped a refusal naming the first of two causes, which is the defect a sibling row nearly shipped. The witness is on RENDERED BYTES with a no-op control, because `Ltx2ConditioningTrace` is filled before denoise and cannot see what the loop does. A keyframe render differs from a no-keyframe render in 59773 of 91169 artifact bytes, two different keyframes differ from each other in 334, and the same request twice is byte-identical. The control is what separates "the append reached the maths" from "the instrument is blind" — a sibling row's first attempt found every arm identical INCLUDING its control, and without that control a blind instrument reads as a real-but-subtle effect. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- docs/FEATURES.md | 2 +- docs/USAGE.md | 34 ++- .../model_executor/models/ltx2_conditioning.h | 63 ++++- include/vllm/multimodal/ltx2_video.h | 28 ++ .../models/ltx2_conditioning.cpp | 85 +++++- src/vllm/multimodal/ltx2_video.cpp | 261 ++++++++++++++---- tests/vllm/models/test_ltx2_vae.cpp | 145 ++++++++++ tests/vllm/multimodal/test_ltx2_video.cpp | 230 +++++++++++++-- 8 files changed, 739 insertions(+), 109 deletions(-) diff --git a/docs/FEATURES.md b/docs/FEATURES.md index d28fd55dc..56a047733 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -162,7 +162,7 @@ in `ltx2_text_encoder.cpp` is the call that would have to change. | Voxtral audio (`VoxtralForConditionalGeneration`) | Voxtral-Mini-3B-2507 | near-tie-robust 16/16 vs vLLM 0.25.0 | decode 0.97x (beats vLLM); encoder FORWARD 15.90x of vLLM's whole TTFT (pin 46.02 ms), or 2.89x with opt-in `VT_WHISPER_ENC_FA2=1` (costs 3 near-tie divergences vs 0). Not a TTFT ratio. Pending | | Whisper audio encoder | openai/whisper-small; whisper-large-v3 (Voxtral cfg) | encoder tower 77/77; large-v3 tower 203/203 | pending | | MiniMax-H3 DiT (`MiniMaxH3DiTModel`, vllm-omni lane) | MiniMax-H3 (33.1B video+audio) | portable 79/79; all three modalities COHERENT on Q4_K_M (§8.20); PRUNED ckpts run, Q8_0 seam 0.9941 (§8.21); ref2va grid was NVFP4 quant error, §8.9 REFUTED; GGUF/NVFP4/bf16 shards stream | FP4/Marlin landed; speed pending; no bf16 render yet. Render from the Q4_K_M GGUF, not the NVFP4 arm. Krea 2 text-to-image (roadmap C11) is scoped to reuse these DiT seams | -| LTX-2.5 DiT (`LTX2VideoTransformer3DModel`, Lightricks lane) | LTX-2.5 (21.00B video+audio) | `SPIKE`. DiT, VAEs+ENCs, cond, pipeline, quant loaders gated, reduced dims. Prompt AdaLN host+dev; Gemma-4->xattn FIXTURE-gated. Img chain PPM->resize->encode->place->noise. Temporal x2 ups gated, UNDRIVEN. Render OWED | `ltx-2.5`/`ltx2-gen`. ~29 GB NVFP4/GB10, FP8 ~44 GB, +24 GB tower. FP8/torchao/NVFP4; kf abs-pos ported; BOTH DiTs load, NO `allow_unported`. IMAGE cond SERVED `crf=0`; DiffVAE/LoRA/keyframe/ref refused. Speed PENDING | +| LTX-2.5 DiT (`LTX2VideoTransformer3DModel`, Lightricks lane) | LTX-2.5 (21.00B video+audio) | `SPIKE`. DiT, VAEs+ENCs, cond, pipeline, quant loaders gated, reduced dims. Prompt AdaLN host+dev; Gemma-4->xattn FIXTURE-gated. Img chain PPM->resize->encode->place->noise. Temporal x2 ups gated, UNDRIVEN. Render OWED | `ltx-2.5`/`ltx2-gen`. ~29 GB NVFP4/GB10, FP8 ~44 GB, +24 GB tower. FP8/torchao/NVFP4; kf abs-pos ported; BOTH DiTs load, NO `allow_unported`. IMAGE+LAST-frame kf SERVED `crf=0`; DiffVAE/LoRA/ref refused. Speed PENDING | | MiniMax-Music3 (`MiniMaxMusic3ForConditionalGeneration`, diffusers lane) | MiniMax-Music3 (8.6B Qwen3 LLM + 0.646B RVQ decoder + 2.4B fp32 DiT + DAC Flow-VAE); diffusers arm, ~28.5 GB | `ACTIVE`. Loader 1413/1413; AR, acoustic and the 8.6B LM forward all gated vs real weights; `SpeechRegistry` + `vllm_speech_*` v20 + `/v1/audio/speech`; GGUF Q4_K depth decoder value-gated. No composed request observed | Not measured. The denominator will be SGLang-Omni in its production configuration (both CUDA graphs, compiled DIT and DAV, batched seeded sampling) | | LTX-2.5 tiled + streaming Conv VAE decode | LTX-2.5 video VAE | gated vs executed upstream `ltx_core` @ `fd4ded7f` (`test_ltx2_tiling` 10/10, 915 assertions); one-tile and untiled-spatial controls BIT-EXACT vs untiled on both causality arms; an untiled frames axis is REFUSED | Streams temporal chunks through upstream's AUTO layout (768/64 px, 80/24 frames); above one tile the pixel volume is never materialized. NO-OP below 768px and 81 frames; 81-120 IS tiled, differing 6.70% of range | | MTP speculator | Qwen3.6-27B, Qwen3.6-35B-A3B | token-identical to vLLM `mtp` at c1 | ~4% faster c1; +16% output tput (MoE) | diff --git a/docs/USAGE.md b/docs/USAGE.md index ec4e093ae..3d4f966f0 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -501,20 +501,28 @@ and documents an explicit `0` as "skip re-compression entirely") but conditions the model on pixels it was not trained to see. That is a render-quality cost, and it is stated rather than applied silently. -Keyframe, reference-image, reference-video and reference-audio conditioning are -still refused, each naming a different missing piece: a last-frame keyframe needs -the token-APPEND machinery — a keyframe is appended to the sequence with its own -positions and a rebuilt attention mask, then trimmed back off, and this engine's -phase loop is fixed at the target grid's token count — while the served -first-frame arm only REPLACES tokens that already exist; the reference arms need -the IC-LoRA's scale factors, which live in LoRA metadata this project does not +A **last-frame keyframe is served** as of the token-APPEND seam. A keyframe is +*appended* to the token sequence with its own pixel positions, denoised as part +of a longer sequence, and trimmed back off before the latent is unpatchified, +where the first-frame arm only REPLACES tokens that already exist. It takes the +same `image_crf=0` and `noise_aug` as the first-frame arm, and both may be +supplied at once. Two things a previous version of this paragraph got wrong are +worth naming, because a reader may have acted on them: there is no rebuilt +attention mask — a supplied keyframe passes `attention_mask=None` and upstream +returns no mask for it — and the sigma schedule keeps reading the TARGET token +count rather than the grown one, because upstream derives its shift from the +unpatchified target. (Until 2026-08-13 this paragraph said a last-frame keyframe +needs the DiT's unported `keyframes_abs_pos_embedding`. That was wrong: a +supplied keyframe is appended unmarked, so the embedding never applies to it. +Where the embedding does bite is the FIRST latent frame of every render, which +was a separate gap; it was closed on 2026-08-14 under issue #658, so the marker +is now applied on every render.) + +Reference-image, reference-video and reference-audio conditioning are still +refused, each naming a different missing piece: the reference arms need the +IC-LoRA's scale factors, which live in LoRA metadata this project does not read; reference audio additionally needs the AUDIO VAE's encoder key filter, -which is not built. (Until 2026-08-13 this said a last-frame keyframe needs the -DiT's unported `keyframes_abs_pos_embedding`. That was wrong: a supplied keyframe -is appended unmarked, so the embedding never applies to it. Where the embedding -does bite is the FIRST latent frame of every render, which was a separate gap; -it was closed on 2026-08-14 under issue #658, so the marker is now applied on -every render.) Three encoder-level limits are worth +which is not built. Three encoder-level limits are worth stating in advance because they are refusals rather than approximations. A reference waveform whose sample rate differs from the audio VAE's is refused rather than resampled, since upstream uses a polyphase kaiser resampler this diff --git a/include/vllm/model_executor/models/ltx2_conditioning.h b/include/vllm/model_executor/models/ltx2_conditioning.h index 506bd6732..ded31a8eb 100644 --- a/include/vllm/model_executor/models/ltx2_conditioning.h +++ b/include/vllm/model_executor/models/ltx2_conditioning.h @@ -53,11 +53,21 @@ namespace vllm { // LatentState (types.py:251-287) at batch 1, carrying only the fields a -// conditioning item touches. `attention_mask` is deliberately absent: every item -// ported here passes `attention_mask=None` and there is no pre-existing mask, so -// `update_attention_mask` returns None (conditioning/mask_utils.py:110-140). An -// item that DID need one would have to grow this struct rather than silently -// dropping it, which is why the omission is stated here. +// conditioning item touches. +// +// `attention_mask` is deliberately absent, and the reason is a fact about the +// ported items rather than a simplification. Both appending VIDEO items pass a +// literal `attention_mask=None` (keyframe_cond.py:68-76, +// reference_video_cond.py:88-96), and `update_attention_mask` returns None when +// its argument is None and the state carries no mask +// (conditioning/mask_utils.py:110-143). The ONLY upstream route to a non-None +// mask is `ConditioningItemAttentionStrengthWrapper`, whose sole application +// site is the IC-LoRA path (ltx-pipelines/iclora_utils.py:169); +// `combined_image_conditionings` (ltx-pipelines/utils/helpers.py:272-308), which +// is the route this engine mirrors, never wraps. So a field here would be one no +// ported item can populate — the unpassed-parameter shape .agents/reachability.md +// enumerates. An item that DID need one would have to grow this struct rather +// than silently dropping it, which is why the omission is stated here. struct Ltx2LatentState { int64_t tokens = 0; int64_t width = 0; // channels per token @@ -66,6 +76,17 @@ struct Ltx2LatentState { std::vector clean; // [tokens, width] std::vector mask; // [tokens] — the denoise mask, 1 = fully denoised std::vector positions; // [pos_dims, tokens, 2] — [start, end) + // `LatentState.keyframes_mask` (types.py:251-287), [tokens] in {0, 1}. EMPTY + // is upstream's `None`, which is what an audio state carries and what a video + // state carries before `create_initial_state` marks the first latent frame. + // + // It lives HERE rather than beside the state because every appending item has + // to extend it — upstream's own docstring says "Every conditioning item that + // appends tokens must call this, otherwise the per-token marker goes out of + // sync with the token sequence" (mask_utils.py:83-85). A marker held next to a + // state that grows is a marker that silently stops describing it, and nothing + // about the render's SHAPE can see the difference. + std::vector keyframes_mask; }; // VideoLatentTools.create_initial_state (tools.py:139-186) at batch 1. @@ -135,6 +156,38 @@ void Ltx2ConditionVideoByReference(Ltx2LatentState* state, const Ltx2LatentVolum int64_t downscale_factor, int64_t temporal_scale_factor, double strength, bool causal_fix); +// `extend_keyframes_mask` (conditioning/mask_utils.py:74-105). Extends the state's +// per-token marker to cover `num_new_tokens` tokens the caller is about to +// append. Call it with the state as it stands BEFORE the append, which is the +// state upstream hands it. +// +// The two None branches are upstream's and are mirrored exactly. No existing +// mask and `marked` false leaves the mask empty (`return None`, :98-99) — an +// audio state and any unmarked append onto an unmarked state. No existing mask +// and `marked` TRUE zero-fills a fresh one first (:100-101), so the appended +// tokens are the only marked ones. +// +// `marked` is TRUE for exactly one upstream construct, `VideoGeneratedKeyframeSlots` +// (conditioning/types/keyframe_slots.py:121); every other appending item passes +// false, because given keyframe content and reference latents are ordinary +// guidance rather than a generated single-pixel-frame slot +// (keyframe_cond.py:85-86, reference_video_cond.py:103-105). +void Ltx2ExtendKeyframesMask(Ltx2LatentState* state, int64_t num_new_tokens, bool marked); + +// `LatentTools.clear_conditioning` (tools.py:88-117), called immediately before +// unpatchify (ltx-pipelines/utils/blocks.py:576, :579). Truncates `latent`, +// `clean` and `positions` back to `target_tokens`, which is +// `patchifier.get_token_count(target_shape)` — so an appending item MUST add its +// tokens at the END, which is what upstream's docstring requires in terms. +// +// TWO THINGS HERE ARE NOT A TRUNCATION and a port that only slices gets both +// wrong. The denoise mask comes back as `torch.ones_like(...)[:, :num_tokens]` +// (:103) — ALL ONES, not the conditioned mask sliced — because the returned +// state describes a finished latent in which every target token is denoised. +// And `keyframes_mask` is dropped to None (:112), because the marker described a +// sequence that no longer exists. +void Ltx2ClearConditioning(Ltx2LatentState* state, int64_t target_tokens); + // AudioConditionByReferenceLatent (reference_audio_cond.py:33-65): APPEND already // patchified reference-audio tokens with their own timings. Takes the patchified // form because the caller holds the encoder's `[channels, frames, mel_bins]` output diff --git a/include/vllm/multimodal/ltx2_video.h b/include/vllm/multimodal/ltx2_video.h index f0d4f20b3..4792303e3 100644 --- a/include/vllm/multimodal/ltx2_video.h +++ b/include/vllm/multimodal/ltx2_video.h @@ -366,6 +366,34 @@ struct Ltx2ConditioningTrace { int64_t image_crf = 0; // the CRF this render actually preprocessed at double image_strength = 0.0; // `ImageConditioningInput.strength` (args.py:64) + // ── the token-APPEND seam (row LTX25-TOKEN-APPEND, issue #930) ─────────── + // + // TWO token counts, because after this row they are no longer the same number + // and the difference between them IS the row. + // + // `video_tokens` is the length of the sequence the DiT forward actually ran + // over on the LAST phase — the target grid plus whatever an appending + // conditioning item added (keyframe_cond.py:79-82). `schedule_tokens` is the + // count the sigma schedule read, which upstream fixes at the TARGET: its shift + // comes from `math.prod(latent.shape[2:])` (schedulers.py:38-39), the + // UNPATCHIFIED target latent, and the pipelines compute sigmas before any state + // exists (ti2vid_one_stage.py:207, distilled.py:200-201). So a render that + // appends must show `video_tokens > schedule_tokens`, and a build that let the + // append re-shift the schedule shows them equal. + // + // Both are written INSIDE the phase loop, like `image_tokens` and unlike every + // field above them. That distinction is the whole reason they exist: the rest + // of this trace is filled before denoise and therefore cannot observe anything + // the loop does, so a witness built on those fields finds every arm identical + // and reads as a weak effect rather than as a blind instrument. + // + // `schedule_tokens` stays 0 on a recipe that carries its own distilled sigmas + // (`Ltx2PhaseRecipe::sigmas` non-empty), because on that path no schedule is + // computed and there is nothing to report. Zero here means "not measured", not + // "zero tokens". + int64_t video_tokens = 0; + int64_t schedule_tokens = 0; + // True only once the `Generate` that produced this conditioning RETURNED. The // trace is filled immediately after the connector and BEFORE the denoise loop, // because that is the only point at which the exact buffers cross-attention diff --git a/src/vllm/model_executor/models/ltx2_conditioning.cpp b/src/vllm/model_executor/models/ltx2_conditioning.cpp index 5abfcca81..af94f84e9 100644 --- a/src/vllm/model_executor/models/ltx2_conditioning.cpp +++ b/src/vllm/model_executor/models/ltx2_conditioning.cpp @@ -59,6 +59,21 @@ void AppendTokens(Ltx2LatentState* state, const std::vector& tokens, int6 const int64_t before = state->tokens; const int64_t after = before + token_count; + // BEFORE anything else, because upstream hands `extend_keyframes_mask` the + // PRE-append state (keyframe_cond.py:85, reference_video_cond.py:103) and the + // None-and-marked branch sizes its fresh mask from that state's denoise mask. + // Both video items ported here pass `marked=False`; the one construct that + // passes true is `VideoGeneratedKeyframeSlots` (keyframe_slots.py:121), which + // is not ported. + // + // It lives INSIDE this helper rather than at the three call sites because + // upstream's docstring makes the call an obligation of appending itself + // (mask_utils.py:83-85), and an obligation spread over three sites is one the + // fourth site forgets. The desynchronisation it prevents is invisible to every + // shape check: the render stays the right size and simply applies a trained + // term to the wrong tokens. + Ltx2ExtendKeyframesMask(state, token_count, /*marked=*/false); + state->latent.resize(static_cast(after * state->width), 0.0f); state->clean.insert(state->clean.end(), tokens.begin(), tokens.end()); state->mask.insert(state->mask.end(), static_cast(token_count), @@ -102,12 +117,80 @@ Ltx2LatentState Ltx2CreateVideoLatentState(const Ltx2VideoLatentShape& shape, in state.positions = VideoPositions(shape, patch_size, factors, causal_fix, tokens); DivideTemporalByFps(&state.positions, tokens, fps); + // tools.py:184 — `create_initial_state` returns + // `replace(state, keyframes_mask=self._first_frame_keyframes_mask(state))` on + // the same line that builds the state, unconditionally. Carried ON the state + // so that an append can extend it; `out_keyframes_mask` stays for the callers + // that only want the vector. + state.keyframes_mask = Ltx2FirstFrameKeyframesMask(shape, patch_size); if (out_keyframes_mask != nullptr) { - *out_keyframes_mask = Ltx2FirstFrameKeyframesMask(shape, patch_size); + *out_keyframes_mask = state.keyframes_mask; } return state; } +void Ltx2ExtendKeyframesMask(Ltx2LatentState* state, int64_t num_new_tokens, bool marked) { + VT_CHECK(state != nullptr, "ltx2 conditioning: null state"); + VT_CHECK(num_new_tokens >= 0, "ltx2 conditioning: cannot extend the keyframes mask by a " + "negative token count"); + // `existing is None and not marked` -> `return None` (mask_utils.py:98-99). + // The empty vector IS None here, so an audio state and an unmarked append onto + // an unmarked state both stay empty rather than materialising a zero mask that + // the DiT would then read as "a marker was supplied". + if (state->keyframes_mask.empty() && !marked) return; + // `existing is None` and marked -> `zeros_like(denoise_mask)` first + // (mask_utils.py:100-101), sized by the state as it stands BEFORE the append. + if (state->keyframes_mask.empty()) { + state->keyframes_mask.assign(static_cast(state->tokens), 0.0f); + } + VT_CHECK(static_cast(state->keyframes_mask.size()) == state->tokens, + "ltx2 conditioning: the keyframes mask must have one value per token BEFORE the " + "append — a mask that already disagrees with the token count has been extended by " + "something that did not go through AppendTokens"); + state->keyframes_mask.insert(state->keyframes_mask.end(), + static_cast(num_new_tokens), marked ? 1.0f : 0.0f); +} + +void Ltx2ClearConditioning(Ltx2LatentState* state, int64_t target_tokens) { + VT_CHECK(state != nullptr, "ltx2 conditioning: null state"); + VT_CHECK(target_tokens >= 0 && target_tokens <= state->tokens, + "ltx2 conditioning: clear_conditioning TRUNCATES to the target token count " + "(tools.py:101-105), so the target cannot exceed what the state carries. A state " + "smaller than its own target means an appending item wrote somewhere other than the " + "END, which is the one thing clear_conditioning's docstring forbids"); + + state->latent.resize(static_cast(target_tokens * state->width)); + state->clean.resize(static_cast(target_tokens * state->width)); + + // ALL ONES, not the conditioned mask sliced (tools.py:104 — + // `torch.ones_like(latent_state.denoise_mask)[:, :num_tokens]`). The returned + // state describes a FINISHED latent, in which every target token is denoised; + // slicing the conditioned mask instead would carry `1 - strength` on the + // conditioned tokens into whatever reads the state next, and on the two-stage + // recipe that next reader is the following phase's initial latent. + state->mask.assign(static_cast(target_tokens), 1.0f); + + // Positions are [pos_dims, tokens, 2], so the truncation is per DIMENSION + // (`positions[:, :, :num_tokens]`, tools.py:105). A plain resize would keep + // the first dimension's appended tokens and drop the last dimension's real + // ones, and the result still type-checks. + std::vector trimmed(static_cast(state->pos_dims * target_tokens * 2)); + for (int64_t d = 0; d < state->pos_dims; ++d) { + std::copy(state->positions.begin() + static_cast(d * state->tokens * 2), + state->positions.begin() + + static_cast(d * state->tokens * 2 + target_tokens * 2), + trimmed.begin() + static_cast(d * target_tokens * 2)); + } + state->positions.swap(trimmed); + + // `keyframes_mask=None` (tools.py:113). The marker described a sequence that + // no longer exists, and a sliced one would claim the trimmed state still + // carries markers for tokens that were removed. + state->keyframes_mask.clear(); + + state->tokens = target_tokens; +} + std::vector Ltx2FirstFrameKeyframesMask(const Ltx2VideoLatentShape& shape, int64_t patch_size) { // tools.py:194-195 — `zeros_like(denoise_mask)` then diff --git a/src/vllm/multimodal/ltx2_video.cpp b/src/vllm/multimodal/ltx2_video.cpp index 56d0c634b..e1c53a015 100644 --- a/src/vllm/multimodal/ltx2_video.cpp +++ b/src/vllm/multimodal/ltx2_video.cpp @@ -177,6 +177,43 @@ struct StreamState { std::vector keyframes_mask; }; +// ── StreamState <-> Ltx2LatentState (row LTX25-TOKEN-APPEND, issue #930) ──── +// +// The conditioning items take `Ltx2LatentState`; the loop runs on `StreamState`. +// ONE statement of the mapping, in both directions, because the first-frame arm +// used to open-code the copy and an appending arm needs three more fields than +// it carried — `positions`, `keyframes_mask` and a `tokens` that comes BACK +// changed. A second open-coded copy is how one of them gets forgotten, and a +// forgotten `keyframes_mask` is invisible to every shape check downstream. +// +// THE POSITIONS ROUND TRIP IS EXACT and that is not luck. `StreamState` holds +// them as `double` only because the DiT's `positions` field takes one; every +// value in it was produced by widening a `float` (see where the temporal axis is +// divided by fps, below), and upstream's own positions are `float32` +// (tools.py:169-174). So double -> float -> double reproduces the bits. +Ltx2LatentState ToLatentState(const StreamState& s, int64_t pos_dims) { + Ltx2LatentState out; + out.tokens = s.tokens; + out.width = s.width; + out.pos_dims = pos_dims; + out.latent = s.latent; + out.clean = s.clean; + out.mask = s.mask; + out.positions.assign(s.positions.begin(), s.positions.end()); + out.keyframes_mask = s.keyframes_mask; + return out; +} + +void FromLatentState(const Ltx2LatentState& in, StreamState* s) { + s->tokens = in.tokens; + s->width = in.width; + s->latent = in.latent; + s->clean = in.clean; + s->mask = in.mask; + s->positions.assign(in.positions.begin(), in.positions.end()); + s->keyframes_mask = in.keyframes_mask; +} + // `post_process_latent` (utils/helpers.py:462-464): // denoised * mask + clean * (1 - mask) // The mask is PER TOKEN and the latent is per token x channel, so the mask @@ -281,7 +318,7 @@ constexpr char kLtx2DurationHeadPathExtra[] = "duration_head_path"; // they are no longer trusted: the list below is derived from this file on every // run and compared, and the failure prints the replacement to paste in. // READER ANCHORS (derived and gated by test_ltx2_video): -// 690 745 841 857 859 929 954 1059 1100 +// 727 782 878 894 896 966 991 1096 1137 const char* const kKnownLoadExtras[] = { kLtx2AudioPromptEmbedsExtra, kLtx2PipelineKindExtra, kLtx2ModelVersionExtra, kLtx2AllowUnportedExtra, kLtx2MaxPhaseExtra, kLtx2DitConfigPathExtra, @@ -1300,39 +1337,23 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { // next reader re-checks the claim instead of re-deriving the refutation. Local // anchors are SYMBOLS, not line numbers in this file: same-file line numbers // drift on every edit, which is how the previous message's citation went stale. - const bool wants_image = !gen.first_frame_path.empty() || !gen.first_frame_ppm.empty(); - if (!gen.last_frame_path.empty()) { - Fail( - "a LAST-frame keyframe is not served. What is missing is the TOKEN-APPEND machinery. " - "`Ltx2ConvVideoEncode` and `Ltx2ConditionVideoByKeyframe` are both ported and gated, " - "and this engine materializes encoder weights through Ltx2VideoVaeEncoderKeyRules, so " - "none of those is the gap. The gap is that `VideoConditionByKeyframeIndex.apply_to` " - "(conditioning/types/keyframe_cond.py:36-90) APPENDS tokens to the sequence: it " - "concatenates onto `latent`, `denoise_mask`, `positions` and `clean_latent` (:79-82), " - "gives the appended tokens their own pixel coordinates offset to `frame_idx` (:46-59), " - "and rebuilds the attention mask through `update_attention_mask` (:68-76) — and then " - "`clear_conditioning` (ltx_core/tools.py:88-105) trims those extra tokens back off " - "before unpatchify. This engine cannot do any of that yet: `Ltx2LatentState` has no " - "attention-mask field at all (see the note on its declaration in ltx2_conditioning.h), " - "and the phase loop is fixed at the target grid's token count — one " - "`Ltx2VideoTokenCount(vshape, 1)` feeds the sigma schedule, the `Ltx2ModalityInput` " - "handed to the DiT, and `Ltx2VideoUnpatchify`, with the clear step an explicit identity " - "because nothing was ever appended. Serving this arm means growing that sequence " - "through the DiT and trimming it back. Conditioning on the FIRST frame needs none of " - "it, which is why that arm IS served: `VideoConditionByLatentIndex` REPLACES tokens " - "that already exist (conditioning/types/latent_cond.py:38-39) and the token count never " - "changes. WHAT IS *NOT* THE REASON, because this refusal used to say it was: " - "`keyframes_abs_pos_embedding`. A SUPPLIED keyframe is appended with `marked=False` " - "(keyframe_cond.py:84-86, whose comment says given keyframe content carries no keyframe " - "marker), and its sole consumer adds `mask * embedding` with `mask = keyframes_mask > 0` " - "(model/transformer/transformer_args.py:42-43, called once at :269) — so on exactly " - "these tokens the embedding contributes nothing, and porting it would not serve this " - "arm. The tokens that DO reach it are the target's own first latent frame, marked " - "unconditionally by `_first_frame_keyframes_mask` (ltx_core/tools.py:184-196) — which " - "is the frame the SERVED first-frame arm writes into. That omission WAS real; row " - "LTX25-KEYFRAMES-ABS-POS closed it on 2026-08-14 (issue #658), so the marker is now " - "applied on every render. It was never what blocks a last-frame keyframe."); - } + // TWO ARMS OF ONE UPSTREAM LOOP. `combined_image_conditionings` + // (ltx-pipelines/utils/helpers.py:272-308) iterates one `images` list and + // branches per item: `frame_idx == 0` takes `VideoConditionByLatentIndex`, + // anything else takes `VideoConditionByKeyframeIndex`. So the CRF, the + // strength polarity, the preprocess and the encode are shared by construction + // upstream, and they are shared here for the same reason rather than + // duplicated per arm. + // + // Row LTX25-TOKEN-APPEND (#930) opened the second branch. The refusal that + // stood here named the token-append machinery, and that refusal was accurate: + // a keyframe APPENDS (keyframe_cond.py:79-82) where an image at latent frame 0 + // REPLACES (latent_cond.py:38-39), and this loop had no way to grow the + // sequence and trim it back. `Ltx2ExtendKeyframesMask` and + // `Ltx2ClearConditioning` are the two halves it was missing. + const bool wants_first_frame = !gen.first_frame_path.empty() || !gen.first_frame_ppm.empty(); + const bool wants_last_frame = !gen.last_frame_path.empty(); + const bool wants_image = wants_first_frame || wants_last_frame; if (!gen.ref_image_paths.empty() || !gen.ref_video_dir.empty()) { Fail( "reference-image / reference-video conditioning is not served. The encoder and the " @@ -1363,7 +1384,7 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { // unsupported request costs nothing and reports the same thing every time. int64_t image_crf = 0; double image_strength = 0.0; - std::string image_bytes; + std::string image_bytes, last_frame_bytes; if (wants_image) { if (!im.has_video_encoder) { Fail( @@ -1392,8 +1413,12 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { "above 1 makes it negative, which the noiser extrapolates PAST the clean latent " "rather than toward it (components/noisers.py:33)"); } - image_bytes = gen.first_frame_ppm.empty() ? ReadFileBytes("first_frame", gen.first_frame_path) - : gen.first_frame_ppm; + if (wants_first_frame) { + image_bytes = gen.first_frame_ppm.empty() + ? ReadFileBytes("first_frame", gen.first_frame_path) + : gen.first_frame_ppm; + } + if (wants_last_frame) last_frame_bytes = ReadFileBytes("last_frame", gen.last_frame_path); im.trace.image_crf = image_crf; im.trace.image_strength = image_strength; } @@ -1558,9 +1583,19 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { } // ── build the two states (create_noised_state, helpers.py:428-445) ─────── + // + // `target_tokens` is `patchifier.get_token_count(target_shape)` — the count + // of the TARGET GRID, which is fixed for this phase. `video.tokens` starts + // equal to it and is GROWN by any appending conditioning item. Row + // LTX25-TOKEN-APPEND (#930) split the two, and the two places that must keep + // reading the target rather than the grown count are the sigma schedule + // (schedulers.py:38-39 derives its shift from the unpatchified target, and + // the pipelines compute sigmas before the state exists at all) and the trim + // at the bottom of the loop (`clear_conditioning`, tools.py:101). + const int64_t target_tokens = Ltx2VideoTokenCount(vshape, 1); StreamState video; video.width = vshape.channels; // patch_size 1 (VideoLatentPatchifier(1)) - video.tokens = Ltx2VideoTokenCount(vshape, 1); + video.tokens = target_tokens; { std::vector volume(static_cast(vshape.channels) * static_cast(vshape.frames) * @@ -1638,43 +1673,55 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { // `height` / `width` at :285-286. Conditioning stage 1 only would let stage 2 re-noise // the pinned frame away; conditioning stage 2 with stage 1's latent would // place a half-resolution image into a full-resolution grid. - if (wants_image) { - const std::vector pixels = Ltx2LoadImageAndPreprocess( - "first_frame", image_bytes, phase_h, phase_w, image_crf); + // ONE ITERATION of upstream's `images` loop (helpers.py:283-291): load, + // preprocess to THIS phase's height and width, encode. Shared by both arms + // because upstream shares it — the branch is on `frame_idx`, below, and it + // is the only thing that differs between them. + auto encode_conditioning_image = [&](const char* label, + const std::string& bytes) -> Ltx2LatentVolume { + const std::vector pixels = + Ltx2LoadImageAndPreprocess(label, bytes, phase_h, phase_w, image_crf); int64_t cropped = 0; const Ltx2LatentVolume encoded = Ltx2ConvVideoEncode( im.video_encoder_cfg, im.video_encoder_weights, pixels, im.video_encoder_cfg.in_channels, /*frame_count=*/1, phase_h, phase_w, &cropped); if (encoded.frames != 1) { Fail("the video VAE encoder returned " + std::to_string(encoded.frames) + - " latent frames for a single image; `VideoConditionByLatentIndex` places one " + " latent frames for a single image; both arms of " + "`combined_image_conditionings` place exactly one " "(ltx-pipelines/utils/helpers.py:294-300)"); } + // The REPLACE arm needs this to hold because upstream raises + // ConditioningError otherwise (latent_cond.py:25-30). The APPEND arm does + // not — `VideoConditionByKeyframeIndex` derives its own shape from the + // keyframe tensor (keyframe_cond.py:41-44) and never compares it to the + // target — so for that arm this is a consistency assertion rather than a + // capability limit, and it cannot falsely fire: the preprocess above + // targets this phase's own height and width, so a disagreement here means + // the encoder's spatial factor and VIDEO_SCALE_FACTORS disagree, which + // would place the keyframe's tokens at the wrong RoPE positions. if (encoded.channels != vshape.channels || encoded.height != vshape.height || encoded.width != vshape.width) { - Fail("the encoded image is " + std::to_string(encoded.channels) + "x" + - std::to_string(encoded.height) + "x" + std::to_string(encoded.width) + + Fail(std::string("the encoded ") + label + " is " + std::to_string(encoded.channels) + + "x" + std::to_string(encoded.height) + "x" + std::to_string(encoded.width) + " but phase '" + phase.name + "' needs " + std::to_string(vshape.channels) + "x" + std::to_string(vshape.height) + "x" + std::to_string(vshape.width) + ". Upstream raises ConditioningError on exactly this " "(conditioning/types/latent_cond.py:25-30): the encoder's spatial factor and the " "pipeline's VIDEO_SCALE_FACTORS must agree, and they do not."); } + return encoded; + }; + + if (wants_first_frame) { + const Ltx2LatentVolume encoded = encode_conditioning_image("first_frame", image_bytes); - // `Ltx2ConditionVideoByLatentIndex` writes `clean` and `mask` and reads - // `tokens` / `width`; `latent` and `positions` are carried so the struct - // is coherent rather than half-filled, not because the item consults them. - Ltx2LatentState state; - state.tokens = video.tokens; - state.width = video.width; - state.pos_dims = 3; - state.latent = video.latent; - state.clean = video.clean; - state.mask = video.mask; + // `frame_idx == 0` -> `VideoConditionByLatentIndex` (helpers.py:295-300). + // It REPLACES tokens that already exist and the token count never changes. + Ltx2LatentState state = ToLatentState(video, /*pos_dims=*/3); Ltx2ConditionVideoByLatentIndex(&state, vshape, /*patch_size=*/1, encoded, image_strength, /*latent_idx=*/0); - video.clean = state.clean; - video.mask = state.mask; + FromLatentState(state, &video); // The witness, taken from the TOKENS THAT WERE WRITTEN rather than from // `encoded` — and the difference is not cosmetic. Digesting the encoder's @@ -1702,9 +1749,61 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { im.trace.image_absmax = AbsMax(written); } + // ── the LAST-frame keyframe (row LTX25-TOKEN-APPEND, issue #930) ──────── + // + // The other branch of the same upstream loop: anything but `frame_idx == 0` + // takes `VideoConditionByKeyframeIndex` (helpers.py:301-305), which APPENDS + // rather than replaces. `frame_idx` is a PIXEL frame, so the last frame of + // the output is `frames - 1`; `num_pixel_frames` stays at upstream's default + // of 1, which is what narrows the appended tokens' temporal extent to + // `[start, start + 1)` instead of the VAE-scaled range + // (keyframe_cond.py:53-56). + // + // AFTER the first-frame arm, because upstream applies conditioning items in + // list order (`state_with_conditionings`, helpers.py:448-458) and both arms + // can be supplied at once — `--image` is repeatable. Order matters here for + // a reason a shape check cannot see: the appended tokens land at the END of + // the sequence, and `clear_conditioning` trims from the end, so an item that + // appended BEFORE a replace would still be trimmed correctly while an item + // that appended before another append would swap their positions. + if (wants_last_frame) { + const Ltx2LatentVolume encoded = encode_conditioning_image("last_frame", last_frame_bytes); + + Ltx2LatentState state = ToLatentState(video, /*pos_dims=*/3); + Ltx2ConditionVideoByKeyframe(&state, encoded, /*patch_size=*/1, factors, fps, + /*frame_idx=*/frames - 1, image_strength, + /*num_pixel_frames=*/1, /*causal_fix=*/true); + FromLatentState(state, &video); + + // THE SEQUENCE GREW, and every consumer below reads `video.tokens` rather + // than the target count. Asserted rather than assumed, because a converter + // that dropped the grown count would leave a state whose buffers are + // longer than the count that describes them — and the DiT would then read + // a prefix, render a plausible clip, and never mention the keyframe. + VT_CHECK(video.tokens > target_tokens, + "ltx2 video: a keyframe conditioning must APPEND tokens " + "(keyframe_cond.py:79-82) and this one left the sequence length unchanged"); + VT_CHECK(static_cast(video.latent.size()) == video.tokens * video.width && + static_cast(video.clean.size()) == video.tokens * video.width && + static_cast(video.mask.size()) == video.tokens && + static_cast(video.keyframes_mask.size()) == video.tokens && + static_cast(video.positions.size()) == 3 * video.tokens * 2, + "ltx2 video: after an append every per-token buffer must have one entry per " + "token. A buffer that did not grow with the others is invisible to the render's " + "SHAPE — the clip comes out the right size and simply describes the wrong " + "tokens."); + } + // The noiser draws VIDEO first, AUDIO second, from one generator // (blocks.py:554-563 builds the video state before the audio one; :576-580, // which this used to cite, is the TEARDOWN and proves nothing about order). + // The length the DiT will actually run over on this phase, recorded BEFORE + // the trim and after every conditioning item, so the last phase's value is + // the one a reader sees. Written inside the loop for the same reason + // `image_tokens` is: the rest of the trace is filled before denoise and + // cannot observe anything that happens in here. + im.trace.video_tokens = video.tokens; + const float noise_scale = static_cast(phase.noise_scale); ApplyGaussianNoise(video, state_noise.Draw(static_cast(video.latent.size())), noise_scale); @@ -1716,7 +1815,25 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { if (sigmas.empty()) { int64_t steps = gen.steps > 0 ? gen.steps : recipe.num_inference_steps; if (steps < 1) Fail("num_inference_steps resolved to " + std::to_string(steps)); - sigmas = Ltx2SigmaSchedule(steps, video.tokens); + // `target_tokens`, NOT `video.tokens`, and this line sits AFTER the + // conditioning block so the distinction is live. Upstream's shift comes + // from `tokens = math.prod(latent.shape[2:])` (schedulers.py:38-39) — the + // UNPATCHIFIED target latent, which by construction cannot see appended + // tokens — and every pipeline computes its sigmas before a state exists at + // all (ti2vid_one_stage.py:207 passes no latent; distilled.py:200-201 uses + // frozen constants). Reading the grown count here would re-shift the whole + // trajectory the moment a keyframe was supplied, which no shape check and + // no frame count can see. + // ONE local feeds both the schedule and the trace, deliberately. If the + // reported number were written independently of the number passed, a + // change to the argument alone would leave the trace still reporting the + // target and the gate still green — the instrument would be describing a + // build that no longer exists. Bound here so the ordinary mutation moves + // both. (It does not defend against an edit to the call argument only; + // nothing local can, and that residual is recorded in the row's spec.) + const int64_t schedule_tokens = target_tokens; + sigmas = Ltx2SigmaSchedule(steps, schedule_tokens); + im.trace.schedule_tokens = schedule_tokens; } else if (gen.steps > 0 && !recipe.allow_request_sigmas) { // `fixed_num_inference_steps` (ltx2_recipes.py:53-87): a distilled recipe's // schedule is trained INTO the model, so honouring a step override would @@ -1854,9 +1971,33 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { } } - // `clear_conditioning` + `unpatchify` (blocks.py:575-580). There are no - // conditioning tokens on this path, so the clear is the identity; the - // unpatchify is not. + // `clear_conditioning` + `unpatchify` (blocks.py:575-580, in that order). + // + // The clear used to be an explicit identity because nothing could append. + // Row LTX25-TOKEN-APPEND (#930) made it real: it truncates `latent`, `clean` + // and `positions` back to the target grid and restores an all-ones denoise + // mask (tools.py:101-105). + // + // WHY THE GUARD IS HERE AND NOT LEFT IMPLICIT. `Ltx2VideoUnpatchify` takes a + // BARE POINTER, so it cannot tell a target-length buffer from a longer one — + // and the appended tokens sit at the tail of a contiguous [tokens, width] + // buffer, so an un-trimmed state would unpatchify the same head bytes and + // render pixel-identical frames. That is exactly the shape of defect this + // project keeps finding: correct output for the wrong reason, with no + // instrument that can see the difference. The check is what turns "the head + // happens to be right" into "the buffer IS the target grid", and it is what + // makes deleting the trim a RED rather than a silent pass. + { + Ltx2LatentState finished = ToLatentState(video, /*pos_dims=*/3); + Ltx2ClearConditioning(&finished, target_tokens); + FromLatentState(finished, &video); + } + VT_CHECK(video.tokens == target_tokens && + static_cast(video.latent.size()) == target_tokens * video.width, + "ltx2 video: the latent handed to unpatchify must be exactly the target grid. " + "`clear_conditioning` (ltx_core/tools.py:88-117) is what establishes that after an " + "appending conditioning item, and `Ltx2VideoUnpatchify` takes a bare pointer that " + "cannot check it."); video_latent_volume = Ltx2VideoUnpatchify(video.latent.data(), vshape, 1); video_lc = vshape.channels; video_lf = vshape.frames; diff --git a/tests/vllm/models/test_ltx2_vae.cpp b/tests/vllm/models/test_ltx2_vae.cpp index 4bc035b25..1a0f8e3b9 100644 --- a/tests/vllm/models/test_ltx2_vae.cpp +++ b/tests/vllm/models/test_ltx2_vae.cpp @@ -2406,6 +2406,151 @@ TEST_CASE("ltx2 conditioning: a REFERENCE VIDEO is translated into the target's CHECK(plain.positions != state.positions); } +// ─── the token-APPEND seam (row LTX25-TOKEN-APPEND, issue #930) ───────────── +// +// UPSTREAM SHIPS NO TESTS. `find /home/mudler/_git/LTX-2 -name 'test_*.py'` +// returns 0 across the whole repository at pin `fd4ded7f`, so there is no suite +// to port and each case below is written against an upstream ANCHOR instead: +// every assertion names the `file:line` that justifies the behaviour it checks. +// +// These are the two halves of an append the conditioning items could not do for +// themselves. The items concatenate; nothing extended the per-token marker +// alongside them, and nothing trimmed the sequence back. + +TEST_CASE("ltx2 conditioning: an APPEND extends the per-token keyframes marker") { + const vllm::Ltx2VideoLatentShape target = CondVideoTarget(); + const vllm::Ltx2ScaleFactors factors; + vllm::Ltx2LatentState state = + vllm::Ltx2CreateVideoLatentState(target, kCondPatch, factors, kCondFps, true); + + // `create_initial_state` returns the marker ON the state (tools.py:184), not + // beside it. A port that only filled the out-parameter would leave an + // appending item with nothing to extend. + REQUIRE(static_cast(state.keyframes_mask.size()) == state.tokens); + const std::vector before = state.keyframes_mask; + + const vllm::Ltx2LatentVolume keyframe = CondVolume("ltx2.cond.keyframe", 4, 1, 2, 2); + vllm::Ltx2ConditionVideoByKeyframe(&state, keyframe, kCondPatch, factors, kCondFps, + vllm_test::kLtx2CondKeyframeFrameIdx, /*strength=*/0.6, + /*num_pixel_frames=*/1, /*causal_fix=*/true); + + // ONE VALUE PER TOKEN, still. This is the invariant `extend_keyframes_mask` + // exists for, in upstream's own words: "otherwise the per-token marker goes + // out of sync with the token sequence" (mask_utils.py:83-85). Out of sync is + // invisible to every shape check downstream — the render stays the right size + // and applies a trained term to the wrong tokens. + CHECK(static_cast(state.keyframes_mask.size()) == state.tokens); + REQUIRE(state.tokens == vllm_test::kLtx2CondKeyframeTokens); + + // The ORIGINAL values are untouched... + for (size_t i = 0; i < before.size(); ++i) { + INFO("target token " << i); + CHECK(state.keyframes_mask[i] == before[i]); + } + // ...and every appended token is UNMARKED. `marked=False` for given keyframe + // content (keyframe_cond.py:85-86, whose comment says given keyframe content + // carries no keyframe marker). Marking them would add a trained bias to + // tokens upstream leaves alone, and the render would still be finite and the + // right shape. + for (size_t i = before.size(); i < state.keyframes_mask.size(); ++i) { + INFO("appended token " << i); + CHECK(state.keyframes_mask[i] == 0.0F); + } +} + +TEST_CASE("ltx2 conditioning: extend_keyframes_mask mirrors BOTH of upstream's None branches") { + // The branches are not symmetric and a port that treats them as one gets the + // generated-slot arm silently wrong (mask_utils.py:96-101). + SUBCASE("no existing mask and UNMARKED stays None") { + // `if existing is None and not marked: return None` + // (conditioning/mask_utils.py:98-99). An audio state carries no marker: + // `AudioLatentTools.create_initial_state` (tools.py:246-280) returns + // `self.patchify(LatentState(...))` with no `keyframes_mask` argument at + // all, where the video tools' own `create_initial_state` sets one on the + // line that builds the state (tools.py:184). Appending reference audio must + // therefore not materialise a zero mask, because a zero mask IS a mask and + // the DiT would read it as one. + const vllm::Ltx2AudioLatentShape target = CondAudioTarget(); + const vllm::Ltx2AudioPatchifierParams params; + vllm::Ltx2LatentState state = vllm::Ltx2CreateAudioLatentState(target, params); + REQUIRE(state.keyframes_mask.empty()); + + vllm::Ltx2ExtendKeyframesMask(&state, /*num_new_tokens=*/3, /*marked=*/false); + CHECK(state.keyframes_mask.empty()); + } + SUBCASE("no existing mask and MARKED zero-fills first, then marks the new tokens") { + // `existing = torch.zeros_like(latent_state.denoise_mask)` (:100-101) sized + // by the state BEFORE the append, then ones for the new tokens. The one + // upstream caller that passes true is `VideoGeneratedKeyframeSlots` + // (keyframe_slots.py:121). + const vllm::Ltx2AudioLatentShape target = CondAudioTarget(); + const vllm::Ltx2AudioPatchifierParams params; + vllm::Ltx2LatentState state = vllm::Ltx2CreateAudioLatentState(target, params); + const int64_t before = state.tokens; + REQUIRE(before > 0); + + vllm::Ltx2ExtendKeyframesMask(&state, /*num_new_tokens=*/3, /*marked=*/true); + REQUIRE(static_cast(state.keyframes_mask.size()) == before + 3); + for (int64_t i = 0; i < before; ++i) { + INFO("pre-existing token " << i); + CHECK(state.keyframes_mask[static_cast(i)] == 0.0F); + } + for (int64_t i = before; i < before + 3; ++i) { + INFO("new token " << i); + CHECK(state.keyframes_mask[static_cast(i)] == 1.0F); + } + } +} + +TEST_CASE("ltx2 conditioning: clear_conditioning TRIMS an append back to the target grid") { + const vllm::Ltx2VideoLatentShape target = CondVideoTarget(); + const vllm::Ltx2ScaleFactors factors; + vllm::Ltx2LatentState state = + vllm::Ltx2CreateVideoLatentState(target, kCondPatch, factors, kCondFps, true); + + const int64_t target_tokens = state.tokens; + REQUIRE(target_tokens == vllm_test::kLtx2CondVideoBaseTokens); + const std::vector target_positions = state.positions; + + const vllm::Ltx2LatentVolume keyframe = CondVolume("ltx2.cond.keyframe", 4, 1, 2, 2); + vllm::Ltx2ConditionVideoByKeyframe(&state, keyframe, kCondPatch, factors, kCondFps, + vllm_test::kLtx2CondKeyframeFrameIdx, /*strength=*/0.6, + /*num_pixel_frames=*/1, /*causal_fix=*/true); + REQUIRE(state.tokens > target_tokens); + // The appended tokens carry `1 - strength` = 0.4, which is what makes the + // all-ones assertion below a real check rather than a restatement. + REQUIRE(state.mask.back() == doctest::Approx(0.4F)); + + vllm::Ltx2ClearConditioning(&state, target_tokens); + + // `latent`, `clean_latent` and `positions` truncated to + // `patchifier.get_token_count(target_shape)` (tools.py:101-105). + CHECK(state.tokens == target_tokens); + CHECK(static_cast(state.latent.size()) == target_tokens * state.width); + CHECK(static_cast(state.clean.size()) == target_tokens * state.width); + + // THE MASK COMES BACK ALL ONES, not the conditioned mask sliced + // (tools.py:104 — `torch.ones_like(latent_state.denoise_mask)[:, :num_tokens]`). + // Slicing instead would leave 0.4 on nothing here, but on the two-stage recipe + // it would carry a conditioned mask into the next phase's initial latent. + REQUIRE(static_cast(state.mask.size()) == target_tokens); + for (int64_t i = 0; i < target_tokens; ++i) { + INFO("mask token " << i); + CHECK(state.mask[static_cast(i)] == 1.0F); + } + + // POSITIONS ARE TRIMMED PER DIMENSION. They are [pos_dims, tokens, 2], so a + // plain resize keeps the first dimension's APPENDED tokens and drops the last + // dimension's real ones — and the result still has the right length. Held to + // the target's own positions byte for byte, which is the only statement that + // can see the difference. + REQUIRE(state.positions.size() == target_positions.size()); + CHECK(state.positions == target_positions); + + // `keyframes_mask=None` (tools.py:113). + CHECK(state.keyframes_mask.empty()); +} + TEST_CASE("ltx2 conditioning: the audio state and its REFERENCE AUDIO append") { const vllm::Ltx2AudioLatentShape target = CondAudioTarget(); const vllm::Ltx2AudioPatchifierParams params; diff --git a/tests/vllm/multimodal/test_ltx2_video.cpp b/tests/vllm/multimodal/test_ltx2_video.cpp index c997b61b6..abb1995b7 100644 --- a/tests/vllm/multimodal/test_ltx2_video.cpp +++ b/tests/vllm/multimodal/test_ltx2_video.cpp @@ -117,6 +117,12 @@ std::vector ReadFloats(const std::string& path) { return out; } +void WriteBytes(const std::string& path, const std::string& bytes) { + std::ofstream out(path, std::ios::binary); + REQUIRE_MESSAGE(out.good(), "cannot write ", path); + out.write(bytes.data(), static_cast(bytes.size())); +} + void WriteFloats(const std::string& path, const std::vector& values) { std::ofstream out(path, std::ios::binary); REQUIRE_MESSAGE(out.good(), "cannot write ", path); @@ -1153,36 +1159,31 @@ TEST_CASE("ltx2 video: keyframe and reference conditioning is refused BY WHAT IS } }; - SUBCASE("a LAST-frame keyframe names the TOKEN-APPEND machinery, not the embedding") { - const std::string msg = refusal("a last-frame keyframe", - [](vllm::multimodal::VideoGenParams& g, const Workspace& w) { - g.last_frame_path = w.paths.video_embeds; - }); - INFO(msg); - // THIS ASSERTION USED TO PIN A FALSE REASON. It required the message to - // blame `keyframes_abs_pos_embedding`, and at pin `fd4ded7f` that is not - // what blocks a supplied keyframe: `apply_to` appends it with - // `marked=False` (keyframe_cond.py:84-86) and the sole consumer adds - // `mask * embedding` (transformer_args.py:42-43, called at :269), so the - // embedding contributes exactly nothing to those tokens. Porting it would - // not serve this arm. The gate enforced the wrong thing, which is worse - // than not gating the message at all. + SUBCASE("a LAST-frame keyframe is no longer refused — it is SERVED") { + // THIS SUBCASE USED TO ASSERT A REFUSAL, and before that it asserted a FALSE + // one: it required the message to blame `keyframes_abs_pos_embedding`, which + // at pin `fd4ded7f` is not what blocks a supplied keyframe — `apply_to` + // appends it with `marked=False` (keyframe_cond.py:84-86) and the sole + // consumer adds `mask * embedding` (transformer_args.py:42-43, called at + // :269), so the embedding contributes nothing to those tokens. // - // What actually blocks it is the append: extended `positions`, - // `update_attention_mask`, extended `clean_latent` / `denoise_mask`, and - // `clear_conditioning` trimming back — none of which this engine's - // fixed-length phase loop can express. - CHECK(msg.find("update_attention_mask") != std::string::npos); - CHECK(msg.find("clear_conditioning") != std::string::npos); - CHECK(msg.find("keyframe_cond.py") != std::string::npos); - CHECK(msg.find("VAE_ENCODER_COMFY_KEYS_FILTER") == std::string::npos); - // The refuted reason may still be NAMED — it is worth telling a reader that - // it was ruled out — but never as the thing that is missing, and only next - // to the issue that tracks where the embedding really does bite (#658). - if (msg.find("keyframes_abs_pos_embedding") != std::string::npos) { - CHECK(msg.find("NOT* THE REASON") != std::string::npos); - CHECK(msg.find("#658") != std::string::npos); - } + // The reason it then named — the token-APPEND machinery — was the true one, + // and row LTX25-TOKEN-APPEND (#930) built it. So the arm is checked here for + // NOT refusing, and what it actually does is gated by "a LAST-frame keyframe + // is APPENDED, and the sequence is trimmed back", which compares rendered + // bytes against a no-op control. + // + // The check is kept in THIS case rather than only in that one because this + // is the case a reader consults to ask "which conditioning arms are refused + // today", and an arm that silently disappeared from it would leave that + // question answered wrongly. + vllm::multimodal::VideoGenParams gen = FixtureGen(ws.root + "/served_last_frame"); + const std::string ppm = ws.root + "/served_last_frame.ppm"; + WriteBytes(ppm, ConditioningPpm(20, 28, 9)); + gen.last_frame_path = ppm; + gen.extras[vllm::multimodal::kLtx2ImageCrfExtra] = "0"; + const vllm::multimodal::VideoResult result = engine->Generate(gen); + CHECK(result.frame_count == 9); } SUBCASE("a reference video names the IC-LoRA metadata this project does not read") { const std::string msg = refusal("a reference video", @@ -2034,6 +2035,25 @@ std::string RenderBytes(vllm::multimodal::VideoModelParams mp, const std::string return all; } +// The same render, driven by a REQUEST the caller chose. `RenderBytes` fixes the +// request at `FixtureGen`, which is what the connector cases want and what a +// conditioning case cannot use. +std::string RenderBytesWithGen(vllm::multimodal::VideoModelParams mp, + const vllm::multimodal::VideoGenParams& gen) { + mp.extras[vllm::multimodal::kLtx2MaxPhaseExtra] = "0"; + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(mp); + const vllm::multimodal::VideoResult result = engine->Generate(gen); + std::string all; + for (int64_t f = 0; f < result.frame_count; ++f) { + char name[64]; + std::snprintf(name, sizeof(name), "/frame_%06lld.ppm", static_cast(f)); + all += ReadAll(gen.output_dir + name); + } + all += ReadAll(result.audio_path); + return all; +} + std::string RefusalOf(const vllm::multimodal::VideoModelParams& mp) { try { (void)vllm::multimodal::LoadVideoEngine(mp); @@ -2144,6 +2164,158 @@ TEST_CASE("ltx2 video: the keyframe marker reaches the PIXELS with no image supp CHECK(RenderBytes(with_marker, ws.root + "/kf_marked2") == with); } +// ─── the token-APPEND seam (row LTX25-TOKEN-APPEND, issue #930) ───────────── +// +// UPSTREAM SHIPS NO TESTS at pin `fd4ded7f` — `find /home/mudler/_git/LTX-2 +// -name 'test_*.py'` returns 0 across the whole repository — so nothing is +// ported here. Every assertion cites the upstream `file:line` that justifies it +// instead. +// +// THE WITNESS IS ON RENDERED BYTES, and that is the whole design. `Ltx2ConditioningTrace` +// is filled before the denoise loop for every field except the handful written +// inside it, so a witness built on the trace cannot observe what the loop does — +// a sibling row's first attempt at exactly this found every arm identical for +// that reason. +// +// AND IT CARRIES A NO-OP CONTROL, which is the correction that made the sibling's +// result diagnosable. Their arms came out identical INCLUDING the control, which +// is what said "the instrument is blind" rather than "the feature is weak". +// Without the control those two read the same, and the wrong one is the one that +// ships. So the comparison set below is {no keyframe, keyframe A, keyframe B}: +// +// * every arm equal, control included => the instrument is blind; +// * kf_a != noop => the append reached the maths; +// * kf_a != kf_b => the appended CONTENT reached it, +// not merely the token count. +TEST_CASE("ltx2 video: a LAST-frame keyframe is APPENDED, and the sequence is trimmed back") { + Workspace ws; + + // Deliberately not the render's own resolution: `load_image_and_preprocess` + // aspect-fills and centre-crops to the phase's height/width + // (media_io/resize.py:41-73). + const std::string kf_a_path = ws.root + "/kf_a.ppm"; + const std::string kf_b_path = ws.root + "/kf_b.ppm"; + WriteBytes(kf_a_path, ConditioningPpm(20, 28, 21)); + WriteBytes(kf_b_path, ConditioningPpm(20, 28, 22)); + + auto request = [&](const std::string& tag, const std::string& keyframe) { + vllm::multimodal::VideoGenParams gen = FixtureGen(ws.root + "/" + tag); + if (!keyframe.empty()) { + gen.last_frame_path = keyframe; + // The codec round trip is unported and an LTX-2.5 checkpoint RESOLVES 18, + // so the supported arm has to be asked for. Same rule as the first-frame + // arm, because upstream resolves the CRF once for the whole `images` list + // (blocks.py:966-983). + gen.extras[vllm::multimodal::kLtx2ImageCrfExtra] = "0"; + } + return gen; + }; + + const vllm::multimodal::VideoModelParams mp = FixtureParams(ws.paths); + const std::string noop = RenderBytesWithGen(mp, request("kf_noop", "")); + const std::string kf_a = RenderBytesWithGen(mp, request("kf_a", kf_a_path)); + const std::string kf_b = RenderBytesWithGen(mp, request("kf_b", kf_b_path)); + REQUIRE(noop.size() == kf_a.size()); + REQUIRE(noop.size() == kf_b.size()); + + // THE CONTROL FIRST. A re-render of the no-keyframe request must be byte + // identical, otherwise every inequality below is noise and this case says + // nothing about appends. + REQUIRE_MESSAGE(RenderBytesWithGen(mp, request("kf_noop2", "")) == noop, + "the same request rendered twice is not byte-identical, so this instrument " + "cannot measure anything"); + + auto differing = [](const std::string& a, const std::string& b) { + size_t n = 0; + for (size_t i = 0; i < a.size(); ++i) { + if (a[i] != b[i]) ++n; + } + return n; + }; + MESSAGE("kf_a vs noop: " << differing(kf_a, noop) << " of " << noop.size() << " bytes; " + << "kf_a vs kf_b: " << differing(kf_a, kf_b)); + + // The appended tokens take part in self-attention over the WHOLE sequence, so + // a keyframe that reached the maths moves the target tokens' own output. This + // is the claim the refusal that stood here was about: the engine could not + // grow the sequence through the DiT. + CHECK_MESSAGE(differing(kf_a, noop) > 0, + "a last-frame keyframe rendered the same bytes as a render with no keyframe at " + "all, so the appended tokens never reached the forward"); + // ...and it is the keyframe's CONTENT that reached it. Two keyframes append + // the same NUMBER of tokens, so a build that grew the sequence with zeros — + // or that appended the wrong buffer — passes the check above and fails this + // one. + CHECK_MESSAGE(differing(kf_a, kf_b) > 0, + "two DIFFERENT last-frame keyframes rendered identical bytes, so the appended " + "tokens carry no content from the keyframe"); + + SUBCASE("the sequence GROWS through the DiT and comes back to the target grid") { + vllm::multimodal::VideoModelParams capped = FixtureParams(ws.paths); + capped.extras[vllm::multimodal::kLtx2MaxPhaseExtra] = "0"; + + auto tokens_of = [&](const std::string& tag, const std::string& keyframe) { + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(capped); + auto* ltx2 = dynamic_cast(engine.get()); + REQUIRE(ltx2 != nullptr); + const vllm::multimodal::VideoResult result = engine->Generate(request(tag, keyframe)); + // The artifact is the other half of the claim: the trim is what lets + // `Ltx2VideoUnpatchify` produce a target-shaped volume, and the frame + // count is that shape observed from outside. + CHECK(result.frame_count == 9); + return ltx2->last_conditioning().video_tokens; + }; + + const int64_t plain = tokens_of("tok_noop", ""); + const int64_t with_kf = tokens_of("tok_kf", kf_a_path); + + // The fixture's phase 0 runs at `spatial_downscale = 2`, so 64x64 pixels is a + // 1x1 latent grid and 9 frames is 2 latent frames: 2 target tokens. One + // encoded keyframe is one latent frame at that grid, so it appends exactly + // `tokens_per_latent_frame` = 1 (tools.py:198-201). + CHECK(plain == 2); + CHECK_MESSAGE(with_kf == plain + 1, + "a keyframe must append one latent frame's worth of tokens " + "(keyframe_cond.py:79-82); got " << with_kf << " against a target of " << plain); + } + + SUBCASE("the sigma schedule keeps reading the TARGET count, not the grown one") { + // The distilled two-stage recipe carries its own frozen sigmas + // (ltx2_recipes.py:125-158), so it never computes a schedule and cannot show + // this. `one_stage` does: `phase.sigmas` is empty, so the engine calls + // `Ltx2SigmaSchedule`, whose shift is a function of the token count + // (schedulers.py:37-39). + // + // Upstream fixes that count at the TARGET twice over: the argument is + // `math.prod(latent.shape[2:])` of the UNPATCHIFIED target, which cannot + // contain appended tokens, and `ti2vid_one_stage.py:207` computes the + // schedule before any state exists. A port that read the grown count would + // re-shift the entire trajectory the moment a keyframe was supplied. + vllm::multimodal::VideoModelParams one_stage = FixtureParams(ws.paths); + one_stage.extras[vllm::multimodal::kLtx2PipelineKindExtra] = "one_stage"; + + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(one_stage); + auto* ltx2 = dynamic_cast(engine.get()); + REQUIRE(ltx2 != nullptr); + + vllm::multimodal::VideoGenParams gen = request("one_stage_kf", kf_a_path); + gen.steps = 2; // one_stage admits a step override; 50 would gate nothing extra + (void)engine->Generate(gen); + const vllm::multimodal::Ltx2ConditioningTrace trace = ltx2->last_conditioning(); + + // Both numbers are MEASURED, and the statement is the relation between them. + // Pinning either to a literal would pass on a build that read the grown + // count everywhere. + CHECK(trace.schedule_tokens > 0); + CHECK_MESSAGE(trace.video_tokens > trace.schedule_tokens, + "the DiT ran over " << trace.video_tokens << " tokens and the schedule was " + << "built for " << trace.schedule_tokens + << "; equal means the append re-shifted the schedule"); + } +} + TEST_CASE("ltx2 video: the connector's positional bound comes from the CONFIG") { // `connector_positional_embedding_max_pos` divides every token index // (rope.py:132-141). LTX-2.5 declares [4096]; the class default is [1], which From bccf44aeade95a9367ca6c32a832e8c51f3a77f7 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 15 Aug 2026 18:59:54 +0000 Subject: [PATCH 3/4] spec(LTX25-TOKEN-APPEND): record what the mutation pass could NOT reach (#930) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three limits a reviewer should press on rather than rediscover, all of them found while running the pass rather than reasoned about afterwards. The sigma-schedule check has a residual. `schedule_tokens` is one local feeding both the schedule and the trace, so the ordinary mutation moves both and REDs — but a mutation editing only the call argument would not be caught. Nothing local can close that, because the instrument and the measured expression would have to become the same thing. The trim is gated on a guard rather than on pixels. Appended tokens sit at the tail of a contiguous buffer and `Ltx2VideoUnpatchify` takes a bare pointer, so an un-trimmed state renders pixel-identical frames. M3 REDs on the `VT_CHECK` at the unpatchify boundary, and saying so is the difference between an invariant this row asserts and a difference this row can observe. `Ltx2ExtendKeyframesMask(marked=true)` lands with a unit driver and no product caller; it was already under `## Owed` and is now also named here. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/specs/ltx25-token-append.md | 32 ++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/.agents/specs/ltx25-token-append.md b/.agents/specs/ltx25-token-append.md index 0f74bf526..2693ef20b 100644 --- a/.agents/specs/ltx25-token-append.md +++ b/.agents/specs/ltx25-token-append.md @@ -297,6 +297,32 @@ Planned mutations: discovered later, per `.agents/reachability.md` `## Landing a slice that is not reached yet`. Owner: row `LTX25-GENERATED-KEYFRAMES`, issue #920. -## 9. Now - -`ACTIVE` — spec committed, implementation to follow on the same branch. +## 9. What the mutation pass could NOT reach + +Recorded because a reviewer should press on it rather than rediscover it. + +**The sigma-schedule binding has a residual.** `schedule_tokens` is a local that +feeds both `Ltx2SigmaSchedule` and the trace, so the ordinary mutation — changing +what the local is initialised from — moves both and REDs (M4). A mutation that +edited only the *call argument* and left the local alone would not be caught by +that field. Nothing local can close this: the instrument and the thing it +measures would have to be the same expression, and then it would measure nothing. +The pixel witness does not close it either, because it has no +correct-schedule render to compare against. + +**The trim is gated on a guard, not on pixels.** Appended tokens sit at the tail +of a contiguous `[tokens, width]` buffer and `Ltx2VideoUnpatchify` takes a bare +pointer, so an un-trimmed state unpatchifies the same head bytes and renders +pixel-identical frames. M3 therefore REDs on the `VT_CHECK` at the unpatchify +boundary rather than on any output difference. That is the honest description: +the trim's correctness on this engine's path is an invariant this row asserts, +not a difference this row can observe. + +**`Ltx2ExtendKeyframesMask(marked=true)` has a unit driver and no production +caller.** See `## Owed`. + +## 10. Now + +`ACTIVE` — spec committed before implementation; implementation, docs and tests +on the same branch; PR [#948](https://github.com/mudler/vllm.cpp/pull/948) open +and awaiting a fresh review. From b5618b305d0fb9269f1d013dcb1712ac37e4eb7d Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sat, 15 Aug 2026 20:12:21 +0000 Subject: [PATCH 4/4] fix(LTX25-TOKEN-APPEND): gate the two guarantees that survived mutation, and the anchors that pointed somewhere real and wrong (#930) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fresh review returned FAIL on six findings. The seam survived every one of them: the upstream design decisions checked out and the pixel witness reproduced exactly. What did not survive was the evidence around it. TWO ADVERTISED GUARANTEES HAD NO MUTATION THAT COULD MOVE THEM. M6 — `Ltx2ClearConditioning` slices the mask instead of restoring all ones — was GREEN. `Ltx2CreateVideoLatentState` already fills every target token's mask with 1.0, and the append writes `1 - strength` only at the TAIL, which the trim drops under either implementation. Over the range the loop walked, slice and restore produce identical bytes, so the case that carries the header's "TWO THINGS HERE ARE NOT A TRUNCATION" claim was asserting a value nothing could change. The case now conditions a token INSIDE the target first, through `Ltx2ConditionVideoByLatentIndex` at `latent_idx = 0`, which writes `1 - strength` at `start .. start + count` (latent_cond.py:41). A slice leaves 0.4 at token 0 and the loop REDs. It also REQUIREs that 0.4 BEFORE the trim, so a later change that stops arming the instrument fails loudly instead of quietly restoring the blind state. M10 — the last-frame arm's `frame_idx` becomes 0 instead of `frames - 1` — was GREEN. Both renders still differed from the no-op control and from each other, and the token count was identical, because a keyframe pinned to the FIRST frame appends exactly as many tokens as one pinned to the last. The pixel witness can see THAT an append happened and WHAT was appended; nothing could see WHERE, which is the entire content of the arm this row exists to lift. The engine now asserts the first appended token's temporal position, recomputed from `frames` and `fps` rather than read back from the `frame_idx` argument, so the check and the thing it checks are independent expressions (keyframe_cond.py:52-58). FOUR ANCHORS RESOLVED TO REAL BUT DIFFERENT UPSTREAM STATEMENTS, which is worse than a dangling one: a reader who checks is misled rather than alerted. `tools.py:103` is `clean_latent = ...[:, :num_tokens]`; the all-ones restore is :104. `tools.py:112` is `attention_mask=None`; the keyframes-mask drop is :113. And `schedulers.py:38-39` is the shift arithmetic, not the `tokens = math.prod(latent.shape[2:])` it was quoted for, which is :32. The issue-index row is fixed first and SHA-anchored, because that file is append-only and the row could not be corrected after landing. The sweep behind those was redone from scratch rather than trusted, deriving each needle from the CLAIM instead of reading it out of the cited span. It covers 117 citations in this row's added lines — 107 explicit and 10 bare `:NN` continuations — and found two the review had not. `ltx2_recipes.py:125-158` cites a file that exists neither upstream nor here; the distilled recipe's frozen sigmas are distilled.py:200-201 over utils/constants.py:17-23. And a bare `(:100-101)` in the extend-mask case resolves against `tools.py`, the nearest file named above it, whose :100-101 is a real and unrelated statement; it means mask_utils.py. The two bare continuations in `Ltx2ClearConditioning`'s declaration are now spelled with their file for the same reason: the nearest name above them is `blocks.py`, and blocks.py:104 and :113 are both real imports. Spec §2 claimed the arm is driven from `last_frame_path` / `last_frame_ppm`. There is no `last_frame_ppm` field — `video_engine.h:90-91` declares the in-memory alternative on the first-frame side only — and the code correctly uses `last_frame_path` alone, so the sentence was wrong rather than the code. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/issue-index.md | 2 +- .agents/specs/ltx25-token-append.md | 59 ++++++++++++++++--- .../model_executor/models/ltx2_conditioning.h | 14 +++-- include/vllm/multimodal/ltx2_video.h | 2 +- src/vllm/multimodal/ltx2_video.cpp | 34 ++++++++++- tests/vllm/models/test_ltx2_vae.cpp | 32 +++++++++- tests/vllm/multimodal/test_ltx2_video.cpp | 4 +- 7 files changed, 127 insertions(+), 20 deletions(-) diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 85f28da44..01c066928 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -249,4 +249,4 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#810](https://github.com/mudler/vllm.cpp/issues/810) | `MODEL-TEXT-nemotron-h-nemotron-hfor-causal-lm` | NemotronH is not reachable through `include/vllm.h`: `GPUModelRunner::initialize_kv_cache` rebuilds the RECURRENT half of the allocation from `config_.linear_*` instead of the `MambaSpec` the model published, so every non-Qwen3.5 hybrid is refused by Qwen3.5's name at `runner.cpp:525`, and per-layer membership comes from `config_.layer_types[l] == "linear_attention"` rather than `KVCacheGroupSpec::layer_names`. Spec [`nemotron-h-abi-e2e.md`](specs/nemotron-h-abi-e2e.md); the attention half at `runner.cpp:539-607` is already spec-driven and is the model to mirror. Note the SAFETY constraint recorded there: neutering the check alone reaches a forward that ignores `attn_kv`/`gdn_state`/`num_reqs`, which is strictly more dangerous than the refusal | bug | | [#873](https://github.com/mudler/vllm.cpp/issues/873) | `GATE-CI-CONCURRENCY` | `main` went RED on six release/registration gates after the #865 `ci.yml` rewrite: `check-release-binary-contract.py` and `check-test-registration.py` credit a checker to CI only through an UNCONDITIONAL job, and #865 gave `agent-record` an `if:`; the closed-PR skip is re-expressed through `needs:` and the byte-exact Windows PR proof schema restored (spec [`ci-concurrency.md`](specs/ci-concurrency.md)) | bug | | [#874](https://github.com/mudler/vllm.cpp/issues/874) | — | `windows-msvc-cpu`/`windows-msvc-vulkan` still start on a CLOSED pull request: `check-release-workflow.py::validate_pr_ci` compares their whole job mapping for equality, so neither an `if:` clause nor a `needs:` guard can be added. Listed under `## Owed` in [`ci-concurrency.md`](specs/ci-concurrency.md) | bug | -| [#930](https://github.com/mudler/vllm.cpp/issues/930) | `LTX25-TOKEN-APPEND` | LTX-2.5's phase loop is fixed at one `Ltx2VideoTokenCount(vshape, 1)`, and that single limitation blocks THREE conditioning arms, not the two #930 was written against: reference video, the LAST-frame keyframe, and generated keyframe slots ([#920](https://github.com/mudler/vllm.cpp/issues/920)). Row `LTX25-TOKEN-APPEND` (spec [`ltx25-token-append.md`](specs/ltx25-token-append.md)) ports the two missing halves of the append — `extend_keyframes_mask` (`mask_utils.py:74-105`), which upstream's own docstring says EVERY appending item must call, and `clear_conditioning` (`tools.py:88-117`), which trims back to the target count and restores an ALL-ONES mask rather than the conditioned one — and lifts the last-frame keyframe as the demonstration. The attention mask is NOT the gap and no field is added for it: both ported video items pass a literal `attention_mask=None` (`keyframe_cond.py:68-76`, `reference_video_cond.py:88-96`) and the only route to a non-None mask is `ConditioningItemAttentionStrengthWrapper`, applied solely at `iclora_utils.py:169`. The sigma schedule must keep reading the TARGET count — `math.prod(latent.shape[2:])` (`schedulers.py:38-39`) is the unpatchified target and cannot see an append — so the engine's `Ltx2SigmaSchedule(steps, video.tokens)` call, which sits AFTER the conditioning block (`src/vllm/multimodal/ltx2_video.cpp:1719 @ bc6433d1b`), re-shifts the whole schedule the moment anything appends. Reference video and generated slots stay refused: at `bc6433d1b` the reference refusal's LoRA-metadata cause is still true because PR [#938](https://github.com/mudler/vllm.cpp/pull/938) is open and unmerged | feature | +| [#930](https://github.com/mudler/vllm.cpp/issues/930) | `LTX25-TOKEN-APPEND` | LTX-2.5's phase loop is fixed at one `Ltx2VideoTokenCount(vshape, 1)`, and that single limitation blocks THREE conditioning arms, not the two #930 was written against: reference video, the LAST-frame keyframe, and generated keyframe slots ([#920](https://github.com/mudler/vllm.cpp/issues/920)). Row `LTX25-TOKEN-APPEND` (spec [`ltx25-token-append.md`](specs/ltx25-token-append.md)) ports the two missing halves of the append — `extend_keyframes_mask` (`mask_utils.py:74-105`), which upstream's own docstring says EVERY appending item must call, and `clear_conditioning` (`tools.py:88-117`), which trims back to the target count and restores an ALL-ONES mask rather than the conditioned one — and lifts the last-frame keyframe as the demonstration. The attention mask is NOT the gap and no field is added for it: both ported video items pass a literal `attention_mask=None` (`keyframe_cond.py:68-76`, `reference_video_cond.py:88-96`) and the only route to a non-None mask is `ConditioningItemAttentionStrengthWrapper`, applied solely at `iclora_utils.py:169`. The sigma schedule must keep reading the TARGET count — `math.prod(latent.shape[2:])` (`schedulers.py:32 @ fd4ded7fa`) is the unpatchified target and cannot see an append — so the engine's `Ltx2SigmaSchedule(steps, video.tokens)` call, which sits AFTER the conditioning block (`src/vllm/multimodal/ltx2_video.cpp:1719 @ bc6433d1b`), re-shifts the whole schedule the moment anything appends. Reference video and generated slots stay refused: at `bc6433d1b` the reference refusal's LoRA-metadata cause is still true because PR [#938](https://github.com/mudler/vllm.cpp/pull/938) is open and unmerged | feature | diff --git a/.agents/specs/ltx25-token-append.md b/.agents/specs/ltx25-token-append.md index 2693ef20b..d5aa60aa9 100644 --- a/.agents/specs/ltx25-token-append.md +++ b/.agents/specs/ltx25-token-append.md @@ -118,8 +118,11 @@ In scope: * The engine's phase loop: a `target_tokens` that the sigma schedule and the trim both read, a grown `video.tokens` through the DiT, and the trim before `Ltx2VideoUnpatchify`. -* The **last-frame keyframe** arm, lifted, driven from `VideoGenParams::last_frame_path` - / `last_frame_ppm`. +* The **last-frame keyframe** arm, lifted, driven from + `VideoGenParams::last_frame_path` **alone**. There is no `last_frame_ppm` + field: `video_engine.h:90-91` declares `first_frame_path, last_frame_path` and + a `first_frame_ppm` for the server's `data:` URLs, and the in-memory + alternative exists on the first-frame side only. Out of scope, and refused as today: @@ -139,8 +142,8 @@ Out of scope, and refused as today: per slot (`types.py:269-273`). The seam this row builds is its prerequisite, not its implementation. * **Reference audio.** Blocked on the audio VAE encoder key filter, untouched. -* Any `include/vllm.h` growth. `last_frame` / `last_frame_ppm` already exist on - the ABI; nothing new is exposed. +* Any `include/vllm.h` growth. `last_frame` already exists on the ABI + (`vllm.h:910`); nothing new is exposed. ## 3. Design @@ -213,8 +216,8 @@ which is the production entry point. 1. **`test_ltx2_vae` — the seam, at the unit.** `Ltx2ExtendKeyframesMask` on both `marked` polarities and on the empty-mask branch (`mask_utils.py:74-105`). `Ltx2ClearConditioning` restores the target token count, restores an - **all-ones** mask rather than the conditioned one (`tools.py:103`), trims - positions per dimension, and drops the keyframes mask (`tools.py:112`). + **all-ones** mask rather than the conditioned one (`tools.py:104`), trims + positions per dimension, and drops the keyframes mask (`tools.py:113`). `AppendTokens` keeps `keyframes_mask.size() == tokens` across an append. 2. **`test_ltx2_video` — the arm, through the entry point.** A last-frame keyframe renders instead of refusing, and the refusal case's last-frame @@ -270,6 +273,36 @@ Planned mutations: | M4 | point the sigma schedule back at `video.tokens` | RED | | M5 | append with `marked=true` instead of `false` | RED | +Two more were added by the fresh review, and both were **GREEN** on the first +implementation. They are recorded here because each names a guarantee this row +advertises, and a guarantee no mutation can move is a comment rather than a +gate: + +| id | mutation | expected | +|---|---|---| +| M6 | `Ltx2ClearConditioning` SLICES the mask instead of restoring all ones | RED | +| M10 | the last-frame arm's `frame_idx` becomes `0` instead of `frames - 1` | RED | + +**Why M6 was green.** `Ltx2CreateVideoLatentState` already fills every target +token's mask with `1.0`, and the append writes `1 - strength` only at the TAIL, +which the trim drops under either implementation. So over the range the test +walked, "restore all ones" and "slice" produce identical bytes. The repair +conditions a token INSIDE the target first, with +`Ltx2ConditionVideoByLatentIndex` at `latent_idx = 0`, which writes +`1 - strength` at `start .. start + count` (`latent_cond.py:41`); a slice then +leaves `0.4` at token 0. The case also `REQUIRE`s that value BEFORE the trim, so +a future change that stops arming the instrument fails loudly rather than +returning the case to gating nothing. + +**Why M10 was green.** Both renders still differed from the no-op control and +from each other, and the token count was identical, because a keyframe pinned to +the FIRST frame appends exactly as many tokens as one pinned to the last. The +pixel witness can see THAT an append happened and WHAT was appended; it cannot +see WHERE. The repair asserts the first appended token's temporal position at +the engine, recomputed from `frames` and `fps` rather than read back from the +`frame_idx` argument, so the check and the thing it checks are independent +expressions. + ## 7. Stop conditions * Stop and report `NEEDS_DECISION` rather than lifting the reference-video arm: @@ -324,5 +357,15 @@ caller.** See `## Owed`. ## 10. Now `ACTIVE` — spec committed before implementation; implementation, docs and tests -on the same branch; PR [#948](https://github.com/mudler/vllm.cpp/pull/948) open -and awaiting a fresh review. +on the same branch; PR [#948](https://github.com/mudler/vllm.cpp/pull/948) open. + +A fresh review returned `FAIL` on six findings and all six are repaired. The +seam itself survived: every upstream design decision checked out and the pixel +witness reproduced exactly. What did not survive was the evidence around it — +two advertised guarantees had no mutation that could move them (§6, M6 and M10), +four records carried anchors that resolve to real-but-different upstream +statements, and `origin/main` advanced to `c2019b0e3` (#935) mid-review so the +branch stopped merging. The anchor sweep was redone from the claims rather than +trusted, and it found two more the review had not: a citation to +`ltx2_recipes.py`, which exists neither upstream nor here, and a bare `:100-101` +whose nearest named file was the wrong one. diff --git a/include/vllm/model_executor/models/ltx2_conditioning.h b/include/vllm/model_executor/models/ltx2_conditioning.h index ded31a8eb..d6852203c 100644 --- a/include/vllm/model_executor/models/ltx2_conditioning.h +++ b/include/vllm/model_executor/models/ltx2_conditioning.h @@ -182,10 +182,16 @@ void Ltx2ExtendKeyframesMask(Ltx2LatentState* state, int64_t num_new_tokens, boo // // TWO THINGS HERE ARE NOT A TRUNCATION and a port that only slices gets both // wrong. The denoise mask comes back as `torch.ones_like(...)[:, :num_tokens]` -// (:103) — ALL ONES, not the conditioned mask sliced — because the returned -// state describes a finished latent in which every target token is denoised. -// And `keyframes_mask` is dropped to None (:112), because the marker described a -// sequence that no longer exists. +// (tools.py:104) — ALL ONES, not the conditioned mask sliced — because the +// returned state describes a finished latent in which every target token is +// denoised. And `keyframes_mask` is dropped to None (tools.py:113), because the +// marker described a sequence that no longer exists. +// +// Both anchors are spelled with their FILE rather than left as bare `:NN` +// continuations. The nearest file named above them is `blocks.py`, and +// `blocks.py:104` and `blocks.py:113` are both real import statements, so a bare +// form would send a reader who checks to a plausible wrong place rather than to +// nothing — which is the failure mode worth spending eight characters on. void Ltx2ClearConditioning(Ltx2LatentState* state, int64_t target_tokens); // AudioConditionByReferenceLatent (reference_audio_cond.py:33-65): APPEND already diff --git a/include/vllm/multimodal/ltx2_video.h b/include/vllm/multimodal/ltx2_video.h index 6cb46cc64..260243a90 100644 --- a/include/vllm/multimodal/ltx2_video.h +++ b/include/vllm/multimodal/ltx2_video.h @@ -411,7 +411,7 @@ struct Ltx2ConditioningTrace { // over on the LAST phase — the target grid plus whatever an appending // conditioning item added (keyframe_cond.py:79-82). `schedule_tokens` is the // count the sigma schedule read, which upstream fixes at the TARGET: its shift - // comes from `math.prod(latent.shape[2:])` (schedulers.py:38-39), the + // comes from `math.prod(latent.shape[2:])` (schedulers.py:32), the // UNPATCHIFIED target latent, and the pipelines compute sigmas before any state // exists (ti2vid_one_stage.py:207, distilled.py:200-201). So a render that // appends must show `video_tokens > schedule_tokens`, and a build that let the diff --git a/src/vllm/multimodal/ltx2_video.cpp b/src/vllm/multimodal/ltx2_video.cpp index ff81abcd9..495973cdc 100644 --- a/src/vllm/multimodal/ltx2_video.cpp +++ b/src/vllm/multimodal/ltx2_video.cpp @@ -1722,7 +1722,8 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { // equal to it and is GROWN by any appending conditioning item. Row // LTX25-TOKEN-APPEND (#930) split the two, and the two places that must keep // reading the target rather than the grown count are the sigma schedule - // (schedulers.py:38-39 derives its shift from the unpatchified target, and + // (schedulers.py:32-39 reads the unpatchified target at :32 and turns it into + // the shift at :39, and // the pipelines compute sigmas before the state exists at all) and the trim // at the bottom of the loop (`clear_conditioning`, tools.py:101). const int64_t target_tokens = Ltx2VideoTokenCount(vshape, 1); @@ -1973,6 +1974,35 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { "token. A buffer that did not grow with the others is invisible to the render's " "SHAPE — the clip comes out the right size and simply describes the wrong " "tokens."); + + // AND IT LANDED ON THE LAST FRAME. Everything above proves the sequence + // grew and stayed self-consistent; none of it can see WHERE the appended + // tokens sit in time, and that is the whole content of this arm. MEASURED: + // mutation M10 changed `frame_idx` from `frames - 1` to `0` and the suite + // stayed GREEN — both renders still differed from the no-op control and + // from each other, and the token count was identical, because a keyframe + // pinned to the FIRST frame appends exactly as many tokens as one pinned + // to the last. + // + // The expectation is recomputed from `frames` and `fps`, NOT read back + // from the `frame_idx` argument above, so the two are independent + // expressions and a mutation of the argument alone moves one and not the + // other. `Ltx2ConditionVideoByKeyframe` offsets the item's temporal + // coordinates by `frame_idx` in integer PIXEL space and then divides the + // temporal axis by fps (keyframe_cond.py:52-58), so the first appended + // token's temporal START is `frame_idx / fps`. Positions are + // [pos_dims, tokens, 2] concatenated PER DIMENSION, so the temporal axis + // is dimension 0 and the first appended token sits at `target_tokens * 2`. + const double want_t0 = static_cast(static_cast( + static_cast(frames - 1) / fps)); + const double got_t0 = video.positions[static_cast(target_tokens * 2)]; + VT_CHECK(std::abs(got_t0 - want_t0) <= 1e-5 * std::max(1.0, std::abs(want_t0)), + "ltx2 video: the last-frame keyframe's appended tokens must carry the temporal " + "position of pixel frame `frames - 1` (" + + std::to_string(want_t0) + "), but the first appended token starts at " + + std::to_string(got_t0) + + ". A keyframe that appends the right number of tokens at the wrong TIME " + "renders a clip of the right length that pins the image to the wrong end"); } // The noiser draws VIDEO first, AUDIO second, from one generator @@ -2015,7 +2045,7 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { if (steps < 1) Fail("num_inference_steps resolved to " + std::to_string(steps)); // `target_tokens`, NOT `video.tokens`, and this line sits AFTER the // conditioning block so the distinction is live. Upstream's shift comes - // from `tokens = math.prod(latent.shape[2:])` (schedulers.py:38-39) — the + // from `tokens = math.prod(latent.shape[2:])` (schedulers.py:32) — the // UNPATCHIFIED target latent, which by construction cannot see appended // tokens — and every pipeline computes its sigmas before a state exists at // all (ti2vid_one_stage.py:207 passes no latent; distilled.py:200-201 uses diff --git a/tests/vllm/models/test_ltx2_vae.cpp b/tests/vllm/models/test_ltx2_vae.cpp index 1a0f8e3b9..12c8c2e0a 100644 --- a/tests/vllm/models/test_ltx2_vae.cpp +++ b/tests/vllm/models/test_ltx2_vae.cpp @@ -2479,7 +2479,9 @@ TEST_CASE("ltx2 conditioning: extend_keyframes_mask mirrors BOTH of upstream's N CHECK(state.keyframes_mask.empty()); } SUBCASE("no existing mask and MARKED zero-fills first, then marks the new tokens") { - // `existing = torch.zeros_like(latent_state.denoise_mask)` (:100-101) sized + // `existing = torch.zeros_like(latent_state.denoise_mask)` + // (mask_utils.py:100-101 — named in full because the nearest file above is + // `tools.py`, whose :100-101 is a real but unrelated statement) sized // by the state BEFORE the append, then ones for the new tokens. The one // upstream caller that passes true is `VideoGeneratedKeyframeSlots` // (keyframe_slots.py:121). @@ -2512,13 +2514,37 @@ TEST_CASE("ltx2 conditioning: clear_conditioning TRIMS an append back to the tar REQUIRE(target_tokens == vllm_test::kLtx2CondVideoBaseTokens); const std::vector target_positions = state.positions; + // A CONDITIONED MASK VALUE INSIDE THE TARGET RANGE, and without it the + // all-ones assertion below gates NOTHING. `Ltx2CreateVideoLatentState` already + // fills every target token's mask with 1.0, and the keyframe append writes its + // `1 - strength` only at the TAIL, which a slice to `target_tokens` drops + // anyway — so "restore all ones" and "slice" produce identical bytes over the + // range the loop walks, and a slicing build passes. MEASURED: mutation M6 + // sliced instead of restoring and this case stayed GREEN. + // + // `Ltx2ConditionVideoByLatentIndex` is the fix because it writes `1 - strength` + // at `start .. start + count` INSIDE the target (latent_cond.py:41; ours at + // ltx2_conditioning.cpp, the `state->mask[i] = 1.0 - strength` loop). At + // `latent_idx = 0` that is `start = 0`, `count = 1*2*2 = 4` of the 12 target + // tokens, so a slice leaves 0.4 at token 0 and the loop below REDs. + const vllm::Ltx2LatentVolume first = CondVolume("ltx2.cond.first", 4, 1, 2, 2); + vllm::Ltx2ConditionVideoByLatentIndex(&state, target, kCondPatch, first, /*strength=*/0.6, + /*latent_idx=*/0); + // THE INSTRUMENT IS ARMED, asserted rather than assumed. If this ever came + // back 1.0 the all-ones loop below would silently return to gating nothing, + // which is the exact state this case was repaired out of. + REQUIRE(state.mask.front() == doctest::Approx(0.4F)); + const vllm::Ltx2LatentVolume keyframe = CondVolume("ltx2.cond.keyframe", 4, 1, 2, 2); vllm::Ltx2ConditionVideoByKeyframe(&state, keyframe, kCondPatch, factors, kCondFps, vllm_test::kLtx2CondKeyframeFrameIdx, /*strength=*/0.6, /*num_pixel_frames=*/1, /*causal_fix=*/true); REQUIRE(state.tokens > target_tokens); - // The appended tokens carry `1 - strength` = 0.4, which is what makes the - // all-ones assertion below a real check rather than a restatement. + // The APPENDED tokens carry `1 - strength` = 0.4 too. This one is a check on + // the append, NOT on the clear: `.back()` sits past `target_tokens`, so the + // trim drops it under either implementation and it can say nothing about + // all-ones-versus-slice. The value that separates those is `mask.front()` + // above. REQUIRE(state.mask.back() == doctest::Approx(0.4F)); vllm::Ltx2ClearConditioning(&state, target_tokens); diff --git a/tests/vllm/multimodal/test_ltx2_video.cpp b/tests/vllm/multimodal/test_ltx2_video.cpp index 1bd84abce..ad2ed0719 100644 --- a/tests/vllm/multimodal/test_ltx2_video.cpp +++ b/tests/vllm/multimodal/test_ltx2_video.cpp @@ -2282,7 +2282,9 @@ TEST_CASE("ltx2 video: a LAST-frame keyframe is APPENDED, and the sequence is tr SUBCASE("the sigma schedule keeps reading the TARGET count, not the grown one") { // The distilled two-stage recipe carries its own frozen sigmas - // (ltx2_recipes.py:125-158), so it never computes a schedule and cannot show + // (distilled.py:200-201 defaults both stages to the `DISTILLED_SIGMAS` / + // `STAGE_2_DISTILLED_SIGMAS` constants of utils/constants.py:17-23), so it + // never computes a schedule and cannot show // this. `one_stage` does: `phase.sigmas` is empty, so the engine calls // `Ltx2SigmaSchedule`, whose shift is a function of the token count // (schedulers.py:37-39).