Repository navigation
fix(bam): prevent duplicate reads from indexed CRAM queries - #178
Draft
NicoDeVeaux wants to merge 5 commits into
Draft
NicoDeVeaux wants to merge 5 commits into
NicoDeVeaux wants to merge 5 commits into
Conversation
… records In a multi-reference CRAM slice, `fetch(tid)` returns every record in the slice whose position overlaps the (whole-reference) interval, whatever its reference: noodles' query filters on the interval only. On the parallel path each such record is counted once per reference fetch (flagstat "in total" 10 instead of 6 for the samtools-written fixture at --threads 4). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
noodles' CRAM `query` filters records on the interval only, so in a multi-reference slice `fetch(tid)` also returned other references' records whose positions fall inside the whole-reference interval. Keep only records whose reference sequence ID is `tid`, as htslib does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Retain the public PR178 ancestry and updated PR177 fixes. Resolve append-only test and fixture documentation conflicts by keeping both complete regressions.
This was referenced Nov 1, 2026
NicoDeVeaux
changed the title
This branch has not been deployed
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.
Indexed CRAM fetches now keep only the requested reference's records and query each reference/container once. This fixes two duplicate-read paths in noodles 0.94.0: multi-reference slices yielded other references' records, and multiple CRAI slice entries for the same reference/container caused the whole container to be decoded repeatedly.
The query index retains one entry per
(reference ID, container offset)when opening the reader and when rebuilding it with a reference FASTA. Records are not deduplicated by name: repeated names can represent distinct physical records and must remain present. The shim queries complete references; the per-record reference filter is preserved.This remains stacked on #177 (
ndv/fix-pr118-unmapped-sweep) for the #118 migration. The updated head includes #177's empty-unmapped-sweep fix through an ordinary merge, preserving both public branch histories. Both PRs should be incorporated into #118 before the migration lands.Validation of
be3976on macOS arm64 / Rust 1.90.0:A separate CI follow-up pins crossbeam-epoch 0.9.20 and replaces an equivalent iterator-exhaustion match with
?for newer stable Clippy. Fresh Linux/upstream CI, package/security validation and a full RNA corpus remain separate checks. #175's counting-candidate Linux evidence does not transfer to this noodles build; integrating those branches requires resolving conflicts and verifying the resulting source. No read-level deduplication, noodles version bump or exit-policy change is introduced.This update is written by GPT-6 on Nico DeVeaux's behalf. The original PR was prepared with Claude Code.