test(ltx-2.5): the register boundary, made numerically reachable (#435)

.agents/porting-inventory.md

@@ -1519,19 +1519,44 @@ Examples: `examples/cli` ✅ (C-API client), `examples/server` ✅ (OpenAI serve
       identical `(video_attn_mask < 1e-6)` and the identical multiply. Checked on both
       BEFORE mirroring it, because the reading a port arrives at by reasoning about
       intent (`>= 0`) is the opposite at padded positions. Gated as the surprising
-      behaviour, so "fixing" it REDs (`test_ltx2_pipeline`, mutation M3).
+      behaviour, so "fixing" it REDs: `test_ltx2_pipeline`, case "ltx2 the processor's
+      binary mask mirrors a comparison that looks backwards".
+    * **THE REGISTER BOUNDARY IS NOW NUMERICALLY REACHABLE (2026-08-13, review
+      finding F1 on the L9c PR).** `prompt_embeds_valid_rows` is the single knob
+      deciding which positions become the connector's TRAINED registers, and the
+      only case covering it asserted that valid=4 renders differently from valid=2.
+      Every monotone corruption of that boundary keeps that true, so a fresh
+      reviewer's `prompt_valid_rows + 1` left the suite fully green — one padded row
+      conditioned on caller junk instead of a register: finite, correctly shaped,
+      plausible, wrong. Closed by asserting WHICH positions are registers, from both
+      sides: perturbing rows `[valid, N)` must move NOTHING (they are substituted at
+      `embeddings_connector.py:148-150`) and perturbing row `valid - 1` must move
+      something. `test_ltx2_video`, case "the register boundary sits EXACTLY at the
+      valid-row count"; RED under `+ 1`, `- 1`, `0` and `v_rows`.
     * **A CONFIG VALUE THAT IS NOT NEAR ITS DEFAULT.** LTX-2.5 declares
       `connector_positional_embedding_max_pos = [4096]` where `Embeddings1DConnector`'s
       class default is `[1]`, and `get_fractional_positions` DIVIDES the token index by
       it (`rope.py:132-141`). No shape can see the difference; the default is every
       RoPE angle wrong. `positional_embedding_theta` is deliberately NOT read from the
       DiT config even though one is declared, because neither configurator passes it —
       reading it would be a re-invention rather than a port.
+    * **ONE KEY WHERE THIS PARSE DIVERGES RATHER THAN MIRRORS (2026-08-13, review
+      finding F3).** `Ltx2ParseConnectorConfig` reads
+      `connector_num_learnable_registers`, which NEITHER configurator does
+      (`embeddings_connector.py:194-219` and `:222-256` both leave it at the class
+      default of 128), so "mirrors both configurators key for key" is not literally
+      true of this one key. The divergence is kept — a checkpoint declaring something
+      else must not be silently run at 128 — and is now ENFORCED rather than asserted:
+      `test_ltx2_video`, case "a connector config that disagrees with the FILE is
+      refused", subcase "a register count the file's TABLE does not carry is refused",
+      which REDs when the read is made inert.
     * **The record changed, not just the code.** The two connector families are no
       longer reported as unported: this port reads them, so naming them would say
       something untrue about the tree and would demand `allow_unported_modules` from a
       caller whose checkpoint is read completely. Asserted as an ABSENCE in
-      `test_ltx2_loader`, so restoring the old behaviour REDs (mutation M4).
+      `test_ltx2_loader`, case "ltx2 loader: the unported families are refused by name,
+      not absorbed" (`CHECK(conn_ck.unported.empty())`), so restoring the old behaviour
+      REDs.
     * **OWED, and precisely:** the Gemma-4 TOWER, so what ENTERS the connector is still
       whatever the caller put in the prompt-embeds file — the link below the tower is
       real, the tower is not. `prompt_embeds_valid_rows` exists because a file carries

src/vllm/multimodal/ltx2_video.cpp

@@ -770,17 +770,31 @@ std::unique_ptr<Ltx2VideoEngine> Ltx2VideoEngine::Load(const VideoModelParams& p
       // honour (embeddings_processor.py:89). `Ltx2ModalityInput` carries no
       // context mask, so a mask with a masked position would be silently dropped
       // — the DiT would attend over register-free padding as if it were caption.
-      // With registers enabled every position is attendable and the mask is all
-      // ones, which is the case the shipped checkpoint is in; anything else is
-      // refused by name rather than ignored.
+      //
+      // THIS LOOP IS UNREACHABLE ON EVERY INPUT EITHER REFERENCE PRODUCES, and
+      // saying which case reaches it is how the claim stays checkable. It is NOT
+      // the `num_learnable_registers = 0` case, which an earlier version of this
+      // comment named and which cannot get here: with registers disabled
+      // `Ltx2ConnectorForward` passes the caller's ADDITIVE mask straight
+      // through, and `_to_binary_mask`'s `encoded_mask < 0.000001`
+      // (embeddings_processor.py:46-48) is satisfied by BOTH values an additive
+      // mask holds — 0.0 and -finfo(f32).max — so the binary mask is one
+      // everywhere there too. With registers enabled :152 returns
+      // `torch.zeros_like(mask)` and the answer is one everywhere for the same
+      // reason. What would reach it is a connector whose output mask carries a
+      // value at or above +1e-6, which no path in `ltx_core` or `diffusers`
+      // emits today. It is kept as a guard on that future, refused by name
+      // rather than ignored, and it is deliberately not gated: a test would have
+      // to fabricate a mask neither reference can produce.
       for (const float m : encoded.mask) {
         if (m == 1.0f) continue;
         Fail(
             "the embeddings connector returned a cross-attention mask with masked "
             "positions, and `Ltx2ModalityInput` carries no context mask to pass it through. "
-            "That happens when the connector runs with num_learnable_registers = 0, where "
-            "padding stays padding instead of becoming a register. Refusing rather than "
-            "dropping the mask, which would condition the DiT on unmasked padding.");
+            "No path in either reference emits such a mask — `_to_binary_mask` is one "
+            "everywhere for both values an additive mask holds — so this is a connector "
+            "whose output mask this port does not model. Refusing rather than dropping "
+            "the mask, which would condition the DiT on unmasked padding.");
       }
       im.video_prompt_embeds = encoded.video;
       im.audio_prompt_embeds = encoded.audio;

tests/vllm/multimodal/test_ltx2_video.cpp

@@ -100,6 +100,24 @@ std::string ReadAll(const std::string& path) {
   return std::string((std::istreambuf_iterator<char>(in)), std::istreambuf_iterator<char>());
 }
 
+// A prompt-embeds file as floats, and back. The register cases need to change
+// ONE row of a supplied stream and leave every other byte alone, which the
+// fixture's seeded writer cannot express.
+std::vector<float> ReadFloats(const std::string& path) {
+  const std::string bytes = ReadAll(path);
+  REQUIRE(bytes.size() % sizeof(float) == 0);
+  std::vector<float> out(bytes.size() / sizeof(float));
+  std::memcpy(out.data(), bytes.data(), bytes.size());
+  return out;
+}
+
+void WriteFloats(const std::string& path, const std::vector<float>& values) {
+  std::ofstream out(path, std::ios::binary);
+  REQUIRE_MESSAGE(out.good(), "cannot write ", path);
+  out.write(reinterpret_cast<const char*>(values.data()),
+            static_cast<std::streamsize>(values.size() * sizeof(float)));
+}
+
 bool Registered(const std::string& family) {
   const std::vector<std::string> all = vllm::multimodal::RegisteredVideoFamilies();
   return std::find(all.begin(), all.end(), family) != all.end();
@@ -841,6 +859,20 @@ TEST_CASE("ltx2 video: an ABI client loads, detects and generates through vllm.h
 // otherwise rather than silently passing: a gate that quietly does nothing when
 // its input is absent is how "we tested the real checkpoint" becomes untrue.
 //
+// HOW FAR THAT ANNOUNCEMENT CARRIES, stated so nobody over-reads it. The
+// `MESSAGE` below is printed by the doctest binary's own run, so `test_ltx2_video`
+// executed directly says SKIPPED on its own output. `ctest` captures a passing
+// test's output and prints only the pass line, so under `ctest` this case is
+// indistinguishable from one that ran. Read "the shipped checkpoints were read"
+// off the binary's output or off an explicitly set env, never off a green ctest
+// row.
+//
+// The env is deliberately NOT the tree-wide `CHECKPOINT_ROOT`. That one "declares
+// an INTENT, not a behaviour" and nothing in the tree reads it (`.env.example`),
+// while this needs a path to one specific publisher tree whose internal layout
+// (`diffusion_models/…`) the case walks. Pointing it at the shared root would
+// make this its first reader and change what that variable means.
+//
 // It deliberately stops short of the DiT FORWARD. At the shipped 21.00B geometry
 // the f32 parity forward needs ~76 GB of weights and ~2.6e14 FLOPs per step, and
 // the device path cannot feed it (see the `device = 1` refusal). What is checked
@@ -1131,6 +1163,28 @@ TEST_CASE("ltx2 video: a connector config that disagrees with the FILE is refuse
     CHECK(msg.find("transformer_1d_blocks.2") != std::string::npos);
   }
 
+  SUBCASE("a register count the file's TABLE does not carry is refused") {
+    // `connector_num_learnable_registers` is the ONE key `Ltx2ParseConnectorConfig`
+    // reads that NEITHER upstream configurator does: `Embeddings1DConnectorConfigurator`
+    // (embeddings_connector.py:194-219) and its audio sibling (:222-256) both leave
+    // it at the class default of 128, so "mirrors both configurators key for key" is
+    // not literally true of this one key and the divergence is deliberate — a
+    // checkpoint declaring something else must not be silently run at 128.
+    //
+    // A read that nothing can falsify is a claim, not a gate, and this is what
+    // falsifies it: the fixture's stored table stays `[2, dim]` while the config
+    // declares 4, and `Ltx2LoadConnectorWeights`' shape check is what has to fire.
+    ltx2_fixture::ReducedDitOptions relabelled;
+    relabelled.transformer_overrides["connector_num_learnable_registers"] = 4;
+    const std::string path = ws.root + "/dit_conn_registers.safetensors";
+    ltx2_fixture::WriteReducedDit(dit, path, relabelled);
+    vllm::multimodal::VideoModelParams mp = FixtureParams(ws.paths);
+    mp.dit_path = path;
+    const std::string msg = RefusalOf(mp);
+    INFO(msg);
+    CHECK(msg.find("learnable_registers") != std::string::npos);
+  }
+
   SUBCASE("gating the config declares but the file does not carry is refused") {
     ltx2_fixture::ReducedDitOptions ungated;
     ungated.connector.gated = false;  // writes NO to_gate_logits tensors...
@@ -1197,3 +1251,96 @@ TEST_CASE("ltx2 video: the valid-row count decides which positions are registers
     CHECK(msg.find(vllm::multimodal::kLtx2PromptValidRowsExtra) != std::string::npos);
   }
 }
+
+TEST_CASE("ltx2 video: the register boundary sits EXACTLY at the valid-row count") {
+  // WHY THE CASE ABOVE IS NOT ENOUGH, and this one exists. It asserts only that
+  // a render at valid=4 differs from one at valid=2. ANY monotone corruption of
+  // the boundary keeps that true — `prompt_valid_rows + 1` still renders
+  // differently from valid=2 — so the whole family passes it. The defect it
+  // cannot see is ONE padded row conditioned on caller junk instead of the
+  // connector's TRAINED register: finite, correctly shaped, plausible, wrong.
+  //
+  // WHAT MAKES THE BOUNDARY NUMERICALLY REACHABLE is the substitution itself.
+  // `binary_mask = additive_attention_mask[:, 0, 0, :] >= 0` then
+  // `binary_mask * hidden_states + (1 - binary_mask) * registers`
+  // (embeddings_connector.py:148-150) REPLACES a masked position's features
+  // outright, so what the caller supplied there cannot reach the render at all.
+  // Positions therefore separate by whether perturbing them moves a byte:
+  //
+  //   rows [valid, N) are REGISTERS -> perturbing them must change NOTHING
+  //   rows [0, valid)  are CAPTION  -> perturbing the last one must change something
+  //
+  // Together those pin the boundary from both sides, which is what "which
+  // positions are registers" means when the conditioning itself is not
+  // observable from outside the engine. `+ 1` breaks the first (row `valid`
+  // keeps caller junk); `- 1` breaks the second (row `valid - 1` becomes a
+  // register and its content stops mattering). The reviewer's mutation was the
+  // former and the suite stayed fully green.
+  //
+  // THE POLARITY AND THE CONTIGUITY ARE BOTH UPSTREAM'S.
+  // `_prepare_attention_mask` builds `(attention_mask - 1) * finfo.max`
+  // (transformer_args.py:203-206), so 0.0 is kept and -finfo.max is padded; and
+  // `_compute_right_pad_order`'s stable descending argsort
+  // (embeddings_processor.py:33-38) makes the padded set the CONTIGUOUS SUFFIX
+  // `[valid, N)` that `prompt_embeds_valid_rows` names. Neither the mask
+  // comparison nor the returned mask can carry this: with registers on, :152
+  // returns `torch.zeros_like(mask)` and `_to_binary_mask`'s `< 0.000001`
+  // (:46-48) is satisfied by BOTH values an additive mask holds, so the mask the
+  // DiT receives is all ones either way. The substitution is the only carrier.
+  Workspace ws;
+  const vllm::Ltx2DitParams dit = ltx2_fixture::ReducedDitParams();
+  const int64_t width = dit.cross_attention_dim;
+  const int64_t rows = 4;   // `ltx2_fixture::WriteFixture`'s `prompt_tokens`
+  const int64_t valid = 2;  // two caption rows, two padded ones
+
+  const std::vector<float> base = ReadFloats(ws.paths.video_embeds);
+  REQUIRE(base.size() == static_cast<size_t>(rows * width));
+
+  // A perturbation big enough that no 8-bit frame byte can absorb it, and
+  // column-varying so it cannot turn one row into a copy of another.
+  auto perturb_rows = [&](const std::string& name, int64_t first, int64_t last) {
+    std::vector<float> values = base;
+    for (int64_t r = first; r < last; ++r) {
+      for (int64_t c = 0; c < width; ++c) {
+        values[static_cast<size_t>(r * width + c)] += 3.0f + 0.25f * static_cast<float>(c);
+      }
+    }
+    const std::string path = ws.root + "/" + name + ".f32";
+    WriteFloats(path, values);
+    return path;
+  };
+
+  vllm::multimodal::VideoModelParams half = FixtureParams(ws.paths);
+  half.extras[vllm::multimodal::kLtx2PromptValidRowsExtra] = std::to_string(valid);
+  const std::string reference = RenderBytes(half, ws.root + "/reg_ref");
+  // Without this an `==` below could pass on two empty reads rather than on two
+  // renders.
+  REQUIRE(reference.size() > 1000);
+  // ...and the same request twice is byte-identical, which is what makes the
+  // equality an assertion about the registers rather than about noise.
+  REQUIRE(RenderBytes(half, ws.root + "/reg_ref2") == reference);
+
+  SUBCASE("every row at or past the count is a register, so its content is inert") {
+    vllm::multimodal::VideoModelParams padded = half;
+    padded.prompt_embeds_path = perturb_rows("perturbed_padding", valid, rows);
+    // REDs at `prompt_valid_rows + 1`: row `valid` would keep the caller's junk
+    // instead of becoming the trained register.
+    CHECK(RenderBytes(padded, ws.root + "/reg_padded") == reference);
+  }
+
+  SUBCASE("the row just BEFORE the count is caption, so its content is not") {
+    vllm::multimodal::VideoModelParams caption = half;
+    caption.prompt_embeds_path = perturb_rows("perturbed_last_caption", valid - 1, valid);
+    // REDs at `prompt_valid_rows - 1`: row `valid - 1` would be replaced by a
+    // register and the perturbation would stop reaching the DiT.
+    CHECK(RenderBytes(caption, ws.root + "/reg_caption") != reference);
+  }
+
+  SUBCASE("the FIRST row is caption too, so the boundary is not the whole prompt") {
+    // Guards the degenerate reading where nothing is caption: a mask of all
+    // -finfo.max renders the register table and only the register table.
+    vllm::multimodal::VideoModelParams first = half;
+    first.prompt_embeds_path = perturb_rows("perturbed_first", 0, 1);
+    CHECK(RenderBytes(first, ws.root + "/reg_first") != reference);
+  }
+}