fix: Kabat CDR deletion orders panicked on single-residue regions - #52
Merged
Conversation
number_with_rules computes to_remove = base_len - len and slices deletion_order[..to_remove]. A region aligning to one residue asks to delete every base position but one, so the table needs base_len - 1 entries. Three Kabat tables were shorter than that and sliced out of bounds: heavy CDR1 num 23-35 base 13, had 11, needs 12 heavy CDR3 num 94-102 base 9, had 7, needs 8 light CDR2 num 50-52 base 3, had 1, needs 2 Every IMGT rule and the remaining Kabat rules already held base_len - 1, which is why the crash was Kabat-only. The orders now follow ANARCI, leaving position 23 for heavy CDR1 and 94 for heavy CDR3. Verified against the reads that triggered this: the single CDR residue lands on 23 and 94 respectively. Found on camel VHH data from the Observed Antibody Space (Li et al. 2017, run SRR3544217) at roughly 1 in 20 000 reads. All were productive, in frame, free of stop codons, and numbered correctly under IMGT -- truncated reads rather than malformed input. Tests: - the six triggering sequences, under Kabat and IMGT - deletion_order holds base_len - 1 entries for every variable rule in every scheme, so a new scheme cannot reintroduce this - number_with_rules returns one position per residue for every region length from 1 to base_len + 4, covering both the deletion and insertion paths The two property tests fail on the pre-fix tables, naming the offending rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ethnum 1.5.2 transmutes () into TryFromIntError, which was zero-sized when that code was written. A recent Rust stdlib gave the type a size, so the transmute is now a hard E0512 error and every build touching the polars feature fails. Unrelated to the Kabat change but blocking its CI: rust-toolchain.toml tracks `channel = "stable"` with no version, so CI floats onto each new release. The last green run on main was 2026-06-18; the break is a toolchain released after it, and unmodified main fails identically. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`channel = "stable"` meant CI floated onto each new Rust release, which is how a
latent unsoundness in a transitive dependency became a build failure with no
change on our side. igx-platform already pins exactly ("1.89"); this brings
immunum in line.
Minor precision, so patch releases are still picked up. rustfmt and clippy are
declared so the pin is self-sufficient for CI.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
None of these are covered by a lockfile, so each run could resolve differently: - release.yml used dtolnay/rust-toolchain@stable, so a crates.io publish could be built with whichever toolchain was current that day. Now @v1 with an explicit toolchain matching rust-toolchain.toml, pinning both the action and the compiler. - bencher.yml tracked bencherdev/bencher@main. A branch can be force-pushed, so this was a supply-chain exposure as well as a reproducibility one. Pinned to the current release, v0.6.11. - The build-system requires polars with no upper bound, and build requirements are not covered by uv.lock -- a source install resolves them fresh. Bounded to <2.0, matching the neighbouring maturin spec. Dependency ranges that resolve through uv.lock or Cargo.lock are left alone; they are already reproducible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Btw, also pinned some versions as the ISO officer (@me) says that's not allowed :P |
ThijsMaas
approved these changes
Aug 4, 2026
ThijsMaas
left a comment
ThijsMaas
left a comment
Collaborator
There was a problem hiding this comment.
Looks good. I made a small change so the publish job reads the toolchain version from the toml, instead of having it declared twice
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.
number_with_rules computes to_remove = base_len - len and slices deletion_order[..to_remove]. A region aligning to one residue asks to delete every base position but one, so the table needs base_len - 1 entries. Three Kabat tables were shorter than that and sliced out of bounds:
heavy CDR1 num 23-35 base 13, had 11, needs 12 heavy CDR3 num 94-102 base 9, had 7, needs 8 light CDR2 num 50-52 base 3, had 1, needs 2
Every IMGT rule and the remaining Kabat rules already held base_len - 1, which is why the crash was Kabat-only.
The orders now follow ANARCI, leaving position 23 for heavy CDR1 and 94 for heavy CDR3. Verified against the reads that triggered this: the single CDR residue lands on 23 and 94 respectively.
Found on camel VHH data from the Observed Antibody Space (Li et al. 2017, run SRR3544217) at roughly 1 in 20 000 reads. All were productive, in frame, free of stop codons, and numbered correctly under IMGT -- truncated reads rather than malformed input.
Tests:
The two property tests fail on the pre-fix tables, naming the offending rule.