Repository navigation
feat(raw-bam): additive API surface - #288
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #288 +/- ##
=======================================
Coverage 90.07% 90.07%
=======================================
Files 124 124
Lines 62247 62247
=======================================
Hits 56071 56071
Misses 6176 6176 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 tasks done
nh13
marked this pull request as ready for review
April 17, 2026 22:28
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (18)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughSwapped 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/fgumi-raw-bam/tests/tags_editor_invariants.rs (1)
1-1:⚠️ Potential issue | 🟠 MajorRestore the crate-level unsafe denial.
This integration test file is its own crate root, so
src/lib.rs’s crate attribute does not cover it.Proposed fix
#![cfg(feature = "test-utils")] +#![deny(unsafe_code)]As per coding guidelines,
**/*.rs: Enforce#![deny(unsafe_code)]- only standard library unsafe code is permitted in Rust files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-raw-bam/tests/tags_editor_invariants.rs` at line 1, This test crate is missing the crate-level safety denial: add the crate attribute preventing unsafe code at the top of the file by inserting the `#![deny(unsafe_code)]` crate attribute (in addition to the existing `#![cfg(feature = "test-utils")]`) so the test file enforces the same unsafe-code prohibition as src/lib.rs; ensure the attribute appears before any items in the file to apply crate-wide.crates/fgumi-raw-bam/src/tags.rs (1)
1256-1307:⚠️ Potential issue | 🟠 MajorKeep in-place setters non-panicking on truncated aux bytes.
find_tag_positioncan return a matching target before its payload is validated. The writes at lines 1263, 1273, 1283, and 1306 can panic on malformed records; returnfalseinstead.Proposed fix
pub fn set_int_in_place(&mut self, tag: &[u8; 2], value: i64) -> bool { let Some((p, val_type)) = find_tag_position(self.0, *tag) else { return false; }; + let Some(size) = tag_value_size(val_type, &self.0[p + 3..]) else { + return false; + }; + if p + 3 + size > self.0.len() { + return false; + } match val_type { b'c' => i8::try_from(value) .map(|v| { @@ pub fn set_float_in_place(&mut self, tag: &[u8; 2], value: f32) -> bool { let Some((p, val_type)) = find_tag_position(self.0, *tag) else { return false; }; - if val_type != b'f' { + if val_type != b'f' || p + 7 > self.0.len() { return false; } self.0[p + 3..p + 7].copy_from_slice(&value.to_le_bytes());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-raw-bam/src/tags.rs` around lines 1256 - 1307, The in-place setters (set_int_in_place and set_float_in_place) currently assume the payload bytes exist and can panic on malformed/truncated aux data; before writing at offsets like self.0[p+3], self.0[p+3..p+5], self.0[p+3..p+7] (set_int_in_place) and self.0[p+3..p+7] (set_float_in_place), validate the buffer has the required length and return false if not; use checked indexing or slice length checks (e.g., self.0.get_mut(p+3) / self.0.get_mut(p+3..p+5) / self.0.get_mut(p+3..p+7) or compare self.0.len() with p + needed_bytes) to avoid panics while keeping the existing type checks from find_tag_position. Ensure each match arm in set_int_in_place only performs its write after verifying the target slice exists and otherwise yields false.
🧹 Nitpick comments (5)
crates/fgumi-raw-bam/tests/noodles_roundtrip.rs (1)
44-64: Strengthen decoded-value assertions in these focused proptests.These tests check only parseability and counts. While
noodles_roundtrip_comprehensive.rsprovides broader coverage, these focused tests should assert decoded CIGAR op kinds/lengths (not just count) and decoded sequence/quality values to clearly validate the round-trip semantics ofset_cigar_opsandset_sequence_and_qualities.For example: decode and assert
buf.cigar()contains the expected ops with correct kinds, andbuf.sequence()/buf.quality_scores()match the inputs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-raw-bam/tests/noodles_roundtrip.rs` around lines 44 - 64, The test only asserts the decoded CIGAR count — update set_cigar_ops_roundtrips to also assert that the decoded CIGAR op kinds and lengths match the expected values derived from cigar_ops (map each u32 word into length and op kind) by iterating buf.cigar() and comparing each op's kind/len; additionally, after calling set_sequence_and_qualities (or using build_record input), decode buf.sequence() and buf.quality_scores() from decode_with_noodles and assert they equal the original sequence bytes and quality bytes used to construct rec; keep existing use of cigar_ops_vec(), decode_with_noodles(), build_record(), set_cigar_ops(), and set_sequence_and_qualities() to locate insertion points.crates/fgumi-raw-bam/src/noodles_compat.rs (1)
106-111: Use or document the ignored header.
raw_record_to_record_bufacceptsheaderbut decodes throughraw_records_to_record_bufs, which constructs a default header internally. Please either thread the supplied header into decoding or document/remove the parameter before this becomes public API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-raw-bam/src/noodles_compat.rs` around lines 106 - 111, raw_record_to_record_buf currently takes a Header but ignores it (raw_records_to_record_bufs builds a default header), so either thread the provided Header through or remove/document the parameter: update raw_records_to_record_bufs to accept a &noodles::sam::Header and use it for decoding (and change its callers), or remove the unused _header parameter from raw_record_to_record_buf (and its signature) if you don't need it, or add a clear doc comment on raw_record_to_record_buf stating the header is intentionally ignored; reference the functions raw_record_to_record_buf and raw_records_to_record_bufs when making the change.crates/fgumi-raw-bam/src/cigar.rs (1)
56-65: Publicconsumes_*can overshift for op_type ≥ 16.Guard removal is fine for spec-compliant 4-bit inputs, and all in-crate callers mask
& 0xF. But these remain publicconst fntakingu32; passing e.g.16yieldsBAM_CIGAR_TYPE >> 32→ debug panic / release wrap (incorrecttrue). The narrowed test (9..=15) no longer catches this. Cheap guard: mask inside the function.Optional defensive tweak
pub const fn consumes_ref(op_type: u32) -> bool { - (BAM_CIGAR_TYPE >> (op_type << 1)) & 2 != 0 + (BAM_CIGAR_TYPE >> ((op_type & 0xF) << 1)) & 2 != 0 } @@ pub const fn consumes_query(op_type: u32) -> bool { - (BAM_CIGAR_TYPE >> (op_type << 1)) & 1 != 0 + (BAM_CIGAR_TYPE >> ((op_type & 0xF) << 1)) & 1 != 0 }Alternatively, document the 0..=15 precondition.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-raw-bam/src/cigar.rs` around lines 56 - 65, Public const fns consumes_ref and consumes_query accept u32 and can overshift when op_type ≥ 16; mask the input inside each function (e.g., use op_type & 0xF) before shifting so the shift amount is bounded to 0..=15 and avoids panic/wrapping, keeping them const and preserving existing logic that checks BAM_CIGAR_TYPE >> (masked_op << 1).crates/fgumi-raw-bam/src/raw_bam_record.rs (1)
123-128:DerefMut<[u8]>+as_mut_vecbypass all record invariants.Intentional per the PR (needed for splice/resize), but worth a one-line safety note in the rustdoc: callers are on the hook for keeping
l_read_name/n_cigar_op/l_seqconsistent with the buffer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-raw-bam/src/raw_bam_record.rs` around lines 123 - 128, Add a one-line safety note to the Rustdoc for RawRecord explaining that the impl of std::ops::DerefMut for RawRecord (and use of as_mut_vec) allows callers to mutate the inner buffer directly and therefore can bypass record invariants; explicitly state callers must ensure l_read_name, n_cigar_op, and l_seq remain consistent with the underlying buffer when using DerefMut/as_mut_vec (mention RawRecord::deref_mut and any public as_mut_vec helper by name) so future readers know they are responsible for preserving invariants.crates/fgumi-raw-bam/tests/noodles_roundtrip_comprehensive.rs (1)
467-533: Proptest float-NaN branch is dead.
-1000.0f32..=1000.0f32never produces NaN, so the|| decoded_f.is_nan() && as_score.is_nan()tail is unreachable. Harmless — drop it or widen the strategy withprop_oneof![Just(f32::NAN), ...]if NaN handling matters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-raw-bam/tests/noodles_roundtrip_comprehensive.rs` around lines 467 - 533, The proptest strategy for as_score in proptest_random_complete_record_roundtrip uses -1000.0f32..=1000.0f32 which cannot generate NaN, so the NaN branch in the AS float assertion is unreachable; either remove the decoded_f.is_nan() && as_score.is_nan() check from the assertion (simplify to (decoded_f - as_score).abs() < 1e-3_f32) or change the strategy for as_score to include NaN (e.g., replace the range with a prop_oneof![Just(f32::NAN), (-1000.0f32..=1000.0f32)] so as_score can be NaN); update the proptest declaration for as_score in the proptest_random_complete_record_roundtrip test accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/fgumi-raw-bam/src/builder.rs`:
- Around line 521-527: The code writes self.bin for all records but doesn't emit
the special UNMAPPED_BIN value for unmapped SamBuilder records; update the
fixed-header bin write in the builder (where
buf.extend_from_slice(&self.bin.to_le_bytes()) is called) to use UNMAPPED_BIN
(4680) when the record is unmapped (e.g., when ref_id == -1 and/or pos == -1)
otherwise keep self.bin; you can compute let bin_to_write = if self.ref_id == -1
&& self.pos == -1 { UNMAPPED_BIN } else { self.bin } and then write
buf.extend_from_slice(&bin_to_write.to_le_bytes()) so SamBuilder matches
UnmappedSamBuilder behavior.
In `@crates/fgumi-raw-bam/src/indexed_reader.rs`:
- Around line 107-114: query_unmapped currently only seeks when
index.last_first_record_start_position() returns Some, so if it returns None the
reader is left at the prior BGZF position; change query_unmapped to seek to the
beginning fallback when last_first_record_start_position() is None by calling
self.inner.get_mut().seek_to_virtual_position(0).context("seeking to unmapped
position")? so UnmappedIter always starts at the documented first-record
fallback; update the branch in query_unmapped to call
seek_to_virtual_position(0) on self.inner when the index returns None.
In `@crates/fgumi-raw-bam/src/tags.rs`:
- Around line 1042-1047: The iterator iter_typed currently rescans RawTagsView
for each entry which causes O(n²) behavior and duplicates; instead, for each
iterated entry use the entry's own type and value bytes to produce the TagValue
directly (use the entry.type_byte and entry.value_bytes to decode/construct a
TagValue for that entry) and yield (entry.tag, decoded_value) without calling
RawTagsView::get again; update iter_typed to map each entry -> (entry.tag,
TagValue::from_bytes / decode using entry.type_byte and entry.value_bytes) so
duplicate keys return their respective values and performance becomes linear.
- Around line 1006-1012: The get() method can panic when handling 'A' tags
because it accesses aux[start] without checking bounds; change the branch
handling val_type == b'A' in function get (and the TagValue::Char construction)
to first ensure aux has at least start+1 bytes (e.g., by checking
aux.get(start)) and return None on truncated data; keep using
find_tag_position(...) to locate p and val_type, but guard the aux[start] access
to avoid panics for truncated "XA:A" entries.
- Around line 437-451: The type_byte() match currently returns the array's
element subtype for Self::Array(a) which is wrong; change the Self::Array(a) arm
in TagValue::type_byte to return the BAM array type byte b'B' (instead of
a.elem_type) so that Array variants report the canonical aux type 'B' while the
element subtype remains available on the Array value itself.
---
Outside diff comments:
In `@crates/fgumi-raw-bam/src/tags.rs`:
- Around line 1256-1307: The in-place setters (set_int_in_place and
set_float_in_place) currently assume the payload bytes exist and can panic on
malformed/truncated aux data; before writing at offsets like self.0[p+3],
self.0[p+3..p+5], self.0[p+3..p+7] (set_int_in_place) and self.0[p+3..p+7]
(set_float_in_place), validate the buffer has the required length and return
false if not; use checked indexing or slice length checks (e.g.,
self.0.get_mut(p+3) / self.0.get_mut(p+3..p+5) / self.0.get_mut(p+3..p+7) or
compare self.0.len() with p + needed_bytes) to avoid panics while keeping the
existing type checks from find_tag_position. Ensure each match arm in
set_int_in_place only performs its write after verifying the target slice exists
and otherwise yields false.
In `@crates/fgumi-raw-bam/tests/tags_editor_invariants.rs`:
- Line 1: This test crate is missing the crate-level safety denial: add the
crate attribute preventing unsafe code at the top of the file by inserting the
`#![deny(unsafe_code)]` crate attribute (in addition to the existing
`#![cfg(feature = "test-utils")]`) so the test file enforces the same
unsafe-code prohibition as src/lib.rs; ensure the attribute appears before any
items in the file to apply crate-wide.
---
Nitpick comments:
In `@crates/fgumi-raw-bam/src/cigar.rs`:
- Around line 56-65: Public const fns consumes_ref and consumes_query accept u32
and can overshift when op_type ≥ 16; mask the input inside each function (e.g.,
use op_type & 0xF) before shifting so the shift amount is bounded to 0..=15 and
avoids panic/wrapping, keeping them const and preserving existing logic that
checks BAM_CIGAR_TYPE >> (masked_op << 1).
In `@crates/fgumi-raw-bam/src/noodles_compat.rs`:
- Around line 106-111: raw_record_to_record_buf currently takes a Header but
ignores it (raw_records_to_record_bufs builds a default header), so either
thread the provided Header through or remove/document the parameter: update
raw_records_to_record_bufs to accept a &noodles::sam::Header and use it for
decoding (and change its callers), or remove the unused _header parameter from
raw_record_to_record_buf (and its signature) if you don't need it, or add a
clear doc comment on raw_record_to_record_buf stating the header is
intentionally ignored; reference the functions raw_record_to_record_buf and
raw_records_to_record_bufs when making the change.
In `@crates/fgumi-raw-bam/src/raw_bam_record.rs`:
- Around line 123-128: Add a one-line safety note to the Rustdoc for RawRecord
explaining that the impl of std::ops::DerefMut for RawRecord (and use of
as_mut_vec) allows callers to mutate the inner buffer directly and therefore can
bypass record invariants; explicitly state callers must ensure l_read_name,
n_cigar_op, and l_seq remain consistent with the underlying buffer when using
DerefMut/as_mut_vec (mention RawRecord::deref_mut and any public as_mut_vec
helper by name) so future readers know they are responsible for preserving
invariants.
In `@crates/fgumi-raw-bam/tests/noodles_roundtrip_comprehensive.rs`:
- Around line 467-533: The proptest strategy for as_score in
proptest_random_complete_record_roundtrip uses -1000.0f32..=1000.0f32 which
cannot generate NaN, so the NaN branch in the AS float assertion is unreachable;
either remove the decoded_f.is_nan() && as_score.is_nan() check from the
assertion (simplify to (decoded_f - as_score).abs() < 1e-3_f32) or change the
strategy for as_score to include NaN (e.g., replace the range with a
prop_oneof![Just(f32::NAN), (-1000.0f32..=1000.0f32)] so as_score can be NaN);
update the proptest declaration for as_score in the
proptest_random_complete_record_roundtrip test accordingly.
In `@crates/fgumi-raw-bam/tests/noodles_roundtrip.rs`:
- Around line 44-64: The test only asserts the decoded CIGAR count — update
set_cigar_ops_roundtrips to also assert that the decoded CIGAR op kinds and
lengths match the expected values derived from cigar_ops (map each u32 word into
length and op kind) by iterating buf.cigar() and comparing each op's kind/len;
additionally, after calling set_sequence_and_qualities (or using build_record
input), decode buf.sequence() and buf.quality_scores() from decode_with_noodles
and assert they equal the original sequence bytes and quality bytes used to
construct rec; keep existing use of cigar_ops_vec(), decode_with_noodles(),
build_record(), set_cigar_ops(), and set_sequence_and_qualities() to locate
insertion points.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8807b8b3-4354-43cf-8ba5-28c39fe2f5af
📒 Files selected for processing (18)
crates/fgumi-consensus/src/codec_caller.rscrates/fgumi-consensus/src/duplex_caller.rscrates/fgumi-consensus/src/vanilla_caller.rscrates/fgumi-raw-bam/Cargo.tomlcrates/fgumi-raw-bam/src/builder.rscrates/fgumi-raw-bam/src/cigar.rscrates/fgumi-raw-bam/src/indexed_reader.rscrates/fgumi-raw-bam/src/lib.rscrates/fgumi-raw-bam/src/noodles_compat.rscrates/fgumi-raw-bam/src/raw_bam_record.rscrates/fgumi-raw-bam/src/sequence.rscrates/fgumi-raw-bam/src/tags.rscrates/fgumi-raw-bam/src/testutil.rscrates/fgumi-raw-bam/tests/noodles_roundtrip.rscrates/fgumi-raw-bam/tests/noodles_roundtrip_comprehensive.rscrates/fgumi-raw-bam/tests/tags_editor_invariants.proptest-regressionscrates/fgumi-raw-bam/tests/tags_editor_invariants.rssrc/lib/commands/extract.rs
nh13
force-pushed
the
nh/raw-bam-api-6-zipper-raw-record-methods
branch
from
April 18, 2026 07:36
7a3a042 to
f820ecd
Compare
Base automatically changed from
nh/raw-bam-api-6-zipper-raw-record-methods
to
main
April 18, 2026 07:39
nh13
force-pushed
the
nh/raw-bam-api-7-additive-api
branch
from
April 18, 2026 08:32
3ff61b1 to
6a93923
Compare
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/fgumi-raw-bam/src/sequence.rs (1)
223-250:⚠️ Potential issue | 🟠 MajorKeep the destination-size check in release builds.
Line 228 is a public precondition. With
debug_assert!, release builds can partially overwritedstbefore a later index panic. Keep the eagerassert!or return aResult.🐛 Proposed fix
- debug_assert!(dst.len() >= packed_len); + assert!( + dst.len() >= packed_len, + "dst too short: got {} bytes, need {packed_len}", + dst.len() + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fgumi-raw-bam/src/sequence.rs` around lines 223 - 250, The public function pack_sequence_into_slice currently uses debug_assert! for the dst size check which is removed in release builds; change this to a real runtime check: either replace debug_assert!(dst.len() >= packed_len) with assert!(dst.len() >= packed_len) to keep the eager panic in all builds, or convert pack_sequence_into_slice to return a Result and return an Err if dst.len() < packed_len (use the packed_len and dst variables to perform the check before writing to dst). Ensure the chosen approach is applied at the start of the function before any writes to dst.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/fgumi-raw-bam/src/cigar.rs`:
- Around line 955-957: The constructor CigarOp::new allows zero-length ops but
spec/docs mark those as invalid; update CigarOp::new to reject len == 0 (e.g.,
assert!(len > 0, "CIGAR op length must be non-zero") or otherwise return an Err)
and mirror the same guard for any other typed constructors that currently accept
a length (refer to CigarOp::new and similar constructors in the file), leaving
from_raw unchanged so it can still represent malformed wire data; add/update
tests to cover rejection of zero-length CIGAR ops.
In `@crates/fgumi-raw-bam/src/indexed_reader.rs`:
- Around line 120-128: query_unmapped currently falls back to BGZF offset 0 when
both index.last_first_record_start_position() and first_record_position are
None, which can make UnmappedIter try to parse the BAM header as a record;
change query_unmapped to detect this missing-state and return an explicit error
instead of VirtualPosition::from(0). Specifically, in the query_unmapped
function (around the virtual_pos computation that uses
index.last_first_record_start_position() and self.first_record_position) replace
the final unwrap_or(bgzf::VirtualPosition::from(0)) path with an early check
that returns an anyhow error (e.g., using anyhow::bail! or
Err(anyhow::anyhow!(...))) when neither marker is present and read_header has
not been called, so callers get a clear failure rather than attempting to read
the header via UnmappedIter.
In `@crates/fgumi-raw-bam/src/noodles_compat.rs`:
- Around line 113-118: raw_record_to_record_buf currently ignores the supplied
header which breaks decoding for mapped records; update the decode path to
thread the Header through: change raw_records_to_record_bufs to accept a
&noodles::sam::Header (or add a new raw_records_to_record_bufs_with_header that
does) and have raw_record_to_record_buf call that variant passing the header
instead of constructing a synthetic empty header, ensuring any header-dependent
decoding in noodles::sam::alignment::RecordBuf is used; then add a round-trip
test that constructs a non-empty Header with a reference dictionary and verifies
a mapped RawRecord round-trips via raw_record_to_record_buf/ back to bytes to
validate the fix.
---
Outside diff comments:
In `@crates/fgumi-raw-bam/src/sequence.rs`:
- Around line 223-250: The public function pack_sequence_into_slice currently
uses debug_assert! for the dst size check which is removed in release builds;
change this to a real runtime check: either replace debug_assert!(dst.len() >=
packed_len) with assert!(dst.len() >= packed_len) to keep the eager panic in all
builds, or convert pack_sequence_into_slice to return a Result and return an Err
if dst.len() < packed_len (use the packed_len and dst variables to perform the
check before writing to dst). Ensure the chosen approach is applied at the start
of the function before any writes to dst.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8097bee9-a192-464f-8540-b5de9494d491
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
crates/fgumi-consensus/src/codec_caller.rscrates/fgumi-consensus/src/duplex_caller.rscrates/fgumi-consensus/src/vanilla_caller.rscrates/fgumi-raw-bam/Cargo.tomlcrates/fgumi-raw-bam/src/builder.rscrates/fgumi-raw-bam/src/cigar.rscrates/fgumi-raw-bam/src/indexed_reader.rscrates/fgumi-raw-bam/src/lib.rscrates/fgumi-raw-bam/src/noodles_compat.rscrates/fgumi-raw-bam/src/raw_bam_record.rscrates/fgumi-raw-bam/src/sequence.rscrates/fgumi-raw-bam/src/tags.rscrates/fgumi-raw-bam/src/testutil.rscrates/fgumi-raw-bam/tests/noodles_roundtrip.rscrates/fgumi-raw-bam/tests/noodles_roundtrip_comprehensive.rscrates/fgumi-raw-bam/tests/tags_editor_invariants.proptest-regressionscrates/fgumi-raw-bam/tests/tags_editor_invariants.rssrc/lib/commands/extract.rs
✅ Files skipped from review due to trivial changes (4)
- crates/fgumi-raw-bam/Cargo.toml
- crates/fgumi-raw-bam/tests/tags_editor_invariants.proptest-regressions
- crates/fgumi-raw-bam/src/testutil.rs
- crates/fgumi-raw-bam/tests/noodles_roundtrip_comprehensive.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/fgumi-raw-bam/tests/noodles_roundtrip.rs
- crates/fgumi-consensus/src/codec_caller.rs
- src/lib/commands/extract.rs
- crates/fgumi-raw-bam/src/raw_bam_record.rs
Additive additions to fgumi-raw-bam that later stack-series PRs consume: - RawBamWriter, write_raw_record, write_raw_records (raw_bam_record.rs) - CigarOp, CigarKind typed CIGAR iteration (cigar.rs) - TagValue zero-copy typed tag dispatch (tags.rs) - Symmetric RawRecord <-> RecordBuf converters + RecordBufEncoder (noodles_compat.rs) - SamBuilder (general-purpose record construction) renamed from MappedBamRecordBuilder; UnmappedSamBuilder renamed from UnmappedBamRecordBuilder (builder.rs) - IndexedRawBamReader, RawQueryIter, UnmappedIter (new indexed_reader.rs) for raw-byte indexed BAM iteration - Matching re-exports in lib.rs Also updates three consensus callers (codec/duplex/vanilla) and commands/extract.rs to use the renamed UnmappedSamBuilder. Adds comprehensive noodles round-trip tests and widens the tag-editor invariant proptests to match the expanded API. Part 2 of 7 in the follow-up series closing #272.
nh13
force-pushed
the
nh/raw-bam-api-7-additive-api
branch
from
April 18, 2026 16:55
6a93923 to
1d47104
Compare
This was referenced Apr 18, 2026
Merged
Merged
This branch was previously 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
1 participant
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.
Summary
Additive additions to `fgumi-raw-bam` that later stack-series PRs consume:
Also updates three consensus callers (`codec_caller`, `duplex_caller`, `vanilla_caller`) and `commands/extract.rs` to use the renamed `UnmappedSamBuilder`.
Adds comprehensive `noodles` round-trip tests and widens the tag-editor invariant proptests to match the expanded API.
Part 2 of 7 in the follow-up series closing #272. Stacks on #286.
Test plan
Stack