feat(isa): sparse keep-set-filtered ISA for tiered indexes + build parallelization - #56
Conversation
|
Warning Review limit reached
More reviews will be available in 27 minutes and 2 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThe PR adds dense and sparse ChangesTiered ISA plumbing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
`prmi build --keep-bed --with-isa` now builds a sparse inverse-suffix-array over
exactly the kept positions, so the ISA launch-hint fast path (BWA-MEME's
no_search) works on the tiered on-target index -- previously the two were
mutually exclusive ("ISA needs the full SA").
A dense refpos-indexed ISA is genome-scale (~31 GB on hg38) regardless of
keep-set size, which would defeat the tiered index's footprint. The new sparse
layout (mode byte 1 in the .isa header; byte 17 was reserved-zero, so existing
dense files read back unchanged) stores num_entries (refpos, rank) pairs sorted
by refpos, mapping each kept doubled-coordinate position to its COMPACTED .sa
rank (the rank space the tiered model predicts). isa_at binary-searches it
(O(log k)); a refpos not in the keep-set returns None and the consumer falls
back to a cold/model launch -- byte-identical, since the hint only seeds the
search (proven by mem_search_warmstart_equals_cold). The sparse ISA's
num_entries equals the tiered .sa entry count, so the loader's existing
count==sa_num check accepts it. The compacted rank is assigned in the same
lex-order filtered scan the .sa write uses, so the two agree.
- isa_file.rs: write_tiered_isa_file + a mode-dispatching IsaFileReader::lookup
(dense direct index / sparse binary search); validate_isa_header branches the
expected file size on mode.
- train/mod.rs: emit the sparse ISA under keep-bed + with-isa (was a warn-skip).
- cli.rs: drop the keep-bed/with-isa incompatibility bail and refresh the docs.
- prmi-sys: prmi_isa_at returns a not-found code (was a panic via expect) when a
refpos has no inverse-SA entry, which a tiered ISA can legitimately report.
Tests: sparse round-trip + off-keep miss (isa_file unit); end-to-end tiered
build whose sparse ISA inverts the compacted SA and misses off-keep positions
(isa_build integration).
…ction The two dominant serial phases of a `--with-isa` mode-2 build are now rayon- parallel; SA construction was already OpenMP-threaded, but these scatter/compute loops ran single-threaded and dominated wall time at genome scale (an hg38-class WG sidecar spends ~an hour here). - write_isa_file: the dense `inv[sa[i]] = i` pack is a parallel scatter. The SA is a permutation of [0, n), so every target offset is distinct -> the 5-byte writes are disjoint and race-free even though they share the mmap body via a raw pointer (documented SAFETY: distinct, in-bounds, non-overlapping). - mode-2 .sa write: `key_for_position_2x` (a 32-base window read per entry) is the CPU cost; compute keys for each chunk's kept entries in parallel (rayon `collect` preserves order) and write the chunk sequentially through the BufWriter. Chunked (~4M entries) to bound peak memory. - tiered-ISA pairs: the keep_pos filter scan (a binary search per entry) is parallelized; the kept entry's index is still its compacted .sa rank. Byte-identical to the serial builder: rebuilding the chr22 full+ISA sidecar (1.3 GB .sa, 508 MB dense ISA, ~101M entries crossing the chunk boundary) and the tiered homology+ISA sidecar both cmp IDENTICAL to the serial-built files; full prmi + prmi-sys suite passes.
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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 (1)
prmi-sys/src/lib.rs (1)
252-264: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMove handle resolution inside the unwind boundary.
Handle::as_ref(handle)andh.idx.has_isa()still run beforecatch_unwind, so a panic there can cross the C ABI. Wrap all Rust-side work after the null checks.Proposed fix
- let h = unsafe { Handle::as_ref(handle) }; - if !h.idx.has_isa() { - set_last_error("prmi_isa_at: sidecar has no .isa (build with --with-isa)"); - return -5; - } + enum IsaAtOutcome { + Found(u64), + Missing, + NoIsa, + } + // isa_at returns None when refpos is out of range (dense) or not in the // keep-set (sparse/tiered). Either way there is no launch hint here; report // not-found rather than indexing out of bounds. A tiered ISA's valid refpos // space is the genome (up to 2*l_pac), NOT sa_num (the kept-entry count), so // we cannot bound-check against sa_num — isa_at does the right check. Wrapped // in catch_unwind so a panic (e.g. the sparse reader's bounds assert) can // never unwind across the extern "C" boundary. - let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| h.idx.isa_at(refpos))); + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let h = unsafe { Handle::as_ref(handle) }; + if !h.idx.has_isa() { + return IsaAtOutcome::NoIsa; + } + match h.idx.isa_at(refpos) { + Some(sa_index) => IsaAtOutcome::Found(sa_index), + None => IsaAtOutcome::Missing, + } + })); match result { - Ok(Some(sa_index)) => { + Ok(IsaAtOutcome::Found(sa_index)) => { unsafe { *out_sa_index = sa_index }; 0 } - Ok(None) => { + Ok(IsaAtOutcome::NoIsa) => { + set_last_error("prmi_isa_at: sidecar has no .isa (build with --with-isa)"); + -5 + } + Ok(IsaAtOutcome::Missing) => { set_last_error( "prmi_isa_at: refpos has no inverse-SA entry (out of range or not in keep-set)", );As per path instructions, every
prmi-sys/src/**/*.rsextern "C"entrypoint requires acatch_unwindboundary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prmi-sys/src/lib.rs` around lines 252 - 264, The extern "C" entrypoint is still doing Rust-side work before the unwind guard, so a panic from Handle::as_ref(handle) or h.idx.has_isa() could escape across the C ABI. Move handle resolution and all subsequent Rust logic inside the existing catch_unwind boundary in prmi_isa_at, keeping only the null/argument checks outside, so every Rust operation after entry is protected.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prmi/src/index/mod.rs`:
- Around line 203-204: The public per-seed hint wrapper in IsaIndex::isa_at is
missing an inline hint, which can block the IsaFileReader::lookup fast path at
this hot boundary. Add #[inline] to isa_at (and keep the wrapper minimal) so
callers can inline through the Option lookup path and preserve the intended
performance behavior for this helper tail.
In `@prmi/src/sidecar/isa_file.rs`:
- Around line 164-177: The two writer-contract checks in write_tiered_isa_file
currently return Error::InvalidInput, but these are internal integrity
invariants for generated pairs, so switch both the uint40 bounds guard and the
strict refpos ordering guard to Error::Internal with the same detail text. Keep
the logic in write_tiered_isa_file and preserve the existing messages; only
change the error variant to match the project’s internal-contract convention
used by the dense writer.
- Around line 117-142: The parallel scatter in the SA-to-body write path only
checks `p < n`, which does not prevent duplicate SA entries from causing
concurrent writes to the same offset in
`sa.par_iter().enumerate().for_each(...)`. Update the `isa_file.rs` logic around
`copy_nonoverlapping` to either add a debug-only distinctness check for `sa`
before the parallel loop (so the safety comment’s permutation assumption is
actually enforced during development) or revise the safety comment to state that
correctness depends entirely on upstream permutation guarantees from
`build_gsa`. Keep the `assert!(p < n)` as the range guard, but make the
uniqueness assumption explicit near the `pack_position`/`body_addr` unsafe
block.
In `@prmi/tests/isa_build.rs`:
- Around line 155-162: The assertion in the `idx.sa_num()` test is too loose for
this deterministic fixture. Update the check in `prmi/tests/isa_build.rs` to
assert the exact compacted count expected from the retained `[500,1500)`
positions plus their RC images and sentinel, using the existing `idx.sa_num()`
and message text to enforce `2001` rather than a range.
- Around line 181-184: The independent-oracle loop in isa_build.rs is skipping
samples when mem_search returns match_len == 0, which hides regressions. Update
the test logic around mem_search so that a zero-length match for these in-keep
queries is treated as a failure instead of continuing, and keep the assertion
inside the isa_build test flow tied to idx.mem_search and match_len.
---
Outside diff comments:
In `@prmi-sys/src/lib.rs`:
- Around line 252-264: The extern "C" entrypoint is still doing Rust-side work
before the unwind guard, so a panic from Handle::as_ref(handle) or
h.idx.has_isa() could escape across the C ABI. Move handle resolution and all
subsequent Rust logic inside the existing catch_unwind boundary in prmi_isa_at,
keeping only the null/argument checks outside, so every Rust operation after
entry is protected.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c22b6094-038c-4202-a1de-3d04a1da1736
📒 Files selected for processing (6)
prmi-sys/src/lib.rsprmi/src/cli.rsprmi/src/index/mod.rsprmi/src/sidecar/isa_file.rsprmi/src/train/mod.rsprmi/tests/isa_build.rs
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prmi/src/sidecar/isa_file.rs`:
- Around line 186-192: The tiered ISA serialization logic only checks `rank`
against `UINT40_MAX`, but it also needs to reject ranks that are outside the
compacted SA length. Move the compacted length value (`n`) before the loop in
the sparse ISA writer and validate each launch-hint `rank` with `rank >= n` or
`rank >= pairs.len()` before emitting the entry, in addition to the existing
uint40 bounds check. Update the out-of-range guard in the code that formats the
tiered ISA entry so invalid sparse ranks cannot serialize a loadable `.isa`
pointing past the compacted `.sa`.
- Line 147: The dense ISA writer in the `IsaFile` packing path currently calls
`pack_position(i as u64)` without checking whether the suffix array length
exceeds the 5-byte uint40 range, which can silently truncate ranks in the
on-disk `.isa`. Add a capacity guard before the loop or before packing in the
`write`/packing logic that rejects `sa.len() > UINT40_MAX` for dense files, and
return `Error::Internal { detail: ... }` with a clear writer/SA integrity
message. Use the existing `pack_position` and `UINT40_MAX` symbols to place the
check near the dense ISA encoding path.
In `@prmi/tests/isa_build.rs`:
- Around line 193-196: The tiered-SA test is using the same mem_search-derived
coordinate as its oracle, so the check is not independent. Update the ISA hint
source in isa_build.rs’s mem_search_from_hint coverage to derive the oracle from
the sampled kept coordinate start as u64 via idx.sa_position_for and idx.isa_at,
rather than from m.sa_start, so the test validates the search primitive through
a different code path.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 72fa4794-948a-498f-9ab9-aea52889e8d7
📒 Files selected for processing (5)
prmi-sys/src/lib.rsprmi/src/index/mod.rsprmi/src/sidecar/isa_file.rsprmi/src/train/mod.rsprmi/tests/isa_build.rs
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
prmi/src/sidecar/isa_file.rs (1)
130-174: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy liftValidate SA distinctness in release before the unsafe scatter.
#[cfg(debug_assertions)]leaves release builds accepting in-range duplicates like[0, 1, 1]; two Rayon workers can then write the same 5-byte slot throughbody_addr, which is a data race from a safepub fn. Move a low-memory distinctness check into all builds before truncating/writing the file, or avoid the parallel raw-pointer scatter unless the permutation proof is enforced.Minimal shape of the fix
- #[cfg(debug_assertions)] - { - let mut seen = vec![false; n]; + { + let mut seen = vec![0u64; n / 64 + usize::from(n % 64 != 0)]; for &p in sa { - let p = p as usize; + let p = usize::try_from(p).map_err(|_| Error::Internal { + detail: format!("SA value {p} does not fit usize; SA must be a permutation"), + })?; assert!( p < n, "SA value {p} out of range (n={n}); SA must be a permutation" ); + let word = p / 64; + let bit = 1u64 << (p % 64); assert!( - !std::mem::replace(&mut seen[p], true), + seen[word] & bit == 0, "SA value {p} appears twice; SA must be a permutation (distinct values)" ); + seen[word] |= bit; } }Based on learnings, writer/SA integrity violations should be treated as internal invariants.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prmi/src/sidecar/isa_file.rs` around lines 130 - 174, The scatter in `isa_file` only checks SA distinctness under `#[cfg(debug_assertions)]`, so release builds can still accept duplicate in-range entries and race two Rayon workers onto the same `body_addr` offset. Add a distinctness/permutation validation that runs in all builds before the unsafe parallel write in this path, or gate the parallel raw-pointer scatter behind a guaranteed permutation proof, so `pub fn` callers cannot trigger an internal invariant violation in release.Source: Learnings
prmi-sys/src/lib.rs (1)
252-264: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winWrap all post-null-check work in the FFI panic boundary.
Handle::as_ref(handle)andhas_isa()run beforecatch_unwind, so a panic there can still crossextern "C". Put handle access,.isapresence check, and lookup in one caught closure, then map outcomes to-5,-2,0, or-3.Proposed restructuring
- let h = unsafe { Handle::as_ref(handle) }; - if !h.idx.has_isa() { - set_last_error("prmi_isa_at: sidecar has no .isa (build with --with-isa)"); - return -5; - } + enum IsaAtOutcome { + NoIsa, + Miss, + Hit(u64), + } + // isa_at returns None when refpos is out of range (dense) or not in the @@ - let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| h.idx.isa_at(refpos))); + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let h = unsafe { Handle::as_ref(handle) }; + if !h.idx.has_isa() { + return IsaAtOutcome::NoIsa; + } + match h.idx.isa_at(refpos) { + Some(sa_index) => IsaAtOutcome::Hit(sa_index), + None => IsaAtOutcome::Miss, + } + })); match result { - Ok(Some(sa_index)) => { + Ok(IsaAtOutcome::Hit(sa_index)) => { unsafe { *out_sa_index = sa_index }; 0 } - Ok(None) => { + Ok(IsaAtOutcome::NoIsa) => { + set_last_error("prmi_isa_at: sidecar has no .isa (build with --with-isa)"); + -5 + } + Ok(IsaAtOutcome::Miss) => { set_last_error( "prmi_isa_at: refpos has no inverse-SA entry (out of range or not in keep-set)", );As per path instructions,
prmi-sys/src/**/*.rsrequires everyextern "C"entrypoint to have acatch_unwindboundary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prmi-sys/src/lib.rs` around lines 252 - 264, `prmi_isa_at` still performs `Handle::as_ref(handle)` and the `has_isa()` check before entering the `catch_unwind` boundary, so move all post-null-check work into a single `std::panic::catch_unwind` closure. Keep the handle access, `.isa` presence validation, and `isa_at(refpos)` lookup together inside that closure, then translate the outcomes to the existing return codes (`-5`, `-2`, `0`, `-3`) after the boundary.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@prmi-sys/src/lib.rs`:
- Around line 252-264: `prmi_isa_at` still performs `Handle::as_ref(handle)` and
the `has_isa()` check before entering the `catch_unwind` boundary, so move all
post-null-check work into a single `std::panic::catch_unwind` closure. Keep the
handle access, `.isa` presence validation, and `isa_at(refpos)` lookup together
inside that closure, then translate the outcomes to the existing return codes
(`-5`, `-2`, `0`, `-3`) after the boundary.
In `@prmi/src/sidecar/isa_file.rs`:
- Around line 130-174: The scatter in `isa_file` only checks SA distinctness
under `#[cfg(debug_assertions)]`, so release builds can still accept duplicate
in-range entries and race two Rayon workers onto the same `body_addr` offset.
Add a distinctness/permutation validation that runs in all builds before the
unsafe parallel write in this path, or gate the parallel raw-pointer scatter
behind a guaranteed permutation proof, so `pub fn` callers cannot trigger an
internal invariant violation in release.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2a407bb8-0903-4060-bedc-a5fe2742bf91
📒 Files selected for processing (5)
prmi-sys/src/lib.rsprmi/src/index/mod.rsprmi/src/sidecar/isa_file.rsprmi/src/train/mod.rsprmi/tests/isa_build.rs
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
prmi/src/sidecar/isa_file.rs (1)
100-158: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate before truncating the
.isafile.Move the dense permutation bitset check above
OpenOptions::truncate(true). Right now a duplicate/out-of-rangesareturnsErronly after destroying any existing sidecar and possibly leaving a correctly sized, partially header-written.isa.Proposed shape
pub fn write_isa_file(path: &Path, sa: &[u64]) -> Result<()> { let n = sa.len(); @@ if n as u64 > UINT40_MAX + 1 { return Err(Error::Internal { @@ }); } + + validate_dense_sa_permutation(sa, n)?; + let io = |e: std::io::Error| Error::Io { path: path.to_path_buf(), source: e, }; @@ - { - let mut seen = vec![0u64; n.div_ceil(64)]; - for &p in sa { - ... - } - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@prmi/src/sidecar/isa_file.rs` around lines 100 - 158, Move the SA permutation validation in the `.isa` writer so it runs before `OpenOptions::truncate(true)` in the sidecar creation path. The issue is that `sa` is only checked for distinctness/range after the file has already been truncated and partially initialized, which can destroy an existing valid sidecar on error. Reorder the logic in the function that opens and writes the mmap-backed `.isa` file so the bitset/permutation check happens first, and only proceed to open with truncate, set length, and write the header/body once the input is confirmed valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prmi-sys/src/lib.rs`:
- Around line 269-276: The ISA lookup path in the extern C entrypoint must
validate the returned suffix-array rank before reporting success. In the
`catch_unwind` closure around `h.idx.isa_at(refpos)`, ensure the
`IsaAtOutcome::Hit(sa_index)` path only succeeds when `sa_index <
h.idx.sa_num()`, and otherwise treat it as a miss or error so `out_sa_index` is
never filled with an out-of-range value. Keep the check local to the
`isa_at`/`IsaAtOutcome` flow so malformed `.isa` bodies cannot propagate invalid
SA indices back to bwa-mem3.
---
Outside diff comments:
In `@prmi/src/sidecar/isa_file.rs`:
- Around line 100-158: Move the SA permutation validation in the `.isa` writer
so it runs before `OpenOptions::truncate(true)` in the sidecar creation path.
The issue is that `sa` is only checked for distinctness/range after the file has
already been truncated and partially initialized, which can destroy an existing
valid sidecar on error. Reorder the logic in the function that opens and writes
the mmap-backed `.isa` file so the bitset/permutation check happens first, and
only proceed to open with truncate, set length, and write the header/body once
the input is confirmed valid.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 64efaafc-6e6e-4d18-83f5-bfb43014cb76
📒 Files selected for processing (5)
prmi-sys/src/lib.rsprmi/src/index/mod.rsprmi/src/sidecar/isa_file.rsprmi/src/train/mod.rsprmi/tests/isa_build.rs
…sertion Address review: write_tiered_isa_file now requires STRICTLY ascending refpos (refpos <= prev rejected) — equal refpos would corrupt the binary-search lookup. Tighten the tiered_build test's sa_num assertion from the near-tautology `< 2*l_pac+1` to a tight ~2000 band (1000 kept positions x 2 strands + sentinel) so a silently-misapplied keep-mask is caught.
Newer nightly rustfmt (unpinned CI fmt toolchain) reformats these; apply it so the fmt check passes. Formatting only.
1 participant
Design-Z stack 5/6 — base:
feat/z-keepset.Sparse keep-set-filtered ISA for tiered indexes (inverts the COMPACTED tiered SA:
isa_at(sa_position_for(i)) == ifor kept positions, misses off-keep), restoring the ISA launch hint that the plain tiered build skipped. Plus parallelization of the dense-ISA write and mode-2.sakey extraction.Dual-reviewed (coderabbit --agent + local CR); fixes applied (reject duplicate refpos in the tiered ISA writer; tighten the build test's sa_num assertion). green.
Summary by CodeRabbit
prmi buildnow supports combining--keep-bedwith--with-isa.