From 78184e36e3a87ebd4a507d4cf3f37cb5fc324580 Mon Sep 17 00:00:00 2001 From: d-laub Date: Mon, 21 Sep 2026 09:25:30 -0700 Subject: [PATCH 01/10] docs(spec): fold the mixed record-backend window decode into the producer (#400) Removes the second VCF/PGEN window decode on the mixed variants+tracks path: read the window the producer already decoded from the drive engine's current slot, via a new with_current_window core primitive and a current_window_realign_inputs pymethod that validates window identity against the engine's own job. Deletes the per-backend _mixed_engine() second engine; keeps window_realign_inputs as a documented test-only oracle for the CSR replication test. SVAR1 deliberately not folded (offsets-only read). Gate is deterministic decode counters, not wall-clock. --- ...reaming-mixed-record-decode-fold-design.md | 236 ++++++++++++++++++ 1 file changed, 236 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-21-streaming-mixed-record-decode-fold-design.md diff --git a/docs/superpowers/specs/2026-09-21-streaming-mixed-record-decode-fold-design.md b/docs/superpowers/specs/2026-09-21-streaming-mixed-record-decode-fold-design.md new file mode 100644 index 00000000..730e9ddc --- /dev/null +++ b/docs/superpowers/specs/2026-09-21-streaming-mixed-record-decode-fold-design.md @@ -0,0 +1,236 @@ +# Fold the record-backend mixed track window decode into the producer + +**Issue:** [#400](https://github.com/mcvickerlab/GenVarLoader/issues/400) +**Roadmap:** `docs/roadmaps/streaming-dataset.md` → Plans table +**Date:** 2026-09-21 +**Branch:** `feat/400-mixed-record-decode-fold` (targets `streaming`) + +## Goal + +Mixed variants+tracks streams on the VCF/BCF and PGEN backends decode every window **twice**: +once synchronously on the consumer thread, to size the deletion-extended track query +(`t_ends_ext`) before the window's track read, and once in the engine's producer thread, which +fills the ping-pong slot the batches are generated from. + +Remove the second decode by reading the window the producer already decoded, from the engine +being driven. Byte-identical parity against `gvl.write()` + `Dataset[r, s]` is preserved on +both halves of the `(haplotypes, tracks)` tuple, and there is no public API change. + +## Summary of decisions + +| Decision | Choice | +|---|---| +| Source of the window's variant data | The drive engine's **current window slot** — no ad-hoc decode | +| New Rust core primitive | `StreamEngineCore::ensure_current_window` (extracted from `advance`) + `with_current_window` | +| New FFI surface | `RecordStreamEngine.current_window_realign_inputs(contig_idx, region_starts, region_ends, s_lo, s_hi)` | +| Window identity | **Validated in Rust** against the current job; mismatch raises `PyValueError` | +| Python seam | `_mixed_track_window(..., engine)` → `_MixedTracksBackend.mixed_realign_window(..., engine=None)` | +| Second engine object | Deleted — `_VcfBackend._mixed_engine` / `_PgenBackend._mixed_engine` go away | +| `window_realign_inputs` | **Kept, documented test-only** — the only independent oracle for the CSR-replication test | +| SVAR1 / SVAR2 | Not folded (SVAR1's re-read is offsets-only, no decode; SVAR2 never had a second decode) | +| Gate | Deterministic decode counters, not wall-clock | + +## 1. The duplication + +In the `"engine"` drive (`python/genvarloader/_dataset/_streaming.py:2270-2287`) each window +composes in this order: + +1. `_mixed_track_window` (`:1677`) calls `backend.mixed_realign_window(...)` (`:1758`), which + for a record backend reaches `_record_mixed_realign_window` (`:735`). +2. That function calls `backend._mixed_engine().window_realign_inputs(...)` (`:812-819`) — + `_mixed_engine` (`:4816`, `:5072`) is a cached, **plan-less** second engine + (`build_engine([], 1, 1)`), and `window_realign_inputs` + (`src/record_stream/engine.rs:1086`) decodes the window it is handed via + `RecordBackend::debug_fill` into a throwaway slot. +3. The decode yields `(v_starts, ilens, geno_v_idxs, geno_offsets)`, from which the mixed path + computes `diffs` → `region_max_del` → `t_ends_ext`, then reads the window's tracks. +4. The batch loop then pulls batches with `next_batch()`, whose producer thread decodes the + **same window** again into the ping-pong slot. + +So a mixed VCF/PGEN sweep pays 2× window decode plus a second engine object per backend (for +PGEN a second `PgenWindowFiller`, hence a second pgenlib reader; for VCF a second +`VcfRecordSource` open per window), and — because the consumer's decode is +`py.detach`'d but still serial with the track read — no overlap between decode and the track +read it feeds. + +## 2. Rust: `ensure_current_window` + `with_current_window` + +`StreamEngineCore::advance` (`src/ffi/stream_core.rs:324-399`) already owns the window +lifecycle: start the producer once, recycle the spent window, `recv` the next, then yield the +next `batch_size` row slice. Split it in two, with no control-flow change: + +- `fn ensure_current_window(&self, state: &mut EngineState) -> Result<(), NextSlice>` + — the start/recycle/`recv` loop, returning `Ok(())` only when + `state.current` exists **and** has rows left (`next_row < n_batch_rows`), leaving + `next_row` untouched. The `Err` side carries only `NextSlice::Done`/`NextSlice::Failed`; + `Ready` is constructed by `advance` alone. +- `fn advance(...) -> NextSlice` — `ensure_current_window`, then the existing slice-and-return + from `current`. Byte-identical to today, including the zero-row-window behavior (such a + window is skipped by the loop, exactly as now). + +Then the accessor primitive: + +```rust +pub(crate) fn with_current_window( + &self, + f: impl FnOnce(usize, &B::Slot) -> anyhow::Result, +) -> Option> +``` + +It locks the state mutex, calls `ensure_current_window`, and on success invokes `f(job_idx, +¤t.filled)` **while holding the lock** — the same discipline `next_batch_core` already +uses for `generate` (`:421-422`). `None` means plan exhaustion; `Some(Err(_))` is the +producer's error/panic, already join-then-classified. No GIL is needed inside `f`; the +blocking `recv` happens under the caller's `py.detach`. + +## 3. Rust: `RecordStreamEngine.current_window_realign_inputs` + +A new `#[pymethods]` entry on `RecordStreamEngine` (`src/record_stream/engine.rs:622`), +mirroring `window_realign_inputs`'s signature and return shape so the Python side is unchanged: + +```rust +fn current_window_realign_inputs<'py>( + &self, py: Python<'py>, + contig_idx: usize, + region_starts: Vec, region_ends: Vec, + s_lo: usize, s_hi: usize, +) -> PyResult>, Bound<'py, PyArray1>, + Bound<'py, PyArray1>, Bound<'py, PyArray1>, +)>> +``` + +Behavior, all inside `py.detach(|| self.core.with_current_window(...))`: + +1. **Identity validation first.** The closure compares the current job + (`RecordBackend::jobs[job_idx]`, fields `contig_idx` / `regions` / `s_lo` / `s_hi`; + `src/record_stream/engine.rs:60-65`) against the requested + `(contig_idx, region_starts|region_ends, s_lo, s_hi)` and bails with a descriptive message + on any difference. This is the change that turns "pair window N's tracks with window M's + variants" from a silent parity break into a loud error, and it is cheap (one region-scale + `Vec` compare per window). It is only sound because the drive builds the engine's jobs and + the track window's bounds from the same `region_offsets` translation + (`_streaming.py:2200-2219` vs `:1715-1725`). +2. **No decode.** A new inherent `RecordBackend::realign_inputs(&self, slot: &DecodedWindow)` + clones the slot's four arrays into `Array1`s — `v_starts`/`ilens`/`geno_v_idxs`/ + `geno_offsets` — exactly the values `window_realign_inputs` returns. The arrays must be + copied rather than moved: the slot is still needed to generate this window's batches + (`EngineBackend::generate` reads `v_starts`/`geno_v_idxs`). The copy is window-scale + (`geno_v_idxs` is the window's CSR entries), i.e. strictly cheaper than the decode that + produced them, let alone a second decode. +3. **Exhaustion** returns `None`; `_record_mixed_realign_window` raises the drive's existing + `"streaming engine exhausted before the plan did"` message, matching the batch loop. + +`RecordBackend::debug_fill` and the existing `window_realign_inputs` pymethod are untouched +(§5). + +## 4. Python: one seam, one engine + +- `_MixedTracksBackend.mixed_realign_window` (`:488`) gains a trailing + `engine: "object | None" = None`. `object` matches this file's existing engine-handle typing + (`_mixed_engine() -> object`, `build_engine(...) -> object`), and the drive's engine class is + imported lazily inside functions. +- `StreamingDataset._mixed_track_window` (`:1677`) gains `engine: "object | None" = None` and + forwards it. Its two call + sites: `:2277` (the `"engine"` drive) passes `engine=engine`; `:2657` (the SVAR2 `"sync"` + drive, no engine exists) passes nothing. +- `_record_mixed_realign_window` (`:735`) takes `engine` and calls + `engine.current_window_realign_inputs(contig_idx, starts, ends, s_lo, s_hi)` where it used to + call `backend._mixed_engine().window_realign_inputs(...)`. With `engine is None` it raises a + `ValueError` naming the required argument — a direct caller (a test) must supply a plan + engine; there is no second-engine fallback, which is the point of the change. +- `_Svar1Backend.mixed_realign_window` (`:4020`) and `_Svar2Backend.mixed_realign_window` + (`:4573`) accept and ignore `engine`, with a one-line docstring note (`SVAR1` reads its own + store's offsets; SVAR2 reads its `read_window` bundle). +- Delete `_VcfBackend._mixed_engine` (`:4816`) and `_PgenBackend._mixed_engine` (`:5072`) plus + their `_mixed_engine_obj` fields (`:4747`, `:5062`), and the `#307` comments that mention + `_mixed_engine()`'s `build_engine([], 1, 1)` (`:4920`, `:5162`) — every remaining + `build_engine` call now carries a real plan, so the `touched_contigs` optimization is + unchanged but its motivating note needs rewording. + +Ordering is preserved without restructuring the drive: the peek itself starts the producer and +receives the window (`ensure_current_window`), leaving `next_row == 0`; the existing +`for lo, hi in _batch_bounds(0, n_rows, batch_size)` loop then drains it. The engine's +`batch_size` is the same value passed to `build_engine`, so the row slices line up. + +## 5. What stays, and why + +- `window_realign_inputs` (`src/record_stream/engine.rs:1086`) **stays**, re-documented as + test-only (precedent: `debug_decode_window`, `:1012`). It is the only independent oracle for + `tests/dataset/test_streaming_tracks_record.py::test_record_window_csr_replicates_across_regions`: + it decodes a caller-named window on a plan-less engine, so the test can keep comparing the + mixed path's replicated CSR against a decode the mixed path did not produce. Deleting it + would force that test to become a self-consistency check, which a consistently-wrong CSR + replica can pass. +- `StreamEngineCore::backend()`'s M1 doc (`src/ffi/stream_core.rs:228-261`) and the + `PgenWindowFiller::reader_lock` doc (`src/record_stream/pgen.rs:415-435`) both describe the + consolidation as a future step and name the production caller of the ad-hoc decode. Both are + now stale: the fold happened, and the remaining second callers are the two test-only decode + accessors (`debug_decode_window` and `window_realign_inputs` themselves). The lock **stays** + (the issue's own instruction): it is what keeps a shared filler correct, and those accessors + still exercise it. + +## 6. Non-goals + +- **SVAR1 is not folded.** Its `mixed_realign_window` calls `self.read_window(r_idx, s_idx)`, + which returns offsets borrowed from the store's `variant_idxs` mmap — no decode, so the fold + would save nothing measurable while touching the parity-critical SVAR1 path. The fold is + record-backend-only by construction: only those backends have a decode inside the engine. +- **The `jitter>0` + `realign_tracks=True` guard is not lifted** (`:2054-2066`). Its stated + rationale (`_Svar1Backend.read_window` re-reads raw `_regions` bounds) survives for SVAR1, + and the record-backend half is untested either way; that is + [#383](https://github.com/mcvickerlab/GenVarLoader/issues/383)'s scope. The guard's comment + gets one added sentence recording that record backends' decode is now jitter-translated. +- No new public symbol, so no `__all__`, `docs/source/api.md`, skill, or FAQ changes. This is a + pure perf/correctness-neutral change to an existing path. +- `_streaming.py` is 5.2k lines and the mixed-tracks seam (`_MixedRealign`, `_RealignWindow`, + `_Svar2Realign`, `_record_mixed_realign_window`, `_TrackWindow`, `_mixed_track_window`, + `_mixed_tracks_batch`) is a natural module-extraction candidate. Deliberately not done here; + it would swamp a review whose whole value is "same bytes, half the decode". + +## 7. Gate + +Standing convention: deterministic counters, never wall-clock on this shared node. + +**Primary gate (new tests, `tests/dataset/test_streaming_tracks_record.py`).** For VCF and +PGEN, drive the same mixed fixture twice — once `realign_tracks=True`, once `False` — with +`transpose_word_reads_reset()` (both) and `pgen_variants_decoded_reset()` (PGEN) around each +sweep, and require the totals be **equal** and non-zero. `realign_tracks` cannot change the +engine's job list, so the decode totals must match; before the fold the realign run decodes +each window twice and the equality fails. The `> 0` half keeps it from passing vacuously. + +**Parity (the safety net, unchanged):** the existing byte-identical suites on the touched paths +— `test_streaming_tracks_record.py`'s mixed parity cases, `test_streaming_tracks.py`, +`test_streaming_tracks_svar2.py`, `test_streaming_parity.py`, and the abandoned-iterator tests +(`test_streaming_abandoned_iter{,_mixed}.py`, #399) since the producer now starts earlier in +each window's composition. + +**New guard test:** a `current_window_realign_inputs` call whose region bounds (or `s_lo`/`s_hi`) +do not match the current job raises `ValueError` — the deliberate-failure half of §3's identity +check, which no parity test can reach. + +**Before/after numbers** are color, captured by running the same counter sweep against the +parent commit; the gate itself is the equality above. + +Full verification: `pixi run -e dev maturin develop --release` before any Python test run, then +`pytest tests/dataset tests/unit -q`, `cargo test`, `cargo clippy`, `ruff check python/ tests/`, +`pyrefly`, and `python scripts/docstring_style.py --check python/genvarloader`. + +Closing bookkeeping: the change is `perf(streaming):` (so it gets a changelog entry), and +`docs/roadmaps/streaming-dataset.md`'s Plans table row for #400 gets its status marker, PR +pointer, and the measured before/after counter numbers. + +## 8. Risks + +1. **Locking.** `with_current_window` holds the engine's state mutex while cloning + window-scale arrays, and the blocking `recv` inside it must stay under `py.detach` + (the #399 deadlock discipline). Any marshaling happens after the lock is released. +2. **Producer start time moves earlier** in each window's composition (before the track read + instead of at the first `next_batch`). Semantically it is the same sequence position; the + abandonment tests are the regression net. +3. **Identity validation is deliberately strict.** A future change that legitimately decodes a + wider window than the track query must relax it explicitly rather than silently; the error + message must name both sides to make that obvious. +4. **Zero-row windows** are skipped by `ensure_current_window` exactly as `advance` skips them + today, so the fold cannot introduce a new skip — and an unexpected skip now trips the + identity check instead of silently mispairing windows. From 41547b4f8206a78c3c6471300d1508ee52b40f65 Mon Sep 17 00:00:00 2001 From: d-laub Date: Mon, 21 Sep 2026 09:34:15 -0700 Subject: [PATCH 02/10] docs(plan): implementation plan for the #400 mixed record decode fold Four TDD tasks: with_current_window in the engine core, the current_window_realign_inputs pymethod, the Python fold plus the deterministic-counter gate, then doc/roadmap retirement and the full-tree sweep. --- ...9-21-streaming-mixed-record-decode-fold.md | 963 ++++++++++++++++++ 1 file changed, 963 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-21-streaming-mixed-record-decode-fold.md diff --git a/docs/superpowers/plans/2026-09-21-streaming-mixed-record-decode-fold.md b/docs/superpowers/plans/2026-09-21-streaming-mixed-record-decode-fold.md new file mode 100644 index 00000000..1535b42a --- /dev/null +++ b/docs/superpowers/plans/2026-09-21-streaming-mixed-record-decode-fold.md @@ -0,0 +1,963 @@ +# Streaming Mixed Record Decode Fold (#400) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fold the record-backend (VCF/PGEN) mixed variants+tracks window decode into the producer engine, so a mixed record stream decodes each window exactly once instead of twice. + +**Architecture:** A new core primitive `StreamEngineCore::ensure_current_window` (split out of `advance`) plus `with_current_window`, exposed as the pymethod `RecordStreamEngine.current_window_realign_inputs`, lets the Python drive read the window the producer has **already** filled. A new inherent `RecordBackend::realign_inputs` clones that slot's four arrays, and the pymethod validates the requested job against the engine's current job so drive/plan drift fails loudly instead of silently pairing one window's tracks with another's variants. Python's record `mixed_realign_window` then takes the drive's `engine` and drops its private zero-job engine. + +**Tech Stack:** Rust (PyO3 0.26-style `py.detach`, `numpy` crate, anyhow), Python 3.10+, pytest, pixi + maturin. + +**Spec:** `docs/superpowers/specs/2026-09-21-streaming-mixed-record-decode-fold-design.md` + +## Global Constraints + +- **The gate is deterministic decode counters, never wall-clock.** For the same mixed fixture sweep, `transpose_word_reads` with `realign_tracks=True` must be **exactly equal** to the `realign_tracks=False` run (and `> 0`); for PGEN, `pgen_variants_decoded` must be equal too. Before the fold the realign run is ~2x. +- **Byte-identical parity is the safety net.** Every other test in `tests/dataset/test_streaming_tracks_record.py`, plus `tests/dataset/test_streaming_tracks.py`, `test_streaming_tracks_svar2.py`, `test_streaming_scale.py`, must stay green **unmodified** — except the two tests this plan explicitly ports. +- **No public API change.** Nothing new in `python/genvarloader/__init__.py::__all__`; do **not** touch `docs/source/api.md` or `skills/genvarloader/SKILL.md`. `RecordStreamEngine` is not exported (no `api.md` entry), so the new pymethod is library-internal. `python/genvarloader/_dataset/_streaming.py` is the only Python module that changes. +- **SVAR1 is NOT folded** — `_Svar1Backend.mixed_realign_window` reads offsets with no variant decode. It gains only the `engine` parameter it ignores. SVAR2 likewise. +- **The `jitter > 0` + `realign_tracks` guard stays** (`_streaming.py:2054-2066`). Do not lift it; issue #383 owns that. +- **Rebuild the extension before every Python test run**: `pixi run -e dev maturin develop --release`. Otherwise pytest imports the stale `.so` and every result is about the old binary. +- **`pyproject.toml` relaxes `missing-attribute`/`bad-argument-type`/`unexpected-keyword`/`bad-return` to WARN** (`[tool.pyrefly.errors]`), so the `object`-typed engine boundary cannot fail the typecheck gate. Do not add `# type: ignore` comments (an unused ignore is itself an error in some envs; the repo prefers WARN). +- **Commits trigger the installed prek hooks.** Never `--no-verify`. Commit type prefixes: `refactor(streaming)` / `feat(streaming)` / `perf(streaming)` / `docs(streaming)`. Use `git commit -F ` for any message containing backticks (zsh `-m` chokes on them). +- Ruff config ignores E501; docstrings are Google style; `python scripts/docstring_style.py --check python/genvarloader` is a gate. **Do not add code comments other than the ones this plan specifies.** +- All commands run from the worktree root (`git rev-parse --show-toplevel`) via `pixi run -e dev `. + +## File Structure + +- `src/ffi/stream_core.rs` — generic engine state machine. Add `ensure_current_window` (private) and `with_current_window` (`pub(crate)`); refactor `advance` onto them; add the `MarkerBackend` stub + cursor test to `mod tests`. Also gains the updated `backend()` doc (Task 4). +- `src/record_stream/engine.rs` — `RecordBackend` owns decode + CSR; `RecordStreamEngine` is the PyO3 surface. Add the inherent `RecordBackend::realign_inputs`; add the `current_window_realign_inputs` pymethod next to `window_realign_inputs`; `window_realign_inputs`'s doc becomes test-only (Task 4). +- `python/genvarloader/_dataset/_streaming.py` — the drive. `engine` parameter on the `_MixedTracksBackend` protocol + all 4 implementations + `_record_mixed_realign_window`; forward it from `_mixed_track_window`; delete `_VcfBackend._mixed_engine`/`_mixed_engine_obj` and `_PgenBackend._mixed_engine`/`_mixed_engine_obj`; `build_engine` doc fixups; the guard comment gains one sentence. +- `tests/dataset/test_streaming_tracks_record.py` — the gate test, the new pymethod test, the ported CSR test, the module docstring, the `window_realign_inputs` test's docstring. +- `docs/roadmaps/streaming-dataset.md` — the two `#400` follow-up mentions become `fixed` entries with the PR number and the measured counter numbers. + +--- + +## Task 1: `ensure_current_window` + `with_current_window` in the engine core + +**Files:** +- Modify: `src/ffi/stream_core.rs` (refactor `advance`, add two methods, extend `#[cfg(test)] mod tests`) +- Test: `src/ffi/stream_core.rs` (`mod tests`) + +**Interfaces:** +- Consumes: existing `EngineState` (`current: Option`, `next_job_idx`, `done`), `ensure_started`, `B::n_batch_rows`, `B::generate`. +- Produces: + - `fn ensure_current_window(&self, state: &mut EngineState) -> anyhow::Result` — private. `Ok(true)` = `state.current` is set with rows left and `next_row` **untouched**; `Ok(false)` = plan exhausted, `state.done` set; `Err(_)` = producer failure/panic, `state.done` set. Never yields a row slice. + - `pub(crate) fn with_current_window(&self, f: impl FnOnce(usize, &B::Slot) -> anyhow::Result) -> Option>` — `None` = exhausted; `Some(Ok(_))` = closure result for the current `(job_idx, slot)`; `Some(Err(_))` = producer failure/panic. Holds the state lock while `f` runs; consumes **no** rows. + +- [ ] **Step 1: Write the failing test** + +Append to the `mod tests` block in `src/ffi/stream_core.rs`, immediately **before** its closing `}` (after `dropping_mid_stream_engine_does_not_deadlock_a_gil_needing_filler`): + +```rust + /// Backend with no filler internals, no GIL and no channels of its own: each filled + /// slot records the job index that produced it (so a test can tell WHICH window the + /// core is holding) and `generate` returns one byte per requested row (so a test can + /// tell which rows the cursor asked for). + struct MarkerBackend { + n_jobs: usize, + rows_per_job: usize, + } + + impl EngineBackend for MarkerBackend { + type Slot = Vec; + + fn n_jobs(&self) -> usize { + self.n_jobs + } + + fn fill(&self, job_idx: usize, slot: &mut Vec) -> anyhow::Result<()> { + slot.clear(); + slot.push(job_idx); + Ok(()) + } + + fn n_batch_rows(&self, _job_idx: usize, _slot: &Vec) -> usize { + self.rows_per_job + } + + fn generate( + &self, + _job_idx: usize, + _slot: &Vec, + row_lo: usize, + row_hi: usize, + ) -> anyhow::Result<( + Array1, + Option>, + Option>, + Array1, + )> { + Ok(( + Array1::from_vec(vec![0u8; row_hi - row_lo]), + None, + None, + Array1::from_vec(vec![0i64, (row_hi - row_lo) as i64]), + )) + } + } + + /// Issue #400: `with_current_window` must hand back the window the producer has + /// already filled WITHOUT consuming any of its rows, and must only move on once that + /// window is spent. This is what lets the Python mixed record path size its track + /// query from the producer's decode instead of decoding the window a second time. + #[test] + fn with_current_window_exposes_the_current_window_without_consuming_rows() { + let core = StreamEngineCore::new( + Arc::new(MarkerBackend { + n_jobs: 3, + rows_per_job: 2, + }), + 1, + ); + + // Starts the producer and receives window 0, consuming nothing. + let first = core + .with_current_window(|job_idx, slot| { + assert_eq!( + slot.as_slice(), + [job_idx], + "slot must carry its own job marker" + ); + Ok(job_idx) + }) + .expect("a current window must be available") + .expect("no producer error"); + assert_eq!(first, 0); + + // Idempotent: window 0 still has all of its rows, so the same window comes back. + let again = core + .with_current_window(|job_idx, _slot| Ok(job_idx)) + .expect("window 0 still has rows") + .expect("no producer error"); + assert_eq!(again, 0, "the peek consumed rows instead of just reading"); + + // The row cursor was untouched: window 0 still yields BOTH of its rows. + let widths: Vec = (0..2) + .map(|_| { + core.next_batch_core() + .expect("row slice") + .expect("no error") + .0 + .len() + }) + .collect(); + assert_eq!(widths, [1, 1], "peeked window lost rows"); + + // Only a SPENT window advances the core's idea of "current". + let second = core + .with_current_window(|job_idx, _slot| Ok(job_idx)) + .expect("window 1 is now current") + .expect("no producer error"); + assert_eq!(second, 1, "the core advanced before the window was spent"); + + // 3 jobs x 2 rows at batch_size 1: 6 batches total, 2 already taken. + let mut remaining = 0usize; + while let Some(batch) = core.next_batch_core() { + batch.expect("no producer error"); + remaining += 1; + } + assert_eq!( + remaining, 4, + "peeking must not change how many rows the plan yields" + ); + + // Fully drained: the peek reports exhaustion rather than hanging or re-yielding. + assert!(core + .with_current_window(|job_idx, _slot| Ok(job_idx)) + .is_none()); + } +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `pixi run -e dev cargo test --release with_current_window` + +Expected: **compile failure** — `no method named 'with_current_window' found for struct 'StreamEngineCore'`. (First `cargo test` in a session compiles the crate; give it a few minutes.) + +- [ ] **Step 3: Implement `ensure_current_window`, `with_current_window`, and the `advance` refactor** + +In `src/ffi/stream_core.rs`, replace the whole `fn advance(...)` (its current body is the `if state.done { ... }` -> `ensure_started` -> `loop { ... }` block, ending with the `match recv` that returns `NextSlice::Ready/ Failed/ Done`) with the three functions below. Keep `advance`'s existing doc-comment text (the "Byte-identical control flow to the pre-refactor `next_batch_core` loop (issue #276/#283)" paragraph) on `advance` itself, extended with the sentence below: + +```rust + /// Ensure `state.current` is a window with rows left, starting the producer, + /// recycling a spent window, and receiving the next one as needed. + /// + /// `Ok(true)` leaves `state.current` set and `state.current.next_row` UNTOUCHED -- + /// the caller decides whether to consume rows (`advance`) or merely read the window + /// (`with_current_window`). `Ok(false)` is `NextSlice::Done` (`state.done` set), and + /// `Err(_)` is `NextSlice::Failed(_)` (`state.done` set); this never yields a row + /// slice. + fn ensure_current_window(&self, state: &mut EngineState) -> anyhow::Result { + if state.done { + return Ok(false); + } + if let Err(e) = self.ensure_started(state) { + state.done = true; + return Err(e); + } + + loop { + if let Some(cur) = state.current.as_ref() { + if cur.next_row < cur.n_batch_rows { + return Ok(true); + } + } + + // Current window is spent (or absent): recycle it, then fetch the next. + if let Some(spent) = state.current.take() { + if let Some(tx) = state.tx_free.as_ref() { + // Always recycle (Err only if the producer already exited) so the + // producer can finish rather than block on rx_free.recv(). + let _ = tx.send(spent.filled); + } + } + + let recv = state + .rx_filled + .as_ref() + .expect("rx_filled set by ensure_started") + .recv(); + match recv { + Ok(slot) => { + let job_idx = state.next_job_idx; + state.next_job_idx += 1; + let n_batch_rows = self.backend.n_batch_rows(job_idx, &slot); + state.current = Some(CurrentWindow { + filled: slot, + job_idx, + next_row: 0, + n_batch_rows, + }); + // Loop back to check the newly received window. + } + Err(_) => { + // Channel closed => producer finished. JOIN FIRST, classify AFTER -- + // never return with the producer live and unjoined. + state.done = true; + if let Some(h) = state.producer.take() { + return match h.join() { + Err(_) => Err(anyhow::anyhow!( + "streaming producer thread panicked" + )), + Ok(Err(e)) => Err(e), + Ok(Ok(())) => Ok(false), + }; + } + return Ok(false); + } + } + } + } + + /// Run `f` against the job index and filled slot of the CURRENT window, ensuring one + /// exists (issue #400). Consumes nothing: a following `next_batch_core` still starts + /// at `state.current.next_row`. + /// + /// `None` = plan exhausted; `Some(Err(_))` = producer error/panic, joined and + /// classified exactly as `next_batch_core` does. `f` runs while the state lock is + /// held -- the slot it reads must not be recycled under it -- so it must not re-enter + /// the engine. + pub(crate) fn with_current_window( + &self, + f: impl FnOnce(usize, &B::Slot) -> anyhow::Result, + ) -> Option> { + let mut state = self.state.lock().unwrap_or_else(|e| e.into_inner()); + match self.ensure_current_window(&mut state) { + Ok(true) => { + let cur = state + .current + .as_ref() + .expect("Ok(true) implies a current window"); + Some(f(cur.job_idx, &cur.filled)) + } + Ok(false) => None, + Err(e) => Some(Err(e)), + } + } + + /// Advance the shared iteration state to the next generatable row slice, or to + /// exhaustion/error. Byte-identical control flow to the pre-refactor `next_batch_core` + /// loop (issue #276/#283): the only change is that the terminal action -- generating + /// bytes from `[row_lo, row_hi)` -- is left to the caller instead of being inlined here, + /// so both the haplotype and variants-output paths can reuse this same cursor walk. + /// + /// The cursor walk itself now lives in `ensure_current_window`, split out so + /// `with_current_window` can read the current window without consuming its rows + /// (issue #400). + fn advance(&self, state: &mut EngineState) -> NextSlice { + match self.ensure_current_window(state) { + Ok(true) => {} + Ok(false) => return NextSlice::Done, + Err(e) => return NextSlice::Failed(e), + } + let cur = state + .current + .as_mut() + .expect("ensure_current_window guarantees a current window"); + let row_lo = cur.next_row; + let row_hi = (row_lo + self.batch_size).min(cur.n_batch_rows); + cur.next_row = row_hi; + NextSlice::Ready { + job_idx: cur.job_idx, + row_lo, + row_hi, + } + } +``` + +Do **not** change `NextSlice`, `EngineState`, `next_batch_core`, `next_batch_variants_core`, or any other `advance` caller: they all keep working because `advance`'s signature and observable behaviour are unchanged. + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `pixi run -e dev cargo test --release with_current_window` +Expected: `test tests::with_current_window_exposes_the_current_window_without_consuming_rows ... ok`, `1 passed`. + +Then run the whole Rust suite for the crate to prove the `advance` refactor is behaviour-preserving: + +Run: `pixi run -e dev cargo test --release` +Expected: all tests pass (`0 failed`) — in particular the two `next_batch`/`advance`-driven tests in `stream_core.rs` and every `#[cfg(test)]` test in `src/record_stream/*`. + +- [ ] **Step 5: Lint and commit** + +Run: `pixi run -e dev cargo clippy --all-targets 2>&1 | tail -n 30` +Expected: no new warning mentioning `stream_core.rs` (the tree may already have unrelated warnings). + +```bash +cat > /tmp/400-task1-commit.txt <<'EOF' +refactor(streaming): split ensure_current_window out of advance (#400) + +with_current_window needs the current window's slot WITHOUT consuming a row, +which advance's fused check-and-advance cannot express. Extract the ensure +half and reimplement advance on top of it; control flow is unchanged, so every +existing advance caller (next_batch_core, next_batch_variants_core, ...) keeps +its exact behaviour. Rust unit test pins the peek semantics on a stub backend. +EOF +git add src/ffi/stream_core.rs +git commit -F /tmp/400-task1-commit.txt +``` + +--- + +## Task 2: `RecordBackend::realign_inputs` + the `current_window_realign_inputs` pymethod + +**Files:** +- Modify: `src/record_stream/engine.rs` (add an inherent method to `impl RecordBackend`, add a pymethod to `#[pymethods] impl RecordStreamEngine`) +- Test: `tests/dataset/test_streaming_tracks_record.py` (new test) + +**Interfaces:** +- Consumes: `StreamEngineCore::with_current_window` (Task 1). +- Produces: + - `RecordBackend::realign_inputs(&self, slot: &DecodedWindow) -> (Vec, Vec, Vec, Vec)` — clones `slot.v_starts`, `slot.ilens`, `slot.geno_v_idxs`, `slot.geno_offsets`. + - `RecordStreamEngine.current_window_realign_inputs(contig_idx: int, region_starts: Sequence[int], region_ends: Sequence[int], s_lo: int, s_hi: int) -> tuple[NDArray[np.int32], NDArray[np.int32], NDArray[np.int32], NDArray[np.int64]] | None` — the current window's arrays, or `None` when exhausted; `ValueError` if the requested job is not the engine's current job; `RuntimeError` if the producer failed/panicked. + +- [ ] **Step 1: Write the failing test** + +Append to `tests/dataset/test_streaming_tracks_record.py`, after `test_record_window_csr_replicates_across_regions` (before the `# --- Issue #375 Track B, Task 8` comment): + +```python +@pytest.mark.parametrize("backend", BACKENDS) +def test_current_window_realign_inputs_reads_the_producers_window( + streaming_record_tracks_fixture, backend +): + """Issue #400: the window the producer has ALREADY filled must be readable from + the drive's own engine -- that is what removes the second decode the track side + used to do. Checked against the independent `window_realign_inputs` decode of the + same window, which runs in the caller's thread (`debug_fill`) and is now + test-only (its `_mixed_engine()` production caller was deleted). + + Also pins the identity guard: a request that does not describe the engine's + current window must raise rather than silently pair one window's tracks with + another's variants. + """ + f = streaming_record_tracks_fixture(backend) + sds = gvl.StreamingDataset( + f.bed, + reference=f.reference_path, + variants=f.variants_path, + tracks=[f.table, f.bigwigs], + ).with_seqs("haplotypes") + b = sds._backend + n_s = sds.n_samples + r_idx = np.arange(len(sds._regions), dtype=np.intp) + contig_idx = int(b._regions[r_idx[0], 0]) + t_starts = np.ascontiguousarray(b._regions[r_idx, 1], np.uint32) + t_ends = np.ascontiguousarray(b._regions[r_idx, 2], np.uint32) + + engine = b.build_engine([(contig_idx, t_starts, t_ends, 0, n_s)], 4, -1) + + # A window that is NOT the engine's current one must fail loudly. + with pytest.raises(ValueError, match="does not match"): + engine.current_window_realign_inputs( + contig_idx, t_starts.tolist(), (t_ends + 1).tolist(), 0, n_s + ) + + got = engine.current_window_realign_inputs( + contig_idx, t_starts.tolist(), t_ends.tolist(), 0, n_s + ) + assert got is not None, "the first window must be available after the peek" + + # Independent source of truth: a plan-less engine decodes this exact window in the + # caller's thread, with no producer and no slot involved. + oracle = b.build_engine([], 1, 1) + expected = oracle.window_realign_inputs( + contig_idx, t_starts.tolist(), t_ends.tolist(), 0, n_s + ) + for name, a, e in zip( + ("v_starts", "ilens", "geno_v_idxs", "geno_offsets"), got, expected + ): + e = np.ascontiguousarray(e) + assert a.dtype == e.dtype, f"{name} dtype drifted" + np.testing.assert_array_equal( + a, e, err_msg=f"{name} drifted from the window's own decode" + ) +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: + +```bash +pixi run -e dev maturin develop --release && \ +pixi run -e dev pytest tests/dataset/test_streaming_tracks_record.py::test_current_window_realign_inputs_reads_the_producers_window -q +``` + +Expected: FAIL — `AttributeError: 'RecordStreamEngine' object has no attribute 'current_window_realign_inputs'` (maturin here only proves the build is current; nothing new is implemented yet). + +- [ ] **Step 3: Implement `RecordBackend::realign_inputs`** + +In `src/record_stream/engine.rs`, insert immediately after the `debug_fill` method (which ends with `self.filler.fill(job, c, &mut slot)?;` ... `Ok(slot)`) inside `impl RecordBackend`: + +```rust + /// Clone the filled window's realign inputs out of the producer's slot: the + /// window-local static table (`v_starts`/`ilens`), the per-hap CSR values + /// (`geno_v_idxs`) and its offsets (`geno_offsets`). Cloned, not moved: the slot stays + /// owned by the engine state so `generate` can still reconstruct this window's + /// batches (issue #400). Returns owned `Vec`s so the caller can convert to numpy + /// after releasing the engine lock. + fn realign_inputs(&self, slot: &DecodedWindow) -> (Vec, Vec, Vec, Vec) { + ( + slot.v_starts.clone(), + slot.ilens.clone(), + slot.geno_v_idxs.clone(), + slot.geno_offsets.clone(), + ) + } +``` + +- [ ] **Step 4: Implement the `current_window_realign_inputs` pymethod** + +In `src/record_stream/engine.rs`, insert immediately after the `window_realign_inputs` method (i.e. just before the closing `}` of the `#[pymethods] impl RecordStreamEngine` block): + +```rust + /// Read the CURRENT window's realign inputs from the producer's already-filled slot + /// (issue #400). + /// + /// Returns the same 4-tuple `window_realign_inputs` returns -- `(v_starts, ilens, + /// geno_v_idxs, geno_offsets)`, all window-local -- but decodes NOTHING: the window was + /// decoded once by the producer (`fill`) and this reads that slot. `None` means the + /// engine's plan is exhausted. + /// + /// The requested job (`contig_idx`, `region_starts`/`region_ends`, `s_lo`/`s_hi`) must + /// EQUAL the engine's current job: the Python drive derives the request from its own + /// plan, and if the two plans have drifted apart, pairing this window's track query + /// with another window's variants would be silently wrong. `ValueError` on mismatch -- + /// loud, because the alternative is wrong bytes. + /// + /// Consumes no rows: the caller can size its track query and then pull the window's + /// batches as usual. + #[pyo3(signature = (contig_idx, region_starts, region_ends, s_lo, s_hi))] + #[allow(clippy::too_many_arguments)] + fn current_window_realign_inputs<'py>( + &self, + py: Python<'py>, + contig_idx: usize, + region_starts: Vec, + region_ends: Vec, + s_lo: usize, + s_hi: usize, + ) -> PyResult>, + Bound<'py, PyArray1>, + Bound<'py, PyArray1>, + Bound<'py, PyArray1>, + )>> { + if region_starts.len() != region_ends.len() { + return Err(PyValueError::new_err( + "current_window_realign_inputs: region_starts and region_ends must have the same length", + )); + } + let regions: Vec<(u32, u32)> = region_starts.into_iter().zip(region_ends).collect(); + let out = py.detach(|| { + self.core.with_current_window(|job_idx, slot| { + Ok((job_idx, self.core.backend().realign_inputs(slot))) + }) + }); + let Some(out) = out else { + return Ok(None); + }; + let (job_idx, (v_starts, ilens, geno_v_idxs, geno_offsets)) = + out.map_err(|e| PyRuntimeError::new_err(e.to_string()))?; + + let backend = self.core.backend(); + let job = &backend.jobs[job_idx]; + if job.contig_idx != contig_idx + || job.regions != regions + || job.s_lo != s_lo + || job.s_hi != s_hi + { + return Err(PyValueError::new_err(format!( + "current_window_realign_inputs: the engine's current window is job {job_idx} \ + (contig {}, regions {:?}, samples {}..{}), which does not match the requested \ + contig {contig_idx}, regions {regions:?}, samples {s_lo}..{s_hi}; the drive and \ + the engine's plan have drifted apart", + job.contig_idx, job.regions, job.s_lo, job.s_hi + ))); + } + Ok(Some(( + Array1::from_vec(v_starts).into_pyarray(py), + Array1::from_vec(ilens).into_pyarray(py), + Array1::from_vec(geno_v_idxs).into_pyarray(py), + Array1::from_vec(geno_offsets).into_pyarray(py), + ))) + } +``` + +`PyValueError`, `PyRuntimeError`, `Array1`, `IntoPyArray`, `PyArray1` and `RecordJob`'s fields are already imported/visible in this file (`use pyo3::exceptions::{PyRuntimeError, PyValueError};`, line 33). + +- [ ] **Step 5: Rebuild and run the test** + +Run: + +```bash +pixi run -e dev maturin develop --release && \ +pixi run -e dev pytest tests/dataset/test_streaming_tracks_record.py::test_current_window_realign_inputs_reads_the_producers_window -q +``` + +Expected: PASS, both `vcf` and `pgen` parametrizations (`2 passed`). + +- [ ] **Step 6: Commit** + +```bash +cat > /tmp/400-task2-commit.txt <<'EOF' +feat(streaming): expose the current window's realign inputs (#400) + +RecordStreamEngine::current_window_realign_inputs reads the producer's +already-decoded slot via the new with_current_window core primitive, so the +Python drive can size a window's track query without a second decode. The +requested job is validated against the engine's current job: a drifted plan +raises ValueError instead of pairing one window's tracks with another's +variants. +EOF +git add src/record_stream/engine.rs tests/dataset/test_streaming_tracks_record.py +git commit -F /tmp/400-task2-commit.txt +``` + +--- + +## Task 3: Fold the Python record mixed path onto the drive's engine + +**Files:** +- Modify: `python/genvarloader/_dataset/_streaming.py` +- Test: `tests/dataset/test_streaming_tracks_record.py` + +**Interfaces:** +- Consumes: `RecordStreamEngine.current_window_realign_inputs` (Task 2). +- Produces: `_MixedTracksBackend.mixed_realign_window(..., engine: object | None = None)` — the drive's engine for the record backends, ignored by SVAR1/SVAR2. `_mixed_track_window(..., engine: object | None = None)` forwards it. `_record_mixed_realign_window(..., engine: object | None = None)` requires it. + +- [ ] **Step 1: Write the failing gate test** + +Append to `tests/dataset/test_streaming_tracks_record.py` (after the test from Task 2): + +```python +@pytest.mark.parametrize("backend", BACKENDS) +def test_mixed_record_realign_decodes_each_window_once( + streaming_record_tracks_fixture, backend +): + """Issue #400 gate: with `realign_tracks=True` the mixed record path must NOT + decode the window a second time. The producer already decodes every window + (`fill`); before this change the track side ALSO decoded it synchronously via a + private plan-less engine (`_mixed_engine()`), roughly doubling every window's + decode. + + Measured with the deterministic decode counters, never wall-clock: this tree + shares a noisy CI node. `transpose_word_reads` counts the genotype transpose + both `VcfWindowFiller` and `PgenWindowFiller` run (`fill_decoded_window`), and + `pgen_variants_decoded` additionally counts PGEN's decoded variants. Both are + process-wide atomics, reset immediately before each sweep and read immediately + after it is fully drained (so no decode is still in flight). + """ + from genvarloader.genvarloader import ( + pgen_variants_decoded, + pgen_variants_decoded_reset, + transpose_word_reads, + transpose_word_reads_reset, + ) + + f = streaming_record_tracks_fixture(backend) + + def sweep(realign: bool) -> tuple[int, int]: + sds = ( + gvl.StreamingDataset( + f.bed, + reference=f.reference_path, + variants=f.variants_path, + tracks=[f.table, f.bigwigs], + ) + .with_seqs("haplotypes") + .with_settings(realign_tracks=realign) + ) + transpose_word_reads_reset() + pgen_variants_decoded_reset() + for _ in sds.to_iter(batch_size=4): + pass + return transpose_word_reads(), pgen_variants_decoded() + + words_without, decoded_without = sweep(False) + words_with, decoded_with = sweep(True) + + assert words_without > 0, "counter is not wired (no window was decoded)" + assert words_with == words_without, ( + f"realign_tracks=True made {words_with} transposed word reads vs " + f"{words_without} with realign_tracks=False; the window is being decoded " + "twice again (issue #400 regression)" + ) + if backend == "pgen": + assert decoded_with == decoded_without, ( + f"realign_tracks=True decoded {decoded_with} PGEN variants vs " + f"{decoded_without} with realign_tracks=False (issue #400 regression)" + ) +``` + +- [ ] **Step 2: Run it to verify it fails, and record the before numbers** + +Run: + +```bash +pixi run -e dev pytest "tests/dataset/test_streaming_tracks_record.py::test_mixed_record_realign_decodes_each_window_once" -q +``` + +Expected: FAIL for BOTH backends with `words_with == 2 * words_without` (the second decode from `_mixed_engine`). **Copy both backends' `words_without`/`words_with` and `decoded_without`/`decoded_with` numbers into your task notes** — Task 4 writes them into the roadmap. + +- [ ] **Step 3: Add `engine` to the protocol and all four implementations** + +In `python/genvarloader/_dataset/_streaming.py`, add the parameter to each of these five signatures, as the last parameter, written **unquoted** exactly like this (the module has `from __future__ import annotations`, and `tests/dataset/test_streaming_tracks.py:809-831` compares parameter names AND annotations across backends with `eval_str=True`, so the text must match everywhere; `object | None` resolves without needing a module-level import): + +```python + engine: object | None = None, +``` + +1. `_MixedTracksBackend.mixed_realign_window` (protocol, ~line 488) — add it after `row_ends`, before the return annotation, and add to its docstring: `The drive's engine, for backends whose window decode the engine owns (the record backends); SVAR1/SVAR2 ignore it.` +2. `_Svar1Backend.mixed_realign_window` (~line 4020) — add the same line plus a docstring sentence: `engine` is accepted for protocol uniformity and ignored: SVAR1's state comes from its on-disk CSR, not from a stream engine. +3. `_Svar2Backend.mixed_realign_window` (~line 4573) — same as SVAR1's note. +4. `_VcfBackend.mixed_realign_window` (~line 4834) — add the line, and change its delegation to: + +```python + return _record_mixed_realign_window( + self, r_idx, s_idx, t_starts, t_ends, row_starts, row_ends, engine + ) +``` + +5. `_PgenBackend.mixed_realign_window` (~line 5090) — exactly the same delegation change. + +- [ ] **Step 4: Make `_record_mixed_realign_window` read the engine's current window** + +In `_record_mixed_realign_window` (~line 735): + +1. Add the parameter as the last one: `engine: object | None = None,`. +2. Add to the docstring, right after the `Shared by \`_VcfBackend\` and \`_PgenBackend\` ...` paragraph: + +``` + The variant tables and CSR come from the DRIVE's engine, which decoded + this window in its producer thread: `current_window_realign_inputs` reads + that already-filled slot and decodes nothing (issue #400 -- this used to + decode the window a second time through a private plan-less engine). It + also validates that the engine's current job is exactly this window, so a + drive/plan drift raises `ValueError` instead of silently pairing one + window's tracks with another's variants. +``` + +3. Replace the `engine = backend._mixed_engine()` / `engine.window_realign_inputs(...)` block with: + +```python + if engine is None: + raise ValueError( + "_record_mixed_realign_window requires the drive's RecordStreamEngine: " + "the window's variant decode is read from the engine's current slot " + "(issue #400). Pass engine=." + ) + window_inputs = engine.current_window_realign_inputs( + contig_idx, + np.ascontiguousarray(t_starts, np.uint32).tolist(), + np.ascontiguousarray(t_ends, np.uint32).tolist(), + int(s_idx[0]), + int(s_idx[-1]) + 1, + ) + if window_inputs is None: + raise RuntimeError("streaming engine exhausted before the plan did") + v_starts, ilens, geno_v_idxs, csr = window_inputs +``` + +4. In the jitter paragraph of the same docstring, change "The engine is queried with `t_starts`/`t_ends`" to "The engine call is validated against `t_starts`/`t_ends`" (the rest of that paragraph stays true). + +- [ ] **Step 5: Forward the engine from the drive** + +1. `_mixed_track_window` (~line 1677): add `engine: object | None = None,` as the last parameter (after `region_offsets`), document it in the Args block (`engine: The drive's engine, forwarded to ``backend.mixed_realign_window``; required by the record backends.`), and append `engine` as the final POSITIONAL argument of its existing `backend.mixed_realign_window(...)` call. Do not reorder or rename the arguments already there (it passes `r_idx`, the sample/track bounds and the row bounds); the composed call must end up shaped like: + +```python + window = backend.mixed_realign_window( + r_idx, + s_idx_w, + t_starts, + t_ends, + row_starts, + row_ends, + engine, + ) +``` + +Keep whatever local variable names that call site already uses; only the trailing `engine` is new. +2. The engine drive's call site (~line 2277, inside `_iter_batches`) becomes: + +```python + if tb is not None: + track_w = self._mixed_track_window( + tb, + backend, + r_idx, + np.arange(s_lo, s_hi, dtype=np.intp), + region_offsets, + engine, + ) +``` + +(`engine` is the `build_engine` result from ~line 2221. The SVAR2 sync call site ~line 2657 stays as-is: it passes no engine, so SVAR2 gets `None`, which it ignores.) + +- [ ] **Step 6: Delete the dead private engines** + +1. Remove `self._mixed_engine_obj = None` from `_VcfBackend.__init__` (~line 4747) and from `_PgenBackend.__init__` (~line 5062). +2. Remove the entire `_mixed_engine` method from `_VcfBackend` (~4816-4832) and from `_PgenBackend` (~5072-5088). +3. In both `build_engine` docstrings (~4920, ~5162), the comment that reads + +``` + # -- including `_mixed_engine()`'s `build_engine([], 1, 1)`, where + # `touched_contigs` is empty and this now allocates nothing at all. +``` + +becomes + +``` + # -- including the plan-less engine the CSR test builds as its + # independent oracle (`build_engine([], 1, 1)`), where + # `touched_contigs` is empty and this allocates nothing at all. +``` + +- [ ] **Step 7: Add the one guard-comment sentence** + +At the `jitter > 0` + `realign_tracks` guard in `_iter_batches` (~line 2054-2066), add one sentence to the existing explanation: `Record backends no longer decode in the consumer thread at all (issue #400): their decode comes from the drive's engine, whose job bounds are the jitter-translated ones. This guard is kept for SVAR1's raw-bounds re-read (#383).` Do not change the guard's behaviour. + +- [ ] **Step 8: Port `test_record_window_csr_replicates_across_regions`** + +The mixed call now needs an engine, and `b._mixed_engine()` no longer exists. In `tests/dataset/test_streaming_tracks_record.py`: + +1. Move `contig_idx = int(b._regions[r_idx[0], 0])` up so it is computed right after `row_ends`. +2. Build the plan engine and pass it: + +```python + engine = b.build_engine( + [(contig_idx, t_starts.astype(np.uint32), t_ends.astype(np.uint32), 0, n_s)], + 4, + -1, + ) + state, t_ends_ext = b.mixed_realign_window( + r_idx, s_idx, t_starts, t_ends, row_starts, row_ends, engine + ) +``` + +3. Replace `engine = b._mixed_engine()` in the independent-oracle block with a plan-less engine, and rename the local so the two engines cannot be confused: + +```python + # Independent source of truth: a plan-less engine decodes this exact window in the + # caller's thread (`debug_fill`), with no producer and no slot involved. + oracle = b.build_engine([], 1, 1) + _, _, _, csr = oracle.window_realign_inputs( + contig_idx, + np.ascontiguousarray(t_starts, np.uint32).tolist(), + np.ascontiguousarray(t_ends, np.uint32).tolist(), + int(s_idx[0]), + int(s_idx[-1]) + 1, + ) +``` + +- [ ] **Step 9: Update the two stale docstrings in that test file** + +1. Module docstring (lines 1-6) becomes: + +```python +"""Rust seam for VCF/PGEN mixed variants+tracks (issue #375, Track B). + +`current_window_realign_inputs` is the production seam: the drive reads the window +the producer has already filled, so a mixed record window is decoded once (issue +#400). `window_realign_inputs` decodes a window it is handed, in the caller's thread; +it is now TEST-ONLY and stays as the independent oracle these tests check the folded +path against. +""" +``` + +2. `test_window_realign_inputs_matches_before_and_during_producer` (~line 112): append to its docstring: `Its production caller (`_mixed_engine()`) was deleted in issue #400, so this test is now the only reason the pymethod stays: it still pins the concurrent-call safety of the shared filler.` + +- [ ] **Step 10: Rebuild and run the file, then the gate** + +Run: + +```bash +pixi run -e dev maturin develop --release && \ +pixi run -e dev pytest tests/dataset/test_streaming_tracks_record.py -q +``` + +Expected: all tests PASS, including `test_mixed_record_realign_decodes_each_window_once` (both backends) and the ported CSR test. + +Then re-run the gate alone to confirm and to read the after numbers: + +```bash +pixi run -e dev pytest tests/dataset/test_streaming_tracks_record.py::test_mixed_record_realign_decodes_each_window_once -q +``` + +Expected: PASS. Record the after numbers for Task 4: `words_with == words_without` for VCF and PGEN, `decoded_with == decoded_without` for PGEN. + +- [ ] **Step 11: Commit** + +```bash +cat > /tmp/400-task3-commit.txt <<'EOF' +perf(streaming): fold the mixed record decode into the producer (#400) + +The VCF/PGEN mixed variants+tracks path decoded every window twice: the +producer filled it for the haplotypes, then the track side re-decoded it +synchronously through a private plan-less engine. The drive's engine is now +the only decoder: _mixed_track_window forwards it and +_record_mixed_realign_window reads the producer's current slot via +current_window_realign_inputs, so realign_tracks=True costs the same decode +as realign_tracks=False (gated on transpose_word_reads and +pgen_variants_decoded, not wall-clock). The private engines are deleted. +EOF +git add python/genvarloader/_dataset/_streaming.py tests/dataset/test_streaming_tracks_record.py +git commit -F /tmp/400-task3-commit.txt +``` + +--- + +## Task 4: Retire the stale docs, tick the roadmap, verify the whole tree + +**Files:** +- Modify: `src/ffi/stream_core.rs` (`backend()` doc), `src/record_stream/engine.rs` (`window_realign_inputs` doc), `src/record_stream/pgen.rs` (`reader_lock` doc), `docs/roadmaps/streaming-dataset.md` +- Test: whole tree + +**Interfaces:** +- Consumes: everything above; the measured counter numbers from Task 3; the PR number you create in Step 5. +- Produces: docs that describe the post-#400 arrangement truthfully. + +- [ ] **Step 1: Fix the three Rust doc comments** + +1. `src/ffi/stream_core.rs`, `backend()`'s doc (~lines 228-261): delete the M1 paragraph ("both callers ... `RecordStreamEngine::window_realign_inputs` (issue #375 Track B) -- a genuine production path ... folding the mixed path onto the drive engine (removing the double decode) is the obvious next step and would make this the live case (final review, M1)"). Replace it with: `- `RecordStreamEngine::window_realign_inputs` and `debug_decode_window` (issues #375/#391) -- now TEST-ONLY: it decodes a window in the caller's thread for parity oracles. Issue #400 removed the production consumer (a private plan-less engine), so the concurrent-with-a-live-producer case is exercised only by `test_window_realign_inputs_matches_before_and_during_producer`. The safety property stands: both callers release the GIL, so both remain safe against a live producer.` Keep the remaining paragraphs about `py.detach` and the two caller conditions. +2. `src/record_stream/engine.rs`, `window_realign_inputs`'s doc (~1049-1083): replace the "Python needs these BEFORE the window's first batch ... Net cost: roughly 2x decode on the mixed path (one per engine); folding this into a single shared engine is a tracked follow-up, not a v1 requirement." paragraphs with a short TEST-ONLY note: the production consumer was deleted in #400 (the drive now reads the producer's slot via `current_window_realign_inputs`), and this stays as the independent CSR oracle used by `test_record_window_csr_replicates_across_regions` and `test_window_realign_inputs_matches_before_and_during_producer`. Keep the bullet list describing the returned arrays' layout. +3. `src/record_stream/pgen.rs`, `PgenWindowFiller::reader_lock`'s doc (~430-434): `fill` is still called concurrently (test-only decodes vs a live producer), so the lock stays, but drop any claim that a production consumer shares the reader: add `Since issue #400 the mixed path decodes only in the producer, so this lock exists for the test-only decode accessors and for any future shared-engine caller.` Keep the LOCK ORDERING line verbatim. + +- [ ] **Step 2: Full Python + Rust verification** + +```bash +pixi run -e dev maturin develop --release +pixi run -e dev pytest tests/dataset tests/unit -q +pixi run -e dev cargo test --release +pixi run -e dev ruff check python/ tests/ && pixi run -e dev ruff format --check python/ tests/ +pixi run -e dev typecheck +pixi run -e dev python scripts/docstring_style.py --check python/genvarloader +``` + +Expected: all green. `typecheck` MAY report `missing-attribute`/`bad-argument-type` WARNINGS (relaxed to warn in `pyproject.toml`); the command must exit 0. If any test outside this plan's scope fails, **stop and report** — do not "fix" parity tests. + +- [ ] **Step 3: Tick the roadmap** + +In `docs/roadmaps/streaming-dataset.md`, two follow-up mentions of #400 must become fixed entries (both are inside the long table rows for the #279 intervals plan at ~line 91 and the `2026-09-11-streaming-mixed-tracks-non-svar1.md` plan at ~line 92). + +In line 91, replace + +```markdown +[#400](https://github.com/mcvickerlab/GenVarLoader/issues/400) (fold the mixed window decode into the producer to remove the 2x decode). +``` + +with + +```markdown +[#400](https://github.com/mcvickerlab/GenVarLoader/issues/400) (fold the mixed window decode into the producer to remove the 2x decode) ✅ **fixed**, PR [#](https://github.com/mcvickerlab/GenVarLoader/pull/) — the drive's `RecordStreamEngine` is now the only decoder: `RecordStreamEngine.current_window_realign_inputs` reads the producer's already-filled slot (validating the requested job against the engine's current job), `_mixed_track_window` forwards `engine=`, and the private plan-less `_mixed_engine()` is deleted. Deterministic-counter gate (not wall-clock): `transpose_word_reads` on the record mixed fixture is now identical with `realign_tracks` on and off ( word reads, both) and PGEN's `pgen_variants_decoded` likewise ( variants, both) — the realign run was ~2x before. +``` + +In line 92, replace + +```markdown +[#400](https://github.com/mcvickerlab/GenVarLoader/issues/400) (fold the mixed window decode into the producer). +``` + +with the same ✅ sentence from line 91 (identical replacement text, ``, `` and `` filled in). + +`` comes from Step 5; `` from Task 3's `words_without`/`words_with` (equal after the fold); `` from Task 3's PGEN `decoded_without`/`decoded_with` (equal after the fold). Do not write placeholders into the file. + +- [ ] **Step 4: Commit the docs** + +```bash +cat > /tmp/400-task4-commit.txt <<'EOF' +docs(streaming): record the #400 decode fold (#400) + +window_realign_inputs and debug_decode_window are now test-only seams: the +record mixed path reads the producer's own slot. Update the three Rust doc +comments that still describe the double decode, and tick the roadmap's #400 +follow-ups with the measured decode-counter numbers. +EOF +git add src/ffi/stream_core.rs src/record_stream/engine.rs src/record_stream/pgen.rs docs/roadmaps/streaming-dataset.md +git commit -F /tmp/400-task4-commit.txt +``` + +- [ ] **Step 5: Push and open the PR** + +Write the PR body first: + +```bash +cat > /tmp/400-pr-body.md <<'EOF' +Closes #400. + +The VCF/PGEN mixed variants+tracks path decoded every window twice: the Rust +producer filled it for the haplotypes, then the track side re-decoded it +synchronously through a private plan-less `_mixed_engine()`. The drive's +`RecordStreamEngine` is now the only decoder. + +- `StreamEngineCore::with_current_window` + `ensure_current_window` (split out of + `advance`): read the current window's slot without consuming a row. +- `RecordStreamEngine.current_window_realign_inputs`: returns that window's + `(v_starts, ilens, geno_v_idxs, geno_offsets)`, validating the requested job + against the engine's current job so a drifted plan raises `ValueError` instead + of pairing one window's tracks with another's variants. +- Python: `engine` threaded through `_mixed_track_window` and + `_RecordStreamEngine`-backed `mixed_realign_window`; the private plan-less + engines are deleted. SVAR1/SVAR2 accept and ignore the parameter. + +Gate is deterministic, not wall-clock: `transpose_word_reads` (and PGEN's +`pgen_variants_decoded`) are now identical with `realign_tracks` on and off, +where the realign run was ~2x before. Every existing mixed parity suite is +unchanged and green. No public API change; `window_realign_inputs` stays as the +test-only oracle. +EOF +git push -u origin feat/400-mixed-record-decode-fold +gh pr create --base streaming --title 'perf(streaming): fold the mixed record decode into the producer (#400)' --body-file /tmp/400-pr-body.md +``` + +Then **re-run Step 3 with the real PR number**, commit that as `docs(streaming): link the #400 PR` (same `git commit -F` discipline), and push again. Add the PR to the **StreamingDataset** GitHub Project (`gh pr edit --add-project StreamingDataset`; if the token lacks `read:project`, ask the user to run `gh auth refresh -s read:project`). + +--- + +## Final Verification Checklist + +- [ ] `pixi run -e dev cargo test --release` — all Rust tests pass, including `with_current_window_exposes_the_current_window_without_consuming_rows`. +- [ ] `pixi run -e dev pytest tests/dataset tests/unit -q` — all pass, and `git diff --stat tests/dataset/test_streaming_tracks.py tests/dataset/test_streaming_tracks_svar2.py tests/dataset/test_streaming_scale.py` shows **no changes** to those files. +- [ ] The gate: `transpose_word_reads` equal with `realign_tracks` on/off for both backends; `pgen_variants_decoded` equal for PGEN; both non-zero without the fold (numbers recorded in the roadmap). +- [ ] `git grep -n '_mixed_engine'` returns nothing. +- [ ] `pixi run -e dev ruff check python/ tests/`, `pixi run -e dev typecheck`, `pixi run -e dev python scripts/docstring_style.py --check python/genvarloader` all exit 0. +- [ ] No `__all__`/`docs/source/api.md`/`skills/` changes: `git diff --stat` lists only the five files in File Structure plus the roadmap. From 9cc91eeff07fd4d1cba85e9b3904e738788f7250 Mon Sep 17 00:00:00 2001 From: d-laub Date: Mon, 21 Sep 2026 09:40:14 -0700 Subject: [PATCH 03/10] refactor(streaming): split ensure_current_window out of advance (#400) with_current_window needs the current window's slot WITHOUT consuming a row, which advance's fused check-and-advance cannot express. Extract the ensure half and reimplement advance on top of it; control flow is unchanged, so every existing advance caller (next_batch_core, next_batch_variants_core, ...) keeps its exact behaviour. Rust unit test pins the peek semantics on a stub backend. --- src/ffi/stream_core.rs | 229 +++++++++++++++++++++++++++++++++++------ 1 file changed, 195 insertions(+), 34 deletions(-) diff --git a/src/ffi/stream_core.rs b/src/ffi/stream_core.rs index 902672a7..cd79a9f5 100644 --- a/src/ffi/stream_core.rs +++ b/src/ffi/stream_core.rs @@ -316,43 +316,31 @@ impl StreamEngineCore { Ok(()) } - /// Advance the shared iteration state to the next generatable row slice, or to - /// exhaustion/error. Byte-identical control flow to the pre-refactor `next_batch_core` - /// loop (issue #276/#283): the only change is that the terminal action — generating - /// bytes from `[row_lo, row_hi)` — is left to the caller instead of being inlined here, - /// so both the haplotype and variants-output paths can reuse this same cursor walk. - fn advance(&self, state: &mut EngineState) -> NextSlice { + /// Ensure `state.current` is a window with rows left, starting the producer, + /// recycling a spent window, and receiving the next one as needed. + /// + /// `Ok(true)` leaves `state.current` set and `state.current.next_row` UNTOUCHED -- + /// the caller decides whether to consume rows (`advance`) or merely read the window + /// (`with_current_window`). `Ok(false)` is `NextSlice::Done` (`state.done` set), and + /// `Err(_)` is `NextSlice::Failed(_)` (`state.done` set); this never yields a row + /// slice. + fn ensure_current_window(&self, state: &mut EngineState) -> anyhow::Result { if state.done { - return NextSlice::Done; + return Ok(false); } if let Err(e) = self.ensure_started(state) { state.done = true; - return NextSlice::Failed(e); + return Err(e); } loop { - // 1. If the current window has rows left, advance the cursor and yield the - // next slice. - let has_rows = match state.current.as_ref() { - Some(cur) => cur.next_row < cur.n_batch_rows, - None => false, - }; - if has_rows { - let cur = state - .current - .as_mut() - .expect("has_rows implies a live current window"); - let row_lo = cur.next_row; - let row_hi = (row_lo + self.batch_size).min(cur.n_batch_rows); - cur.next_row = row_hi; - return NextSlice::Ready { - job_idx: cur.job_idx, - row_lo, - row_hi, - }; + if let Some(cur) = state.current.as_ref() { + if cur.next_row < cur.n_batch_rows { + return Ok(true); + } } - // 2. Current window is spent (or absent): recycle it, then fetch the next. + // Current window is spent (or absent): recycle it, then fetch the next. if let Some(spent) = state.current.take() { if let Some(tx) = state.tx_free.as_ref() { // Always recycle (Err only if the producer already exited) so the @@ -377,27 +365,82 @@ impl StreamEngineCore { next_row: 0, n_batch_rows, }); - // Loop back to yield from the newly received window. + // Loop back to check the newly received window. } Err(_) => { - // Channel closed => producer finished. JOIN FIRST, classify AFTER — + // Channel closed => producer finished. JOIN FIRST, classify AFTER -- // never return with the producer live and unjoined. state.done = true; if let Some(h) = state.producer.take() { return match h.join() { - Err(_) => NextSlice::Failed(anyhow::anyhow!( + Err(_) => Err(anyhow::anyhow!( "streaming producer thread panicked" )), - Ok(Err(e)) => NextSlice::Failed(e), - Ok(Ok(())) => NextSlice::Done, + Ok(Err(e)) => Err(e), + Ok(Ok(())) => Ok(false), }; } - return NextSlice::Done; + return Ok(false); } } } } + /// Run `f` against the job index and filled slot of the CURRENT window, ensuring one + /// exists (issue #400). Consumes nothing: a following `next_batch_core` still starts + /// at `state.current.next_row`. + /// + /// `None` = plan exhausted; `Some(Err(_))` = producer error/panic, joined and + /// classified exactly as `next_batch_core` does. `f` runs while the state lock is + /// held -- the slot it reads must not be recycled under it -- so it must not re-enter + /// the engine. + pub(crate) fn with_current_window( + &self, + f: impl FnOnce(usize, &B::Slot) -> anyhow::Result, + ) -> Option> { + let mut state = self.state.lock().unwrap_or_else(|e| e.into_inner()); + match self.ensure_current_window(&mut state) { + Ok(true) => { + let cur = state + .current + .as_ref() + .expect("Ok(true) implies a current window"); + Some(f(cur.job_idx, &cur.filled)) + } + Ok(false) => None, + Err(e) => Some(Err(e)), + } + } + + /// Advance the shared iteration state to the next generatable row slice, or to + /// exhaustion/error. Byte-identical control flow to the pre-refactor `next_batch_core` + /// loop (issue #276/#283): the only change is that the terminal action — generating + /// bytes from `[row_lo, row_hi)` — is left to the caller instead of being inlined here, + /// so both the haplotype and variants-output paths can reuse this same cursor walk. + /// + /// The cursor walk itself now lives in `ensure_current_window`, split out so + /// `with_current_window` can read the current window without consuming its rows + /// (issue #400). + fn advance(&self, state: &mut EngineState) -> NextSlice { + match self.ensure_current_window(state) { + Ok(true) => {} + Ok(false) => return NextSlice::Done, + Err(e) => return NextSlice::Failed(e), + } + let cur = state + .current + .as_mut() + .expect("ensure_current_window guarantees a current window"); + let row_lo = cur.next_row; + let row_hi = (row_lo + self.batch_size).min(cur.n_batch_rows); + cur.next_row = row_hi; + NextSlice::Ready { + job_idx: cur.job_idx, + row_lo, + row_hi, + } + } + /// The consumer, GIL-free. Returns: /// * `Some(Ok((data, offsets)))` — the next batch; /// * `Some(Err(_))` — a producer error/panic (join-then-classified); @@ -576,4 +619,122 @@ mod tests { .expect("EngineState::drop deadlocked: it joined a GIL-needing producer without releasing the GIL (issue #399)"); worker.join().expect("worker thread should not panic"); } + + /// Backend with no filler internals, no GIL and no channels of its own: each filled + /// slot records the job index that produced it (so a test can tell WHICH window the + /// core is holding) and `generate` returns one byte per requested row (so a test can + /// tell which rows the cursor asked for). + struct MarkerBackend { + n_jobs: usize, + rows_per_job: usize, + } + + impl EngineBackend for MarkerBackend { + type Slot = Vec; + + fn n_jobs(&self) -> usize { + self.n_jobs + } + + fn fill(&self, job_idx: usize, slot: &mut Vec) -> anyhow::Result<()> { + slot.clear(); + slot.push(job_idx); + Ok(()) + } + + fn n_batch_rows(&self, _job_idx: usize, _slot: &Vec) -> usize { + self.rows_per_job + } + + fn generate( + &self, + _job_idx: usize, + _slot: &Vec, + row_lo: usize, + row_hi: usize, + ) -> anyhow::Result<( + Array1, + Option>, + Option>, + Array1, + )> { + Ok(( + Array1::from_vec(vec![0u8; row_hi - row_lo]), + None, + None, + Array1::from_vec(vec![0i64, (row_hi - row_lo) as i64]), + )) + } + } + + /// Issue #400: `with_current_window` must hand back the window the producer has + /// already filled WITHOUT consuming any of its rows, and must only move on once that + /// window is spent. This is what lets the Python mixed record path size its track + /// query from the producer's decode instead of decoding the window a second time. + #[test] + fn with_current_window_exposes_the_current_window_without_consuming_rows() { + let core = StreamEngineCore::new( + Arc::new(MarkerBackend { + n_jobs: 3, + rows_per_job: 2, + }), + 1, + ); + + // Starts the producer and receives window 0, consuming nothing. + let first = core + .with_current_window(|job_idx, slot| { + assert_eq!( + slot.as_slice(), + [job_idx], + "slot must carry its own job marker" + ); + Ok(job_idx) + }) + .expect("a current window must be available") + .expect("no producer error"); + assert_eq!(first, 0); + + // Idempotent: window 0 still has all of its rows, so the same window comes back. + let again = core + .with_current_window(|job_idx, _slot| Ok(job_idx)) + .expect("window 0 still has rows") + .expect("no producer error"); + assert_eq!(again, 0, "the peek consumed rows instead of just reading"); + + // The row cursor was untouched: window 0 still yields BOTH of its rows. + let widths: Vec = (0..2) + .map(|_| { + core.next_batch_core() + .expect("row slice") + .expect("no error") + .0 + .len() + }) + .collect(); + assert_eq!(widths, [1, 1], "peeked window lost rows"); + + // Only a SPENT window advances the core's idea of "current". + let second = core + .with_current_window(|job_idx, _slot| Ok(job_idx)) + .expect("window 1 is now current") + .expect("no producer error"); + assert_eq!(second, 1, "the core advanced before the window was spent"); + + // 3 jobs x 2 rows at batch_size 1: 6 batches total, 2 already taken. + let mut remaining = 0usize; + while let Some(batch) = core.next_batch_core() { + batch.expect("no producer error"); + remaining += 1; + } + assert_eq!( + remaining, 4, + "peeking must not change how many rows the plan yields" + ); + + // Fully drained: the peek reports exhaustion rather than hanging or re-yielding. + assert!(core + .with_current_window(|job_idx, _slot| Ok(job_idx)) + .is_none()); + } } From 37b44d864b95dd8183449105c4c8b369e17362ef Mon Sep 17 00:00:00 2001 From: d-laub Date: Mon, 21 Sep 2026 09:47:22 -0700 Subject: [PATCH 04/10] feat(streaming): expose the current window's realign inputs (#400) RecordStreamEngine::current_window_realign_inputs reads the producer's already-decoded slot via the new with_current_window core primitive, so the Python drive can size a window's track query without a second decode. The requested job is validated against the engine's current job: a drifted plan raises ValueError instead of pairing one window's tracks with another's variants. --- src/record_stream/engine.rs | 87 +++++++++++++++++++ tests/dataset/test_streaming_tracks_record.py | 57 ++++++++++++ 2 files changed, 144 insertions(+) diff --git a/src/record_stream/engine.rs b/src/record_stream/engine.rs index 6474b724..f1544196 100644 --- a/src/record_stream/engine.rs +++ b/src/record_stream/engine.rs @@ -142,6 +142,21 @@ impl RecordBackend { Ok(slot) } + /// Clone the filled window's realign inputs out of the producer's slot: the + /// window-local static table (`v_starts`/`ilens`), the per-hap CSR values + /// (`geno_v_idxs`) and its offsets (`geno_offsets`). Cloned, not moved: the slot stays + /// owned by the engine state so `generate` can still reconstruct this window's + /// batches (issue #400). Returns owned `Vec`s so the caller can convert to numpy + /// after releasing the engine lock. + fn realign_inputs(&self, slot: &DecodedWindow) -> (Vec, Vec, Vec, Vec) { + ( + slot.v_starts.clone(), + slot.ilens.clone(), + slot.geno_v_idxs.clone(), + slot.geno_offsets.clone(), + ) + } + /// The per-row CSR walk + region/AF keep shared by `generate_variants` and /// `generate_variant_windows` (Wave B PR-B4, #304) — factored out of what was /// previously `generate_variants`' own inline loop so BOTH output modes select and @@ -1124,6 +1139,78 @@ impl RecordStreamEngine { Array1::from_vec(slot.geno_offsets).into_pyarray(py), )) } + + /// Read the CURRENT window's realign inputs from the producer's already-filled slot + /// (issue #400). + /// + /// Returns the same 4-tuple `window_realign_inputs` returns -- `(v_starts, ilens, + /// geno_v_idxs, geno_offsets)`, all window-local -- but decodes NOTHING: the window was + /// decoded once by the producer (`fill`) and this reads that slot. `None` means the + /// engine's plan is exhausted. + /// + /// The requested job (`contig_idx`, `region_starts`/`region_ends`, `s_lo`/`s_hi`) must + /// EQUAL the engine's current job: the Python drive derives the request from its own + /// plan, and if the two plans have drifted apart, pairing this window's track query + /// with another window's variants would be silently wrong. `ValueError` on mismatch -- + /// loud, because the alternative is wrong bytes. + /// + /// Consumes no rows: the caller can size its track query and then pull the window's + /// batches as usual. + #[pyo3(signature = (contig_idx, region_starts, region_ends, s_lo, s_hi))] + #[allow(clippy::too_many_arguments)] + fn current_window_realign_inputs<'py>( + &self, + py: Python<'py>, + contig_idx: usize, + region_starts: Vec, + region_ends: Vec, + s_lo: usize, + s_hi: usize, + ) -> PyResult>, + Bound<'py, PyArray1>, + Bound<'py, PyArray1>, + Bound<'py, PyArray1>, + )>> { + if region_starts.len() != region_ends.len() { + return Err(PyValueError::new_err( + "current_window_realign_inputs: region_starts and region_ends must have the same length", + )); + } + let regions: Vec<(u32, u32)> = region_starts.into_iter().zip(region_ends).collect(); + let out = py.detach(|| { + self.core.with_current_window(|job_idx, slot| { + Ok((job_idx, self.core.backend().realign_inputs(slot))) + }) + }); + let Some(out) = out else { + return Ok(None); + }; + let (job_idx, (v_starts, ilens, geno_v_idxs, geno_offsets)) = + out.map_err(|e| PyRuntimeError::new_err(e.to_string()))?; + + let backend = self.core.backend(); + let job = &backend.jobs[job_idx]; + if job.contig_idx != contig_idx + || job.regions != regions + || job.s_lo != s_lo + || job.s_hi != s_hi + { + return Err(PyValueError::new_err(format!( + "current_window_realign_inputs: the engine's current window is job {job_idx} \ + (contig {}, regions {:?}, samples {}..{}), which does not match the requested \ + contig {contig_idx}, regions {regions:?}, samples {s_lo}..{s_hi}; the drive and \ + the engine's plan have drifted apart", + job.contig_idx, job.regions, job.s_lo, job.s_hi + ))); + } + Ok(Some(( + Array1::from_vec(v_starts).into_pyarray(py), + Array1::from_vec(ilens).into_pyarray(py), + Array1::from_vec(geno_v_idxs).into_pyarray(py), + Array1::from_vec(geno_offsets).into_pyarray(py), + ))) + } } #[cfg(test)] diff --git a/tests/dataset/test_streaming_tracks_record.py b/tests/dataset/test_streaming_tracks_record.py index 0e4e933d..85d38cc6 100644 --- a/tests/dataset/test_streaming_tracks_record.py +++ b/tests/dataset/test_streaming_tracks_record.py @@ -321,6 +321,63 @@ def test_record_window_csr_replicates_across_regions( ) +@pytest.mark.parametrize("backend", BACKENDS) +def test_current_window_realign_inputs_reads_the_producers_window( + streaming_record_tracks_fixture, backend +): + """Issue #400: the window the producer has ALREADY filled must be readable from + the drive's own engine -- that is what removes the second decode the track side + used to do. Checked against the independent `window_realign_inputs` decode of the + same window, which runs in the caller's thread (`debug_fill`) and is now + test-only (its `_mixed_engine()` production caller was deleted). + + Also pins the identity guard: a request that does not describe the engine's + current window must raise rather than silently pair one window's tracks with + another's variants. + """ + f = streaming_record_tracks_fixture(backend) + sds = gvl.StreamingDataset( + f.bed, + reference=f.reference_path, + variants=f.variants_path, + tracks=[f.table, f.bigwigs], + ).with_seqs("haplotypes") + b = sds._backend + n_s = sds.n_samples + r_idx = np.arange(len(sds._regions), dtype=np.intp) + contig_idx = int(b._regions[r_idx[0], 0]) + t_starts = np.ascontiguousarray(b._regions[r_idx, 1], np.uint32) + t_ends = np.ascontiguousarray(b._regions[r_idx, 2], np.uint32) + + engine = b.build_engine([(contig_idx, t_starts, t_ends, 0, n_s)], 4, -1) + + # A window that is NOT the engine's current one must fail loudly. + with pytest.raises(ValueError, match="does not match"): + engine.current_window_realign_inputs( + contig_idx, t_starts.tolist(), (t_ends + 1).tolist(), 0, n_s + ) + + got = engine.current_window_realign_inputs( + contig_idx, t_starts.tolist(), t_ends.tolist(), 0, n_s + ) + assert got is not None, "the first window must be available after the peek" + + # Independent source of truth: a plan-less engine decodes this exact window in the + # caller's thread, with no producer and no slot involved. + oracle = b.build_engine([], 1, 1) + expected = oracle.window_realign_inputs( + contig_idx, t_starts.tolist(), t_ends.tolist(), 0, n_s + ) + for name, a, e in zip( + ("v_starts", "ilens", "geno_v_idxs", "geno_offsets"), got, expected + ): + e = np.ascontiguousarray(e) + assert a.dtype == e.dtype, f"{name} dtype drifted" + np.testing.assert_array_equal( + a, e, err_msg=f"{name} drifted from the window's own decode" + ) + + # --- Issue #375 Track B, Task 8: VCF/PGEN mixed parity, edge cases, docs --- From 36da60013b450fa3ad3246b026a7f99458f83c1b Mon Sep 17 00:00:00 2001 From: d-laub Date: Mon, 21 Sep 2026 09:51:17 -0700 Subject: [PATCH 05/10] test(streaming): make the #400 seam docstring true at this commit (#400) The new test's docstring claimed window_realign_inputs was already test-only; at this commit _mixed_engine still drives it. Reword to the future tense so the claim lands only once this branch's fold deletes that caller. Also pin the exhausted-plan path: a plan-less engine has no current window and must return None. --- tests/dataset/test_streaming_tracks_record.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/dataset/test_streaming_tracks_record.py b/tests/dataset/test_streaming_tracks_record.py index 85d38cc6..e4a29849 100644 --- a/tests/dataset/test_streaming_tracks_record.py +++ b/tests/dataset/test_streaming_tracks_record.py @@ -328,8 +328,9 @@ def test_current_window_realign_inputs_reads_the_producers_window( """Issue #400: the window the producer has ALREADY filled must be readable from the drive's own engine -- that is what removes the second decode the track side used to do. Checked against the independent `window_realign_inputs` decode of the - same window, which runs in the caller's thread (`debug_fill`) and is now - test-only (its `_mixed_engine()` production caller was deleted). + same window, which runs in the caller's thread (`debug_fill`) and becomes + test-only once this branch's fold (#400) deletes its `_mixed_engine()` + production caller). Also pins the identity guard: a request that does not describe the engine's current window must raise rather than silently pair one window's tracks with @@ -365,6 +366,12 @@ def test_current_window_realign_inputs_reads_the_producers_window( # Independent source of truth: a plan-less engine decodes this exact window in the # caller's thread, with no producer and no slot involved. oracle = b.build_engine([], 1, 1) + assert ( + oracle.current_window_realign_inputs( + contig_idx, t_starts.tolist(), t_ends.tolist(), 0, n_s + ) + is None + ), "a plan-less engine has no current window" expected = oracle.window_realign_inputs( contig_idx, t_starts.tolist(), t_ends.tolist(), 0, n_s ) From 9bacfd4627e220a41049574238a59c82f88bef98 Mon Sep 17 00:00:00 2001 From: d-laub Date: Mon, 21 Sep 2026 10:05:05 -0700 Subject: [PATCH 06/10] perf(streaming): fold the mixed record decode into the producer (#400) The VCF/PGEN mixed variants+tracks path decoded every window twice: the producer filled it for the haplotypes, then the track side re-decoded it synchronously through a private plan-less engine. The drive's engine is now the only decoder: _mixed_track_window forwards it and _record_mixed_realign_window reads the producer's current slot via current_window_realign_inputs, so realign_tracks=True costs the same decode as realign_tracks=False (gated on transpose_word_reads and pgen_variants_decoded, not wall-clock). The private engines are deleted. --- python/genvarloader/_dataset/_streaming.py | 96 ++++++++++--------- tests/dataset/test_streaming_tracks_record.py | 89 +++++++++++++++-- 2 files changed, 128 insertions(+), 57 deletions(-) diff --git a/python/genvarloader/_dataset/_streaming.py b/python/genvarloader/_dataset/_streaming.py index 25f078ab..baf1c59b 100644 --- a/python/genvarloader/_dataset/_streaming.py +++ b/python/genvarloader/_dataset/_streaming.py @@ -493,12 +493,16 @@ def mixed_realign_window( t_ends: NDArray[np.int32], row_starts: NDArray[np.int32], row_ends: NDArray[np.int32], + engine: object | None = None, ) -> tuple[_MixedRealign, NDArray[np.int32]]: """Per-window realign state + deletion-extended track ends. See `_Svar1Backend.mixed_realign_window` for the full contract every conforming backend must satisfy (index spaces, row ordering, shapes, and the exact 2-tuple return order). + + The `engine` argument is the drive's engine, for backends whose window + decode the engine owns (the record backends); SVAR1/SVAR2 ignore it. """ ... @@ -740,12 +744,21 @@ def _record_mixed_realign_window( t_ends: NDArray[np.int32], row_starts: NDArray[np.int32], row_ends: NDArray[np.int32], + engine: object | None = None, ) -> "tuple[_MixedRealign, NDArray[np.int32]]": """One record-stream window's fused-kernel realign state (issue #375, Track B). Shared by `_VcfBackend` and `_PgenBackend`: the two differ only in their Rust `WindowFiller`, so the Python side is one function, not two. + The variant tables and CSR come from the DRIVE's engine, which decoded + this window in its producer thread: `current_window_realign_inputs` reads + that already-filled slot and decodes nothing (issue #400 -- this used to + decode the window a second time through a private plan-less engine). It + also validates that the engine's current job is exactly this window, so a + drive/plan drift raises `ValueError` instead of silently pairing one + window's tracks with another's variants. + Produces exactly the same `_RealignWindow` shape `_Svar1Backend` produces, with two differences the kernel cannot observe: @@ -761,7 +774,7 @@ def _record_mixed_realign_window( wants per (region, sample, ploid) rows, so the CSR is replicated across the window's regions here. - The engine is queried with `t_starts`/`t_ends` -- the bounds the DRIVE + The engine call is validated against `t_starts`/`t_ends` -- the bounds the DRIVE actually used -- not with `backend._regions[r_idx, 1:3]`. Under jitter the two differ: the drive draws `region_offsets` once and translates the track bounds through `_jitter_region_bounds`, and the same translated bounds went @@ -809,14 +822,22 @@ def _record_mixed_realign_window( f"contract narrowed by this record-backend implementation; got {s_idx!r}." ) - engine = backend._mixed_engine() - v_starts, ilens, geno_v_idxs, csr = engine.window_realign_inputs( + if engine is None: + raise ValueError( + "_record_mixed_realign_window requires the drive's RecordStreamEngine: " + "the window's variant decode is read from the engine's current slot " + "(issue #400). Pass engine=." + ) + window_inputs = engine.current_window_realign_inputs( contig_idx, np.ascontiguousarray(t_starts, np.uint32).tolist(), np.ascontiguousarray(t_ends, np.uint32).tolist(), int(s_idx[0]), int(s_idx[-1]) + 1, ) + if window_inputs is None: + raise RuntimeError("streaming engine exhausted before the plan did") + v_starts, ilens, geno_v_idxs, csr = window_inputs v_starts = np.ascontiguousarray(v_starts, np.int32) ilens = np.ascontiguousarray(ilens, np.int32) geno_v_idxs = np.ascontiguousarray(geno_v_idxs, np.int32) @@ -1681,6 +1702,7 @@ def _mixed_track_window( r_idx: NDArray[np.intp], s_idx_w: NDArray[np.intp], region_offsets: "NDArray[np.int64] | None", + engine: object | None = None, ) -> "_TrackWindow": """Read one window's tracks, plus its realign state when re-aligning. @@ -1706,6 +1728,9 @@ def _mixed_track_window( `None` -- `_iter_batches` rejects jitter for SVAR2 before the drive runs, so its bounds are always the raw `self._regions` slice. + engine: The drive's engine, forwarded to + ``backend.mixed_realign_window``; required by the record + backends. Returns: The window's `_TrackWindow`; its `realign` is set only when @@ -1762,6 +1787,7 @@ def _mixed_track_window( np.ascontiguousarray(t_ends, np.int32), row_starts_w, row_ends_w, + engine, ) else: # Un-realigned tracks stay in reference coordinates -- no @@ -2051,6 +2077,10 @@ def _iter_batches(self, batch_size: int) -> Iterator[tuple]: # boundary deletion and under-extend the track query. Fail fast rather # than silently under-size the track buffer, matching the tracks-only # branch's identical jitter guard just above in this file. + # Record backends no longer decode in the consumer thread at all + # (issue #400): their decode comes from the drive's engine, whose + # job bounds are the jitter-translated ones. This guard is kept for + # SVAR1's raw-bounds re-read (#383). if ( self._track_backend is not None and self._jitter > 0 @@ -2280,6 +2310,7 @@ def _iter_batches(self, batch_size: int) -> Iterator[tuple]: r_idx, np.arange(s_lo, s_hi, dtype=np.intp), region_offsets, + engine, ) else: track_w = None @@ -4025,6 +4056,7 @@ def mixed_realign_window( t_ends: NDArray[np.int32], row_starts: NDArray[np.int32], row_ends: NDArray[np.int32], + engine: object | None = None, ) -> tuple[_MixedRealign, NDArray[np.int32]]: """One window's fused-kernel realign state + deletion-extended track ends. @@ -4084,6 +4116,8 @@ def mixed_realign_window( row_ends: `(n_regions * n_samples,)` int32, same region-major/sample-minor layout as `row_starts`, one row end per (region, sample) pair. + engine: Accepted for protocol uniformity and ignored: SVAR1's state + comes from its on-disk CSR, not from a stream engine. Returns: A 2-tuple `(realign_window, t_ends_ext)`, in that exact order: @@ -4578,6 +4612,7 @@ def mixed_realign_window( t_ends: NDArray[np.int32], row_starts: NDArray[np.int32], row_ends: NDArray[np.int32], + engine: object | None = None, ) -> tuple[_MixedRealign, NDArray[np.int32]]: """One window's split-kernel realign state + deletion-extended track ends. @@ -4599,6 +4634,9 @@ def mixed_realign_window( today. Relaxing that guard requires threading `row_starts`/`row_ends` into the gather here, mirroring how SVAR1's `get_diffs_sparse` call takes `q_starts=row_starts, q_ends=row_ends`. + + `engine` is accepted for protocol uniformity and ignored: SVAR2's state + comes from its read-bound kernel, not from a stream engine. """ from .._threads import should_parallelize from ..genvarloader import hap_diffs_from_svar2_readbound @@ -4744,8 +4782,6 @@ def __init__( sp.bed.sort(bed_df), ContigNormalizer(self._contigs) ) - self._mixed_engine_obj = None - @property def has_cached_af(self) -> bool: """Whether this VCF's ``INFO/AF`` is usable for AF filtering. @@ -4813,24 +4849,6 @@ def _af_is_unambiguous(self) -> bool: return False return True - def _mixed_engine(self) -> object: - """A zero-job engine kept solely for `window_realign_inputs` calls. - - `window_realign_inputs` decodes the window it is handed and never - touches the engine's own plan, so a plan-less engine is enough -- and - keeping it separate from the drive's engine means the mixed track read - cannot perturb the producer/consumer lockstep. Built lazily and cached - on the backend, paid once per `StreamingDataset`, not once per window. - `jobs=[]` means `touched_contigs` in `build_engine` is empty, so the - #307 placeholder optimization makes this call allocate no reference - bytes at all -- without it, this would otherwise pull the whole - reference (and, for PGEN, a full second `.pvar` scan) into a second - Rust engine (issue #375 Track B final review, H1). - """ - if self._mixed_engine_obj is None: - self._mixed_engine_obj = self.build_engine([], 1, 1) - return self._mixed_engine_obj - def mixed_realign_window( self, r_idx: NDArray[np.intp], @@ -4839,6 +4857,7 @@ def mixed_realign_window( t_ends: NDArray[np.int32], row_starts: NDArray[np.int32], row_ends: NDArray[np.int32], + engine: object | None = None, ) -> tuple[_MixedRealign, NDArray[np.int32]]: """See `_Svar1Backend.mixed_realign_window` for the full contract. @@ -4847,7 +4866,7 @@ def mixed_realign_window( `WindowFiller`, which the engine hides. """ return _record_mixed_realign_window( - self, r_idx, s_idx, t_starts, t_ends, row_starts, row_ends + self, r_idx, s_idx, t_starts, t_ends, row_starts, row_ends, engine ) def build_engine( @@ -4917,7 +4936,8 @@ def build_engine( # `contig_refs[job.contig_idx]`, so untouched contigs are never read. # Materializing every contig would pull the whole reference into # Python (and again into the Rust engine) on every `build_engine` call - # -- including `_mixed_engine()`'s `build_engine([], 1, 1)`, where + # -- including the plan-less engine the CSR test builds as its + # independent oracle (`build_engine([], 1, 1)`), where # `touched_contigs` is empty and this now allocates nothing at all. # Untouched contigs get an empty placeholder to keep the per-contig # arrays index-aligned (the engine requires equal per-contig lengths). @@ -5059,8 +5079,6 @@ def __init__( sp.bed.sort(bed_df), ContigNormalizer(self._contigs) ) - self._mixed_engine_obj = None - @property def has_cached_af(self) -> bool: """PGEN record streams carry no INFO -> no AF (Wave B PR-B2, #319). @@ -5069,24 +5087,6 @@ def has_cached_af(self) -> bool: """ return False - def _mixed_engine(self) -> object: - """A zero-job engine kept solely for `window_realign_inputs` calls. - - `window_realign_inputs` decodes the window it is handed and never - touches the engine's own plan, so a plan-less engine is enough -- and - keeping it separate from the drive's engine means the mixed track read - cannot perturb the producer/consumer lockstep. Built lazily and cached - on the backend, paid once per `StreamingDataset`, not once per window. - `jobs=[]` means `touched_contigs` in `build_engine` is empty, so the - #307 placeholder optimization makes this call allocate no reference - bytes at all -- without it, this would otherwise pull the whole - reference (and, for PGEN, a full second `.pvar` scan) into a second - Rust engine (issue #375 Track B final review, H1). - """ - if self._mixed_engine_obj is None: - self._mixed_engine_obj = self.build_engine([], 1, 1) - return self._mixed_engine_obj - def mixed_realign_window( self, r_idx: NDArray[np.intp], @@ -5095,6 +5095,7 @@ def mixed_realign_window( t_ends: NDArray[np.int32], row_starts: NDArray[np.int32], row_ends: NDArray[np.int32], + engine: object | None = None, ) -> tuple[_MixedRealign, NDArray[np.int32]]: """See `_Svar1Backend.mixed_realign_window` for the full contract. @@ -5103,7 +5104,7 @@ def mixed_realign_window( `WindowFiller`, which the engine hides. """ return _record_mixed_realign_window( - self, r_idx, s_idx, t_starts, t_ends, row_starts, row_ends + self, r_idx, s_idx, t_starts, t_ends, row_starts, row_ends, engine ) def build_engine( @@ -5159,7 +5160,8 @@ def build_engine( # `contig_refs[job.contig_idx]`, so untouched contigs are never read. # Materializing every contig would pull the whole reference into # Python (and again into the Rust engine) on every `build_engine` call - # -- including `_mixed_engine()`'s `build_engine([], 1, 1)`, where + # -- including the plan-less engine the CSR test builds as its + # independent oracle (`build_engine([], 1, 1)`), where # `touched_contigs` is empty and this now allocates nothing at all. # Untouched contigs get an empty placeholder to keep the per-contig # arrays index-aligned (the engine requires equal per-contig lengths). diff --git a/tests/dataset/test_streaming_tracks_record.py b/tests/dataset/test_streaming_tracks_record.py index e4a29849..e6cd93f4 100644 --- a/tests/dataset/test_streaming_tracks_record.py +++ b/tests/dataset/test_streaming_tracks_record.py @@ -1,8 +1,10 @@ """Rust seam for VCF/PGEN mixed variants+tracks (issue #375, Track B). -`window_realign_inputs` is the pymethod the Python mixed path uses to size a -window's deletion-extended track query before pulling the window's first -batch. Task 6 consumes it; this file pins its contract. +`current_window_realign_inputs` is the production seam: the drive reads the window +the producer has already filled, so a mixed record window is decoded once (issue +#400). `window_realign_inputs` decodes a window it is handed, in the caller's thread; +it is now TEST-ONLY and stays as the independent oracle these tests check the folded +path against. """ from __future__ import annotations @@ -125,6 +127,10 @@ def test_window_realign_inputs_matches_before_and_during_producer( the first `next_batch()` call returns, maximizing the chance this test actually overlaps the two decodes rather than merely running twice sequentially. + + Its production caller (`_mixed_engine()`) was deleted in issue #400, so this + test is now the only reason the pymethod stays: it still pins the + concurrent-call safety of the shared filler. """ f = pgen_snp_ins_del_multi contig_len = int(f.regions["chromEnd"][0]) @@ -273,20 +279,24 @@ def test_record_window_csr_replicates_across_regions( t_ends = np.ascontiguousarray(sds._regions[r_idx, 2], np.int32) row_starts = np.repeat(t_starts, n_s).astype(np.int32) row_ends = np.repeat(t_ends, n_s).astype(np.int32) + contig_idx = int(b._regions[r_idx[0], 0]) + engine = b.build_engine( + [(contig_idx, t_starts.astype(np.uint32), t_ends.astype(np.uint32), 0, n_s)], + 4, + -1, + ) state, t_ends_ext = b.mixed_realign_window( - r_idx, s_idx, t_starts, t_ends, row_starts, row_ends + r_idx, s_idx, t_starts, t_ends, row_starts, row_ends, engine ) assert state.geno_offsets.shape == (2, n_reg * n_s * P) assert state.diffs.shape == (n_reg * n_s, P) assert t_ends_ext.shape == (n_reg,) assert (t_ends_ext >= t_ends).all() - # Independent source of truth: query the engine directly for this exact - # (contig, t_starts/t_ends, sample sub-range) rather than trusting the - # mixed path's own replication of it. - contig_idx = int(b._regions[r_idx[0], 0]) - engine = b._mixed_engine() - _, _, _, csr = engine.window_realign_inputs( + # Independent source of truth: a plan-less engine decodes this exact window in the + # caller's thread (`debug_fill`), with no producer and no slot involved. + oracle = b.build_engine([], 1, 1) + _, _, _, csr = oracle.window_realign_inputs( contig_idx, np.ascontiguousarray(t_starts, np.uint32).tolist(), np.ascontiguousarray(t_ends, np.uint32).tolist(), @@ -680,3 +690,62 @@ def test_record_mixed_multi_contig_parity( assert seen == { (r, s) for r in range(written.shape[0]) for s in range(written.shape[1]) } + + +@pytest.mark.parametrize("backend", BACKENDS) +def test_mixed_record_realign_decodes_each_window_once( + streaming_record_tracks_fixture, backend +): + """Issue #400 gate: with `realign_tracks=True` the mixed record path must NOT + decode the window a second time. The producer already decodes every window + (`fill`); before this change the track side ALSO decoded it synchronously via a + private plan-less engine (`_mixed_engine()`), roughly doubling every window's + decode. + + Measured with the deterministic decode counters, never wall-clock: this tree + shares a noisy CI node. `transpose_word_reads` counts the genotype transpose + both `VcfWindowFiller` and `PgenWindowFiller` run (`fill_decoded_window`), and + `pgen_variants_decoded` additionally counts PGEN's decoded variants. Both are + process-wide atomics, reset immediately before each sweep and read immediately + after it is fully drained (so no decode is still in flight). + """ + from genvarloader.genvarloader import ( + pgen_variants_decoded, + pgen_variants_decoded_reset, + transpose_word_reads, + transpose_word_reads_reset, + ) + + f = streaming_record_tracks_fixture(backend) + + def sweep(realign: bool) -> tuple[int, int]: + sds = ( + gvl.StreamingDataset( + f.bed, + reference=f.reference_path, + variants=f.variants_path, + tracks=[f.table, f.bigwigs], + ) + .with_seqs("haplotypes") + .with_settings(realign_tracks=realign) + ) + transpose_word_reads_reset() + pgen_variants_decoded_reset() + for _ in sds.to_iter(batch_size=4): + pass + return transpose_word_reads(), pgen_variants_decoded() + + words_without, decoded_without = sweep(False) + words_with, decoded_with = sweep(True) + + assert words_without > 0, "counter is not wired (no window was decoded)" + assert words_with == words_without, ( + f"realign_tracks=True made {words_with} transposed word reads vs " + f"{words_without} with realign_tracks=False; the window is being decoded " + "twice again (issue #400 regression)" + ) + if backend == "pgen": + assert decoded_with == decoded_without, ( + f"realign_tracks=True decoded {decoded_with} PGEN variants vs " + f"{decoded_without} with realign_tracks=False (issue #400 regression)" + ) From 77a7912e4a83c9389cb7cba5de6f5a16f97eb6fa Mon Sep 17 00:00:00 2001 From: d-laub Date: Mon, 21 Sep 2026 10:16:15 -0700 Subject: [PATCH 07/10] docs(streaming): record the #400 decode fold (#400) window_realign_inputs and debug_decode_window are now test-only seams: the record mixed path reads the producer's own slot. Update the three Rust doc comments that still describe the double decode, and tick the roadmap's #400 follow-ups with the measured decode-counter numbers. --- docs/roadmaps/streaming-dataset.md | 4 +-- src/ffi/stream_core.rs | 22 +++++--------- src/record_stream/engine.rs | 29 ++++--------------- src/record_stream/pgen.rs | 13 ++------- .../test_streaming_abandoned_iter_mixed.py | 5 ++-- tests/dataset/test_streaming_tracks_record.py | 5 ++-- 6 files changed, 22 insertions(+), 56 deletions(-) diff --git a/docs/roadmaps/streaming-dataset.md b/docs/roadmaps/streaming-dataset.md index d55867d6..7a7ebc3b 100644 --- a/docs/roadmaps/streaming-dataset.md +++ b/docs/roadmaps/streaming-dataset.md @@ -88,8 +88,8 @@ only** (no map-style random access). | `docs/superpowers/plans/2026-07-19-streaming-svar2-phase2-pr3.md` | SVAR2 Phase 2 **PR 3** (read↔reconstruct pipeline engine): `Svar2StreamEngine` Rust producer thread (`src/ffi/svar2_stream_engine.rs`) running the whole fill chain (`find_ranges` → gather/decode → super-batch reconstruct) GIL-free, ping-ponging two `Svar2ReconBuf`s through `bounded(2)` channels (mirrors SVAR1's `Svar1StreamEngine`); selected via the existing `_prefetch_strategy` seam as `"svar2_engine"`. Deterministic order preserved (no relaxed-order doc change). Spec: `2026-07-19-streaming-svar2-phase2-pr3-design.md`. | ✅ **implemented, gated off by measurement — default stays `"sync"`.** Byte-identical parity (`test_svar2_engine_matches_written`, `test_svar2_engine_matches_sync_bytewise`) and the #284 cohort-independence gate (`test_svar2_engine_output_is_flat_in_cohort_size`) both green under the engine. **Cold-cache A/B** (`benchmarking/streaming/svar2_cold_cache.py`, vcfixture bulk store, fresh store + `posix_fadvise(DONTNEED)` per (rep, strategy), best-of-3, `batch_size=32`, shared 8-core dev node): at n_samples=500, `sync` runs `[0.148, 0.138, 0.119]` (range `[0.119, 0.148]`) vs `svar2_engine` runs `[0.123, 0.110, 0.107]` (range `[0.107, 0.123]`); at n_samples=2000, `sync` runs `[0.613, 0.539, 0.596]` (range `[0.539, 0.613]`) vs `svar2_engine` runs `[0.381, 0.611, 0.345]` (range `[0.345, 0.611]`). **Ranges overlap at both cohort sizes** (engine's best-of-3 edges into/past sync's range each time, and at n=2000 the engine's own worst rep, 0.611s, is slower than sync's worst) — the SVAR1 Task-9 non-overlapping-ranges bar is not cleared, and n=2000 in particular reads as node noise dominating any real effect, not a stable win. Per the ship rule (`docs/superpowers/specs/2026-07-19-streaming-svar2-phase2-pr3-design.md` → "Gating"), **kept `_Svar2Backend._default_strategy = "sync"`** — a marginal/noisy result does not meet the bar, and the rule is to bias toward the safe, already-parity-anchored default on any doubt. The engine ships as off-by-default infrastructure (selectable via `_prefetch_strategy="svar2_engine"`), matching how SVAR1's Design C shipped off-by-default when it lost its own A-vs-C call — it remains available for future re-measurement (larger cohorts, a quieter node, or once #276's VCF/PGEN decode cost gives the GIL-free producer more to hide). Draft PR [#306](https://github.com/mcvickerlab/GenVarLoader/pull/306) (stacked on `278-svar2-phase2-pr2`); deferred non-blocking follow-ups [#307](https://github.com/mcvickerlab/GenVarLoader/issues/307) — **addressed** in the stacked PR-3 follow-up: `build_engine` now materializes reference bytes only for contigs a job touches (SVAR1 + SVAR2, empty placeholder for the rest, keeps the cohort-independent memory story on the reference axis); the SVAR2 producer's `phys` alloc is deferred to the find-ranges cache-miss path; and `svar2_fill_super_batch` uses `require_contiguous_1d` for the friendly `ref_` message. Byte-identical parity preserved. Issue [#278](https://github.com/mcvickerlab/GenVarLoader/issues/278). | | `docs/superpowers/plans/2026-07-19-streaming-output-mode-breadth-wave-a.md` — issue [#277](https://github.com/mcvickerlab/GenVarLoader/issues/277) | Output-mode breadth, Wave A: `with_len` (int/ragged, no "variable"), read-time jitter (`with_settings(jitter=, rng=, deterministic=)`), `with_seqs("annotated")`. Spec: `docs/superpowers/specs/2026-07-19-streaming-output-mode-breadth-wave-a-design.md` | ✅ **done (Tasks 1-6 all landed)** — PR pending | | `docs/superpowers/plans/2026-07-18-streaming-vcf-pgen-optimization.md` — issue [#276](https://github.com/mcvickerlab/GenVarLoader/issues/276) | Optimization pass. Spec: `docs/superpowers/specs/2026-07-18-streaming-vcf-pgen-optimization-design.md` | ✅ **done (Tasks 1-7 all landed)** — counters (Task 1), `--compare-dataset` arm (Task 2), baseline + profile (Task 3, `docs/roadmaps/streaming-optimization-baseline.md`), PGEN `var_start`/`var_end` narrowing (Task 4), word-level transpose (Task 5), VCF sample-index reuse (Task 6), docs/gate (Task 7, see results below) | -| `docs/superpowers/plans/2026-09-08-streaming-intervals.md` — issue [#279](https://github.com/mcvickerlab/GenVarLoader/issues/279) | Interval (BigWigs/Table) streaming + mixed variants+tracks scheduler. Spec: `docs/superpowers/specs/2026-09-08-streaming-intervals-design.md` | ✅ **done (Tasks 1-10 all landed)**, PR [#374](https://github.com/mcvickerlab/GenVarLoader/pull/374) — per-contig `RustTable` COITree cache (Task 1), track parity fixtures (Task 2), `_TrackBackend` window reads (Task 3), `iteration_order`/`_plan()` region-vs-sample-major swap + `iteration_order_is_active` (Task 4), `tracks=` construction incl. tracks-only datasets (Task 5), tracks-without-variants reconstruction (Task 6), mixed SVAR1 variants+re-aligned-tracks fused-kernel wiring (Task 7), `with_seqs`/`realign_tracks`/jitter guards mirroring the written path (Task 8), `max_mem` track-byte accounting (`TRACK_BYTES_PER_CELL`) + `with_insertion_fill`/`with_settings(realign_tracks=)` (Task 9), docs (Task 10, this commit). Byte-identical parity gated on `extend_to_length=False, max_jitter=None`; `extend_to_length=True`/`max_jitter>0` and non-SVAR1 mixed variants+tracks are deferred follow-ups, filed as [#375](https://github.com/mcvickerlab/GenVarLoader/issues/375)-[#384](https://github.com/mcvickerlab/GenVarLoader/issues/384). **Follow-ups landed since:** [#379](https://github.com/mcvickerlab/GenVarLoader/issues/379) (`n_batches` under-reported for the super-batched SVAR2 drives — the counter re-derived a flat `range` instead of walking the drive's `super_batch`->`batch` nesting; both now share `_super_batch_bounds`/`_batch_bounds`, so they cannot drift again); [#380](https://github.com/mcvickerlab/GenVarLoader/issues/380) (the mixed variants+tracks drive reconstructed haplotype bytes and then DISCARDED them, yielding tracks alone — it now yields the `(haplotypes, tracks)` pair the written path's `HapsTracks` returns. This was not only an API asymmetry and wasted work: with the bytes discarded, NO mixed fixture could compare the haplotype half, so streaming's mixed reconstruction was un-oracled. All 10 mixed parity tests now assert both halves byte-identical vs `Dataset[r, s]`, verified non-vacuous by a 1-bit mutation of the haplotype buffer failing every one of them); [#375](https://github.com/mcvickerlab/GenVarLoader/issues/375) Track B ([#391](https://github.com/mcvickerlab/GenVarLoader/issues/391), PR [#397](https://github.com/mcvickerlab/GenVarLoader/pull/397)) — VCF/PGEN mixed variants+tracks, byte-identical parity on both backends via a per-hap CSR the record engine exposes per window; also ported the #307 `touched_contigs` optimization to both record backends (they were materializing every contig's reference bytes per engine, measured 201.2 MB -> 0.0 MB for a plan-less engine). Review follow-ups filed: [#398](https://github.com/mcvickerlab/GenVarLoader/issues/398) (`with_len` re-aligned-track parity gap, pre-existing and shared with SVAR1, documented as a known gap), [#399](https://github.com/mcvickerlab/GenVarLoader/issues/399) (abandoned record-stream `to_iter` generator wedges the producer) ✅ **fixed**, PR [#402](https://github.com/mcvickerlab/GenVarLoader/pull/402) — `EngineState::drop` joined the producer while *holding* the GIL (it runs during Python deallocation), and `PgenWindowFiller::fill` needs the GIL to drive pgenlib, so the two waited on each other. Closing the channels first, which `drop` already did, frees a producer parked on `rx_free.recv()`/`tx_filled.send()` but not one parked inside `fill`. The join now runs under `py.detach`, the same discipline `next_batch`/`window_realign_inputs` already follow. VCF never wedged (`VcfWindowFiller` touches Python not at all) and neither did non-mixed streams (their producer parks in `rx_free.recv()`, not in `fill`) — both verified, not assumed. Regression tests: a `stream_core` unit test that drops a mid-stream engine from inside `Python::attach` (verified non-vacuous — it fails at its 30 s bound with the `detach` removed), plus 10 `tests/dataset/test_streaming_abandoned_iter{,_mixed}.py` cases guarded by a `faulthandler` C-level watchdog, since a GIL deadlock never reaches a bytecode boundary and `pytest-timeout`'s signal handler therefore never runs), [#400](https://github.com/mcvickerlab/GenVarLoader/issues/400) (fold the mixed window decode into the producer to remove the 2x decode). ✅ **#375 Track A (SVAR2 mixed variants+tracks)**, PR [#395](https://github.com/mcvickerlab/GenVarLoader/pull/395) — `_Svar2Realign`/`_Svar2Backend.mixed_realign_window` wired into the "sync" read drive, byte-identical to `Dataset[r, s]` for both halves. With Track A and Track B both landed, **every variant backend (SVAR1, SVAR2, VCF/BCF, PGEN) supports mixed variants+tracks**. ✅ De-duplicating the mixed drive across the `"engine"` and `"sync"` branches landed as [#394](https://github.com/mcvickerlab/GenVarLoader/issues/394), PR [#401](https://github.com/mcvickerlab/GenVarLoader/pull/401) — the ~110 duplicated lines are now one `_mixed_track_window` (per-window bounds, `mixed_realign_window`, the single `read_window`) plus one `_mixed_tracks_batch` (`regions_batch`, the written path's `base_seed` xor-reduce, `realign_batch`/`_tracks_from_intervals`) serving both drives; the three differences (`s_idx_w`, `out_lengths`, and jitter via `region_offsets=None` for SVAR2) became parameters. Deliberately sequenced after both tracks so the two independent byte-identical parity suites could serve as the safety net — before it, each copy was executed by only one suite, so a shared-logic fix applied to one would silently miss the other with both green. | -| `docs/superpowers/plans/2026-09-11-streaming-mixed-tracks-non-svar1.md` — issue [#375](https://github.com/mcvickerlab/GenVarLoader/issues/375) | Mixed variants+tracks for the **non-SVAR1** backends (SVAR2, VCF, PGEN) — the deferred half of #279's mixed path. Decomposed into a serial prelude plus two parallel tracks. **Prelude:** extract a backend-agnostic re-align seam (`_MixedRealign` bundles one window's state with the kernel that consumes it; `_MixedTracksBackend` + a `supports_mixed_tracks` flag replace an `isinstance` guard) out of the SVAR1-only `to_iter` drive — issue [#389](https://github.com/mcvickerlab/GenVarLoader/issues/389). **Track A** (Python-only): `_Svar2Realign` + `_Svar2Backend.mixed_realign_window`, wired into SVAR2's `"sync"` super-batch drive rather than the `"engine"` drive the seam first served — issue [#390](https://github.com/mcvickerlab/GenVarLoader/issues/390). **Track B**: a Rust `window_realign_inputs` pymethod + `_record_mixed_realign_window`, serving VCF and PGEN together — issue [#391](https://github.com/mcvickerlab/GenVarLoader/issues/391). The split is forced by the kernels, not by convenience: SVAR1 and the record backends share the fused `intervals_and_realign_track_fused` kernel, while SVAR2 has none and must split into `intervals_to_tracks` + `shift_and_realign_tracks_from_svar2_readbound`. | ✅ **done — prelude and both tracks landed in `streaming`.** Prelude PR [#393](https://github.com/mcvickerlab/GenVarLoader/pull/393) (`c8b3b2d3`), Track B PR [#397](https://github.com/mcvickerlab/GenVarLoader/pull/397) (`c96acbdb`), Track A PR [#395](https://github.com/mcvickerlab/GenVarLoader/pull/395) (`b15ebb0a`); the two parallel tracks were reconciled in the merge `01c0d6d9`, where every conflict turned out to be the same question — *which backends support mixed variants+tracks?* — and the answer is now all four. **Byte-identical parity vs `gvl.write()` + `Dataset[r, s]` on both halves of the `(haplotypes, tracks)` tuple, for SVAR1, SVAR2, VCF/BCF and PGEN.** De-duplicating the resulting two copies of the mixed drive was deliberately sequenced last and landed as [#394](https://github.com/mcvickerlab/GenVarLoader/issues/394), PR [#401](https://github.com/mcvickerlab/GenVarLoader/pull/401). Still open: [#398](https://github.com/mcvickerlab/GenVarLoader/issues/398) (`with_len` re-aligned-track parity gap, pre-existing), [#399](https://github.com/mcvickerlab/GenVarLoader/issues/399) (abandoned record-stream `to_iter` generator wedges the producer) ✅ **fixed**, PR [#402](https://github.com/mcvickerlab/GenVarLoader/pull/402) — `EngineState::drop` joined the producer while *holding* the GIL (it runs during Python deallocation), and `PgenWindowFiller::fill` needs the GIL to drive pgenlib, so the two waited on each other. Closing the channels first, which `drop` already did, frees a producer parked on `rx_free.recv()`/`tx_filled.send()` but not one parked inside `fill`. The join now runs under `py.detach`, the same discipline `next_batch`/`window_realign_inputs` already follow. VCF never wedged (`VcfWindowFiller` touches Python not at all) and neither did non-mixed streams (their producer parks in `rx_free.recv()`, not in `fill`) — both verified, not assumed. Regression tests: a `stream_core` unit test that drops a mid-stream engine from inside `Python::attach` (verified non-vacuous — it fails at its 30 s bound with the `detach` removed), plus 10 `tests/dataset/test_streaming_abandoned_iter{,_mixed}.py` cases guarded by a `faulthandler` C-level watchdog, since a GIL deadlock never reaches a bytecode boundary and `pytest-timeout`'s signal handler therefore never runs), [#400](https://github.com/mcvickerlab/GenVarLoader/issues/400) (fold the mixed window decode into the producer). One correction to the prelude's own description above: the capability guard is enforced at **runtime** by the `isinstance` narrowing only — the `unsafe-overlap` static half was re-tested during #394 (swapped return tuple and wrong parameter dtype, before and after the refactor) and pyrefly reports nothing for any of them with this repo's current config. | +| `docs/superpowers/plans/2026-09-08-streaming-intervals.md` — issue [#279](https://github.com/mcvickerlab/GenVarLoader/issues/279) | Interval (BigWigs/Table) streaming + mixed variants+tracks scheduler. Spec: `docs/superpowers/specs/2026-09-08-streaming-intervals-design.md` | ✅ **done (Tasks 1-10 all landed)**, PR [#374](https://github.com/mcvickerlab/GenVarLoader/pull/374) — per-contig `RustTable` COITree cache (Task 1), track parity fixtures (Task 2), `_TrackBackend` window reads (Task 3), `iteration_order`/`_plan()` region-vs-sample-major swap + `iteration_order_is_active` (Task 4), `tracks=` construction incl. tracks-only datasets (Task 5), tracks-without-variants reconstruction (Task 6), mixed SVAR1 variants+re-aligned-tracks fused-kernel wiring (Task 7), `with_seqs`/`realign_tracks`/jitter guards mirroring the written path (Task 8), `max_mem` track-byte accounting (`TRACK_BYTES_PER_CELL`) + `with_insertion_fill`/`with_settings(realign_tracks=)` (Task 9), docs (Task 10, this commit). Byte-identical parity gated on `extend_to_length=False, max_jitter=None`; `extend_to_length=True`/`max_jitter>0` and non-SVAR1 mixed variants+tracks are deferred follow-ups, filed as [#375](https://github.com/mcvickerlab/GenVarLoader/issues/375)-[#384](https://github.com/mcvickerlab/GenVarLoader/issues/384). **Follow-ups landed since:** [#379](https://github.com/mcvickerlab/GenVarLoader/issues/379) (`n_batches` under-reported for the super-batched SVAR2 drives — the counter re-derived a flat `range` instead of walking the drive's `super_batch`->`batch` nesting; both now share `_super_batch_bounds`/`_batch_bounds`, so they cannot drift again); [#380](https://github.com/mcvickerlab/GenVarLoader/issues/380) (the mixed variants+tracks drive reconstructed haplotype bytes and then DISCARDED them, yielding tracks alone — it now yields the `(haplotypes, tracks)` pair the written path's `HapsTracks` returns. This was not only an API asymmetry and wasted work: with the bytes discarded, NO mixed fixture could compare the haplotype half, so streaming's mixed reconstruction was un-oracled. All 10 mixed parity tests now assert both halves byte-identical vs `Dataset[r, s]`, verified non-vacuous by a 1-bit mutation of the haplotype buffer failing every one of them); [#375](https://github.com/mcvickerlab/GenVarLoader/issues/375) Track B ([#391](https://github.com/mcvickerlab/GenVarLoader/issues/391), PR [#397](https://github.com/mcvickerlab/GenVarLoader/pull/397)) — VCF/PGEN mixed variants+tracks, byte-identical parity on both backends via a per-hap CSR the record engine exposes per window; also ported the #307 `touched_contigs` optimization to both record backends (they were materializing every contig's reference bytes per engine, measured 201.2 MB -> 0.0 MB for a plan-less engine). Review follow-ups filed: [#398](https://github.com/mcvickerlab/GenVarLoader/issues/398) (`with_len` re-aligned-track parity gap, pre-existing and shared with SVAR1, documented as a known gap), [#399](https://github.com/mcvickerlab/GenVarLoader/issues/399) (abandoned record-stream `to_iter` generator wedges the producer) ✅ **fixed**, PR [#402](https://github.com/mcvickerlab/GenVarLoader/pull/402) — `EngineState::drop` joined the producer while *holding* the GIL (it runs during Python deallocation), and `PgenWindowFiller::fill` needs the GIL to drive pgenlib, so the two waited on each other. Closing the channels first, which `drop` already did, frees a producer parked on `rx_free.recv()`/`tx_filled.send()` but not one parked inside `fill`. The join now runs under `py.detach`, the same discipline `next_batch`/`window_realign_inputs` already follow. VCF never wedged (`VcfWindowFiller` touches Python not at all) and neither did non-mixed streams (their producer parks in `rx_free.recv()`, not in `fill`) — both verified, not assumed. Regression tests: a `stream_core` unit test that drops a mid-stream engine from inside `Python::attach` (verified non-vacuous — it fails at its 30 s bound with the `detach` removed), plus 10 `tests/dataset/test_streaming_abandoned_iter{,_mixed}.py` cases guarded by a `faulthandler` C-level watchdog, since a GIL deadlock never reaches a bytecode boundary and `pytest-timeout`'s signal handler therefore never runs), [#400](https://github.com/mcvickerlab/GenVarLoader/issues/400) (fold the mixed window decode into the producer to remove the 2x decode) ✅ **fixed** — the drive's `RecordStreamEngine` is now the only decoder: `RecordStreamEngine.current_window_realign_inputs` reads the producer's already-filled slot (validating the requested job against the engine's current job), `_mixed_track_window` forwards `engine=`, and the private plan-less `_mixed_engine()` is deleted. Deterministic-counter gate (not wall-clock): `transpose_word_reads` on the record mixed fixture is now identical with `realign_tracks` on and off (2 word reads, both) and PGEN's `pgen_variants_decoded` likewise (5 variants, both) — the realign run was ~2x before, at 4 word reads and 10 variants. ✅ **#375 Track A (SVAR2 mixed variants+tracks)**, PR [#395](https://github.com/mcvickerlab/GenVarLoader/pull/395) — `_Svar2Realign`/`_Svar2Backend.mixed_realign_window` wired into the "sync" read drive, byte-identical to `Dataset[r, s]` for both halves. With Track A and Track B both landed, **every variant backend (SVAR1, SVAR2, VCF/BCF, PGEN) supports mixed variants+tracks**. ✅ De-duplicating the mixed drive across the `"engine"` and `"sync"` branches landed as [#394](https://github.com/mcvickerlab/GenVarLoader/issues/394), PR [#401](https://github.com/mcvickerlab/GenVarLoader/pull/401) — the ~110 duplicated lines are now one `_mixed_track_window` (per-window bounds, `mixed_realign_window`, the single `read_window`) plus one `_mixed_tracks_batch` (`regions_batch`, the written path's `base_seed` xor-reduce, `realign_batch`/`_tracks_from_intervals`) serving both drives; the three differences (`s_idx_w`, `out_lengths`, and jitter via `region_offsets=None` for SVAR2) became parameters. Deliberately sequenced after both tracks so the two independent byte-identical parity suites could serve as the safety net — before it, each copy was executed by only one suite, so a shared-logic fix applied to one would silently miss the other with both green. | +| `docs/superpowers/plans/2026-09-11-streaming-mixed-tracks-non-svar1.md` — issue [#375](https://github.com/mcvickerlab/GenVarLoader/issues/375) | Mixed variants+tracks for the **non-SVAR1** backends (SVAR2, VCF, PGEN) — the deferred half of #279's mixed path. Decomposed into a serial prelude plus two parallel tracks. **Prelude:** extract a backend-agnostic re-align seam (`_MixedRealign` bundles one window's state with the kernel that consumes it; `_MixedTracksBackend` + a `supports_mixed_tracks` flag replace an `isinstance` guard) out of the SVAR1-only `to_iter` drive — issue [#389](https://github.com/mcvickerlab/GenVarLoader/issues/389). **Track A** (Python-only): `_Svar2Realign` + `_Svar2Backend.mixed_realign_window`, wired into SVAR2's `"sync"` super-batch drive rather than the `"engine"` drive the seam first served — issue [#390](https://github.com/mcvickerlab/GenVarLoader/issues/390). **Track B**: a Rust `window_realign_inputs` pymethod + `_record_mixed_realign_window`, serving VCF and PGEN together — issue [#391](https://github.com/mcvickerlab/GenVarLoader/issues/391). The split is forced by the kernels, not by convenience: SVAR1 and the record backends share the fused `intervals_and_realign_track_fused` kernel, while SVAR2 has none and must split into `intervals_to_tracks` + `shift_and_realign_tracks_from_svar2_readbound`. | ✅ **done — prelude and both tracks landed in `streaming`.** Prelude PR [#393](https://github.com/mcvickerlab/GenVarLoader/pull/393) (`c8b3b2d3`), Track B PR [#397](https://github.com/mcvickerlab/GenVarLoader/pull/397) (`c96acbdb`), Track A PR [#395](https://github.com/mcvickerlab/GenVarLoader/pull/395) (`b15ebb0a`); the two parallel tracks were reconciled in the merge `01c0d6d9`, where every conflict turned out to be the same question — *which backends support mixed variants+tracks?* — and the answer is now all four. **Byte-identical parity vs `gvl.write()` + `Dataset[r, s]` on both halves of the `(haplotypes, tracks)` tuple, for SVAR1, SVAR2, VCF/BCF and PGEN.** De-duplicating the resulting two copies of the mixed drive was deliberately sequenced last and landed as [#394](https://github.com/mcvickerlab/GenVarLoader/issues/394), PR [#401](https://github.com/mcvickerlab/GenVarLoader/pull/401). Still open: [#398](https://github.com/mcvickerlab/GenVarLoader/issues/398) (`with_len` re-aligned-track parity gap, pre-existing), [#399](https://github.com/mcvickerlab/GenVarLoader/issues/399) (abandoned record-stream `to_iter` generator wedges the producer) ✅ **fixed**, PR [#402](https://github.com/mcvickerlab/GenVarLoader/pull/402) — `EngineState::drop` joined the producer while *holding* the GIL (it runs during Python deallocation), and `PgenWindowFiller::fill` needs the GIL to drive pgenlib, so the two waited on each other. Closing the channels first, which `drop` already did, frees a producer parked on `rx_free.recv()`/`tx_filled.send()` but not one parked inside `fill`. The join now runs under `py.detach`, the same discipline `next_batch`/`window_realign_inputs` already follow. VCF never wedged (`VcfWindowFiller` touches Python not at all) and neither did non-mixed streams (their producer parks in `rx_free.recv()`, not in `fill`) — both verified, not assumed. Regression tests: a `stream_core` unit test that drops a mid-stream engine from inside `Python::attach` (verified non-vacuous — it fails at its 30 s bound with the `detach` removed), plus 10 `tests/dataset/test_streaming_abandoned_iter{,_mixed}.py` cases guarded by a `faulthandler` C-level watchdog, since a GIL deadlock never reaches a bytecode boundary and `pytest-timeout`'s signal handler therefore never runs), [#400](https://github.com/mcvickerlab/GenVarLoader/issues/400) (fold the mixed window decode into the producer to remove the 2x decode) ✅ **fixed** — the drive's `RecordStreamEngine` is now the only decoder: `RecordStreamEngine.current_window_realign_inputs` reads the producer's already-filled slot (validating the requested job against the engine's current job), `_mixed_track_window` forwards `engine=`, and the private plan-less `_mixed_engine()` is deleted. Deterministic-counter gate (not wall-clock): `transpose_word_reads` on the record mixed fixture is now identical with `realign_tracks` on and off (2 word reads, both) and PGEN's `pgen_variants_decoded` likewise (5 variants, both) — the realign run was ~2x before, at 4 word reads and 10 variants. One correction to the prelude's own description above: the capability guard is enforced at **runtime** by the `isinstance` narrowing only — the `unsafe-overlap` static half was re-tested during #394 (swapped return tuple and wrong parameter dtype, before and after the refactor) and pyrefly reports nothing for any of them with this repo's current config. | ### Optimization pass (#276) results — Task 7 (2026-07-19) diff --git a/src/ffi/stream_core.rs b/src/ffi/stream_core.rs index cd79a9f5..a5b3aca5 100644 --- a/src/ffi/stream_core.rs +++ b/src/ffi/stream_core.rs @@ -231,21 +231,13 @@ impl StreamEngineCore { /// thread (if started) is concurrently doing the same against the same `Arc`-shared /// backend. Two callers: /// - /// - `RecordStreamEngine::debug_decode_window` (issue #276 task 7) — test/debug-only, - /// parity testing. - /// - `RecordStreamEngine::window_realign_inputs` (issue #375 Track B) — a genuine - /// production path for mixed VCF/PGEN variants+tracks streams, called per window. - /// As shipped, Python's `_mixed_engine()` calls it against a separate, - /// plan-less engine (`jobs=[]`) built just for this, so in production - /// there is no producer thread on that engine's `Arc`-shared backend to - /// contend with -- the concurrent-with-a-live-producer case this doc - /// describes is currently exercised only by - /// `test_window_realign_inputs_matches_before_and_during_producer`, - /// which deliberately drives one engine both ways. It stays a genuine - /// safety property of this accessor regardless, since folding the mixed - /// path onto the drive engine (removing the double decode) is the - /// obvious next step and would make this the live case (final review, - /// M1). + /// - `RecordStreamEngine::window_realign_inputs` and `debug_decode_window` (issues + /// #375/#391) -- now TEST-ONLY: it decodes a window in the caller's thread for + /// parity oracles. Issue #400 removed the production consumer (a private plan-less + /// engine), so the concurrent-with-a-live-producer case is exercised only by + /// `test_window_realign_inputs_matches_before_and_during_producer`. The safety + /// property stands: both callers release the GIL, so both remain safe against a + /// live producer. /// /// Both release the GIL (`py.detach`) around their call in here, so both are safe to /// call against a live producer — safety does not rest on a call-site convention like diff --git a/src/record_stream/engine.rs b/src/record_stream/engine.rs index f1544196..97ad7573 100644 --- a/src/record_stream/engine.rs +++ b/src/record_stream/engine.rs @@ -1072,30 +1072,11 @@ impl RecordStreamEngine { /// [`RecordBackend::kept_v_idxs`] assumes (`h = si * ploidy + p`). The /// caller replicates rows across regions. /// - /// Python needs these BEFORE the window's first batch, to size the - /// deletion-extended track query, so this cannot ride along on - /// `next_batch` (which is sub-window) and cannot read the producer's slot - /// (which the consumer owns). It therefore does its own synchronous decode - /// in the calling thread via `debug_fill`, the same path - /// `debug_decode_window` uses. That means a mixed VCF/PGEN stream decodes - /// each window TWICE: once here for the track sizing, once in the producer - /// for the haplotypes. - /// - /// As shipped, Python's `_mixed_engine()` gives this call its OWN engine - /// (a separate `PgenWindowFiller`/`reader_lock` or `VcfWindowFiller` from - /// the drive's), so in production this decode never actually contends - /// with the producer -- the two run against independent fillers with no - /// shared mutable state. `PgenWindowFiller`'s `reader_lock` (see its doc - /// comment) would still serialize this call against a producer sharing - /// the SAME filler, which is why it stays: `debug_fill` is a genuine - /// production entry point (not test-only), and the lock is what makes the - /// obvious future consolidation onto one engine safe. Today only - /// `test_window_realign_inputs_matches_before_and_during_producer` - /// exercises that pairing (final review, M1). VCF needs no lock either - /// way -- `VcfWindowFiller::fill` opens a fresh record source per call, so - /// there is no shared mutable reader to race. Net cost: roughly 2x decode - /// on the mixed path (one per engine); folding this into a single shared - /// engine is a tracked follow-up, not a v1 requirement. + /// TEST-ONLY since issue #400: the production consumer was deleted (the drive + /// now reads the producer's own slot via `current_window_realign_inputs`), so + /// this stays as the independent CSR oracle used by + /// `test_record_window_csr_replicates_across_regions` and + /// `test_window_realign_inputs_matches_before_and_during_producer`. #[pyo3(signature = (contig_idx, region_starts, region_ends, s_lo, s_hi))] #[allow(clippy::too_many_arguments)] fn window_realign_inputs<'py>( diff --git a/src/record_stream/pgen.rs b/src/record_stream/pgen.rs index d63eb6e3..57a10379 100644 --- a/src/record_stream/pgen.rs +++ b/src/record_stream/pgen.rs @@ -417,16 +417,9 @@ pub struct PgenWindowFiller { /// concurrent `fill`s would let one thread's `change_sample_subset` land between the /// other's subset-set and read -- silently decoding the wrong sample columns. /// - /// As shipped, the mixed variants+tracks path (`_mixed_engine()` in - /// `_streaming.py`) builds its own separate engine/filler/reader rather than - /// sharing this one with the producer, so today the producer thread and a - /// consumer-thread caller of `RecordStreamEngine::window_realign_inputs` - /// never actually contend on this lock in production -- only - /// `test_window_realign_inputs_matches_before_and_during_producer` drives - /// one filler both ways. The lock stays regardless: it is what makes the - /// obvious future consolidation (one filler serving both callers) safe, so - /// removing it now would silently arm that race for whoever does the - /// consolidation later (final review, M1). + /// Since issue #400 the mixed path decodes only in the producer, so this lock + /// exists for the test-only decode accessors and for any future shared-engine + /// caller. /// /// LOCK ORDERING: take this lock BEFORE acquiring the GIL, never after. Callers holding /// the GIL must release it (`py.detach`) before entering `fill`, or the producer -- which diff --git a/tests/dataset/test_streaming_abandoned_iter_mixed.py b/tests/dataset/test_streaming_abandoned_iter_mixed.py index b48a4a00..d2744e48 100644 --- a/tests/dataset/test_streaming_abandoned_iter_mixed.py +++ b/tests/dataset/test_streaming_abandoned_iter_mixed.py @@ -72,8 +72,9 @@ def test_failing_assert_inside_mixed_loop_does_not_wedge( """The literal shape #399 describes: an assertion fires inside the loop. Reusing the SAME dataset object for the second drive matters -- the mixed - path caches a second engine on the backend (`_mixed_engine_obj`), so a - fresh `StreamingDataset` would sidestep exactly the state under suspicion. + path now shares the drive's engine (#400 deleted the cached second engine), + so a fresh `StreamingDataset` would sidestep exactly the engine state under + suspicion. """ f = streaming_record_tracks_fixture(backend) diff --git a/tests/dataset/test_streaming_tracks_record.py b/tests/dataset/test_streaming_tracks_record.py index e6cd93f4..9575ca2e 100644 --- a/tests/dataset/test_streaming_tracks_record.py +++ b/tests/dataset/test_streaming_tracks_record.py @@ -338,9 +338,8 @@ def test_current_window_realign_inputs_reads_the_producers_window( """Issue #400: the window the producer has ALREADY filled must be readable from the drive's own engine -- that is what removes the second decode the track side used to do. Checked against the independent `window_realign_inputs` decode of the - same window, which runs in the caller's thread (`debug_fill`) and becomes - test-only once this branch's fold (#400) deletes its `_mixed_engine()` - production caller). + same window, which runs in the caller's thread (`debug_fill`) and is now + test-only: issue #400 deleted its `_mixed_engine()` production caller. Also pins the identity guard: a request that does not describe the engine's current window must raise rather than silently pair one window's tracks with From ad8c43df5ce326a045fb66c0dbb876856facc392 Mon Sep 17 00:00:00 2001 From: d-laub Date: Mon, 21 Sep 2026 10:16:39 -0700 Subject: [PATCH 08/10] docs(streaming): link the #400 PR --- docs/roadmaps/streaming-dataset.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/roadmaps/streaming-dataset.md b/docs/roadmaps/streaming-dataset.md index 7a7ebc3b..74a43f9a 100644 --- a/docs/roadmaps/streaming-dataset.md +++ b/docs/roadmaps/streaming-dataset.md @@ -88,8 +88,8 @@ only** (no map-style random access). | `docs/superpowers/plans/2026-07-19-streaming-svar2-phase2-pr3.md` | SVAR2 Phase 2 **PR 3** (read↔reconstruct pipeline engine): `Svar2StreamEngine` Rust producer thread (`src/ffi/svar2_stream_engine.rs`) running the whole fill chain (`find_ranges` → gather/decode → super-batch reconstruct) GIL-free, ping-ponging two `Svar2ReconBuf`s through `bounded(2)` channels (mirrors SVAR1's `Svar1StreamEngine`); selected via the existing `_prefetch_strategy` seam as `"svar2_engine"`. Deterministic order preserved (no relaxed-order doc change). Spec: `2026-07-19-streaming-svar2-phase2-pr3-design.md`. | ✅ **implemented, gated off by measurement — default stays `"sync"`.** Byte-identical parity (`test_svar2_engine_matches_written`, `test_svar2_engine_matches_sync_bytewise`) and the #284 cohort-independence gate (`test_svar2_engine_output_is_flat_in_cohort_size`) both green under the engine. **Cold-cache A/B** (`benchmarking/streaming/svar2_cold_cache.py`, vcfixture bulk store, fresh store + `posix_fadvise(DONTNEED)` per (rep, strategy), best-of-3, `batch_size=32`, shared 8-core dev node): at n_samples=500, `sync` runs `[0.148, 0.138, 0.119]` (range `[0.119, 0.148]`) vs `svar2_engine` runs `[0.123, 0.110, 0.107]` (range `[0.107, 0.123]`); at n_samples=2000, `sync` runs `[0.613, 0.539, 0.596]` (range `[0.539, 0.613]`) vs `svar2_engine` runs `[0.381, 0.611, 0.345]` (range `[0.345, 0.611]`). **Ranges overlap at both cohort sizes** (engine's best-of-3 edges into/past sync's range each time, and at n=2000 the engine's own worst rep, 0.611s, is slower than sync's worst) — the SVAR1 Task-9 non-overlapping-ranges bar is not cleared, and n=2000 in particular reads as node noise dominating any real effect, not a stable win. Per the ship rule (`docs/superpowers/specs/2026-07-19-streaming-svar2-phase2-pr3-design.md` → "Gating"), **kept `_Svar2Backend._default_strategy = "sync"`** — a marginal/noisy result does not meet the bar, and the rule is to bias toward the safe, already-parity-anchored default on any doubt. The engine ships as off-by-default infrastructure (selectable via `_prefetch_strategy="svar2_engine"`), matching how SVAR1's Design C shipped off-by-default when it lost its own A-vs-C call — it remains available for future re-measurement (larger cohorts, a quieter node, or once #276's VCF/PGEN decode cost gives the GIL-free producer more to hide). Draft PR [#306](https://github.com/mcvickerlab/GenVarLoader/pull/306) (stacked on `278-svar2-phase2-pr2`); deferred non-blocking follow-ups [#307](https://github.com/mcvickerlab/GenVarLoader/issues/307) — **addressed** in the stacked PR-3 follow-up: `build_engine` now materializes reference bytes only for contigs a job touches (SVAR1 + SVAR2, empty placeholder for the rest, keeps the cohort-independent memory story on the reference axis); the SVAR2 producer's `phys` alloc is deferred to the find-ranges cache-miss path; and `svar2_fill_super_batch` uses `require_contiguous_1d` for the friendly `ref_` message. Byte-identical parity preserved. Issue [#278](https://github.com/mcvickerlab/GenVarLoader/issues/278). | | `docs/superpowers/plans/2026-07-19-streaming-output-mode-breadth-wave-a.md` — issue [#277](https://github.com/mcvickerlab/GenVarLoader/issues/277) | Output-mode breadth, Wave A: `with_len` (int/ragged, no "variable"), read-time jitter (`with_settings(jitter=, rng=, deterministic=)`), `with_seqs("annotated")`. Spec: `docs/superpowers/specs/2026-07-19-streaming-output-mode-breadth-wave-a-design.md` | ✅ **done (Tasks 1-6 all landed)** — PR pending | | `docs/superpowers/plans/2026-07-18-streaming-vcf-pgen-optimization.md` — issue [#276](https://github.com/mcvickerlab/GenVarLoader/issues/276) | Optimization pass. Spec: `docs/superpowers/specs/2026-07-18-streaming-vcf-pgen-optimization-design.md` | ✅ **done (Tasks 1-7 all landed)** — counters (Task 1), `--compare-dataset` arm (Task 2), baseline + profile (Task 3, `docs/roadmaps/streaming-optimization-baseline.md`), PGEN `var_start`/`var_end` narrowing (Task 4), word-level transpose (Task 5), VCF sample-index reuse (Task 6), docs/gate (Task 7, see results below) | -| `docs/superpowers/plans/2026-09-08-streaming-intervals.md` — issue [#279](https://github.com/mcvickerlab/GenVarLoader/issues/279) | Interval (BigWigs/Table) streaming + mixed variants+tracks scheduler. Spec: `docs/superpowers/specs/2026-09-08-streaming-intervals-design.md` | ✅ **done (Tasks 1-10 all landed)**, PR [#374](https://github.com/mcvickerlab/GenVarLoader/pull/374) — per-contig `RustTable` COITree cache (Task 1), track parity fixtures (Task 2), `_TrackBackend` window reads (Task 3), `iteration_order`/`_plan()` region-vs-sample-major swap + `iteration_order_is_active` (Task 4), `tracks=` construction incl. tracks-only datasets (Task 5), tracks-without-variants reconstruction (Task 6), mixed SVAR1 variants+re-aligned-tracks fused-kernel wiring (Task 7), `with_seqs`/`realign_tracks`/jitter guards mirroring the written path (Task 8), `max_mem` track-byte accounting (`TRACK_BYTES_PER_CELL`) + `with_insertion_fill`/`with_settings(realign_tracks=)` (Task 9), docs (Task 10, this commit). Byte-identical parity gated on `extend_to_length=False, max_jitter=None`; `extend_to_length=True`/`max_jitter>0` and non-SVAR1 mixed variants+tracks are deferred follow-ups, filed as [#375](https://github.com/mcvickerlab/GenVarLoader/issues/375)-[#384](https://github.com/mcvickerlab/GenVarLoader/issues/384). **Follow-ups landed since:** [#379](https://github.com/mcvickerlab/GenVarLoader/issues/379) (`n_batches` under-reported for the super-batched SVAR2 drives — the counter re-derived a flat `range` instead of walking the drive's `super_batch`->`batch` nesting; both now share `_super_batch_bounds`/`_batch_bounds`, so they cannot drift again); [#380](https://github.com/mcvickerlab/GenVarLoader/issues/380) (the mixed variants+tracks drive reconstructed haplotype bytes and then DISCARDED them, yielding tracks alone — it now yields the `(haplotypes, tracks)` pair the written path's `HapsTracks` returns. This was not only an API asymmetry and wasted work: with the bytes discarded, NO mixed fixture could compare the haplotype half, so streaming's mixed reconstruction was un-oracled. All 10 mixed parity tests now assert both halves byte-identical vs `Dataset[r, s]`, verified non-vacuous by a 1-bit mutation of the haplotype buffer failing every one of them); [#375](https://github.com/mcvickerlab/GenVarLoader/issues/375) Track B ([#391](https://github.com/mcvickerlab/GenVarLoader/issues/391), PR [#397](https://github.com/mcvickerlab/GenVarLoader/pull/397)) — VCF/PGEN mixed variants+tracks, byte-identical parity on both backends via a per-hap CSR the record engine exposes per window; also ported the #307 `touched_contigs` optimization to both record backends (they were materializing every contig's reference bytes per engine, measured 201.2 MB -> 0.0 MB for a plan-less engine). Review follow-ups filed: [#398](https://github.com/mcvickerlab/GenVarLoader/issues/398) (`with_len` re-aligned-track parity gap, pre-existing and shared with SVAR1, documented as a known gap), [#399](https://github.com/mcvickerlab/GenVarLoader/issues/399) (abandoned record-stream `to_iter` generator wedges the producer) ✅ **fixed**, PR [#402](https://github.com/mcvickerlab/GenVarLoader/pull/402) — `EngineState::drop` joined the producer while *holding* the GIL (it runs during Python deallocation), and `PgenWindowFiller::fill` needs the GIL to drive pgenlib, so the two waited on each other. Closing the channels first, which `drop` already did, frees a producer parked on `rx_free.recv()`/`tx_filled.send()` but not one parked inside `fill`. The join now runs under `py.detach`, the same discipline `next_batch`/`window_realign_inputs` already follow. VCF never wedged (`VcfWindowFiller` touches Python not at all) and neither did non-mixed streams (their producer parks in `rx_free.recv()`, not in `fill`) — both verified, not assumed. Regression tests: a `stream_core` unit test that drops a mid-stream engine from inside `Python::attach` (verified non-vacuous — it fails at its 30 s bound with the `detach` removed), plus 10 `tests/dataset/test_streaming_abandoned_iter{,_mixed}.py` cases guarded by a `faulthandler` C-level watchdog, since a GIL deadlock never reaches a bytecode boundary and `pytest-timeout`'s signal handler therefore never runs), [#400](https://github.com/mcvickerlab/GenVarLoader/issues/400) (fold the mixed window decode into the producer to remove the 2x decode) ✅ **fixed** — the drive's `RecordStreamEngine` is now the only decoder: `RecordStreamEngine.current_window_realign_inputs` reads the producer's already-filled slot (validating the requested job against the engine's current job), `_mixed_track_window` forwards `engine=`, and the private plan-less `_mixed_engine()` is deleted. Deterministic-counter gate (not wall-clock): `transpose_word_reads` on the record mixed fixture is now identical with `realign_tracks` on and off (2 word reads, both) and PGEN's `pgen_variants_decoded` likewise (5 variants, both) — the realign run was ~2x before, at 4 word reads and 10 variants. ✅ **#375 Track A (SVAR2 mixed variants+tracks)**, PR [#395](https://github.com/mcvickerlab/GenVarLoader/pull/395) — `_Svar2Realign`/`_Svar2Backend.mixed_realign_window` wired into the "sync" read drive, byte-identical to `Dataset[r, s]` for both halves. With Track A and Track B both landed, **every variant backend (SVAR1, SVAR2, VCF/BCF, PGEN) supports mixed variants+tracks**. ✅ De-duplicating the mixed drive across the `"engine"` and `"sync"` branches landed as [#394](https://github.com/mcvickerlab/GenVarLoader/issues/394), PR [#401](https://github.com/mcvickerlab/GenVarLoader/pull/401) — the ~110 duplicated lines are now one `_mixed_track_window` (per-window bounds, `mixed_realign_window`, the single `read_window`) plus one `_mixed_tracks_batch` (`regions_batch`, the written path's `base_seed` xor-reduce, `realign_batch`/`_tracks_from_intervals`) serving both drives; the three differences (`s_idx_w`, `out_lengths`, and jitter via `region_offsets=None` for SVAR2) became parameters. Deliberately sequenced after both tracks so the two independent byte-identical parity suites could serve as the safety net — before it, each copy was executed by only one suite, so a shared-logic fix applied to one would silently miss the other with both green. | -| `docs/superpowers/plans/2026-09-11-streaming-mixed-tracks-non-svar1.md` — issue [#375](https://github.com/mcvickerlab/GenVarLoader/issues/375) | Mixed variants+tracks for the **non-SVAR1** backends (SVAR2, VCF, PGEN) — the deferred half of #279's mixed path. Decomposed into a serial prelude plus two parallel tracks. **Prelude:** extract a backend-agnostic re-align seam (`_MixedRealign` bundles one window's state with the kernel that consumes it; `_MixedTracksBackend` + a `supports_mixed_tracks` flag replace an `isinstance` guard) out of the SVAR1-only `to_iter` drive — issue [#389](https://github.com/mcvickerlab/GenVarLoader/issues/389). **Track A** (Python-only): `_Svar2Realign` + `_Svar2Backend.mixed_realign_window`, wired into SVAR2's `"sync"` super-batch drive rather than the `"engine"` drive the seam first served — issue [#390](https://github.com/mcvickerlab/GenVarLoader/issues/390). **Track B**: a Rust `window_realign_inputs` pymethod + `_record_mixed_realign_window`, serving VCF and PGEN together — issue [#391](https://github.com/mcvickerlab/GenVarLoader/issues/391). The split is forced by the kernels, not by convenience: SVAR1 and the record backends share the fused `intervals_and_realign_track_fused` kernel, while SVAR2 has none and must split into `intervals_to_tracks` + `shift_and_realign_tracks_from_svar2_readbound`. | ✅ **done — prelude and both tracks landed in `streaming`.** Prelude PR [#393](https://github.com/mcvickerlab/GenVarLoader/pull/393) (`c8b3b2d3`), Track B PR [#397](https://github.com/mcvickerlab/GenVarLoader/pull/397) (`c96acbdb`), Track A PR [#395](https://github.com/mcvickerlab/GenVarLoader/pull/395) (`b15ebb0a`); the two parallel tracks were reconciled in the merge `01c0d6d9`, where every conflict turned out to be the same question — *which backends support mixed variants+tracks?* — and the answer is now all four. **Byte-identical parity vs `gvl.write()` + `Dataset[r, s]` on both halves of the `(haplotypes, tracks)` tuple, for SVAR1, SVAR2, VCF/BCF and PGEN.** De-duplicating the resulting two copies of the mixed drive was deliberately sequenced last and landed as [#394](https://github.com/mcvickerlab/GenVarLoader/issues/394), PR [#401](https://github.com/mcvickerlab/GenVarLoader/pull/401). Still open: [#398](https://github.com/mcvickerlab/GenVarLoader/issues/398) (`with_len` re-aligned-track parity gap, pre-existing), [#399](https://github.com/mcvickerlab/GenVarLoader/issues/399) (abandoned record-stream `to_iter` generator wedges the producer) ✅ **fixed**, PR [#402](https://github.com/mcvickerlab/GenVarLoader/pull/402) — `EngineState::drop` joined the producer while *holding* the GIL (it runs during Python deallocation), and `PgenWindowFiller::fill` needs the GIL to drive pgenlib, so the two waited on each other. Closing the channels first, which `drop` already did, frees a producer parked on `rx_free.recv()`/`tx_filled.send()` but not one parked inside `fill`. The join now runs under `py.detach`, the same discipline `next_batch`/`window_realign_inputs` already follow. VCF never wedged (`VcfWindowFiller` touches Python not at all) and neither did non-mixed streams (their producer parks in `rx_free.recv()`, not in `fill`) — both verified, not assumed. Regression tests: a `stream_core` unit test that drops a mid-stream engine from inside `Python::attach` (verified non-vacuous — it fails at its 30 s bound with the `detach` removed), plus 10 `tests/dataset/test_streaming_abandoned_iter{,_mixed}.py` cases guarded by a `faulthandler` C-level watchdog, since a GIL deadlock never reaches a bytecode boundary and `pytest-timeout`'s signal handler therefore never runs), [#400](https://github.com/mcvickerlab/GenVarLoader/issues/400) (fold the mixed window decode into the producer to remove the 2x decode) ✅ **fixed** — the drive's `RecordStreamEngine` is now the only decoder: `RecordStreamEngine.current_window_realign_inputs` reads the producer's already-filled slot (validating the requested job against the engine's current job), `_mixed_track_window` forwards `engine=`, and the private plan-less `_mixed_engine()` is deleted. Deterministic-counter gate (not wall-clock): `transpose_word_reads` on the record mixed fixture is now identical with `realign_tracks` on and off (2 word reads, both) and PGEN's `pgen_variants_decoded` likewise (5 variants, both) — the realign run was ~2x before, at 4 word reads and 10 variants. One correction to the prelude's own description above: the capability guard is enforced at **runtime** by the `isinstance` narrowing only — the `unsafe-overlap` static half was re-tested during #394 (swapped return tuple and wrong parameter dtype, before and after the refactor) and pyrefly reports nothing for any of them with this repo's current config. | +| `docs/superpowers/plans/2026-09-08-streaming-intervals.md` — issue [#279](https://github.com/mcvickerlab/GenVarLoader/issues/279) | Interval (BigWigs/Table) streaming + mixed variants+tracks scheduler. Spec: `docs/superpowers/specs/2026-09-08-streaming-intervals-design.md` | ✅ **done (Tasks 1-10 all landed)**, PR [#374](https://github.com/mcvickerlab/GenVarLoader/pull/374) — per-contig `RustTable` COITree cache (Task 1), track parity fixtures (Task 2), `_TrackBackend` window reads (Task 3), `iteration_order`/`_plan()` region-vs-sample-major swap + `iteration_order_is_active` (Task 4), `tracks=` construction incl. tracks-only datasets (Task 5), tracks-without-variants reconstruction (Task 6), mixed SVAR1 variants+re-aligned-tracks fused-kernel wiring (Task 7), `with_seqs`/`realign_tracks`/jitter guards mirroring the written path (Task 8), `max_mem` track-byte accounting (`TRACK_BYTES_PER_CELL`) + `with_insertion_fill`/`with_settings(realign_tracks=)` (Task 9), docs (Task 10, this commit). Byte-identical parity gated on `extend_to_length=False, max_jitter=None`; `extend_to_length=True`/`max_jitter>0` and non-SVAR1 mixed variants+tracks are deferred follow-ups, filed as [#375](https://github.com/mcvickerlab/GenVarLoader/issues/375)-[#384](https://github.com/mcvickerlab/GenVarLoader/issues/384). **Follow-ups landed since:** [#379](https://github.com/mcvickerlab/GenVarLoader/issues/379) (`n_batches` under-reported for the super-batched SVAR2 drives — the counter re-derived a flat `range` instead of walking the drive's `super_batch`->`batch` nesting; both now share `_super_batch_bounds`/`_batch_bounds`, so they cannot drift again); [#380](https://github.com/mcvickerlab/GenVarLoader/issues/380) (the mixed variants+tracks drive reconstructed haplotype bytes and then DISCARDED them, yielding tracks alone — it now yields the `(haplotypes, tracks)` pair the written path's `HapsTracks` returns. This was not only an API asymmetry and wasted work: with the bytes discarded, NO mixed fixture could compare the haplotype half, so streaming's mixed reconstruction was un-oracled. All 10 mixed parity tests now assert both halves byte-identical vs `Dataset[r, s]`, verified non-vacuous by a 1-bit mutation of the haplotype buffer failing every one of them); [#375](https://github.com/mcvickerlab/GenVarLoader/issues/375) Track B ([#391](https://github.com/mcvickerlab/GenVarLoader/issues/391), PR [#397](https://github.com/mcvickerlab/GenVarLoader/pull/397)) — VCF/PGEN mixed variants+tracks, byte-identical parity on both backends via a per-hap CSR the record engine exposes per window; also ported the #307 `touched_contigs` optimization to both record backends (they were materializing every contig's reference bytes per engine, measured 201.2 MB -> 0.0 MB for a plan-less engine). Review follow-ups filed: [#398](https://github.com/mcvickerlab/GenVarLoader/issues/398) (`with_len` re-aligned-track parity gap, pre-existing and shared with SVAR1, documented as a known gap), [#399](https://github.com/mcvickerlab/GenVarLoader/issues/399) (abandoned record-stream `to_iter` generator wedges the producer) ✅ **fixed**, PR [#402](https://github.com/mcvickerlab/GenVarLoader/pull/402) — `EngineState::drop` joined the producer while *holding* the GIL (it runs during Python deallocation), and `PgenWindowFiller::fill` needs the GIL to drive pgenlib, so the two waited on each other. Closing the channels first, which `drop` already did, frees a producer parked on `rx_free.recv()`/`tx_filled.send()` but not one parked inside `fill`. The join now runs under `py.detach`, the same discipline `next_batch`/`window_realign_inputs` already follow. VCF never wedged (`VcfWindowFiller` touches Python not at all) and neither did non-mixed streams (their producer parks in `rx_free.recv()`, not in `fill`) — both verified, not assumed. Regression tests: a `stream_core` unit test that drops a mid-stream engine from inside `Python::attach` (verified non-vacuous — it fails at its 30 s bound with the `detach` removed), plus 10 `tests/dataset/test_streaming_abandoned_iter{,_mixed}.py` cases guarded by a `faulthandler` C-level watchdog, since a GIL deadlock never reaches a bytecode boundary and `pytest-timeout`'s signal handler therefore never runs), [#400](https://github.com/mcvickerlab/GenVarLoader/issues/400) (fold the mixed window decode into the producer to remove the 2x decode) ✅ **fixed**, PR [#420](https://github.com/mcvickerlab/GenVarLoader/pull/420) — the drive's `RecordStreamEngine` is now the only decoder: `RecordStreamEngine.current_window_realign_inputs` reads the producer's already-filled slot (validating the requested job against the engine's current job), `_mixed_track_window` forwards `engine=`, and the private plan-less `_mixed_engine()` is deleted. Deterministic-counter gate (not wall-clock): `transpose_word_reads` on the record mixed fixture is now identical with `realign_tracks` on and off (2 word reads, both) and PGEN's `pgen_variants_decoded` likewise (5 variants, both) — the realign run was ~2x before, at 4 word reads and 10 variants. ✅ **#375 Track A (SVAR2 mixed variants+tracks)**, PR [#395](https://github.com/mcvickerlab/GenVarLoader/pull/395) — `_Svar2Realign`/`_Svar2Backend.mixed_realign_window` wired into the "sync" read drive, byte-identical to `Dataset[r, s]` for both halves. With Track A and Track B both landed, **every variant backend (SVAR1, SVAR2, VCF/BCF, PGEN) supports mixed variants+tracks**. ✅ De-duplicating the mixed drive across the `"engine"` and `"sync"` branches landed as [#394](https://github.com/mcvickerlab/GenVarLoader/issues/394), PR [#401](https://github.com/mcvickerlab/GenVarLoader/pull/401) — the ~110 duplicated lines are now one `_mixed_track_window` (per-window bounds, `mixed_realign_window`, the single `read_window`) plus one `_mixed_tracks_batch` (`regions_batch`, the written path's `base_seed` xor-reduce, `realign_batch`/`_tracks_from_intervals`) serving both drives; the three differences (`s_idx_w`, `out_lengths`, and jitter via `region_offsets=None` for SVAR2) became parameters. Deliberately sequenced after both tracks so the two independent byte-identical parity suites could serve as the safety net — before it, each copy was executed by only one suite, so a shared-logic fix applied to one would silently miss the other with both green. | +| `docs/superpowers/plans/2026-09-11-streaming-mixed-tracks-non-svar1.md` — issue [#375](https://github.com/mcvickerlab/GenVarLoader/issues/375) | Mixed variants+tracks for the **non-SVAR1** backends (SVAR2, VCF, PGEN) — the deferred half of #279's mixed path. Decomposed into a serial prelude plus two parallel tracks. **Prelude:** extract a backend-agnostic re-align seam (`_MixedRealign` bundles one window's state with the kernel that consumes it; `_MixedTracksBackend` + a `supports_mixed_tracks` flag replace an `isinstance` guard) out of the SVAR1-only `to_iter` drive — issue [#389](https://github.com/mcvickerlab/GenVarLoader/issues/389). **Track A** (Python-only): `_Svar2Realign` + `_Svar2Backend.mixed_realign_window`, wired into SVAR2's `"sync"` super-batch drive rather than the `"engine"` drive the seam first served — issue [#390](https://github.com/mcvickerlab/GenVarLoader/issues/390). **Track B**: a Rust `window_realign_inputs` pymethod + `_record_mixed_realign_window`, serving VCF and PGEN together — issue [#391](https://github.com/mcvickerlab/GenVarLoader/issues/391). The split is forced by the kernels, not by convenience: SVAR1 and the record backends share the fused `intervals_and_realign_track_fused` kernel, while SVAR2 has none and must split into `intervals_to_tracks` + `shift_and_realign_tracks_from_svar2_readbound`. | ✅ **done — prelude and both tracks landed in `streaming`.** Prelude PR [#393](https://github.com/mcvickerlab/GenVarLoader/pull/393) (`c8b3b2d3`), Track B PR [#397](https://github.com/mcvickerlab/GenVarLoader/pull/397) (`c96acbdb`), Track A PR [#395](https://github.com/mcvickerlab/GenVarLoader/pull/395) (`b15ebb0a`); the two parallel tracks were reconciled in the merge `01c0d6d9`, where every conflict turned out to be the same question — *which backends support mixed variants+tracks?* — and the answer is now all four. **Byte-identical parity vs `gvl.write()` + `Dataset[r, s]` on both halves of the `(haplotypes, tracks)` tuple, for SVAR1, SVAR2, VCF/BCF and PGEN.** De-duplicating the resulting two copies of the mixed drive was deliberately sequenced last and landed as [#394](https://github.com/mcvickerlab/GenVarLoader/issues/394), PR [#401](https://github.com/mcvickerlab/GenVarLoader/pull/401). Still open: [#398](https://github.com/mcvickerlab/GenVarLoader/issues/398) (`with_len` re-aligned-track parity gap, pre-existing), [#399](https://github.com/mcvickerlab/GenVarLoader/issues/399) (abandoned record-stream `to_iter` generator wedges the producer) ✅ **fixed**, PR [#402](https://github.com/mcvickerlab/GenVarLoader/pull/402) — `EngineState::drop` joined the producer while *holding* the GIL (it runs during Python deallocation), and `PgenWindowFiller::fill` needs the GIL to drive pgenlib, so the two waited on each other. Closing the channels first, which `drop` already did, frees a producer parked on `rx_free.recv()`/`tx_filled.send()` but not one parked inside `fill`. The join now runs under `py.detach`, the same discipline `next_batch`/`window_realign_inputs` already follow. VCF never wedged (`VcfWindowFiller` touches Python not at all) and neither did non-mixed streams (their producer parks in `rx_free.recv()`, not in `fill`) — both verified, not assumed. Regression tests: a `stream_core` unit test that drops a mid-stream engine from inside `Python::attach` (verified non-vacuous — it fails at its 30 s bound with the `detach` removed), plus 10 `tests/dataset/test_streaming_abandoned_iter{,_mixed}.py` cases guarded by a `faulthandler` C-level watchdog, since a GIL deadlock never reaches a bytecode boundary and `pytest-timeout`'s signal handler therefore never runs), [#400](https://github.com/mcvickerlab/GenVarLoader/issues/400) (fold the mixed window decode into the producer to remove the 2x decode) ✅ **fixed**, PR [#420](https://github.com/mcvickerlab/GenVarLoader/pull/420) — the drive's `RecordStreamEngine` is now the only decoder: `RecordStreamEngine.current_window_realign_inputs` reads the producer's already-filled slot (validating the requested job against the engine's current job), `_mixed_track_window` forwards `engine=`, and the private plan-less `_mixed_engine()` is deleted. Deterministic-counter gate (not wall-clock): `transpose_word_reads` on the record mixed fixture is now identical with `realign_tracks` on and off (2 word reads, both) and PGEN's `pgen_variants_decoded` likewise (5 variants, both) — the realign run was ~2x before, at 4 word reads and 10 variants. One correction to the prelude's own description above: the capability guard is enforced at **runtime** by the `isinstance` narrowing only — the `unsafe-overlap` static half was re-tested during #394 (swapped return tuple and wrong parameter dtype, before and after the refactor) and pyrefly reports nothing for any of them with this repo's current config. | ### Optimization pass (#276) results — Task 7 (2026-07-19) From 55c07abf2560098dce13d46823e02cb792b88005 Mon Sep 17 00:00:00 2001 From: d-laub Date: Mon, 21 Sep 2026 10:24:31 -0700 Subject: [PATCH 09/10] docs(streaming): make the retired oracle's docs self-consistent (#400) --- src/ffi/stream_core.rs | 16 +++++++++------- src/record_stream/engine.rs | 4 ++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/ffi/stream_core.rs b/src/ffi/stream_core.rs index a5b3aca5..efb25000 100644 --- a/src/ffi/stream_core.rs +++ b/src/ffi/stream_core.rs @@ -231,13 +231,15 @@ impl StreamEngineCore { /// thread (if started) is concurrently doing the same against the same `Arc`-shared /// backend. Two callers: /// - /// - `RecordStreamEngine::window_realign_inputs` and `debug_decode_window` (issues - /// #375/#391) -- now TEST-ONLY: it decodes a window in the caller's thread for - /// parity oracles. Issue #400 removed the production consumer (a private plan-less - /// engine), so the concurrent-with-a-live-producer case is exercised only by - /// `test_window_realign_inputs_matches_before_and_during_producer`. The safety - /// property stands: both callers release the GIL, so both remain safe against a - /// live producer. + /// - `RecordStreamEngine::debug_decode_window` (issue #276 task 7) and + /// `RecordStreamEngine::window_realign_inputs` (issues #375/#391) -- now + /// TEST-ONLY: they decode a window in the caller's thread for parity + /// oracles. Issue #400 removed the production consumer (a private + /// plan-less engine), so the concurrent-with-a-live-producer case is + /// exercised only by + /// `test_window_realign_inputs_matches_before_and_during_producer`. The + /// safety property stands: both callers release the GIL, so both remain + /// safe against a live producer. /// /// Both release the GIL (`py.detach`) around their call in here, so both are safe to /// call against a live producer — safety does not rest on a call-site convention like diff --git a/src/record_stream/engine.rs b/src/record_stream/engine.rs index 97ad7573..909f951b 100644 --- a/src/record_stream/engine.rs +++ b/src/record_stream/engine.rs @@ -1061,8 +1061,8 @@ impl RecordStreamEngine { )) } - /// Decode one window's genotype table + CSR for the Python mixed - /// variants+tracks path (issue #375, Track B). + /// Decode one window's genotype table + CSR (issue #375, Track B; the + /// test-only oracle since issue #400 -- see below). /// /// Returns `(v_starts, ilens, geno_v_idxs, geno_offsets)`, all WINDOW-LOCAL: /// `geno_v_idxs` holds column indices into `v_starts`/`ilens`, and From 3c115dcf4e06c9b43abe305630866015b580259c Mon Sep 17 00:00:00 2001 From: d-laub Date: Mon, 21 Sep 2026 10:33:41 -0700 Subject: [PATCH 10/10] test(streaming): tighten the #400 gate and guard tests (#400) --- src/ffi/stream_core.rs | 3 ++- tests/dataset/test_streaming_tracks_record.py | 21 ++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/ffi/stream_core.rs b/src/ffi/stream_core.rs index efb25000..76f20454 100644 --- a/src/ffi/stream_core.rs +++ b/src/ffi/stream_core.rs @@ -387,7 +387,8 @@ impl StreamEngineCore { /// `None` = plan exhausted; `Some(Err(_))` = producer error/panic, joined and /// classified exactly as `next_batch_core` does. `f` runs while the state lock is /// held -- the slot it reads must not be recycled under it -- so it must not re-enter - /// the engine. + /// the engine. Callers must release the GIL (`py.detach`) before calling in: this + /// can park in `recv` (and start the producer), which may itself need the GIL. pub(crate) fn with_current_window( &self, f: impl FnOnce(usize, &B::Slot) -> anyhow::Result, diff --git a/tests/dataset/test_streaming_tracks_record.py b/tests/dataset/test_streaming_tracks_record.py index 9575ca2e..6e903c33 100644 --- a/tests/dataset/test_streaming_tracks_record.py +++ b/tests/dataset/test_streaming_tracks_record.py @@ -361,11 +361,21 @@ def test_current_window_realign_inputs_reads_the_producers_window( engine = b.build_engine([(contig_idx, t_starts, t_ends, 0, n_s)], 4, -1) - # A window that is NOT the engine's current one must fail loudly. - with pytest.raises(ValueError, match="does not match"): - engine.current_window_realign_inputs( - contig_idx, t_starts.tolist(), (t_ends + 1).tolist(), 0, n_s - ) + # A window that is NOT the engine's current one must fail loudly -- once per + # perturbed guard field, so dropping any one field from the Rust comparison + # cannot leave this suite green. + for field, bad in ( + ("contig_idx", (contig_idx + 1, t_starts.tolist(), t_ends.tolist(), 0, n_s)), + ( + "region_starts", + (contig_idx, (t_starts + 1).tolist(), t_ends.tolist(), 0, n_s), + ), + ("region_ends", (contig_idx, t_starts.tolist(), (t_ends + 1).tolist(), 0, n_s)), + ("s_lo", (contig_idx, t_starts.tolist(), t_ends.tolist(), 1, n_s)), + ("s_hi", (contig_idx, t_starts.tolist(), t_ends.tolist(), 0, n_s + 1)), + ): + with pytest.raises(ValueError, match="does not match"): + engine.current_window_realign_inputs(*bad) got = engine.current_window_realign_inputs( contig_idx, t_starts.tolist(), t_ends.tolist(), 0, n_s @@ -744,6 +754,7 @@ def sweep(realign: bool) -> tuple[int, int]: "twice again (issue #400 regression)" ) if backend == "pgen": + assert decoded_without > 0, "PGEN decode counter is not wired" assert decoded_with == decoded_without, ( f"realign_tracks=True decoded {decoded_with} PGEN variants vs " f"{decoded_without} with realign_tracks=False (issue #400 regression)"