feat(streaming): variants min_af/max_af — Wave B PR-B2 (#317, closes #319)#323
Merged
Conversation
…317) Design for adding min_af/max_af AF filtering to streaming with_seqs("variants") on SVAR1/VCF/PGEN, byte-identical to the written oracle. Inline-folds the AF keep into generate_variants' existing per-variant region-clip predicate. Splits PR-B2 out of #304. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…parity, #317) Code investigation showed gvl.write never computes AF (VCF needs a pre-built info=["AF"] index; PGEN has no INFO path at all), so AF filtering is effectively SVAR-only on the written path. Rescope PR-B2: full SVAR1 support + parity via cache_afs; VCF/PGEN reproduce the written RuntimeError guard. Defer VCF live-INFO AF to #319 (fixed on both paths together). Keeps streaming<->written interchangeable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
8-task TDD plan: with_settings params, output-mode guard, SVAR1 Rust keep-fold + afs table, Python _afs loading + build_engine threading, AF-missing guard parity, byte-identical parity test, docs, full-suite+PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…th sides Expand the B2 spec + plan from SVAR-only to also support VCF/BCF min_af/max_af by reading the VCF INFO/AF field on both paths: gvl.write caches AF into the .gvi (info=["AF"]) and the streaming filler requests an AF FieldSpec (info_staged -> DecodedWindow.afs -> record keep). Same genoray resolve_scalar ALT resolution => byte-identical parity. PGEN stays guard-parity. No genoray change (verified against pinned rev). Closes #319 into #317. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…p (PR-B2, #317) Adds an optional GLOBAL AF table (afs: Option<Array1<f32>>) plus min_af/max_af bounds to Svar1Backend, folded into generate_variants' existing region-overlap keep as an AND-composed, inclusive-bounds AF term. Threaded through Svar1StreamEngine::build and the #[new] pyclass constructor (all default None -> pure no-op, preserving byte-for-byte parity with every existing call site). Adds a next_batch_variants_core test accessor (mirrors next_batch_core) and a unit test proving the AND-compose from both directions: AF alone deciding survivorship within a region, and a variant that clears the region clip but is then dropped by AF.
…ine (PR-B2, #317) Load per-variant AF from the .svar index (via SparseVar(attrs=["AF"]) since a plain open omits cached AF), expose has_cached_af, and thread min_af/max_af through the shared build_engine call site into the Svar1StreamEngine constructor. VCF/PGEN build_engine accept-and-ignore the new kwargs (behavior wired in Task 8) so the shared call site stays valid. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…, #319) gvl.write now caches a POS-aligned AF column into a VCF-sourced dataset's .gvi/variants.arrow whenever the source VCF header declares INFO/AF, so a written VCF Dataset can min_af/max_af-filter like the streaming SVAR1 path. AF-less VCFs are unchanged. Works around a genoray bug (d-laub/genoray#139) where VCF._write_gvi_index(info=[...])'s general info= parameter silently drops the requested INFO column; fetches AF via the (unaffected) private _fetch_info_cols helper instead and attaches it onto the base index with a POS-alignment cross-check.
…review, #317, #319) Number=A INFO/AF comes back from oxbow as List(Float32); without coercion the column fails the downstream is_numeric() filter and variants.info["AF"] is silently dropped. Coerce via list.first() (lossless under gvl's bi-allelic precondition; guard raises if a list carries >1 value). Add a Number=A fixture and a value-round-trip test (was presence-only). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ndow.afs (PR-B2, #319) Add DecodedWindow.afs, populated by fill_decoded_window from chunk.info_staged[0] (a StagedColumn::Float) when present, else left empty -- afs.is_empty() is the no-filter no-op contract, mirroring SVAR1's afs: None and PGEN's always-empty info_staged. VcfWindowFiller::new gains a want_af: bool that gates whether the AF INFO FieldSpec (Info/Float/F32) is requested from genoray, so no-filter jobs pay no INFO-decode cost. All current call sites pass false; Task 8 wires the real min_af/max_af-derived flag from Python.
… assembly (PR-B2, #319) RecordBackend.generate_variants ANDs an AF keep term onto the existing per-variant region-overlap clip, reading slot.afs (Task 6's DecodedWindow.afs channel) by the same window-local vidx used for v_starts/ilens. Empty afs or unset min_af/max_af is a no-op, preserving pre-PR-B2 behavior exactly -- mirrors the SVAR1 fold (e6f4284) for the record-stream (VCF/PGEN) path. min_af/max_af are threaded through RecordBackend, new_rs, and the #[new] pyclass constructor (default None), with VcfWindowFiller's want_af left false pending Task 8, which wires want_af and min_af/max_af together from Python. Adds two unit tests: one proving AND-composition (a variant passing only the region clip or only the AF bound is dropped; only the variant passing both is kept) and one proving the afs-present-but-unbounded case reduces to the pre-PR-B2 region-only clip.
…-B2, #317, #319) Activate VCF streaming AF filtering: RecordStreamEngine derives want_af from min_af/max_af, so the AF FieldSpec is requested exactly when filtering is active. _VcfBackend.has_cached_af reads the VCF header INFO/AF (same detection as the written path); _PgenBackend.has_cached_af is False. _iter_batches raises the written path's exact AF-missing RuntimeError when filtering is requested without cached AF. Also fix svar1_multicontig_fixture: samples=["S0","S1","S2"] made SparseVar.from_vcf cache AF as a subsetting side effect, so the fixture was not actually un-cached; samples=None keeps the same sample order without caching AF. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-B2, #319) Adds test_streaming_vcf_af_matches_written: authors a VCF with a multiallelic record (A -> C,G, AF=0.1,0.2), splits it via `bcftools norm -m -any -f ref`, and proves the streaming StreamingDataset(min_af/max_af) path (live Rust FieldSpec AF) agrees byte-for-byte with the written Dataset path (`_fetch_info_cols`-cached AF) across 3 filter bands, including an explicit per-hap assertion that the split ALT->AF mapping (C=0.1, G=0.2) is resolved identically on both sides.
…NFO/AF (PR-B2, #317, #319) Document the shipped StreamingDataset.with_settings(min_af, max_af) feature: per-backend AF source (SVAR cache_afs(), VCF/BCF INFO/AF, PGEN unsupported), identical guard behavior to the written Dataset path, and gvl.write() now caching an AF column for VCF/BCF sources that declare INFO/AF. Updates the genvarloader skill, dataset.md/faq.md/write.md, and ticks PR-B2 in the streaming-dataset roadmap.
…R-B2 review, #319) Address Task 11 review: write.md's AF-caching paragraph omitted the new _attach_af_column ValueError for a multi-valued (un-normalized multiallelic) INFO/AF, which SKILL.md's "preconditions not validated" claim could contradict. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…R-B2, #319) The streaming VCF/BCF INFO/AF read rides genoray's htslib reader, which must stay built with libdeflate. Record the invariant + the cargo tree -i libdeflate-sys verification at the rev-bump site so it can't silently regress. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…NFO/AF (PR-B2, #319) The Task-5 review fix raised ValueError when a record's INFO/AF carried >1 value, assuming that only happens on un-normalized multiallelic ALT. But a bi-allelic record can legitimately carry a multi-value Number=. AF (e.g. a `bcftools norm -m` split that left the AF list un-subset: `G>A` with AF=0.333,0.667). Such VCFs write fine for non-AF use and are common in the test corpus, so raising regressed 74 unrelated tests (test_output_format, test_flat_mode_equivalence, test_unphased_union). Now gvl.write logs a warning and declines to cache AF when the ALT->AF mapping is ambiguous (>1 value): the write succeeds unchanged and AF filtering raises the usual "AF not cached" guard. Bi-allelic single-value AF still caches as before. Adds a regression test with a multi-value Number=. AF fixture. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rgence (PR-B2 review, #319) Final-review follow-up: streaming reads INFO/AF live (resolving a multi-value Number=. AF to the first value) while gvl.write declines to cache ambiguous multi-value AF, so the two can diverge on un-normalized VCFs. Document the normalize-first requirement; a symmetric streaming decline is tracked separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
d-laub
marked this pull request as ready for review
July 23, 2026 05:45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Streaming
with_seqs("variants")min_af/max_afallele-frequency filtering (Wave B PR-B2, #317; closes #319). Byte-identical to the writtenDatasetvariants path atjitter=0.What
afstable folded intoSvar1Backend::generate_variants' per-variant inline keep (region ∧ AF, inclusive bounds). AF fromSparseVar.cache_afs().INFO/AFon both paths:FieldSpec→DecodedWindow.afs→ the record-path variant keep;.gvigains anAFcolumn (gvl.write), so a writtenDatasetAF-filters the same source.bcftools norm -m -any, per-ALT AF verified on both paths).RuntimeError.NotImplementedError; AF unavailable → byte-identicalRuntimeError.Notes / deviations
#139workaround._write_gvi_index(info=["AF"])silently drops the column at the pinned rev;gvl.writeinstead attaches AF via genoray's own_fetch_info_cols(POS-aligned, atomic write), mirroring genoray's SV-field pattern. No genoray change / no rev bump. Filed get_record_info(fields=[...], info=[...]) silently drops the requested INFO column d-laub/genoray#139.INFO/AF. For an ambiguous multi-value AF (e.g. aNumber=.field left un-subset afterbcftools norm -m),gvl.writedeclines to cache AF (warns; write still succeeds), while streaming reads it live (first value). Documented as a normalize-first requirement (docs/source/dataset.md); a symmetric streaming decline is tracked in streaming: symmetric decline for multi-value INFO/AF (streaming vs written parity) #324.cargo tree -i libdeflate-syscheck documented at the genoray dep inCargo.toml.with_settingskwargs;api.md/__all__unchanged.Verification
cargo test: 0 failed.ruff/pyreflyclean. SVAR1 + VCF/BCF (incl. multiallelic) byte-parity gates green.Follow-ups
INFO/AF(streaming currently reads first value; written declines). Documented as a normalize-first limitation..bcftwin fixture for the VCF AF parity test.Closes #317, #319. Relates to #304. Design:
docs/superpowers/specs/2026-07-21-streaming-variants-min-max-af-b2-design.md.🤖 Generated with Claude Code