perf(spectrum): boundary-search backward, .kmt k-mer table, lockstep batch + oracle, streaming trainer - #16
Merged
Merged
Conversation
|
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 selected for processing (36)
💤 Files with no reviewable changes (5)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (18)
📝 WalkthroughWalkthroughAdds a ChangesCore spectrum & sidecar
Training and verification
Benchmarks, examples, and tests
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
|
nh13
force-pushed
the
feat/v0.2-boundary-kmt
branch
from
June 8, 2026 01:41
1bfe813 to
145de09
Compare
Author
|
@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: 11
🧹 Nitpick comments (3)
prmi-sys/src/lib.rs (1)
850-893: 🏗️ Heavy liftSerial backward batch now has avoidable peak-memory amplification.
The
lockstep == falsebranch still collects every task’sVec<SmemStep>intoall_stepsbefore writing outputs. For largentasks, this can inflate memory and increase OOM risk versus streaming per-task writeback.🤖 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 850 - 893, The else branch currently builds all_steps by collecting every task’s Vec<SmemStep>, causing peak-memory blowup; change it to stream per-task results instead: replace the tasks_s.iter().map(...).collect() in the non-lockstep branch with a simple for/for_each over tasks_s that calls h.idx.backward_spectrum(...) for each task and immediately performs the same per-task writeback logic that is done later for `all_steps` (i.e., process the returned Vec<SmemStep> for that task right away) so you no longer accumulate all SmemStep vectors in `all_steps`; keep the lockstep path using backward_spectrum_lockstep unchanged and retain the same handling for `SmemStep` elements and indices (references: backward_spectrum, backward_spectrum_lockstep, tasks_s, all_steps, SmemStep).prmi/src/sidecar/mod.rs (1)
4-5: ⚡ Quick winDocument the optional
.kmtin the sidecar path contract.The module summary and
from_prefixexample still describe only.meta/.sa/.l1/.l2, butSidecarPathsnow publicly includeskmt. Keeping these docs aligned will make the sidecar surface less misleading.Suggested doc update
-//! On-disk sidecar format: TOML meta + binary `.sa` / `.l1` / `.l2`. +//! On-disk sidecar format: TOML meta + binary `.sa` / `.l1` / `.l2`, +//! plus an optional `.kmt` accelerator sidecar. @@ -/// `meta = "/data/hg38.fa.prmi.meta"`, `.sa`, `.l1`, `.l2`. +/// `meta = "/data/hg38.fa.prmi.meta"`, `.sa`, `.l1`, `.l2`, `.kmt`.Also applies to: 17-17
🤖 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/mod.rs` around lines 4 - 5, Update the module docstring and the example usage of from_prefix to include the optional `.kmt` sidecar file so the documentation matches the public API: mention `.kmt` alongside `.meta`, `.sa`, `.l1`, `.l2` in the top-level comment and update the from_prefix example to show SidecarPaths including the kmt field; ensure the text and example reference the SidecarPaths struct and its kmt member so readers see the full sidecar contract.prmi/src/index/mod.rs (1)
32-35: ⚡ Quick winFix the
kmtfield docs for SHM-backed indexes.This comment still says
open_shmleaveskmtempty because the blob does not carry.kmt, but the new SHM path now loadsblob.kmt_*and can returnhas_kmt() == true. The public type docs should match that behavior.Suggested doc update
- /// Optional forward k-mer table (shallow-band accelerator). `None` for - /// sidecars built without `--kmer-table-k`, or loaded via `open_shm` - /// (the shm blob does not yet carry the `.kmt`). + /// Optional forward k-mer table (shallow-band accelerator). `None` for + /// sidecars built without `--kmer-table-k`, or when best-effort loading + /// rejects an absent, corrupt, or mismatched `.kmt`.🤖 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/index/mod.rs` around lines 32 - 35, The kmt field docs are outdated about SHM-backed indexes: update the comment on the kmt: Option<KmtFileReader> field in mod.rs to state that open_shm may populate kmt from SHM blobs (e.g., blob.kmt_*), so kmt may be Some when has_kmt() is true; remove the claim that open_shm always leaves kmt empty and clearly describe that kmt is None when the index was built without k-mer table or not provided in the SHM blob. Reference the kmt field, KmtFileReader type, open_shm path, and has_kmt() behavior in the brief doc text.
🤖 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/benches/README.md`:
- Around line 128-131: The fenced code block in README.md is missing a language
tag; update the triple-backtick fence that wraps the cargo/build and run
commands to include a language identifier such as "sh" or "text" (e.g., change
``` to ```sh) so markdown linting (MD040) passes while leaving the commands
inside (cargo build --release... and ./target/release/examples/...) unchanged.
In `@prmi/examples/confirm_forward_deep_probes.rs`:
- Around line 68-103: The code can panic: computing max_start = l_pac as usize -
query_len will underflow if query_len > l_pac, and computing stride = (max_start
/ corpus_size) will divide by zero if corpus_size == 0. Before building the
corpus (around the variables l_pac, query_len, corpus_size, max_start, stride,
corpus), validate the CLI inputs: ensure corpus_size > 0 (or return/exit with a
clear error) and ensure query_len <= l_pac (or clamp/query_len = l_pac and/or
return an error); alternatively use safe arithmetic like saturating_sub for
max_start and explicitly handle corpus_size == 0 by returning an error, then
compute stride = (max_start / corpus_size).max(1) only after these checks to
prevent underflow/divide-by-zero when constructing corpus.
In `@prmi/examples/profile_spectrum.rs`:
- Around line 80-112: The current argv parsing loop increments i then directly
accesses argv[i] for flags like "--sidecar", "--fasta", "--n-fwd", "--n-bwd",
"--query-len", "--corpus-size", and "--pac", which will panic on a trailing
flag; update the parser to first check that i + 1 < argv.len() (or use an
iterator/Peekable) before consuming the next value and return a clear
usage/error message if the value is missing, and replace .parse().expect(...)
calls for n_fwd/n_bwd/query_len/corpus_size with proper Result handling that
emits user-friendly errors rather than panicking; leave flags that take no value
(e.g., "--phase-time") as-is without incrementing i.
In `@prmi/src/index/mod.rs`:
- Around line 322-330: Remove the initial Path::exists() gate in
load_kmt_best_effort and instead call KmtFileReader::open(kmt_path)
unconditionally; treat Err(e) by matching e.kind() ==
std::io::ErrorKind::NotFound => return None, and for all other error kinds log
the existing warning (including the error) and return None to preserve the
best-effort forward-search behavior. Apply the same fix pattern to
write_shm_blob: stop using Path::exists(), attempt to open/read the file and
only treat NotFound as missing while logging other IO/permission errors and
proceeding with the fallback.
In `@prmi/src/index/spectrum.rs`:
- Around line 1072-1091: Validate the BwdTask bounds before calling
begin_left_step in bwd_stepper_new: check that t.occ_count != 0 AND t.pivot +
t.anchor_len <= t.read.len() (or equivalent safe bound) before invoking
s.begin_left_step(self); if the bounds are violated set s.finished = true (as
the serial path would produce an empty result) so malformed tasks cannot panic
in lockstep. This references bwd_stepper_new, BwdTask fields (pivot, anchor_len,
read, occ_count) and the begin_left_step call.
- Around line 947-960: The public function forward_spectrum_tabled currently
allows packed-PAC traversal without validating the packed data; add the same
packed-PAC validation used by forward_spectrum/forward_spectrum_auto: call
validate_packed_pac(self.l_pac(), pac, enc) near the start of
forward_spectrum_tabled (after obtaining l_pac and enc/k) and if it returns
false, return the empty steps Vec immediately to avoid unpacking panics on
truncated data. Ensure you reference the existing validate_packed_pac helper and
the forward_spectrum_tabled signature (query: &[u8], pac: &[u8], enc:
PacEncoding, table: &impl KmerBounds) when making the change.
In `@prmi/src/train/config.rs`:
- Around line 121-127: The doc comment above the public method with_kmer_table_k
is incorrect (it still references MemoryMode); update the rustdoc for
with_kmer_table_k to clearly describe that it returns a copy of the config with
a k-mer table (.kmt) of order k (i.e., config builder that enables a k-mer table
/ forward-spectrum shallow-band accelerator of the given k) and remove the
leftover MemoryMode text so generated docs accurately reflect the method's
behavior.
In `@prmi/src/train/mod.rs`:
- Around line 244-247: The current unconditional swallow of
std::fs::remove_file(&paths.kmt) hides real permission/IO errors; replace that
line with explicit error handling that ignores only NotFound and propagates any
other error. For example, call std::fs::remove_file(&paths.kmt) and if it
returns Err(e) check e.kind() against std::io::ErrorKind::NotFound — if
NotFound, continue; otherwise propagate the error from the enclosing function
(or return Err(e.into())/use the ? operator if the function returns a compatible
Result). Ensure you reference and update the call to
std::fs::remove_file(&paths.kmt) and use std::io::ErrorKind::NotFound for the
check.
- Around line 336-345: The code currently coerces config.kmer_table_k == Some(0)
into a 1 by using requested_k.min(k_max).max(1); change this to treat 0 as
invalid: if requested_k == 0 return Err(Error::InvalidInput(...)) (or map to
your crate's InvalidInput variant) before computing k_max and applying the upper
cap; update the logic around the variables requested_k, k_max and k in the block
that reads config.kmer_table_k so callers get an explicit error instead of
silently getting a 1-mer table.
In `@prmi/src/train/trainer.rs`:
- Around line 639-651: The computed radius for trailing empty leaves is off by
one for odd-length tails because it uses (hi - lo + 1).div_ceil(2); change the
radius computation in the ModelEntry creation (around variables lo, hi, mid and
the ModelEntry instantiation) to radius = hi.saturating_sub(lo).div_ceil(2) so
err equals ceil((hi - lo)/2) (which yields 1 for lo=10, hi=12) instead of the
current value; update the line that defines radius and ensure ModelEntry { err:
radius, ... } uses the new value.
---
Nitpick comments:
In `@prmi-sys/src/lib.rs`:
- Around line 850-893: The else branch currently builds all_steps by collecting
every task’s Vec<SmemStep>, causing peak-memory blowup; change it to stream
per-task results instead: replace the tasks_s.iter().map(...).collect() in the
non-lockstep branch with a simple for/for_each over tasks_s that calls
h.idx.backward_spectrum(...) for each task and immediately performs the same
per-task writeback logic that is done later for `all_steps` (i.e., process the
returned Vec<SmemStep> for that task right away) so you no longer accumulate all
SmemStep vectors in `all_steps`; keep the lockstep path using
backward_spectrum_lockstep unchanged and retain the same handling for `SmemStep`
elements and indices (references: backward_spectrum, backward_spectrum_lockstep,
tasks_s, all_steps, SmemStep).
In `@prmi/src/index/mod.rs`:
- Around line 32-35: The kmt field docs are outdated about SHM-backed indexes:
update the comment on the kmt: Option<KmtFileReader> field in mod.rs to state
that open_shm may populate kmt from SHM blobs (e.g., blob.kmt_*), so kmt may be
Some when has_kmt() is true; remove the claim that open_shm always leaves kmt
empty and clearly describe that kmt is None when the index was built without
k-mer table or not provided in the SHM blob. Reference the kmt field,
KmtFileReader type, open_shm path, and has_kmt() behavior in the brief doc text.
In `@prmi/src/sidecar/mod.rs`:
- Around line 4-5: Update the module docstring and the example usage of
from_prefix to include the optional `.kmt` sidecar file so the documentation
matches the public API: mention `.kmt` alongside `.meta`, `.sa`, `.l1`, `.l2` in
the top-level comment and update the from_prefix example to show SidecarPaths
including the kmt field; ensure the text and example reference the SidecarPaths
struct and its kmt member so readers see the full sidecar contract.
🪄 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: 313fd140-fe4a-4d2b-bc8a-e3e176d75ab7
📒 Files selected for processing (35)
prmi-sys/src/lib.rsprmi-sys/tests/spectrum_ffi.rsprmi/Cargo.tomlprmi/benches/README.mdprmi/benches/spectrum_bench.rsprmi/examples/confirm_forward_deep_probes.rsprmi/examples/profile_spectrum.rsprmi/proptest-regressions/index/spectrum.txtprmi/src/cli.rsprmi/src/index/mod.rsprmi/src/index/shm.rsprmi/src/index/spectrum.rsprmi/src/sidecar/isa_file.rsprmi/src/sidecar/kmt_file.rsprmi/src/sidecar/magic.rsprmi/src/sidecar/mod.rsprmi/src/train/config.rsprmi/src/train/mod.rsprmi/src/train/trainer.rsprmi/src/train/training_set.rsprmi/src/train/verify.rsprmi/tests/build_from_pac.rsprmi/tests/isa_open.rsprmi/tests/mask.rsprmi/tests/memory_modes.rsprmi/tests/shm_loader.rsprmi/tests/sidecar_magic.rsprmi/tests/sidecar_paths.rsprmi/tests/spectrum_oracle.proptest-regressionsprmi/tests/spectrum_oracle.rsprmi/tests/train_prmi.rsprmi/tests/train_proptest.rsprmi/tests/train_sidecar_writer.rsprmi/tests/train_training_set.rsprmi/tests/train_verify.rs
💤 Files with no reviewable changes (5)
- prmi/tests/isa_open.rs
- prmi/src/sidecar/isa_file.rs
- prmi/tests/sidecar_paths.rs
- prmi/tests/train_sidecar_writer.rs
- prmi/tests/build_from_pac.rs
nh13
force-pushed
the
feat/v0.2-boundary-kmt
branch
from
June 8, 2026 04:03
145de09 to
822ef64
Compare
Author
|
@coderabbitai Worked through all 11 inline findings + the 3 nitpicks. Amended into the single commit and force-pushed. Fixed (inline threads, resolved):
Fixed (nitpicks):
Deferred (with reasons):
|
|
Tip For best results, initiate chat on the files or code changes.
The two deferrals are both reasonable:
I'll approve the PR. [approve] 🐇 ✨ ✏️ Learnings added
🧠 Learnings used |
Author
|
@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/index/shm.rs (1)
364-374:⚠️ Potential issue | 🟠 MajorUse checked conversions for SHM header offsets/lengths (
u64 -> usize).
Inprmi/src/index/shm.rs(around lines 364-374), the blob header fields are parsed withLittleEndian::read_u64(... ) as usize, which truncates on 32-bit targets. The subsequent layout checks (checked_add+end > blob.len()) and the derived component slices inprmi/src/index/mod.rsoperate on those truncated values, so out-of-rangeu64header values can be incorrectly accepted and point to unintended components. Switch these reads tousize::try_from(LittleEndian::read_u64(...))and reject when they don’t fit (or explicitly gate to 64-bit targets).🤖 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/index/shm.rs` around lines 364 - 374, The header fields currently parsed with LittleEndian::read_u64(... ) as usize (meta_offset, meta_len, sa_offset, sa_len, l1_offset, l1_len, l2_offset, l2_len, kmt_offset, kmt_len) must use checked conversions to avoid truncation on 32-bit targets: replace the casts with usize::try_from(LittleEndian::read_u64(...)) and propagate or return an error when try_from fails (or explicitly gate this parsing to 64-bit builds); ensure the calling function (the parser in prmi/src/index/shm.rs that produces these offsets/lengths and any subsequent layout checks using checked_add) rejects values that don’t fit into usize so out-of-range u64 header values cannot be accepted.
🤖 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/shm.rs`:
- Around line 389-394: The current loop in read_shm_blob skips validation
whenever len == 0, which lets core components (e.g., meta, sa) be treated as
empty slices; change the logic to reject zero-length for all mandatory
components and only allow skipping when the component is the optional .kmt.
Specifically, inside the loop that examines each component (the block that
currently checks if len == 0 { continue; }), replace the unconditional continue
with a check on the component identifier (the variable used to identify the
component, e.g., component_name or comp) and return a malformed-wrapper error
for zero length on mandatory components (meta, sa, etc.), only continuing
silently if the component is ".kmt"; keep the existing strict layout/offset
validation for any present (len > 0) component.
---
Outside diff comments:
In `@prmi/src/index/shm.rs`:
- Around line 364-374: The header fields currently parsed with
LittleEndian::read_u64(... ) as usize (meta_offset, meta_len, sa_offset, sa_len,
l1_offset, l1_len, l2_offset, l2_len, kmt_offset, kmt_len) must use checked
conversions to avoid truncation on 32-bit targets: replace the casts with
usize::try_from(LittleEndian::read_u64(...)) and propagate or return an error
when try_from fails (or explicitly gate this parsing to 64-bit builds); ensure
the calling function (the parser in prmi/src/index/shm.rs that produces these
offsets/lengths and any subsequent layout checks using checked_add) rejects
values that don’t fit into usize so out-of-range u64 header values cannot be
accepted.
🪄 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: d1187774-ab92-468c-96df-3d299c0886a6
📒 Files selected for processing (36)
prmi-sys/src/lib.rsprmi-sys/tests/spectrum_ffi.rsprmi/Cargo.tomlprmi/benches/README.mdprmi/benches/spectrum_bench.rsprmi/examples/confirm_forward_deep_probes.rsprmi/examples/profile_spectrum.rsprmi/proptest-regressions/index/spectrum.txtprmi/src/cli.rsprmi/src/index/mod.rsprmi/src/index/shm.rsprmi/src/index/spectrum.rsprmi/src/sidecar/isa_file.rsprmi/src/sidecar/kmt_file.rsprmi/src/sidecar/magic.rsprmi/src/sidecar/mod.rsprmi/src/train/config.rsprmi/src/train/mod.rsprmi/src/train/trainer.rsprmi/src/train/training_set.rsprmi/src/train/verify.rsprmi/tests/build_from_pac.rsprmi/tests/isa_open.rsprmi/tests/mask.rsprmi/tests/memory_modes.rsprmi/tests/shm_loader.rsprmi/tests/sidecar_magic.rsprmi/tests/sidecar_paths.rsprmi/tests/spectrum_oracle.proptest-regressionsprmi/tests/spectrum_oracle.rsprmi/tests/train_2x_sidecar.rsprmi/tests/train_prmi.rsprmi/tests/train_proptest.rsprmi/tests/train_sidecar_writer.rsprmi/tests/train_training_set.rsprmi/tests/train_verify.rs
💤 Files with no reviewable changes (5)
- prmi/tests/sidecar_paths.rs
- prmi/tests/train_sidecar_writer.rs
- prmi/src/sidecar/isa_file.rs
- prmi/tests/isa_open.rs
- prmi/tests/build_from_pac.rs
✅ Files skipped from review due to trivial changes (3)
- prmi/tests/spectrum_oracle.proptest-regressions
- prmi/proptest-regressions/index/spectrum.txt
- prmi/src/sidecar/magic.rs
🚧 Files skipped from review as they are similar to previous changes (25)
- prmi/tests/mask.rs
- prmi/tests/train_verify.rs
- prmi/tests/sidecar_magic.rs
- prmi/Cargo.toml
- prmi/tests/train_prmi.rs
- prmi/tests/train_proptest.rs
- prmi/src/train/config.rs
- prmi/tests/memory_modes.rs
- prmi/src/cli.rs
- prmi/tests/shm_loader.rs
- prmi/src/sidecar/mod.rs
- prmi/src/train/trainer.rs
- prmi/tests/spectrum_oracle.rs
- prmi/tests/train_training_set.rs
- prmi/examples/confirm_forward_deep_probes.rs
- prmi/src/train/verify.rs
- prmi/src/sidecar/kmt_file.rs
- prmi-sys/tests/spectrum_ffi.rs
- prmi/src/index/mod.rs
- prmi/benches/spectrum_bench.rs
- prmi-sys/src/lib.rs
- prmi/examples/profile_spectrum.rs
- prmi/src/train/training_set.rs
- prmi/src/index/spectrum.rs
- prmi/src/train/mod.rs
…p batch + oracle; streaming/parallel trainer
nh13
force-pushed
the
feat/v0.2-boundary-kmt
branch
from
June 8, 2026 18:37
822ef64 to
c64daf4
Compare
Author
|
@coderabbitai Addressed both remaining findings (amended into the single commit).
build / clippy |
Author
|
@coderabbitai review |
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
This was referenced Jun 21, 2026
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.
PR #3 of the v0.2 stack (carries commit
2e048de). Base isfeat/v0.2-c-abi(#15), notmain— this is stacked on #2 and should merge after it. Tracked inV0.2_PR_STACK.mdrow #3.What this does
The heaviest PR in the stack — the boundary-search backward spectrum engine plus its supporting machinery. Four interdependent concerns that cannot be cleanly separated (lockstep calls
backward_spectrum_lockstep, which uses the boundary-search; the spectrum imports the.kmtKmerBounds;forward_spectrum_autodispatches on the loaded.kmt), so they ship together:backward_spectrum)..kmtk-mer table sidecar (kmt_file.rs, magicPMKT) — a forward-search accelerator, loaded best-effort (ignored with a warning if corrupt or ref-mismatched). Replaces the.isasidecar, which is removed here and reintroduced (build/load/FFI) at stack feat(prmi): cleanroom trainer (uniform weighting) + shared lookup math #6 (3acd729).backward_spectrum_lockstepdrives all tasks through batched probe loads (memory-level parallelism); byte-identical to the serial path..pacbuild (no ~51 GB key/target materialization on the uniform-prior, no-mask path).Plus the brute-force
spectrum_oracleproptests that gate byte-identity.Carry-forward of the merged review fixes (the conflict resolution)
2e048dewas authored on the pre-squash foundation, so it reverts four review fixes that now live onmain/#2. I re-applied each onto #3's rewritten code (compiler + full test suite as the safety net):train/mod.rs) — re-applied to the materialized training path (the streamed path only runs when no N-mask is in effect).backward_spectrumbounds — re-applied tobackward_spectrum_innerand its test-only reference oracle; Upstream RMI primitives: latent bugs to revisit (deferred from #2) #3's rewrite also slicesread[.. pivot + anchor_len], so the guard now checks the full anchored window fitsread(stronger than the oldpivot > read.len()).forward_spectrum,forward_spectrum_auto(covers the.kmt-tabled path), and the backward walkers.prmi-sys) — re-applied thepacked_pac_byteshelper +usize::try_fromguards forn_out,ntasks, packed-pac length, and per-taskquery_off/read_off, across the single, batch, and the new sharedbackward_spectrum_batch_impl(covers bothprmi_backward_spectrum_batchand_lockstep). Doc-2/-3enumerations updated to match.Two #14 test fixes were orphaned by the
.isaremoval and adapted rather than dropped:tests/sidecar_magic.rs— theISA_MAGICassertion becameKMT_MAGIC(PMKT=0x544B4D50), preserving magic-contract coverage for the new sidecar.tests/train_sidecar_writer.rs— the.isa-exists assertion removed (no.isauntil feat(prmi): cleanroom trainer (uniform weighting) + shared lookup math #6).tests/shm_loader.rs— Upstream RMI primitives: latent bugs to revisit (deferred from #2) #3 deleted theread_shm_blobwrapper-layout invariant tests; I restored them (they still apply toread_shm_blob), per the green-pass "relocate misplaced test/code" rule.Green-pass
cargo build --workspace✅cargo clippy --workspace --all-targets --all-features -- -D warnings✅cargo +nightly fmt --all -- --check✅ (also reformatted some of2e048de's own code that wasn't nightly-fmt-clean)cargo test --workspace✅ — 139 lib tests + all integration/FFI suites pass (spectrum oracle proptests, lockstep≡serial, restored shm invariant tests, FFI arena/overflow guards); only the Plan-3 deferral tests remainignored(re-enabled later infeat/v0.2-extend-match).Summary by CodeRabbit
New Features
Improvements
Removals
Testing & Benchmarking