diff --git a/.agents/issue-index.md b/.agents/issue-index.md index fd6abcc0c..be88abad9 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -240,6 +240,7 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#907](https://github.com/mudler/vllm.cpp/issues/907) | `BACKEND-CUDA-COMP-CORE` | Five tests are red on dgx.casa (GB10, sm_121a) at `main`, proven PRE-EXISTING by a control build at `10b8bbdaa`: `test_capi` (SIGSEGV in an ABI v8 custom-logits-processor case, plausibly [#547](https://github.com/mudler/vllm.cpp/issues/547) or [#844](https://github.com/mudler/vllm.cpp/issues/844)), `test_cuda_ops` 439/440, `test_linear_method` 83/85, `test_ops_gdn` 4899/4900 ([#614](https://github.com/mudler/vllm.cpp/issues/614)), `test_qwen3_5_gdn_spec_routing` 119/123. Three of the five had no issue at all, which is why this exists | bug | | [#912](https://github.com/mudler/vllm.cpp/issues/912) | `ENG-EXPERT-STREAM` | Stream routed experts from NVMe so a model larger than device memory runs. Target `Qwen/Qwen3.8-2.4T-A95B`, REGISTERED against `Qwen3_5MoeForCausalLM` and blocked only on capacity: 370 GiB at UD-Q1_0 against 128 GB of unified memory on GB10. The only one of the three offload rows that helps on a unified-memory host, because `ENG-WEIGHT-OFFLOAD` and `ENG-HYBRID-PLACEMENT` both move bytes inside one physical pool. Cheaper than the spec assumed: on the GGUF path the mmap'd file already IS the bank and the per-expert slicer landed 2026-07-22 ([#824](https://github.com/mudler/vllm.cpp/issues/824)) | feature | | [#911](https://github.com/mudler/vllm.cpp/issues/911) | — | A `file:line` anchor into a file the row is ITSELF editing is stale by default, and spec BODIES are checked by nothing: `check-agent-record.py`'s `MATRIX_PATHS` (`:521`, `:529-530`) covers the five matrices, `feature-matrix.md` and `specs/model-family-inventory.md`, leaving 4772 line-carrying citations across 315 `.agents/specs/*.md` unexamined (positive control: 2314 line-less `.cpp` mentions match the same shape). `ltx25-prompt-adaln.md` shipped EIGHT stale repo-local anchors across two repair commits, moved by its own `020381676` and by `98f8e046d` (#658), then SEVEN more that were correct at `00613767d` and wrong at the merge of `origin/main`, because `0785cfc4d` (#882) added 70 lines to `ltx2_video.cpp` and 306 to `test_ltx2_video.cpp` ahead of every one. The obvious checker is a TAUTOLOGY — reading the span out of the file it validates reports 27 of 27 fresh on the same tree where reading the spans against their CLAIMS finds seven stale. Remedy is already in use and unwritten: `path:NN @ ` for a historical claim, claim-sourced uniqueness re-derivation for a live one, re-run after the merge. Narrower than [#632](https://github.com/mudler/vllm.cpp/issues/632) on surface and sharper on mechanism. Listed under `## Owed` in [`ltx25-prompt-adaln.md`](specs/ltx25-prompt-adaln.md) | bug | +| [#920](https://github.com/mudler/vllm.cpp/issues/920) | `LTX25-GENERATED-KEYFRAMES` | Generated keyframe slots — upstream `VideoGeneratedKeyframeSlots` (`conditioning/types/keyframe_slots.py:27-174` @ `fd4ded7f`), reached as `--num-generated-keyframes` (`ltx-pipelines/utils/args.py:833-844`) and documented at `ltx-pipelines/docs/conditioning.md:29-61` — have no REQUEST surface in this tree, so asking for them falls through the per-generation extras check and gets `unknown per-generation extra`, a message that says the family does not define the key and sends the reader looking for a typo. This is the ONLY upstream conditioning item that passes `marked=True` to `extend_keyframes_mask` (`:121` against `keyframe_cond.py:84-86`), so it is the only user-facing feature that puts #658's trained bias on a token other than the target's own first latent frame; `KeyframeInterpolationPipeline` does NOT (no `generated_keyframe` reference in its 362 lines, and absent from the feature's own applies-to list at `conditioning.md:47-51`). ONE blocker, the `GeneratedKeyframeLayout` READBACK: the slots are the OUTPUT, so they must be located by the layout the append recorded rather than assumed to trail, extracted before the extra tokens are trimmed, and each decoded as a STANDALONE one-frame clip (`ltx_core/types.py:269-272`). The token-append machinery is NOT a second blocker, and this row states that rather than restating the refusal it replaces: `c7cb59fbb` (#930) landed the seam and serves the LAST-frame supplied arm through it. Owed to THIS row rather than to #930 is a production caller for the `marked=true` branch of `Ltx2ExtendKeyframesMask` (`include/vllm/model_executor/models/ltx2_conditioning.h:175` @ `e5351776c`), whose only driver today is the unit case at `tests/vllm/models/test_ltx2_vae.cpp:2494` @ `e5351776c`. Spec [`ltx25-generated-keyframes.md`](specs/ltx25-generated-keyframes.md), campaign #644 | bug | | [#919](https://github.com/mudler/vllm.cpp/issues/919) | `LTX25-RESOLUTION-ENVELOPE` | `vllm_video_generate` integer-divides `width`/`height` into the latent grid (`ltx2_video.cpp:1456-1463 @ 5a0ffe9e3`) with no divisibility check, so a 100x100 request on the distilled two-stage recipe silently renders 96x96. The only geometry guard in the LTX path is a LOWER bound (`ltx2_video.cpp:1464-1471 @ 5a0ffe9e3`). Every repo-local anchor in this row is SHA-pinned because the fix edits the files it cites, and inserts lines above both spans (#911). Upstream hard-validates and raises at the top of a pipeline `__call__` — `assert_resolution` (`ltx-pipelines utils/helpers.py:540-551` @ `fd4ded7f2`), 64 for two-stage and 32 for one-stage, NINE invocations including `ti2vid_two_stages.py:184` and `ti2vid_two_stages_hq.py:199` — so mirroring means refusing, not flooring. Nine, not the 21 lines a grep for the name returns (9 invocations + 1 definition + 10 imports + 1 `__all__` string), and not every pipeline: 13 pipeline `__call__`s take a resolution and the three `*_mgpu.py` variants plus `hdr_ic_lora.py:352` skip the guard. `docs/USAGE.md:626-629 @ 5a0ffe9e3` already documents the rule as though it were enforced. Frames are the OPPOSITE answer: upstream floors an explicit `num_frames` exactly as we do (`ltx_core/types.py:113`) and validates it nowhere, so that half is a doc correction | bug | | [#921](https://github.com/mudler/vllm.cpp/issues/921) | — | The res_2s DENOISING LOOP (`ltx-pipelines utils/samplers.py:206-447` @ `fd4ded7f2`) is unported, so `TI2VidTwoStagesHQPipeline` cannot be served. What exists is one substep's SDE arithmetic: `Ltx2Res2sSdeCoeff`/`Ltx2Res2sStep` (`ltx2_pipeline.cpp:307-360 @ 5a0ffe9e3`, the two functions in full) mirror `Res2sDiffusionStep` (`diffusion_steps.py:118-190`) and are gated. Absent are the `phi`/`get_res2s_coefficients` exponential integrator (`utils/res2s.py:4-62`), the SECOND transformer evaluation per step at `sub_sigma = sqrt(sigma * sigma_next)` (`samplers.py:315` and `samplers.py:380-386`, spelt out because a bare `:NN` after a res2s.py citation reads as res2s.py) against our once-per-step loop (`ltx2_video.cpp:1735 @ 5a0ffe9e3`, with its single forward at `ltx2_video.cpp:1813-1817 @ 5a0ffe9e3`), the bong anchor refinement (`samplers.py:357-364`), and any `Ltx2StepperKind` enumerator to select it. The sampler IS the HQ variant, so this arm must refuse by name rather than substitute Euler and render something plausible that is quietly not HQ. Listed under `## Owed` in [`ltx25-resolution-envelope.md`](specs/ltx25-resolution-envelope.md) | feature | | [#922](https://github.com/mudler/vllm.cpp/issues/922) | `LTX25-A2V-AUDIO-INPUT` | LTX-2.5 audio-to-video (`A2VidPipelineTwoStage`, `a2vid_two_stage.py:53,143`) is absent: `vllm_video_params` carries no field or extra that accepts a driving waveform and `ltx2-gen` has no `--audio-path`, so nothing turns a file on disk into the audio latent the DiT's audio stream consumes. Distinct from reference-audio conditioning, which is correctly refused by name at `ltx2_video.cpp:1348-1355 @ 5a0ffe9e`; the two share one blocking dependency, the audio VAE ENCODER load path (`ltx2_loader.cpp:1295-1300` materializes `audio_vae.decoder.` only). The analysis half is already ported and unreached — `Ltx2AudioEncoderForward` (`ltx2_audio_vae.cpp:1114`), `Ltx2WaveformToLogMel` (`:1019`), `Ltx2SlaneyMelFilterbank` (`:970`) — and the engine applies ONE `phase.noise_scale` to both streams (`ltx2_video.cpp:1708-1712 @ 5a0ffe9e`) where upstream's `ModalitySpec` carries `noise_scale` and `frozen` per modality (`utils/types.py:99-112`). Spec [`ltx25-a2v-audio-input.md`](specs/ltx25-a2v-audio-input.md) | feature | diff --git a/.agents/specs/ltx25-generated-keyframes.md b/.agents/specs/ltx25-generated-keyframes.md new file mode 100644 index 000000000..70b210cf9 --- /dev/null +++ b/.agents/specs/ltx25-generated-keyframes.md @@ -0,0 +1,429 @@ +# LTX25-GENERATED-KEYFRAMES — refuse generated keyframe slots by what is missing + +Issue: [#920](https://github.com/mudler/vllm.cpp/issues/920). Campaign: +[#644](https://github.com/mudler/vllm.cpp/issues/644). Sibling that landed the +marker itself: [#658](https://github.com/mudler/vllm.cpp/issues/658), +[`ltx25-keyframes-abs-pos.md`](ltx25-keyframes-abs-pos.md), which put "keyframe +*conditioning* as a user-facing feature (supplying keyframe slots)" explicitly +**out** of its scope. This row picks up that half. + +Oracle: Lightricks `LTX-2` at pin `fd4ded7f2d88d3da713abcdd4ad41ecc4a9314ca`, +checked out at `/home/mudler/_git/LTX-2` and verified at that SHA with a clean +tree before any anchor below was read. + +## 0. The two features that both say "keyframe" + +This row exists because one word covers two upstream features, and conflating +them has already cost this campaign one falsely-pinned test assertion +(`test_ltx2_video.cpp:1162-1170` records the previous one). + +| | Supplied keyframes | **Generated keyframe slots** | +|---|---|---| +| Upstream item | `VideoConditionByKeyframeIndex` | `VideoGeneratedKeyframeSlots` | +| Anchor | `conditioning/types/keyframe_cond.py:36-90` | `conditioning/types/keyframe_slots.py:27-174` | +| Who supplies content | the caller, an image per frame index | nobody — the model generates it | +| `keyframes_mask` | `marked=False` (`keyframe_cond.py:84-86`) | `marked=True` (`keyframe_slots.py:121`) | +| Touches the trained bias | **no** | **yes** | +| State here | **SERVED** as of `c7cb59fbb` (row LTX25-TOKEN-APPEND, #930) | no request surface | + +The class spans `27-174`; `apply_to` is `71-150` and `_slot_positions` `153-174`. +The row's first draft cited `27-150` for the class, which stops at the end of +`apply_to` and excludes the very span the same draft cited separately as class +content. + +`marked` is the whole difference in one argument. `extend_keyframes_mask` +(`conditioning/mask_utils.py:76-107`) documents the polarity: *"True only for +generated keyframe slots; given-content conditioning tokens (image guidance, +reference latents) are never marked, matching the reference implementation."* + +So `VideoGeneratedKeyframeSlots` is the **only** conditioning item upstream that +marks anything, and therefore the only user-facing feature that puts the trained +`keyframes_abs_pos_embedding` on a token other than the target's own first +latent frame. `KeyframeInterpolationPipeline` +(`ltx-pipelines/keyframe_interpolation.py`) does **not** do this: it builds only +`VideoConditionByKeyframeIndex` items through +`image_conditionings_by_adding_guiding_latent` (`utils/helpers.py:343-367`), +contains no reference to `generated_keyframe` or `keyframes_abs_pos` anywhere in +its 362 lines, and is absent from the feature's own "where it applies" list +(`ltx-pipelines/docs/conditioning.md:47-51`, which names +`TI2VidOneStagePipeline`, `TI2VidTwoStagesPipeline`, `TI2VidTwoStagesHQPipeline`, +`DistilledPipeline` and the multi-GPU runners). Recorded here because the +opposite was the working assumption when this row was dispatched. + +## 1. What is wrong today — measured, not inferred + +**Re-measured at the merge with main at `e5351776c`, because the first +measurement went stale under this row.** At the branch's merge base +(`5a0ffe9e3`) `git grep -i generatedkeyframe` returned **0** lines, and the spec +said so. At `e5351776c` it returns **7 lines in 4 files** — three of them outside +`.agents/`: `include/vllm/model_executor/models/ltx2_conditioning.h`, +`src/vllm/model_executor/models/ltx2_conditioning.cpp` and +`tests/vllm/models/test_ltx2_vae.cpp`. Row LTX25-TOKEN-APPEND (#930) put them +there. So the concept is no longer absent from the tree; what is absent is the +**request surface** — no key, no refusal, no test — and the **readback**. + +A count that was true when written and false at review is exactly the failure +this row's own refusal is about, which is why it is recorded here rather than +quietly corrected. + +The per-generation extras check in `Ltx2VideoEngine::Generate` accepts exactly +one key and rejects every other by the same generic message: + +``` +unknown per-generation extra 'num_generated_keyframes'. This family defines: image_crf +``` + +That is the wrong message, and it is wrong in a way this campaign has already +paid for once. It asserts the family does not define the key, which sends the +reader looking for a typo. `CheckUnservedExtras` exists on the load side +precisely because of that distinction (#611): *"Deliberately NOT the 'unknown +load extra' path above. That message says the family does not define the key, +which is false here and would send the reader looking for a typo instead of for +the unported head."* + +The obligation is stated twice in policy. [`AGENTS.md`](../../AGENTS.md) +`## Shared seams`: *"Refuse an unimplemented arm with a message that names the +missing part. Record the arm as owed. Never leave the missing path to be +discovered later."* [`porting-a-model.md:81`](../porting-a-model.md): an arm is +*"explicitly refused with a message naming the missing piece."* + +## 2. What upstream does, with anchors + +Every anchor re-derived at the pin and asserted unique before this spec was +committed (`## 8`). + +**The item.** `VideoGeneratedKeyframeSlots.__init__` +(`keyframe_slots.py:47-69`) takes `pixel_frame_indices` — non-empty, strictly +increasing, non-negative — and an optional `initial_keyframes` of shape +`(B, C, K, H, W)` with `K == len(indices)`. `apply_to` (`:71-150`): + +- refuses an index at or beyond the target's pixel-frame count (`:77-81`); +- sizes each slot at one latent frame of tokens, `patchifier.get_token_count` + over `target_shape._replace(frames=1)` (`:83-84`); +- builds slot positions whose temporal span is exactly `[t, t+1)` with + `causal_fix=False`, because the span is set explicitly (`:152-174`); +- appends to `latent`, `denoise_mask`, `positions`, `clean_latent` (`:136-140`); +- sets `denoise_mask = 1` on the new tokens so the noiser lerps from the slot + `latent` and ignores `clean_latent` (`:118-119`); +- marks them: `extend_keyframes_mask(..., marked=True)` (`:121`); +- rebuilds the attention mask via `update_attention_mask` (`:123-131`); +- refuses a second application (`:133-134`); +- records `GeneratedKeyframeLayout(pixel_frame_indices, tokens_per_keyframe, + first_token)` (`:143-147`), so the slots are located exactly rather than + assumed to be trailing (`types.py:220-247`). + +**Readback.** `clear_conditioning` (`ltx_core/tools.py:88-117`) extracts the +denoised slot content into `generated_keyframes` *before* trimming the extra +tokens (`:97`, `:115`), validating the layout against the live token count and +the target resolution (`extract_generated_keyframes`, `tools.py:203-230` — the +first draft wrote `203-241`, which runs 11 lines past the method and into an +unrelated `AudioLatentTools` dataclass at `233`). Each frame must then be decoded as a +standalone one-frame clip — `types.py:269-272` and `conditioning.md:59-61` both +warn that a K-frame causal decode blends slots that were never adjacent. + +**The request surface.** `--num-generated-keyframes`, `type=int`, `default=0` +(`ltx-pipelines/utils/args.py:833-844`). Opt-in per CLI *"only pipelines that +actually forward the value to their first diffusion stage should advertise the +flag, otherwise it would parse and be silently ignored"* (`:828-831`) — the same +rule this row is applying. `resolve_generated_keyframes` +(`utils/helpers.py:394-411`): an `int` requests that many evenly spaced interior +positions, a sequence gives explicit indices, `0`/empty means off. +`evenly_spaced_keyframe_positions` (`:370-381`) refuses a negative count +(*"num_keyframes must be non-negative"*) and a target shorter than +`num_keyframes + 2`. `has_generated_keyframes` (`:384-391`) exists so callers do +not test truthiness of a value that may be a tensor. + +**The admission gate.** `DiffusionStage.supports_generated_keyframes` +(`ltx-pipelines/utils/blocks.py:395-403`) reads the **declared** config flag +only, *"answered before any weights are built"*. +`assert_generated_keyframes_supported` (`:405-419`) raises naming +`use_keyframes_abs_pos_embedding`; `_assert_supports_conditionings` (`:421-425`) +re-checks as a backstop for callers that build items directly. The reason it +refuses rather than degrades is stated at `keyframe_slots.py:9-12`: on a +checkpoint without the marker *"the slots would be denoised as unmarked tokens +and the extra compute would be wasted"* — and each slot costs one latent frame +of tokens to buy one pixel frame (`docs/conditioning.md:43-46`: about +16% +tokens at 512x768/241 frames, +31% at 1088x1920/121). + +## 3. Scope + +**In.** + +1. Define `num_generated_keyframes` as a key this family **knows**, spelled as + upstream spells it, on the per-generation surface (`vllm_video_params` + extras), which is where upstream takes it — a `__call__` argument, not a load + option. +2. Mirror upstream's `0 = off` default exactly: an explicit `0` is upstream's + own default and must **not** refuse. This is the half most likely to be + ported as "any mention refuses", which would break a caller that passes the + default through. +3. Mirror `evenly_spaced_keyframe_positions`' negative-count `ValueError` + (`helpers.py:372-373`) for a negative value. +4. For a positive count, **refuse by name**, naming the one missing piece from + `## 4`, the upstream symbols a later reader can go and check, and the two + reasons that are ruled out together with what ruled each one out. +5. State the refusal's claims about **this** tree in a form a gate can + re-derive, and gate them. See `## 4a`. +6. Record the arm as owed under `## Owed` with issue #920. + +**Out.** + +- Building the token-append machinery. Row LTX25-TOKEN-APPEND (#930) built it + and it landed at `c7cb59fbb`; nothing is owed here. The one piece of that seam + still owed is a production caller for `Ltx2ExtendKeyframesMask(..., marked=true)`, + and that belongs to **this** row — see `## Owed`. +- `initial_keyframes` seeding, `GeneratedKeyframeLayout` readback, and the + standalone single-frame decode. All owed to this row, none of them landed here. +- Any change to the marker itself. #658 landed it and it is applied on every + render; `_first_frame_keyframes_mask` (`ltx_core/tools.py:184-196`) marks the + target's first latent frame **unconditionally**, so the embedding is consulted + on the default text-to-video path and must not be made conditional on this + key. +- `Ltx2AdoptDeclaredDitParams`. See `## 5` — it is already correct. + +## 4. What the refusal must name + +**One blocker: readback with a standalone decode.** Unique to this arm; the +supplied arm needs none of it. `apply_to` records a `GeneratedKeyframeLayout` +that locates the slots exactly, `clear_conditioning` extracts them into +`generated_keyframes` before it trims, and each frame is then decoded on its own. +Without all three the slots would be generated and then thrown away, which is +worse than refusing. + +**Two reasons are RULED OUT, and each must name what ruled it out**, in the shape +this file's own `keyframes_abs_pos_embedding` clause already uses. A refusal that +merely omits a plausible reason leaves the next reader to re-derive the +refutation; the point of naming it is that they re-CHECK it instead. + +1. **NOT the token-append machinery.** The first draft of this row named it as + blocker 1, and by the time the row reached review that was false in every + particular: `c7cb59fbb` (row LTX25-TOKEN-APPEND, #930) landed + `Ltx2ExtendKeyframesMask` and `Ltx2ClearConditioning`, bound a `target_tokens` + in the phase loop, made the clear step a real trim, and **served** the + LAST-frame supplied-keyframe arm through all of it. Two small pieces of that + seam are still owed, and they are owed to THIS row rather than to #930: the + `marked=true` branch has no production caller, and `update_attention_mask` has + no local counterpart, because `Ltx2LatentState` deliberately carries no + attention-mask field (`include/vllm/model_executor/models/ltx2_conditioning.h:58-70` + @ `e5351776c` argues that omission from upstream's own polarity, and #930 kept + it deliberately). Neither is the blocker. +2. **NOT `keyframes_abs_pos_embedding`.** Ported under #658 and applied on every + render, because `_first_frame_keyframes_mask` marks the target's first latent + frame unconditionally. + +## 4a. And the refusal's LOCAL claims must be gated + +The stale half of the first draft was never the upstream half. Upstream symbol +names do not move when this tree changes, so a suite that asserts only on +`update_attention_mask` and `clear_conditioning` stays green through the exact +event that falsifies the message. Measured, not argued: the reviewer's mutation +M7 replaced the local-cause sentence with a self-declared falsehood, left the +upstream names alone, and the suite reported 18/18, exit 0. + +So the message carries its claims about **this** tree as two explicit lists — +`DECLARED HERE:` and `ABSENT HERE:` — and `test_ltx2_video` parses them **out of +the thrown message** and re-derives each against +`include/vllm/model_executor/models/ltx2_conditioning.h`. Three properties make +that check real rather than decorative: + +- **A different file.** The claim lives in `ltx2_video.cpp`; the truth is read + from `ltx2_conditioning.h`. Reading a claim out of the file that makes it is + the tautology [#911](https://github.com/mudler/vllm.cpp/issues/911) records. +- **Comment lines stripped.** `ltx2_conditioning.h:170` names upstream's + `VideoGeneratedKeyframeSlots` in prose, so an unstripped search for + `GeneratedKeyframe` finds a hit and the ABSENT half silently inverts. The + stripping is itself controlled: the case requires + `struct Ltx2LatentState` to survive it. +- **A count floor.** An empty parsed list satisfies every `for each` and reports + a pass over nothing. Both lists have a minimum size, so a message that dropped + its clause cannot report a pass over an empty set. + +The check fires in both directions. If the readback lands, `GeneratedKeyframe` +appears in the header, ABSENT goes red, and whoever landed it is told the refusal +is now false. If the append seam is renamed or removed, DECLARED goes red. + +## 5. #902 — answered, and it is not a code hole + +[#902](https://github.com/mudler/vllm.cpp/issues/902) asks which way +`Ltx2AdoptDeclaredDitParams` resolves a checkpoint that declares +`use_keyframes_abs_pos_embedding=true` and carries no keyframe tensor, and +whether that is upstream's behaviour. Answered from upstream, not from our code, +as the issue asks: + +Upstream resolves the same contradiction **two ways at two layers, by design**: + +- `LTXModel.supports_keyframes_abs_pos_embedding` + (`model/transformer/model.py:166-173`) reads the **materialized tensor**: + `embedding is not None and not embedding.is_meta`. `_init_video:216-218` + builds the parameter whenever the flag is declared, models are built on `meta` + and loaded `strict=False, assign=True`, so a declare-true / carry-nothing + checkpoint leaves it on `meta` and the property is **False**. Its own + docstring names our exact case. +- `DiffusionStage.supports_generated_keyframes` + (`ltx-pipelines/utils/blocks.py:395-403`) reads the **declared flag only**, so + on the same checkpoint it returns True and the admission gate would let a + request through. + +Ours resolves to shapes (`ltx2_loader.cpp`, `Ltx2AdoptDeclaredDitParams`), +matching the first and asserted at `tests/vllm/multimodal/test_ltx2_video.cpp`. +The hypothesised live hole — that the `ltx2_dit.cpp` guard should have refused +the #902 render and did not — **does not exist**, because the flag resolves +false and the guard never arms on that checkpoint. + +If anything ours is the safer of the two. `apply_keyframes_absolute_embedding` +(`transformer_args.py:23-43`) skips only on a `None` provider, and +`_keyframes_embedding` (`model.py:158-164`) returns the meta parameter rather +than `None`, so upstream on that checkpoint would reach `embedding.to(dtype=...)` +on a meta tensor. `enable_keyframes_abs_pos_embedding` (`model.py:175-200`) +exists to materialize real zeros for exactly this, and it has **one hit +repo-wide at the pin: its own definition, no caller.** Its docstring is explicit +that this only makes the marker *harmless*, not meaningful. + +Residual on #902 is checkpoint availability, not code. Reported on the issue; no +change owed here. + +## 6. Tests + +RED-first, and the RED must be the intended failure. Every case enters through +the **production entry point** — `vllm::multimodal::LoadVideoEngine` then +`VideoEngine::Generate`, the chain `vllm_video_generate` takes — never by +constructing a type ([`reachability.md`](../reachability.md)). + +1. **A positive count is refused, and the message names the READBACK and both + ruled-out reasons.** Assert on the upstream symbols a reader can go and check + (`VideoGeneratedKeyframeSlots`, `keyframe_slots.py`, `GeneratedKeyframeLayout`, + `extract_generated_keyframes`, `generated_keyframes`, `update_attention_mask`, + `clear_conditioning`) and on `#930`, not on prose. +1a. **Its claims about THIS tree are re-derived from this tree.** Parse the + `DECLARED HERE:` and `ABSENT HERE:` lists out of the thrown message and check + each against `ltx2_conditioning.h`'s declarations with comment lines stripped. + This is the case `## 4a` exists for: every assertion in case 1 is on an + upstream name, and no change to this tree can move one. +2. **The refusal is NOT the generic one.** Assert the message does not contain + `unknown per-generation extra`. This is the assertion the row exists for, and + without it case 1 passes against the message we already have. +3. **`0` does not refuse**, mirroring upstream's default (`args.py:836`). The + render must complete. This is the half a naive port breaks. +4. **A negative value refuses with upstream's own reason**, mirroring + `evenly_spaced_keyframe_positions` (`helpers.py:372-373`), and NOT with the + unported-machinery message — a malformed request and an unported arm are + different answers. +5. **The refuted reason may be named but never as the blocker.** If the message + mentions `keyframes_abs_pos_embedding`, it must also say it is not what is + missing and cite #658, mirroring the guard the LAST-frame case already + carries at `test_ltx2_video.cpp:1180-1186`. + +**Mutations that must be run and recorded**, each with three facts — +`git diff --stat` after applying, whether it BUILT with any compile error beside +it, and the exit code: + +- delete the production refusal call site (the reachability mutation) — cases + 1, 1a, 2, 4 must go RED; +- make the refusal fire on `0` — case 3 must go RED; +- make it fire on any presence of the key regardless of value — case 3 RED; +- accept a positive count silently — cases 1, 1a, 2 RED; +- give a negative value the unported-machinery message — case 4 RED; +- **M7, the one this repair adds**: replace the message's LOCAL-cause clause with + a self-declared falsehood and leave every upstream symbol in place — case 1a + RED. Against the first draft this mutation left the suite GREEN at 18/18, + exit 0, which is the whole reason `## 4a` exists. + +## 7. Risks + +- **`ltx2_video.cpp` carries derived READER ANCHORS** (the comment above + `kKnownLoadExtras`, gated by `test_ltx2_video`) whose values are line numbers + in that file. A clean `git merge` will not warn when they go stale. This row + keeps its `.cpp` edit **below** every anchored line and puts the new key + constant in the header, so no anchor should move — but that is a prediction, + and the gate is the check. Re-derive at the final tree. +- **Three other agents are editing `ltx2_video.cpp`, `ltx2.h`, + `docs/FEATURES.md` and `.agents/issue-index.md` concurrently**, and + `origin/main` moves several times an hour. Keep the footprint minimal and + merge often. `docs/FEATURES.md` is a keyed record: reapply the scoped edit by + key and prove unrelated keys byte-identical rather than accepting a three-way + merge. +- **Refusing too broadly.** An explicit `0` is upstream's default and a caller + that plumbs defaults through would hit a refusal that upstream does not raise. + Test 3 is the guard. +- **The refusal's REASON going stale under the row.** This happened: the row was + written against a merge base where the token-append seam did not exist, and + `c7cb59fbb` landed it while the branch sat in review, and by the merge the + branch was fifteen commits behind. The + refusal, the index row, `docs/USAGE.md`, the PR body and this spec all named it + as blocker 1, and nothing went red, because every assertion was on an upstream + symbol name. `## 4a` is the guard, and it is the only one of the three risks + here with a test behind it. + +## 8. Anchor discipline + +Every `file:line` in this spec was re-derived at the pin, and the needle was +derived from the **claim** rather than read out of the cited span — that check +is circular and has reported 27/27 FRESH while five anchors pointed at unrelated +code. Uniqueness is asserted, not existence. + +## 9. Gates + +CPU-only. The GPU is not used: `dgx.casa` is running a long render under `flock` +and OOM-reboots when the 119 GiB unified pool is exhausted. + +``` +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 +``` + +Known-red on main and to be proved pre-existing rather than asserted: the #873 +family of checkers and `windows-msvc-*` (#584). Load-dependent and to be re-run +alone before being charged: `test_openai_conformance`, `test_cpu_threadpool`, +`test_engine_core_proc`, `test_async_llm` (#294), `test_cpu_x86_llamacpp_floor`. + +## 10. Arms + +| Arm | State | +|---|---| +| bf16 / f32 reference | refusal reached and gated; the arm itself is owed | +| FP8 | same refusal, same path — the key is checked before any DiT arm is selected | +| NVFP4 | same | +| GGUF k-quants | not applicable to this key; owed for LTX-2.5 as a whole under #644 | + +The refusal is resolved on the request, ahead of arm selection, so no arm can +reach the unported readback by a different route. That is the property test 1 +pins. + +## Owed + +- [#920](https://github.com/mudler/vllm.cpp/issues/920) — the generated keyframe + slots arm itself: `GeneratedKeyframeLayout` readback, extraction into + `generated_keyframes` before the trim, and the standalone single-frame decode + of each slot. This row lands the refusal and the request surface only. Campaign + [#644](https://github.com/mudler/vllm.cpp/issues/644). +- [#920](https://github.com/mudler/vllm.cpp/issues/920) — **a production caller + for `Ltx2ExtendKeyframesMask`'s `marked=true` branch.** Row LTX25-TOKEN-APPEND + landed that branch with a unit driver and no production caller, and assigned it + here by ID and issue + ([`ltx25-token-append.md` `## 8`](ltx25-token-append.md)). Its only driver in + the tree is `tests/vllm/models/test_ltx2_vae.cpp:2494` @ `e5351776c`, which is + a unit case rather than reach. `AGENTS.md` `## Nothing lands dead` permits a + staged unreached slice only while the owning row's spec lists it under + `## Owed`, and until this repair it was named in #930's spec and in no other, + so the permission rested on a bullet this row never wrote. The only upstream + construct that passes `true` is `VideoGeneratedKeyframeSlots` + (`keyframe_slots.py:121`), so no other row can claim it. + +## Stop conditions + +- If the readback turns out to be reachable without a layout — if the slots can + be assumed to trail — stop: the reading of `keyframe_slots.py:143-147` and + `ltx_core/types.py:220-247` in `## 2` is wrong and the refusal names the wrong + blocker. +- If the refusal cannot be placed before arm selection, stop and report — a + refusal reachable on only some arms is worse than none. +- If the READER ANCHORS move, re-derive them in this row rather than editing the + gate. + +## Now + +`ACTIVE`. Refusal and request surface in review on `row/LTX25-GENERATED-KEYFRAMES`, +merged onto `e5351776c` and repaired: the refusal now names the READBACK as its +one blocker, the token-append seam is a ruled-out clause rather than a blocker, +and `## 4a` gates the message's local claims. The arm itself is owed above. diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 0d688c2a1..5734cfc33 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+LAST kf SERVED `crf=0`, A2V WAV; DiffVAE/LoRA/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`. IMG+LAST kf SERVED `crf=0`, A2V WAV; GENkf/DiffVAE/LoRA/ref refused. 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. HTTP request OBSERVED (#852) | 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 9d3e5bc2c..68e950b2e 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -518,6 +518,23 @@ 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.) +**Generated keyframe slots are a different feature, and they are refused.** +Upstream also lets the model *generate* extra frames at interior positions — +`--num-generated-keyframes N` there, the per-generation extra +`num_generated_keyframes` here. That is not a keyframe you supply; it is one you +ask the model to invent, and each slot buys one pixel frame at the cost of a +full latent frame of tokens. `0` is upstream's own default and means off, so +passing it explicitly renders normally. A positive count is refused by name, and +what the refusal names is the **readback**: the slots are the OUTPUT, so they +have to be located by the layout the append recorded, extracted before the extra +tokens are trimmed off, and then each decoded as a standalone one-frame clip, +because a multi-frame causal decode would blend slots that were never temporally +adjacent. Two plausible reasons are ruled out rather than left for a reader to +re-derive: the token-APPEND machinery, which the last-frame arm above uses +today, and the trained keyframe marker, which issue #658 landed and which every +render already applies. A negative count is refused separately with upstream's +own reason, since a malformed request and an unported arm are different answers. + 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 diff --git a/include/vllm/multimodal/ltx2_video.h b/include/vllm/multimodal/ltx2_video.h index 260243a90..1e586b049 100644 --- a/include/vllm/multimodal/ltx2_video.h +++ b/include/vllm/multimodal/ltx2_video.h @@ -295,6 +295,39 @@ inline constexpr char kLtx2AudioStartTimeExtra[] = "audio_start_time"; // audio as the clip is long. inline constexpr char kLtx2AudioMaxDurationExtra[] = "audio_max_duration"; +// GENERATED keyframe slots — the OTHER upstream feature called "keyframe", and +// the one this port does not serve. Row LTX25-GENERATED-KEYFRAMES (#920). +// +// Not to be confused with the SUPPLIED keyframe arm. The two differ by one +// argument, and it is the argument that decides whether the trained marker is +// applied at all: +// +// supplied `VideoConditionByKeyframeIndex` — the caller hands in an image +// for a frame index; appended `marked=False` (keyframe_cond.py:84-86) +// GENERATED `VideoGeneratedKeyframeSlots` — the MODEL generates extra frames +// at interior positions; appended `marked=True` (keyframe_slots.py:121) +// +// `extend_keyframes_mask` (conditioning/mask_utils.py:76-107) documents the +// polarity, and `keyframe_slots.py:121` is upstream's ONLY call site that passes +// True. So this is the only user-facing feature that puts +// `keyframes_abs_pos_embedding` on a token other than the target's own first +// latent frame — which `Ltx2FirstFrameKeyframesMask` already marks on every +// render, unconditionally, mirroring `tools.py:184-196`. +// +// THIS KEY IS DEFINED AND NOT SERVED, and it is defined precisely so the refusal +// can name what is missing. Falling through the per-generation extras check +// would produce "unknown per-generation extra", which asserts the family does +// not define the key and sends the reader hunting a typo — the distinction +// `CheckUnservedExtras` exists for on the load side (#611). +// +// Spelled and typed as upstream's CLI spells it: `--num-generated-keyframes`, +// `type=int`, `default=0` (ltx-pipelines/utils/args.py:833-844). It is a +// per-CALL argument upstream, forwarded to the FIRST diffusion stage only, so it +// belongs on the per-generation surface rather than on load. `0` is upstream's +// own default and means OFF (`has_generated_keyframes`, utils/helpers.py:384-391) +// — an explicit 0 must therefore RENDER, not refuse. +inline constexpr char kLtx2GeneratedKeyframesExtra[] = "num_generated_keyframes"; + // WHAT THE LAST `Generate()` ACTUALLY HANDED THE DiT's CROSS-ATTENTION. // // Every field is read off the exact f32 buffers `Ltx2ModalityInput::context` diff --git a/src/vllm/multimodal/ltx2_video.cpp b/src/vllm/multimodal/ltx2_video.cpp index ce438a27c..3943e02c3 100644 --- a/src/vllm/multimodal/ltx2_video.cpp +++ b/src/vllm/multimodal/ltx2_video.cpp @@ -1207,25 +1207,123 @@ std::unique_ptr Ltx2VideoEngine::Load(const VideoModelParams& p return engine; } +namespace { + +// GENERATED keyframe slots, refused BY WHAT IS MISSING. +// +// Deliberately a second anonymous namespace rather than an addition to the one +// at the top of this file: the READER ANCHORS comment above `kKnownLoadExtras` +// carries derived LINE NUMBERS into this file and is gated by +// `test_ltx2_video`, so a definition inserted up there would move every anchor +// under it and break that gate for a reason that has nothing to do with this +// row. Everything here sits below the last anchored line. +// +// This resolves the request BEFORE any arm is selected, so the FP8, NVFP4 and +// bf16 arms cannot reach the unported machinery by different routes — there is +// one answer for the family, not one per arm. +void CheckGeneratedKeyframes(const std::map& extras) { + if (VideoExtra(extras, kLtx2GeneratedKeyframesExtra).empty()) return; + const int64_t count = ExtraInt(extras, kLtx2GeneratedKeyframesExtra, 0); + + // ZERO IS UPSTREAM'S DEFAULT, AND IT IS OFF. `args.py:836` is `default=0` and + // `has_generated_keyframes` (utils/helpers.py:384-391) reads 0 as "no slots + // requested". A caller that plumbs the default through must get a render. + // Refusing on the mere presence of the key is one line shorter and wrong. + if (count == 0) return; + + // A MALFORMED REQUEST AND AN UNPORTED ARM ARE DIFFERENT ANSWERS, and upstream + // gives this one first: `evenly_spaced_keyframe_positions` raises + // "num_keyframes must be non-negative" (utils/helpers.py:372-373) before + // anything looks at the checkpoint. Collapsing the two would tell a caller who + // typed -1 to go and read about attention masks. + if (count < 0) { + Fail("the '" + std::string(kLtx2GeneratedKeyframesExtra) + "' extra is " + + std::to_string(count) + ", and num_keyframes must be non-negative — upstream's own " + "refusal, raised by `evenly_spaced_keyframe_positions` " + "(ltx-pipelines/utils/helpers.py:370-381) before the checkpoint is consulted. Use 0 " + "to turn generated keyframes off, which is upstream's default (utils/args.py:836)."); + } + + Fail( + "generated keyframe slots are not served. This is upstream's " + "`VideoGeneratedKeyframeSlots` (ltx-core/conditioning/types/keyframe_slots.py:27-174), " + "reached from the CLI as `--num-generated-keyframes` (ltx-pipelines/utils/args.py:833-844) " + "and documented at ltx-pipelines/docs/conditioning.md:29-61 — the model GENERATES extra " + "frames at interior positions, which is a different feature from a SUPPLIED keyframe " + "image and is refused for a different reason. WHAT IS MISSING IS THE READBACK, and the " + "supplied arm needs none of it. The slots are the OUTPUT rather than conditioning, so " + "`apply_to` (keyframe_slots.py:71-150) records a `GeneratedKeyframeLayout` (:143-147, " + "defined at ltx_core/types.py:220-247) that locates them EXACTLY rather than assuming they " + "trail — items are applied in list order and each appends, so a state carrying slots AND a " + "supplied keyframe has no fixed trailing layout. `clear_conditioning` (ltx_core/" + "tools.py:88-117) then extracts them into `LatentState.generated_keyframes` as " + "(B, C, K, H, W) BEFORE it trims the extra tokens (tools.py:97, :115, by " + "`extract_generated_keyframes` at :203-230, which validates the layout against the live " + "token count and the target resolution), and each frame must then be decoded as a " + "STANDALONE one-frame clip — a K-frame causal decode would blend slots that were never " + "temporally adjacent (ltx_core/types.py:269-272, docs/conditioning.md:59-61). None of that " + "exists here, so a port that grew the sequence and stopped would generate the slots and " + "then throw them away, which is worse than refusing. " + "TWO PLAUSIBLE REASONS ARE RULED OUT, each with what ruled it out, so the next reader " + "re-checks the claim instead of re-deriving the refutation. FIRST, NOT the TOKEN-APPEND " + "machinery. Row LTX25-TOKEN-APPEND (issue #930) landed it and the LAST-frame " + "supplied-keyframe arm is SERVED through it today: `Ltx2ExtendKeyframesMask` and " + "`Ltx2ClearConditioning` grow the sequence and trim it back, and the phase loop binds a " + "`target_tokens` the grown count is measured against, so `apply_to`'s concatenation onto " + "`latent`, `denoise_mask`, `positions` and `clean_latent` (keyframe_slots.py:136-140), its " + "explicit [t, t+1) span with `causal_fix=False` (:152-174) and its `denoise_mask = 1` " + "(:118-119) all have a seam to land on. Two SMALL pieces of that seam are owed to THIS row " + "rather than to #930, and neither is the blocker above: the `marked=true` branch of " + "`Ltx2ExtendKeyframesMask` (upstream `keyframe_slots.py:121`) has no production caller yet, " + "and `update_attention_mask` (:123-131) has no local counterpart because `Ltx2LatentState` " + "carries no attention-mask field — deliberately, since the only upstream route to a " + "non-None mask is the IC-LoRA wrapper this engine does not mirror. SECOND, and this is the " + "one this campaign pinned falsely once already, WHAT IS *NOT* THE REASON: " + "`keyframes_abs_pos_embedding`. It is ported and applied on every render (row " + "LTX25-KEYFRAMES-ABS-POS, issue #658), because `_first_frame_keyframes_mask` " + "(ltx_core/tools.py:184-196) marks the target's first latent frame unconditionally. What " + "generated slots would add is MORE marked tokens — `keyframe_slots.py:121` is upstream's " + "only `extend_keyframes_mask(..., marked=True)` call site, against `marked=False` for " + "supplied content at keyframe_cond.py:84-86 — not the marker itself. " + "LOCAL FACTS, and the gate re-derives every one of them from " + "`ltx2_conditioning.h`'s DECLARATIONS with comment lines stripped, because a comment can " + "name a symbol the header does not declare. This clause exists because the reason above is " + "the part that goes stale: the refusal this one replaces named a token-append gap that " + "#930 had already closed, and the suite stayed GREEN through it because every assertion was " + "on an UPSTREAM symbol name, which no change to this tree can move. DECLARED HERE: " + "Ltx2ExtendKeyframesMask, Ltx2ClearConditioning, Ltx2LatentState. ABSENT HERE: " + "GeneratedKeyframe, generated_keyframe. " + "Tracked as owed by issue #920."); +} + +} // namespace + VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { Impl& im = *impl_; std::lock_guard guard(im.mutex); if (gen.output_dir.empty()) Fail("output_dir is required"); for (const auto& kv : gen.extras) { - // `image_crf` is the only per-generation extra this family defines (row - // LTX25-IMAGE-COND). Everything else is refused rather than ignored, for the + // The per-generation extras this family DEFINES, and the list is the one + // below rather than this sentence: `image_crf` (row LTX25-IMAGE-COND), the + // three audio-to-video knobs (row LTX25-A2V-AUDIO-INPUT, #922) and + // `num_generated_keyframes` (row LTX25-GENERATED-KEYFRAMES, #920). DEFINED + // is not SERVED — the last one is defined so that its own refusal can name + // what is missing, exactly as `CheckUnservedExtras` does on the load side + // (#611). Everything OUTSIDE the list is refused rather than ignored, for the // reason `CheckKnownExtras` gives for the load side: a mistyped knob that is // silently dropped renders the DEFAULT and looks like the feature not // working — and for THIS knob the default is a refusal, so a typo would turn // a served request into an unexplained one. const bool known = kv.first == kLtx2ImageCrfExtra || kv.first == kLtx2AudioPathExtra || kv.first == kLtx2AudioStartTimeExtra || - kv.first == kLtx2AudioMaxDurationExtra; + kv.first == kLtx2AudioMaxDurationExtra || + kv.first == kLtx2GeneratedKeyframesExtra; if (!known) { Fail("unknown per-generation extra '" + kv.first + "'. This family defines: " + std::string(kLtx2ImageCrfExtra) + ", " + kLtx2AudioPathExtra + ", " + - kLtx2AudioStartTimeExtra + ", " + kLtx2AudioMaxDurationExtra); + kLtx2AudioStartTimeExtra + ", " + kLtx2AudioMaxDurationExtra + ", " + + kLtx2GeneratedKeyframesExtra); } } // The two audio WINDOW knobs only mean something alongside a file. Accepting @@ -1241,6 +1339,7 @@ VideoResult Ltx2VideoEngine::Generate(const VideoGenParams& gen) { } } } + CheckGeneratedKeyframes(gen.extras); if (!gen.prompt.empty() && !im.has_encoder) { Fail( "a prompt was supplied but no text tower is loaded, so it cannot condition this " diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2a0c0f819..1b0625a01 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -263,8 +263,15 @@ vllm_cpp_add_test(test_ltx2_video vllm/multimodal/test_ltx2_video.cpp) # (a merge of origin/main moved all nine of these by 25 lines mid-review), which # is why the anchors are derived from the source and compared with the recorded # ones rather than trusted. +# +# The second path is the conditioning header, and it is a DIFFERENT file from the +# one the refusal it checks lives in. The generated-keyframe refusal states its +# claims about this tree as a list of names, and the suite re-derives each one +# from these declarations. Reading the claim out of the file that makes it is the +# tautology issue #911 records, so the two files are deliberately not the same. target_compile_definitions(test_ltx2_video PRIVATE - LTX2_VIDEO_SOURCE_PATH="${CMAKE_SOURCE_DIR}/src/vllm/multimodal/ltx2_video.cpp") + LTX2_VIDEO_SOURCE_PATH="${CMAKE_SOURCE_DIR}/src/vllm/multimodal/ltx2_video.cpp" + LTX2_CONDITIONING_HEADER_PATH="${CMAKE_SOURCE_DIR}/include/vllm/model_executor/models/ltx2_conditioning.h") # LTX-2.5 image conditioning (row LTX25-IMAGE-COND, issue #644): the CHAIN from # PPM bytes to conditioned, noised tokens, gated against upstream ltx_core AND # ltx_pipelines executed at reduced dimensions. ltx2_image_cond_goldens.inc diff --git a/tests/vllm/multimodal/test_ltx2_video.cpp b/tests/vllm/multimodal/test_ltx2_video.cpp index aa36dc852..1430e0305 100644 --- a/tests/vllm/multimodal/test_ltx2_video.cpp +++ b/tests/vllm/multimodal/test_ltx2_video.cpp @@ -1484,6 +1484,207 @@ TEST_CASE("ltx2 video: keyframe and reference conditioning is refused BY WHAT IS } } +TEST_CASE("ltx2 video: GENERATED keyframe slots are refused BY WHAT IS MISSING, not as a typo") { + // Row LTX25-GENERATED-KEYFRAMES (#920). This is the OTHER feature called + // "keyframe", and the distinction is the whole point of the case above: + // + // supplied -> `VideoConditionByKeyframeIndex`, content from the caller, + // appended with `marked=False` (keyframe_cond.py:84-86) + // GENERATED -> `VideoGeneratedKeyframeSlots`, content from the MODEL, + // appended with `marked=True` (keyframe_slots.py:121) + // + // `extend_keyframes_mask`'s `marked` argument (conditioning/mask_utils.py:76-107) + // is the only difference, and `keyframe_slots.py:121` is upstream's only call + // site that passes True. So generated slots are the ONLY user-facing feature + // that puts #658's trained marker on a token other than the target's own first + // latent frame -- and `KeyframeInterpolationPipeline` is NOT that feature: it + // builds only `VideoConditionByKeyframeIndex` items through + // `image_conditionings_by_adding_guiding_latent` (utils/helpers.py:343-367) + // and does not appear in the feature's own applies-to list + // (ltx-pipelines/docs/conditioning.md:47-51). + // + // Upstream reaches it as `--num-generated-keyframes`, `type=int`, `default=0` + // (ltx-pipelines/utils/args.py:833-844), forwarded to the FIRST diffusion + // stage only. That is a per-CALL argument, so it is a per-generation extra + // here rather than a load option. + Workspace ws; + vllm::multimodal::VideoModelParams mp = FixtureParams(ws.paths); + // Phase 0 only: this case is about the REQUEST being resolved, and the + // zero-is-off arm has to complete a render to prove it. The upsampler is a + // second, unrelated refusal. + mp.extras[vllm::multimodal::kLtx2MaxPhaseExtra] = "0"; + const std::unique_ptr engine = + vllm::multimodal::LoadVideoEngine(mp); + REQUIRE(engine != nullptr); + + auto refusal = [&](const char* value, const char* dir) { + vllm::multimodal::VideoGenParams gen = FixtureGen(ws.root + "/" + dir); + gen.extras[vllm::multimodal::kLtx2GeneratedKeyframesExtra] = value; + try { + (void)engine->Generate(gen); + FAIL_CHECK("num_generated_keyframes=" << value << " must be refused, never dropped"); + return std::string(); + } catch (const std::exception& e) { + return std::string(e.what()); + } + }; + + SUBCASE("a positive count names the READBACK, and rules the other reasons out") { + const std::string msg = refusal("2", "gk2"); + INFO(msg); + // The upstream symbols, so a later reader can go and check whether the + // reason still holds rather than re-deriving it. This is the bar the + // LAST-frame refusal set and the one six refusals in this campaign failed. + CHECK(msg.find("VideoGeneratedKeyframeSlots") != std::string::npos); + CHECK(msg.find("keyframe_slots.py") != std::string::npos); + // The BLOCKER: the readback, which the SUPPLIED arm needs none of. + CHECK(msg.find("GeneratedKeyframeLayout") != std::string::npos); + CHECK(msg.find("generated_keyframes") != std::string::npos); + CHECK(msg.find("extract_generated_keyframes") != std::string::npos); + // The two RULED-OUT reasons, each of which a reader would otherwise reach + // for first. Both must be present and both must be marked as ruled out. + CHECK(msg.find("RULED OUT") != std::string::npos); + CHECK(msg.find("update_attention_mask") != std::string::npos); + CHECK(msg.find("clear_conditioning") != std::string::npos); + CHECK(msg.find("#930") != std::string::npos); + const bool names_the_served_arm = msg.find("last-frame") != std::string::npos || + msg.find("LAST-frame") != std::string::npos; + CHECK(names_the_served_arm); + // The refuted reason may be NAMED -- it is worth telling a reader the marker + // was ruled out -- but never as the thing that is missing. Same guard the + // LAST-frame case carries. + 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); + } + } + SUBCASE("its claims about THIS tree are re-derived from this tree") { + // THE ASSERTION THIS REPAIR EXISTS FOR, and it is a different assertion from + // every one above. + // + // The refusal this one replaces named the token-append machinery as its + // first blocker. That was true when it was written and FALSE by the time it + // reached review: `c7cb59fbb` (row LTX25-TOKEN-APPEND, #930) landed the + // append seam, served the LAST-frame arm through it, and turned the clear + // step from an identity into a real trim. Nothing went red. Every assertion + // in the case above is on an UPSTREAM symbol name -- `update_attention_mask`, + // `clear_conditioning` -- and no change to THIS tree can move one, so the + // suite could not see it. A reviewer's mutation confirmed the hole directly: + // replacing the local-cause sentence with a self-declared falsehood, leaving + // the upstream names alone, kept the suite GREEN at 18/18. + // + // So the message states its local claims in a form the gate can re-derive, + // and re-derives them from `ltx2_conditioning.h` -- a DIFFERENT file from the + // one the message lives in, which is what stops the check from being the + // tautology .agents/issue-index.md #911 records (reading a span out of the + // file it validates reports everything fresh). + // + // Comment lines are stripped before the scan. Without that the ABSENT half + // is answered by prose: this very header's comment at `Ltx2ExtendKeyframesMask` + // names upstream's `VideoGeneratedKeyframeSlots`, so an unstripped search for + // `GeneratedKeyframe` finds a hit and the check silently inverts. + const std::string msg = refusal("2", "gkclaims"); + INFO(msg); + const std::vector header_lines = + SplitLines(ReadSourceFile(LTX2_CONDITIONING_HEADER_PATH)); + REQUIRE(header_lines.size() > 100); + std::string declarations; + for (const std::string& line : header_lines) { + const size_t first = line.find_first_not_of(" \t"); + if (first != std::string::npos && line.compare(first, 2, "//") == 0) continue; + declarations += line; + declarations += '\n'; + } + // The header must still be mostly declarations after stripping, or a + // reformat that turned it into one comment block would answer ABSENT for + // free. Positive control on the instrument itself. + REQUIRE_MESSAGE(declarations.find("struct Ltx2LatentState") != std::string::npos, + "comment stripping removed the declarations it was meant to keep"); + + // The claimed names are read OUT OF THE MESSAGE, never listed here: a list + // here would be a second place the claim lives, and the two would drift. + auto names_after = [&](const std::string& marker) { + std::vector out; + const size_t at = msg.find(marker); + REQUIRE_MESSAGE(at != std::string::npos, + "the refusal no longer carries its '" << marker << "' clause"); + const size_t start = at + marker.size(); + const size_t stop = msg.find('.', start); + REQUIRE(stop != std::string::npos); + std::string list = msg.substr(start, stop - start); + size_t from = 0; + while (from <= list.size()) { + const size_t comma = list.find(',', from); + std::string name = list.substr(from, comma == std::string::npos ? std::string::npos + : comma - from); + const size_t b = name.find_first_not_of(" \t"); + const size_t e = name.find_last_not_of(" \t"); + if (b != std::string::npos) out.push_back(name.substr(b, e - b + 1)); + if (comma == std::string::npos) break; + from = comma + 1; + } + return out; + }; + + const std::vector declared = names_after("DECLARED HERE: "); + const std::vector absent = names_after("ABSENT HERE: "); + // A count floor, because an empty list satisfies every "for each" below and + // reports a pass over nothing. + CHECK(declared.size() >= 3); + CHECK(absent.size() >= 2); + for (const std::string& name : declared) { + INFO("claimed DECLARED: " << name); + CHECK_MESSAGE(declarations.find(name) != std::string::npos, + "the refusal claims ltx2_conditioning.h declares '" + << name << "', and it does not. The message is stale about THIS tree"); + } + for (const std::string& name : absent) { + INFO("claimed ABSENT: " << name); + CHECK_MESSAGE(declarations.find(name) == std::string::npos, + "the refusal claims ltx2_conditioning.h has no '" + << name + << "', and it does. If the readback landed, this refusal is no longer " + "true and must be rewritten or removed"); + } + } + SUBCASE("and it is NOT the generic unknown-extra message") { + // THE ASSERTION THIS ROW EXISTS FOR. Without it the case above passes + // against the message the tree already has, because "unknown per-generation + // extra 'num_generated_keyframes'. This family defines: image_crf" contains + // no upstream symbol at all -- and a refusal that says the family does not + // define the key sends the reader looking for a typo instead of for the + // unported machinery. That is the distinction `CheckUnservedExtras` was + // written for on the load side (#611). + const std::string msg = refusal("1", "gk1"); + INFO(msg); + CHECK(msg.find("unknown per-generation extra") == std::string::npos); + } + SUBCASE("zero is upstream's DEFAULT and must not refuse") { + // args.py:836 is `default=0`, and `has_generated_keyframes` + // (utils/helpers.py:384-391) reads 0 as off. A caller that plumbs the + // default through must get a render, not a refusal. This is the half a + // naive port breaks -- "the key is present, so refuse" is one line shorter + // and wrong. + vllm::multimodal::VideoGenParams gen = FixtureGen(ws.root + "/gk0"); + gen.extras[vllm::multimodal::kLtx2GeneratedKeyframesExtra] = "0"; + const vllm::multimodal::VideoResult result = engine->Generate(gen); + CHECK(result.frame_count == 9); + CHECK(result.width == 32); + CHECK(result.height == 32); + } + SUBCASE("a negative count gets upstream's OWN reason, not the unported one") { + // `evenly_spaced_keyframe_positions` raises "num_keyframes must be + // non-negative" (utils/helpers.py:372-373) before anything looks at the + // checkpoint. A malformed request and an unported arm are different + // answers, and collapsing them would tell a caller who typed `-1` to go + // read about attention masks. + const std::string msg = refusal("-1", "gkneg"); + INFO(msg); + CHECK(msg.find("non-negative") != std::string::npos); + CHECK(msg.find("update_attention_mask") == std::string::npos); + } +} + TEST_CASE("ltx2 video: an image at crf 0 conditions the render, and the ENCODER weights are read") { // The arm row LTX25-IMAGE-COND (#644) opened. Two separate claims are made // here and they are NOT the same claim: