fix(LTX25-POSITION-CONTRACT): gate the tower positions as integers, and stop a note claiming a detection the same file measures as absent - #1797
Merged
Conversation
…eally closer #1467 measured the position-renumbering mutant scoring closer to the LTX-2.5 tower oracle than the port, and filed that as an instrument defect. The honest reading of the same table is that our positions are wrong, and nobody had checked. This spec checks it before anything is repaired, because the answer decides whether the repair is a comment or a correctness fix. It is a comment. Upstream passes no `position_ids` on this path, so transformers derives them from `torch.arange` over the PADDED width and the real tokens start at `first_valid` -- which is what the port writes and what the mutant does not. The `attention_mask.cumsum(-1) - 1` renumbering exists only inside `generate()`, which a plain module call never reaches. Asked of the oracle directly, against its own left-padded run, the port's absolute numbering is closer at 12 of the 13 states in bf16 and the mutant at none; the end-to-end table reads the other way only because our bf16 error and the mutant's perturbation are the same size and can cancel. `scripts/probe-ltx2-tower-positions.py` is that measurement. It imports the committed golden generator and calls the generator's own `build_tower` and `run_tower`, so the fixture behind the verdict cannot drift away from the fixture behind the goldens. It reproduces identically under transformers 5.12.1 (the goldens' oracle) and 5.14.1 (the parity pin). FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
…nd stop a note claiming a detection the same file measures as absent `test_ltx2_text_encoder.cpp` contradicted itself. The note heading the `"ltx2 prompt -> conditioning: the VALUES"` case told the reader that renumbering the tower's positions from zero "does red this case, but only at 1.10x the audio floor". A measured table 140 lines below it, in the same file, showed that is false post-`4712dac40`: the correct code sits at 1.209x/1.313x of the floor and the mutant at 0.683x/0.931x, so the comparison had inverted. A reader who stopped at the note trusted a detection that does not exist. Re-measured at this HEAD before touching the note, and it is worse than #1467 recorded: with `positions[i] = i` in `ltx2_text_encoder.cpp` the whole 27-case suite is green, 4118 of 4118 assertions. Not one case detects it. The coverage moves rather than being dropped. `Ltx2PromptConditioning` now carries the `positions` vector the tower actually ran on -- by reference, not as a copy built beside it -- and the case REQUIREs it against the golden's own pad count. That is an integer contract, upstream of every rounding that could absorb it. No tolerance is widened and none is moved; #1668 forbids exactly that, and no constant could recover a detection whose ordering has reversed. The question the inverted table raises is whether our positions are wrong, since the mutant is measurably closer to the oracle. They are not, and the spec carries the work. Upstream passes no `position_ids`, transformers derives them from `torch.arange` over the padded width, and the real tokens therefore start at `first_valid`. Asked of the oracle directly, against its own left-padded run, the port's numbering is closer at 12 of 13 states in bf16 and the mutant at none of them -- 2.64x. The end-to-end conditioning reads the other way because our bf16 realization and the oracle's sit on opposite sides of one f32 trajectory, so a perturbation of the same size can land closer by cancellation. No new issue is filed because there is no defect to file. Closes #1467. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
…es and what the reachability mutation does not Four notes from the fresh review, none blocking, all about a record claiming more than it measured. The reference binding on `out.positions` is the whole contract and nothing gates it. The review mutated it: turn the `&` into a copy, fill the field correctly and hand the tower a renumbered vector, and the suite stays green at 27/27 and 4127/4127 while the value arms visibly report the mutant's ratios. The comment now says that, so a refactor that drops the `&` is not a silent one. The spec's `## Design` item 4 was headed "Reachability" and delivered something narrower. Deleting the `ltx2_video.cpp:2278` call site does NOT red this case, because the case enters at `Ltx2EncodePromptToConditioning` rather than at `Ltx2VideoEngine::Generate` -- pre-existing test architecture that this row adds three assertions to rather than introduces. Recorded plainly instead of left for a reader to mistake `M1` for a reachability proof. The evidence table labelled its first two rows "HEAD" when their assertion counts and the recorded pristine sha both belong to the base tree. Relabelled, and row 3 now also records that the value arms still PASS under the mutation, which is the point of the row. The probe script is run by no gate and cannot be, because CI has neither torch nor a `transformers` that registers `gemma4_unified`. It reads the golden generator's function signatures, so it rots silently if those change. Named as owed rather than assumed durable. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
Brings in `VT-CONV1D-BLOCK-CONDITION` (#1770), which touches `vt::cpu::Conv1dTimeBlock` and `tests/vllm/models/test_ltx2_vae.cpp` -- a different kernel and a different test binary from this row's path, so the interaction risk is low. Merged and re-gated rather than assumed: after the merge `test_ltx2_text_encoder` is `SUCCESS` at 27/27 cases and 4127 of 4127 assertions. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tests/vllm/models/test_ltx2_text_encoder.cppcontradicted itself in one file.The note heading the
"ltx2 prompt -> conditioning: the VALUES, against the left-padded oracle"case told the reader that renumbering the tower's positionsfrom zero "does red this case, but only at 1.10x the audio floor". A measured
table 140 lines below it, in the same file, showed that is false post-
4712dac40:the correct code sits at 1.209x/1.313x of the propagated floor and the mutant at
0.683x/0.931x, so the comparison had INVERTED. A reader who stopped at the note
trusted a detection that does not exist.
Re-measured at this base before the note was touched, and it is worse than #1467
recorded. With
positions[i] = iinsrc/vllm/model_executor/models/ltx2_text_encoder.cppthe whole 27-case suite is green, 4118 of 4118 assertions. Not one case detects
the defect.
The layer-two question, asked and answered
That table says the MUTANT is measurably closer to the oracle than the port is.
The honest reading is that our position handling is wrong, and nobody had chased
it. It is not wrong, and the chase is
.agents/specs/ltx25-position-contract.md.Upstream passes no
position_idson this path. LTX-2fd4ded7fcalls the towerwith three arguments at
text_encoders/gemma/encoders/base_encoder.py:64-68, anddiffusers
3a2f35d4does the same atpipelines/ltx2/pipeline_ltx2.py:347-349.So transformers derives them, and
modeling_gemma4_unified.py:1092-1096derivestorch.arange(inputs_embeds.shape[1]) + past_seen_tokensover the PADDED width:the pads consume
0..P-1and the real tokens start atP. Theattention_mask.cumsum(-1) - 1renumbering exists only insidegenerate()(
generation/utils.py:713-727, sole base call site:2483), which a plainmodule call never reaches. transformers 5.14.1, the parity pin, carries that
branch byte-identically. vLLM defines nothing on this path:
grep -rn -i "ltx"over
vllm/model_executor/models/, over--include='*.py'for the wholecheckout at
5559679229, plus-i "lightricks"and"LTXVideo\|ltx_video\|LTX2\|LTX-2", all return zero.Asked of the oracle directly, against its OWN left-padded run, worst of 13 states:
In f32 both are round-off on values of magnitude 14.35, which is the physics:
rotary embedding depends only on
m - nand the pads are masked, so a uniformshift cancels exactly. In bf16 renumbering is 2.64x FURTHER from upstream's own
answer and closer at none of the states. The port is right and the mutant is
worse. The end-to-end conditioning reads the other way because
out.conditioningand
want_bf16are two different bf16 realizations of one f32 trajectory, so aperturbation of the same order as the gap between them can land on either side of
it. That is a property of the instrument, not of the port, and it is what
gen-ltx2-gemma-tower-goldens.py:363-375already records forpartial_rotary_factor. No new issue is filed because there is no defect to file.scripts/probe-ltx2-tower-positions.pyis that measurement. It imports thecommitted golden generator and calls the generator's own
build_towerandrun_tower, so the fixture behind the verdict cannot drift from the fixturebehind the goldens. Identical under transformers 5.12.1 with torch 2.12.1+cpu
and under 5.14.1 with torch 2.11.0+cu130.
What changes
Ltx2PromptConditioningcarries thepositionsvector the tower actually ranon, bound by reference rather than copied beside it, and the case REQUIREs it
against the golden's own pad count. That is an integer contract, upstream of
every rounding that could absorb it. The stale note becomes what the table below
it measures and points at the new assertion.
No tolerance is widened and none is moved. The four
<= 2.0 * floorvalueassertions are byte-identical to base and the measured table is kept intact. A
constant could not recover this detection anyway, because the ordering of correct
and mutant has reversed, and reaching for one is what #1668 forbids.
Evidence
One build directory, CPU-only Release (
-DVLLM_CPP_CUDA=OFF, NDEBUG), Ninja,x86_64, gcc 13. Compile rc 0 on every arm.
ltx2_text_encoder.cpppristinesha256 8911296882e03ea072a3d6b1898b7fff88503c2f048683802a2a7a43de07a59c,restored and re-verified.
SUCCESS27/27, 4118 of 4118positions[i] = iSUCCESS27/27, 4118 of 4118FAILURE, 8 of 4127 failedtest_ltx2_text_encoder.cpp:2398, all T=8 positionsSUCCESS27/27, 4127 of 4127The full build is
rc=0with zero errors across all 1141 targets, andctest -R ltx2is 12 of 12 passed.test_cpu_x86_llamacpp_flooris FLAKY on this box under load. It failed threetimes during this work, always on
CpuX86FloorHarnessTests.test_a_contended_leg_is_discarded_and_never_summarisedwith
NO_QUIET_WINDOWat loads between 40 and 170 from other sessions, and itpassed 10 of 10 standalone in 19.4 s in the one window that was quiet. This row
changes no CPU kernel and no harness -- the diff touches nothing under
tests/scripts/orsrc/vt/. It is neither caused nor repaired here. The finalscripts/agent-preflight.shon the merged head reportsAll gates green.Fresh review
Reviewed at
fc357094fby an agent that did not write it, in its own detachedworktree, tree restored and
sha256sum-verified after each mutation. VerdictFINDINGS, none blocking. It reproduced every cell of the oracle table on bothtransformers stacks, verified all fifteen upstream and in-repo anchors, and
confirmed that zero non-comment test lines were removed and that the four
2.0 * floorassertions are byte-identical to base.Its four notes are repaired in
522097f01:out.positionsis the whole contract and nothinggates it -- the review mutated it (copy instead of
&, tower fed a renumberedvector) and the suite stayed green at 27/27. The comment now says so.
## Designitem 4 was headed "Reachability" and delivered something narrower:deleting the
ltx2_video.cpp:2278call site does NOT red this case, becausethe case enters one level below
Ltx2VideoEngine::Generate. That entry pointis pre-existing and this row adds assertions to it rather than a capability.
Recorded plainly so
M1is not mistaken for a reachability proof.the recorded pristine sha belong to the base tree. Relabelled.
scripts/probe-ltx2-tower-positions.pyis run by no gate and cannot be, sinceCI has neither torch nor a
transformersthat registersgemma4_unified.Named as owed rather than assumed durable.
The fifth note was that the branch had fallen behind
main. MergedVT-CONV1D-BLOCK-CONDITION(#1770) and re-gated on the merged head: the focusedsuite is
SUCCESSat 27/27 and 4127 of 4127, and thepositions[i] = imutation still reds it at exactly 8 of 4127, restored byte-for-byte after.
Closes #1467.
FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5 [claude-code]