Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions .github/workflows/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,20 @@ jobs:
uses: taiki-e/install-action@0631aa6515c7d545823c67cfae7ef4fc7f490154 # v2.81.8
with:
tool: nextest
# Install bwa + bwa-mem3 from bioconda for the AAM
# (--start-from align-and-merge) integration tests. Local dev
# without these binaries gets gracefully-skipped tests via
# `which::which` checks in the test code; CI installs them so
# the real-aligner parity tests actually exercise the chain.
- name: Install aligners (bwa-mem3, bwa) from bioconda
# Install bwa + bwa-mem3 + samtools from bioconda. bwa / bwa-mem3 back the
# AAM (--start-from align-and-merge) parity tests (still `#[ignore]`'d, run
# explicitly below). samtools backs the sort-correctness oracle: the
# queryname-natural / template-coordinate order checks cross-reference
# `samtools sort -n` / `samtools sort --template-coordinate`, and the
# sort --write-index region-query tests. Those are NOT `#[ignore]`'d — they
# run in the default `cargo ci-test` step below and are gated only by a
# runtime `which samtools` check, so installing samtools here makes them
# actually exercise the independent order oracle in CI (X3-005).
#
# samtools is pinned because it is the oracle: an unpinned bioconda solve
# could silently change sort-order semantics with no repo change. Bump the
# pin deliberately (and re-validate the parity expectations) when updating.
- name: Install bioconda tools (bwa-mem3, bwa, samtools)
uses: mamba-org/setup-micromamba@06375d89d211a1232ef63355742e9e2e564bc7f7 # v2.0.7
with:
micromamba-version: '2.0.5-0'
Expand All @@ -46,16 +54,19 @@ jobs:
-c bioconda -c conda-forge
bwa
bwa-mem3
samtools=1.23.1
init-shell: bash
cache-environment: true
cache-downloads: true
- name: Verify aligner binaries on PATH
- name: Verify bioconda binaries on PATH
shell: bash -el {0}
run: |
which bwa
which bwa-mem3
which samtools
bwa 2>&1 | head -3 || true
bwa-mem3 2>&1 | head -3 || true
samtools --version 2>&1 | head -2 || true
- name: Unit tests
shell: bash -el {0}
run: cargo ci-test
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ docs/design/*
# Tracked: cited from read_ahead.rs (S3-015 deferral); must be committed so the
# code's PERF NOTE citation does not dangle.
!docs/design/sort-queryname-arena-deferral.md
# Tracked: the simulate-aligner replay design doc (committed alongside the
# `simulate aligner` subcommand).
!docs/design/2026-06-18-simulate-aligner.md
scripts/bench-*.sh
scripts/parity-*.sh
/TRACKER.md
Expand Down
7 changes: 5 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file.

- [**breaking**] `fgumi runall` no longer accepts the `--raw-tag`/`-t` and `--assign-tag`/`-T` flags. They were validated but never propagated into any stage — group reads the UMI from `RX` and emits `MI`, and the consensus callers read `MI`, all via shared constants — so the flags were silent no-ops that misleadingly implied configurability. Removing them frees the `-t`/`-T` short flags. There is no replacement: the UMI/assign tags are fixed (`RX`/`MI`) across the fused chain.
- [**breaking**] `fgumi zipper`'s `--skip-pa-tags` flag (and `runall`'s `--zipper::skip-pa-tags`) is renamed to `--skip-tc-tags`. The flag never controlled a `pa` tag — it gates the `TC` (template-coordinate) tag added to secondary/supplementary reads — so the old name and help were misleading. Behavior is unchanged; update any scripts passing `--skip-pa-tags`.
- [**breaking**] The separate `simplex`, `duplex`, and `codec` Cargo features on the root `fgumi` crate are collapsed into a single default-on `consensus` umbrella feature, which gates all consensus calling (the `simplex`, `duplex`, `codec`, `runall`, `simplex-metrics`, and `duplex-metrics` subcommands). Default builds are unaffected. Embedders that built with a subset of the old features should use `--features consensus` (or `--no-default-features` to drop all consensus code); the granular `simplex`/`duplex`/`codec` features still exist one level down in the `fgumi-consensus` crate for reduced library builds.

- `fgumi sort --write-index --order coordinate` BAM output bytes now match the corresponding `--write-index`-off run for the same input — both paths now share the same writer backend. Pre-`#330` Phase 4 the `--write-index` path used a *different* writer backend (the in-sort indexer wrote through noodles' `MultithreadedWriter` so it could track per-record virtual offsets during write) than the off path (`PooledBamWriter`); the two backends emit different BGZF block boundaries, so the BAMs diverged byte-for-byte even though both were multi-threaded. The indexer now runs as a post-write pass via `IndexBamFinalizeHook`, so the `--write-index`-on and -off runs go through the identical `PooledBamWriter` path. BAI content (record-set returned by samtools region queries) is unchanged; only the BAM's BGZF block layout differs from prior releases.
- `fgumi extract` output BAM block layout may differ from prior releases. When `--threads` is unset, the typed-step framework's BGZF compression schedule produces different block boundaries than the legacy `process_singlethreaded` path. Record content (queryname + sequence + quality + tags) is unchanged.
Expand All @@ -21,7 +22,9 @@ All notable changes to this project will be documented in this file.

### Features

- `fgumi runall` now supports `--start-from extract` so users can run FASTQ-to-consensus in a single fused invocation without intermediate BAM files.
- New `fgumi runall` command: runs the full FASTQ/BAM-to-consensus pipeline as a single fused invocation, streaming the stages together (no intermediate stage BAMs on disk — the in-pipeline sort may still spill temporary chunks under memory pressure). Use `--start-from`/`--stop-after` to run a sub-range of stages; per-stage tuning is exposed as prefixed flags (`--sort::max-memory`, `--group::strategy`, …). With `--start-from extract` it runs FASTQ-to-consensus end to end without intermediate stage BAM files.
- New `fgumi simulate aligner` subcommand (gated behind the `simulate` feature): a fake streaming replay aligner that replays a pre-recorded BAM, for benchmarking the `runall` align-and-merge chain without a real aligner.
- The intermediate `fgumi sort` stage now runs as an in-pipeline streaming sort within `runall` (streaming, not file-to-file), and the standalone `fgumi sort` command is re-homed into the new `fgumi-sort-cli` crate (`fgumi sort`'s CLI surface is unchanged).

### Bug Fixes

Expand All @@ -30,7 +33,7 @@ All notable changes to this project will be documented in this file.

### Refactor

- Extracted the sort engine into the new `fgumi-sort` crate and the BAM-pipeline I/O layer into the new `fgumi-bam-io` crate. The main `fgumi` binary now consumes both as workspace dependencies; behavior is unchanged.
- Extracted the typed-step pipeline engine into the new `fgumi-pipeline-core` crate, the BAM-pipeline I/O layer into `fgumi-pipeline-io`, shared CLI plumbing into `fgumi-cli-common`, the multi-options proc-macro into `fgumi-cli-macros`, and the standalone sort CLI into `fgumi-sort-cli`. The main `fgumi` binary consumes these as workspace dependencies; behavior is unchanged.
- Unified chain-builder refactor (`#330`): the typed-step pipeline framework is the single execution path for all multi-stage commands. `fgumi runall`'s 15 fused dispatchers collapse into a declarative `ChainSpec` consumed by `chains::build_for`; `fgumi sort --write-index` lifts to a post-pipeline `IndexBamFinalizeHook` keyed off `SinkSpec::BamWithIndex` (see Behavioural changes above for the user-visible BAM-layout consequence). Internal-only otherwise.
- `fgumi extract` migrated onto the typed-step `chains::build_for` framework. The 5462-LOC custom FASTQ-pipeline framework (`unified_pipeline/fastq.rs`) is deleted; `Extract::execute` collapses to a ~25-line `ChainSpec` construction. Net: −3879 LOC.

Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ Commands implement the `Command` trait dispatched via enum:
- **Thread Pooling:** Work-stealing with per-command thread optimization
- **2-bit Encoding:** DNA bases packed efficiently for fast operations
- **Typed-Step Pipeline Framework (`fgumi_pipeline_core`, re-exported as `pipeline::core`):** All multi-threaded commands run through the typed-step framework (`Pipeline::builder().chain(step1).chain(step2).…build().run(...)`). The execution engine lives in the `fgumi-pipeline-core` crate (`crates/fgumi-pipeline-core/`) and is re-exported as `crate::pipeline::core`; the `chains` and `steps` layers remain in the `fgumi` crate. Steps declare `StepKind` (Serial/Parallel/Exclusive), input/output handle types, and an `on_input_drained` callback for end-of-stream cleanup. The `--use-new-pipeline` flag and the legacy `run_bam_pipeline_from_reader{,_with_mi_assign}` drivers were removed in Phase 1 of issue #330; the typed-step framework is the execution mechanism for sort, group, simplex, duplex, codec, correct, zipper, clip, filter, and dedup. New commands should follow the typed-step pattern.
- **Chain-builder façade (`unified_pipeline::chains`):** Phase 2 introduced a single declarative chain-construction entry point — `chains::build_for(spec) -> Result<BuiltPipeline>`. Phase 3 introduced the stage-by-stage `ChainBuilder`: `build_for` validates the spec, then constructs a `ChainBuilder`, calls `add_source()`, walks `spec.stages` calling `chain.add_stage(stage, position)` for each, and finishes with `add_sink()` + `build()`. Each `add_<stage>` method (one per `Stage` variant — `add_dedup`, `add_filter`, `add_clip`, `add_sort`, `add_group`, `add_simplex`, `add_duplex`, `add_codec`, `add_correct`, `add_zipper`, `add_align`) reads `spec.stage_opts.<stage>` (already validated present), pushes the canonical step sequence via factories in `chains::commands::<command>::build_*_step`, and registers a `<Command>FinalizeHook`. `StagePosition::{Terminal, Intermediate}` gates the serialize step so intermediate stages leave their typed output for the next `add_<stage>`. Shared config wiring (threads, deadlock_timeout, queue_memory, pipeline stats) lives in `chains::build_helpers::build_pipeline_config_for_chain`. The chain-level `StageTimingFinalizeHook` is registered by `ChainBuilder::build()`; `PipelineStatsFinalizeHook` (gated on `--pipeline-stats`) is the next hook in order. `BuiltPipeline::run()` executes the pipeline then drains hooks in registration order. **New commands MUST add: one `Stage` variant, one bag slot, one validator entry, one `add_<stage>` method, and one factory per per-stage step. Do NOT construct chains inline in command `execute` methods.** `runall::execute` routes every chain shape — including chains with an intermediate Sort — through `build_for`; `ChainBuilder::add_sort` performs the intermediate sort in-pipeline (streaming, not file-to-file), so there is no file-to-file fallback dispatcher in `commands::runall`.
- **Chain-builder façade (`pipeline::chains`):** Phase 2 introduced a single declarative chain-construction entry point — `chains::build_for(spec) -> Result<BuiltPipeline>`. Phase 3 introduced the stage-by-stage `ChainBuilder`: `build_for` validates the spec, then constructs a `ChainBuilder`, calls `add_source()`, walks `spec.stages` calling `chain.add_stage(stage, position)` for each, and finishes with `add_sink()` + `build()`. Each `add_<stage>` method (one per `Stage` variant — `add_dedup`, `add_filter`, `add_clip`, `add_sort`, `add_group`, `add_simplex`, `add_duplex`, `add_codec`, `add_correct`, `add_zipper`, `add_align`, `add_extract`) reads `spec.stage_opts.<stage>` (already validated present), pushes the canonical step sequence via factories in `chains::commands::<command>::build_*_step`, and registers a `<Command>FinalizeHook`. `StagePosition::{Terminal, Intermediate}` gates the serialize step so intermediate stages leave their typed output for the next `add_<stage>`. Shared config wiring (threads, deadlock_timeout, queue_memory, pipeline stats) lives in `chains::build_helpers::build_pipeline_config_for_chain`. The chain-level `StageTimingFinalizeHook` is registered by `ChainBuilder::build()`; `PipelineStatsFinalizeHook` (gated on `--pipeline-stats`) is the next hook in order. `BuiltPipeline::run()` executes the pipeline then drains hooks in registration order. **New commands MUST add: one `Stage` variant, one bag slot, one validator entry, one `add_<stage>` method, and one factory per per-stage step. Do NOT construct chains inline in command `execute` methods.** `runall::execute` routes every chain shape — including chains with an intermediate Sort — through `build_for`; `ChainBuilder::add_sort` performs the intermediate sort in-pipeline (streaming, not file-to-file), so there is no file-to-file fallback dispatcher in `commands::runall`.
- **Multi-Options Macro (`crates/fgumi-cli-macros`):** Per-stage tuning options used by both a standalone command (`fgumi sort`, `fgumi group`, …) and the fused `fgumi runall` command live in a single `<Stage>Options` struct annotated with `#[multi_options("stage", "Help Heading")]`. The standalone command flattens `<Stage>Options` directly so its CLI surface is unchanged (`--max-memory`, `--strategy`, …). The proc-macro generates a sibling `Multi<Stage>Options` struct that runall flattens, exposing the same fields as prefixed `--<stage>::<flag>` flags (`--sort::max-memory`, `--group::strategy`, …) grouped under a `--help` heading. The Multi struct carries a `validate(self) -> Result<<Stage>Options>` method that runall calls before executing a stage; required-without-default fields (e.g. `--group::strategy`) become `Option<T>` on the Multi side and the validator surfaces a clear "required when `<stage>` is selected" error if missing. The convention is established in `commands::{sort,group,duplex,codec}` and is the way to expose new per-stage options on runall going forward.

## Development Practices
Expand Down
Loading
Loading