fix(FIX-UNALIGNED-CONSUMERS-2540): take the comma out of the new EXL3 case name, because doctest's -tc filter splits on it - #2602
Merged
Conversation
… case name, because doctest's -tc filter splits on it Closes #2601. #2581 landed `exl3 gemm: suh, svh and the trellis at ODD byte addresses decode the same`. doctest splits `-tc=` on commas, so that name can never select that case: the filter resolves to three patterns, none of them matches, and the binary prints `test cases: 0`, `assertions: 0` and `Status: SUCCESS!` at rc 0. `ctest` runs the whole binary, so nothing in this repository is unmeasured today and no gate was ever wrong. The exposure is the next focused run. An implementer or reviewer who filters this case by name reads a green that measured nothing, and this is the case that holds the odd-address decode for #2558 -- the one it matters most not to be wrong about. The comment above the case now states the constraint, so the next author does not put a comma back. Measured on this tree, `build-sanitize` (`address,undefined`), with the case name reverted to the one on `main`: ./test_exl3_gemm -tc="exl3 gemm: suh, svh and the trellis at ODD byte addresses decode the same" [doctest] test cases: 0 | 0 passed | 0 failed | 15 skipped [doctest] assertions: 0 | 0 passed | 0 failed | [doctest] Status: SUCCESS! rc 0 and with the name this change gives it: ./test_exl3_gemm -tc="exl3 gemm: the suh/svh scales and the trellis at ODD byte addresses decode the same" [doctest] test cases: 1 | 1 passed | 0 failed | 14 skipped [doctest] assertions: 8 | 8 passed | 0 failed | [doctest] Status: SUCCESS! rc 0 Nothing else moves. The whole binary is 15 cases and 210 assertions before and after, and the case body, its `REQUIRE`s on address parity and its byte-for-byte comparison are untouched. The other case #2581 added, `rmsnorm: a bf16 gamma at an ODD byte address is read as its bytes`, carries no comma and needs nothing. `grep` finds 1202 case names tree-wide that do carry one, four of them in the two files #2581 touched; none of those is this change's to rename, and a checker that refuses the shape wants its own row. Also names #2597 under the spec's `## Owed`. Widening five weight-side operands to `const void*` is what let the trellis become a byte cursor, and it also removed the compiler's ability to reject a wrongly typed argument at those five call boundaries. Every caller still passes the int16 words as stored, so it is a latent hazard rather than a defect; the repair is a one-member wrapper whose own alignment is 1, it crosses the public ABI in `include/vt/ops.h`, and it wants its own row rather than riding in a rename. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5-1m [claude-code]
This was referenced Sep 2, 2026
…e leaves behind an owner, because filing one without an owner is the gate failure The fresh review of #2602 found the record, not the test. Three separate findings, all of them mine and none of them a code change. `check-agent-record.py` exits 1 on the reviewed head. #2601 was filed with ``Row: `-` `` and a sentence claiming it was owed by `.agents/specs/unaligned-safetensors-consumers.md` under `## Owed`. That claim was FALSE -- the section listed #2579 and #2597 and nothing else -- so of the three places AGENTS.md requires to agree, only the pull request body named the row. #2601 now carries ``Row: `FIX-UNALIGNED-CONSUMERS-2540` `` and the false sentence is gone. This row does fix it, in this flow, so the row line is the honest half of that pair and the `## Owed` entry would have been the wrong one. The count was wrong, and `squash_merge_commit_message = PR_BODY` would have made an arithmetic error about counting permanent in a `main` commit message. The old case name carries exactly ONE comma, so `-tc=` resolves to TWO patterns and not three. Corrected here and in #2601. The durable follow-up had no owner. Both the body and #2601 said a checker that refuses the shape "wants its own row" and nothing recorded that, which is the invisible debt AGENTS.md names. It is #2605 now, listed under `## Owed` with the mechanism evidence the review measured against a PRE-EXISTING comma case: `exl3 policy: the shape table is upstream's, value for value` selects 0 cases and 0 assertions by name and 1 case and 25 assertions through a `*` glob, and `-tc="nomatchXYZ,exl3 policy: shape compatibility and the empty-block clamp"` selects 1 case and 7 assertions. The filter splits into independently full-matched patterns; a space after the comma drops the selection back to 0. Renaming the 1202 existing names stays out of scope. #2606 is listed beside it and is the larger one. `check-agent-record`'s ownership check reads an untracked snapshot, and its own docstring says an absent snapshot is a SKIP the caller must report and `--fail-on-skip` must redden. Neither is implemented: `skips` is populated and never read, the flag does not exist, and no workflow step runs `agent-issue-index.py --refresh`, so `.github/workflows/ci.yml:187` always runs the gate with its input absent. Measured on this head, one file moved and nothing else changed: python3 scripts/agent-issue-index.py --refresh python3 scripts/check-agent-record.py ERROR: ... references #2601, which names no owning row rc 1 mv .agents/issue-index.generated.md /tmp/ python3 scripts/check-agent-record.py --report agent record OK: ENGINE=178 MODEL=379 ... rc 0 Zero lines containing "skip" in the second run. The local operator following the documented procedure gets the red and CI gets the green, which is the wrong polarity for the surface that decides whether work lands, and it is the same failure class as the defect this row repairs: an absent input reading as a clean result. Not fixed here on purpose -- a record gate's semantics and a CI workflow are not a test rename, and bundling them would hide them. The test and its name do not move. The reviewer confirmed the rename is correct, minimal, unique, selects 1 case and not 2, and moved the name rather than the body: 8 assertions on both sides. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5-1m [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
2 participants
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.
fix(FIX-UNALIGNED-CONSUMERS-2540): take the comma out of the new EXL3 case name, because doctest's -tc filter splits on it
Closes #2601.
#2581 landed
exl3 gemm: suh, svh and the trellis at ODD byte addresses decode the same. doctest splits-tc=on commas, so that name can never select that case. It carries exactly ONE comma, so the filter resolves to TWO patterns --exl3 gemm: suhandsvh and the trellis at ODD byte addresses decode the same-- each of which must match a case name in full. Neither does, and the binary printstest cases: 0,assertions: 0andStatus: SUCCESS!at rc 0.ctestruns the whole binary, so nothing in this repository is unmeasured today and no gate was ever wrong. The exposure is the next focused run. An implementer or reviewer who filters this case by name reads a green that measured nothing, and this is the case that holds the odd-address decode for #2558 -- the one it matters most not to be wrong about. The comment above the case now states the constraint, so the next author does not put a comma back.Evidence
cmake -S . -B build-sanitize -DVLLM_CPP_BUILD_TESTS=ON -DVLLM_CPP_CUDA=OFF -DVLLM_CPP_SANITIZE='address,undefined',-j 4, run withVT_POOL_BYPASS=1 UBSAN_OPTIONS=print_stacktrace=1. Both rows measured on this tree, the first with the case name reverted to the one onmainand nothing else changed:-tc=exl3 gemm: suh, svh and the trellis at ODD byte addresses decode the sameSUCCESS!exl3 gemm: the suh/svh scales and the trellis at ODD byte addresses decode the sameSUCCESS!Nothing else moves. The whole binary is 15 cases and 210 assertions before and after, and the case body, its
REQUIREs on address parity and its byte-for-byte comparison are untouched: this moves the NAME, not the test.test_qwen35_exl3(10 / 3836) andtest_ops_rmsnorm_weight_dtype(7 / 266) are green beside it at rc 0, with zeromisaligned addressand zeroruntime errorlines.The mechanism was reproduced independently against doctest 2.5.2 on a PRE-EXISTING comma case, so it does not rest on the case this change renames:
exl3 policy: the shape table is upstream's, value for valueselects 0 cases and 0 assertions by name, and 1 case and 25 assertions through a*glob.-tc="nomatchXYZ,exl3 policy: shape compatibility and the empty-block clamp"selects 1 case and 7 assertions, which is what proves the filter splits into independently full-matched patterns; adding a space after the comma drops the selection back to 0.The other case #2581 added,
rmsnorm: a bf16 gamma at an ODD byte address is read as its bytes, carries no comma and needs nothing.The three debts, and who owns each
Each is named under
.agents/specs/unaligned-safetensors-consumers.md's## Owed, because each changes something a rename does not touch. An issue filed without an owner is the gate failure, not the habit.#2597 -- widening five weight-side operands to
const void*is what let the trellis become a byte cursor in #2581, and it also removed the compiler's ability to reject a wrongly typed argument at those five call boundaries. Every caller still passes the int16 words as stored, so it is a latent hazard rather than a defect. The repair is a one-member wrapper whose own alignment is 1 -- never a re-typedconst uint16_t*, which is the undefined load #2558 names -- and it crosses the public ABI ininclude/vt/ops.h.#2605 -- 1202 case names tree-wide carry a comma, four of them in the two files #2581 touched. The durable answer is a checker that refuses the shape, diff-scoped or baselined so those 1202 do not red ordinary work. That changes gate semantics and wants its own spec and a red-before test. Renaming the 1202 is explicitly not asked for.
#2606 -- and this one is larger than this pull request.
check-agent-record's ownership check reads an untracked snapshot, and its own docstring says an absent snapshot is a SKIP the caller must report and--fail-on-skipmust redden. Neither is implemented:skipsis populated and never read, the flag does not exist, and no workflow step runsagent-issue-index.py --refresh, so.github/workflows/ci.yml:187always runs the gate with its input absent. Measured on this head, one file moved and nothing else changed:Zero lines containing "skip" in the second run. A local operator following the documented procedure gets the red and CI gets the green, which is the wrong polarity for the surface that decides whether work can land -- and it is the same failure class as the defect this change repairs: an absent input reading as a clean result. Not fixed here on purpose. A record gate's semantics and a CI workflow are not a test rename, and bundling them would hide them.
Provenance
The rename is the fresh review of #2581's blocking finding 1. It missed that pull request's merge (
a8246badd) by two minutes and lands on its own; #2581's own body and evidence are unchanged and still describe what merged there.scripts/agent-preflight.shreports 0 gate failures on this head with the 5 environmental SKIPs the base reports.check-agent-record.pyexits 0 with the snapshot refreshed as AGENTS.md instructs.FOLLOWING_AGENTS_PROTOCOL
Following-Agents-Protocol: true
AI-Assisted: true
Assisted-by: AGENT:claude-opus-5-1m [claude-code]