Skip to content

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

Closed
localai-bot wants to merge 1 commit into
row/LTX25-L9C-CONNECTOR-DRAINfrom
row/LTX25-L9C-REGISTER-GATE
Closed

test(ltx-2.5): the register boundary, made numerically reachable (#435)#597
localai-bot wants to merge 1 commit into
row/LTX25-L9C-CONNECTOR-DRAINfrom
row/LTX25-L9C-REGISTER-GATE

Conversation

@localai-bot

Copy link
Copy Markdown
Collaborator

Closes review finding F1 (MEDIUM) on PR #574, plus the sibling F3 (LOW) and F4 (INFO). Issue: #435.

Base is row/LTX25-L9C-CONNECTOR-DRAIN at 708e05b56.

The gap

prompt_embeds_valid_rows is the single knob deciding which token positions become the connector's trained learnable_registers (104 of 128 rows on the accepted render). A fresh reviewer changed s = im.prompt_valid_rows to s = im.prompt_valid_rows + 1 at src/vllm/multimodal/ltx2_video.cpp:744 and test_ltx2_video stayed fully green — 23/23 cases, 253/253 assertions, exit 0. Reproduced here before anything was written.

The existing case asserts only that a render at valid=4 differs from one at valid=2. Any monotone corruption of the boundary keeps that true. The defect it cannot see is one padded row conditioned on caller junk instead of a trained register: finite, correctly shaped, plausible, wrong.

The closure

The substitution — not the mask — is what carries the boundary. binary_mask * hidden_states + (1 - binary_mask) * registers (embeddings_connector.py:148-150) replaces a masked position's features, so what the caller supplied there cannot reach the render. The mask carries nothing either way: :152 returns torch.zeros_like(mask), and _to_binary_mask's < 0.000001 (embeddings_processor.py:46-48) is satisfied by both values an additive mask holds.

So positions separate by whether perturbing them moves a byte, and the boundary is pinned from both sides:

  • rows [valid, N) are registers → perturbing them must change nothing
  • rows [0, valid) are caption → perturbing the last one must change something

Polarity from (attention_mask - 1) * finfo.max (transformer_args.py:203-206); contiguity from _compute_right_pad_order's stable descending argsort (embeddings_processor.py:33-38), which is what makes the padded set the suffix the extra names.

RED evidence

Each mutation applied to the product code, built and run in one chain, then restored — git diff empty and HEAD unchanged after every one.

mutation failing subcase doctest
prompt_valid_rows + 1 every row at or past the count is a register 1/1 failed, 110/111 assertions, Status: FAILURE!, exit 1
prompt_valid_rows - 1 the row just BEFORE the count is caption 1/1 failed, 110/111 assertions, Status: FAILURE!, exit 1
0 both caption arms 1/1 failed, 109/111 assertions, Status: FAILURE!, exit 1
v_rows every row at or past the count is a register 1/1 failed, 110/111 assertions, Status: FAILURE!, exit 1

And + 1 over the whole suite: 24 | 23 passed | 1 failed, 364 | 363 passed | 1 failed. Only the new case moves — the reviewer's finding reproduced and closed in one run.

Siblings

F3. Ltx2ParseConnectorConfig reads connector_num_learnable_registers; neither configurator does (embeddings_connector.py:194-219, :222-256 both take the class default of 128), so "key for key" is not literally true of that key. The divergence is deliberate and stays, but a read nothing can falsify is a claim rather than a gate — so it now has one: the fixture's stored table stays [2, dim] while the config declares 4, and the shape check fires. REDs when the read is made inert (4/5 assertions).

The correction lands in porting-inventory.md and the test, not as a comment beside the parse: check-doc-checkpoint classifies src/vllm/model_executor/models/ as feature_surface and would demand a docs/FEATURES.md entry for a comment that changes no feature. The parse's existing comment already says "NOT read by either configurator".

F4. The masked-mask refusal claimed it fires at num_learnable_registers = 0. It cannot — with registers disabled the additive mask passes straight through and < 0.000001 is still true at every position. The comment and message now say the branch is unreachable on every input either reference produces, name what would reach it, and say why it is not gated.

porting-inventory.md's "mutation M3/M4" labels existed only in a PR body; replaced with the test case names, which are in the tree.

Not reproduced: the shipped-checkpoint case does not skip silently — MESSAGE: SKIPPED: set LTX2_CHECKPOINT_ROOT ... prints in the baseline run. It is invisible under ctest's output capture, and its env is deliberately not the tree-wide CHECKPOINT_ROOT (.env.example: that one "declares an INTENT, not a behaviour" and nothing reads it). Both now stated at the case.

Gate

Clean-configured CPU-only Release build (-DCMAKE_BUILD_TYPE=Release -DVLLM_CPP_CUDA=OFF), rebuilt from a wiped CMakeCache.txt/CMakeFiles:

  • BUILD_EXIT=0, enospc_hits=0, werror_hits=0, df 89% throughout
  • ctest -NTotal Tests: 399
  • ctest -j4100% tests passed, 0 tests failed out of 399, CTEST_EXIT=0
  • test_ltx2_video24 | 24 passed | 0 failed, 365 | 365 passed | 0 failed, Status: SUCCESS! (baseline was 23 | 23, 253 | 253)
  • check-doc-checkpoint.py --base 708e05b56 --head HEADOK

🤖 Generated with Claude Code

FOLLOWING_AGENTS_PROTOCOL

A fresh review of the L9c connector PR passed the change and found a hole in its
gate. `prompt_embeds_valid_rows` is the single knob deciding which token
positions become the connector's TRAINED `learnable_registers` and which carry
caller-supplied conditioning -- on the accepted render, 104 of 128 rows are
registers. The reviewer changed `s = im.prompt_valid_rows` to
`s = im.prompt_valid_rows + 1` at `ltx2_video.cpp:744` and `test_ltx2_video`
stayed fully green: 23/23 cases, 253/253 assertions, exit 0. Reproduced here
before anything was written.

WHY THE COVERAGE COULD NOT SEE IT. The only case on that knob asserted that a
render at valid=4 differs from one at valid=2. ANY monotone corruption of the
boundary keeps that true, so the whole family passes it. The defect it cannot
see is ONE padded row conditioned on caller junk instead of a trained register:
finite, correctly shaped, plausible, and wrong.

WHAT MAKES THE BOUNDARY REACHABLE IS THE SUBSTITUTION, NOT THE MASK.
`binary_mask * hidden_states + (1 - binary_mask) * registers`
(embeddings_connector.py:148-150) REPLACES a masked position's features, so what
the caller supplied there cannot reach the render at all. The mask carries
nothing: with registers on, :152 returns `torch.zeros_like(mask)`, and
`_to_binary_mask`'s `encoded_mask < 0.000001` (embeddings_processor.py:46-48) is
satisfied by BOTH values an additive mask holds, so what the DiT receives is all
ones either way.

Positions therefore separate by whether perturbing them moves a byte, and the
boundary is pinned from BOTH sides:

  rows [valid, N) are REGISTERS -> perturbing them must change NOTHING
  rows [0, valid)  are CAPTION  -> perturbing the last one must change something

The polarity is `(attention_mask - 1) * finfo.max`
(transformer_args.py:203-206) and the contiguity is `_compute_right_pad_order`'s
stable descending argsort (embeddings_processor.py:33-38), which is what makes
the padded set the suffix `[valid, N)` that the extra names.

RED FIRST, on the product code, restored byte-for-byte after each run
(`git diff` empty, HEAD unchanged, verified per mutation):

  + 1      "every row at or past the count is a register" FAILS   110/111
  - 1      "the row just BEFORE the count is caption"     FAILS   110/111
  0        both caption arms                              FAIL    109/111
  v_rows   "every row at or past the count is a register" FAILS   110/111

and `+ 1` over the WHOLE suite: 24 cases, 23 passed, 1 failed; 364 assertions,
363 passed, 1 failed. Only the new case moves, which is the finding reproduced
and closed in one run.

TWO SIBLING FINDINGS FOLDED IN.

F3 is a correction of a claim, and then a gate under it. The L9c commit message
said `Ltx2ParseConnectorConfig` "mirrors both configurators key for key". It
does not: `connector_num_learnable_registers` is read here and by NEITHER
configurator (embeddings_connector.py:194-219 and :222-256 both take the class
default of 128). The divergence is deliberate and stays -- a checkpoint
declaring something else must not be silently run at 128 -- but a read nothing
can falsify is a claim rather than a gate, so it now has one: the fixture's
stored table stays `[2, dim]` while the config declares 4, and
`Ltx2LoadConnectorWeights`' shape check fires. REDs when the read is made inert.

The correction itself lands in `porting-inventory.md` and in the test, NOT as a
comment beside the parse. `check-doc-checkpoint` classifies any path under
`src/vllm/model_executor/models/` as `feature_surface` and would demand a
`docs/FEATURES.md` entry for a comment that changes no feature -- and inventing
one to satisfy a checker is a false record. The parse's existing comment already
says "NOT read by either configurator", so nothing there was untrue; what was
missing was the gate, and the gate is what this adds.

F4 is an unreachable branch that named the wrong reason. The refusal on a masked
cross-attention mask said it fires "when the connector runs with
num_learnable_registers = 0". It cannot: with registers disabled the caller's
additive mask passes straight through and `< 0.000001` is still true at every
position, so the branch is unreachable on every input either reference produces.
The comment and the message now say that, name what WOULD reach it, and say why
it is not gated.

Also in the record: `porting-inventory.md` cited "mutation M3/M4" by labels that
existed only in a PR body, which git cannot resolve -- replaced with the test
case names, which are in the tree.

NOT REPRODUCED, and recorded rather than dropped: the shipped-checkpoint case
does not skip silently. `MESSAGE: SKIPPED: set LTX2_CHECKPOINT_ROOT ...` prints
in the binary's own output, in the baseline run above. It IS invisible under
`ctest`, which captures a passing test's output, and its env is deliberately not
the tree-wide `CHECKPOINT_ROOT` -- `.env.example` says that one "declares an
INTENT, not a behaviour" and nothing reads it. Both are now stated at the case
so a green ctest row cannot be read as "the shipped checkpoints were read".

Gate: clean-configured CPU-only Release build, BUILD_EXIT=0, no
`No space left`/`BFD assertion`/kill in the log, `ctest -N` 399 tests.

Issue: #435

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
localai-bot pushed a commit that referenced this pull request Aug 13, 2026
…lly was one

Collapses the L9 stack. The register-gate fix (#597, which closed the reviewer's
prompt_valid_rows finding two-sided) is already folded in below this.

Two conflicts, and they wanted opposite treatments, which is the whole reason
the "union it" reflex is wrong on record files.

.agents/benchmark-record.md IS genuinely append-only: both sides appended
distinct sections at the same anchor, nothing keyed collided. Unioned in
chronological order, older (the 2026-08-11 BENCHMARKS.md overflow move) before
newer (L9c's drain measurement), and verified both survive.

docs/FEATURES.md is the same single keyed row for the THIRD time this campaign,
and again neither side was a superset. L9C knew the render had become a scene at
320x192/25f with the connector wired; L9B knew the coverage had grown to both
VAE encoders and the conditioning items, and that torchao-NVFP4 loads while the
first-party file does not. Composed the facts true of the merged tree.

The row then failed check-public-doc-tables at 221 chars in one cell against the
220 cap -- worth recording that the budget bites at exactly the point a merge
tempts you to concatenate two descriptions instead of writing one.

Issue: #435

FOLLOWING_AGENTS_PROTOCOL

Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [Claude Code]
@localai-bot

Copy link
Copy Markdown
Collaborator Author

Landed on row/MODEL-DIFFUSION-LTX25 at aa6aa0ecd846301cfb is an ancestor (verified with git merge-base --is-ancestor), folded in via the L9-stack collapse at 2c1dda5df / 25bfd6640.

Closing rather than merging because the base branch it targeted (row/LTX25-L9C-CONNECTOR-DRAIN) has itself been collapsed into the campaign; merging here would re-open a stack that no longer exists. Nothing is lost — the commits are on the campaign branch and reach main through #437.

The finding it closed is worth restating, since it is the fourth instance of one class in this campaign: mutating prompt_valid_rows by +1 left the entire suite green, because the only case asserted that two renders DIFFER, and any monotone corruption of the boundary satisfies that. It is now pinned two-sided — +1, -1, 0 and v_rows each red exactly one subcase, and under +1 across the whole suite only the new case moves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants