Skip to content

feat(simulate): add the aligner replay subcommand - #935

Merged
nh13 merged 1 commit into
mainfrom
nh/simulate-aligner
Sep 8, 2026
Merged

nh13 merged 1 commit into
mainfrom
nh/simulate-aligner

Conversation

@nh13

@nh13 nh13 commented Sep 7, 2026 •

Copy link
Copy Markdown
Member

What

Adds fgumi simulate aligner, a fake streaming aligner for benchmarking fgumi runall's align stage. It is invoked as the aligner subprocess via --aligner::command: it drains the interleaved FASTQ on stdin (discarding it) while concurrently streaming a pre-captured aligned BAM (--replay-bam) verbatim to stdout. Reading and writing run on independent threads, so it never deadlocks AlignAndMerge the way a single-threaded lockstep replay (the shell replay-aligner.sh it replaces) does. It performs no real alignment — it replays alignments captured earlier (e.g. by bwa mem), letting a benchmark measure fgumi's own pipeline overhead without a real aligner's compute dominating every run.

Why

The benchmark needs a drop-in for a real streaming aligner that adds as little overhead as possible. The concurrent drain/emit design is exactly what a real aligner does; the single-threaded shell replay it replaces deadlocked the align stage (the writer fills its in-flight gate feeding FASTQ and blocks, while the reader stalls waiting for aligner output the mid-drain shell can't emit).

Details

  • Streams the replay BAM byte-for-byte (header first; every record, including secondary/supplementary, in stored order), so the replay must be in input / queryname-grouped order (as bwa mem emits it), which is how AlignAndMerge regroups records into templates by queryname.
  • Trailing positional tokens (the {ref} / {threads} a command template substitutes) are accepted and ignored, so the command is a drop-in for real-aligner templates. --replay-bam must precede any positional token — documented on the field, in docs/simulate-cli.md, and pinned by a test.
  • The emitter locks stdout once and block-buffers it: the replay is BGZF, dense in newline bytes, so the default line-buffered stdout would split nearly every copy chunk at its last newline and roughly double the write syscall count.
  • The exit paths are documented and each has a dedicated no-hang test: full emit joins the (detached) stdin drainer so the caller finishes feeding FASTQ before exit; a broken stdout pipe and an emit error both return without joining, since joining could otherwise block forever on stdin that never reaches EOF.

Testing

  • Unit tests cover the concurrent drain/emit (including a deadlock reproduction against a single-threaded write-then-read caller over real OS pipes), broken-pipe clean exit, the emit-error-without-join path, verbatim streaming, a full BAM round-trip through the project's BAM reader, and the argument-ordering constraint.
  • A subprocess integration test drives the real binary through OS pipes with payloads larger than a pipe buffer.
  • The integration module is gated on the simulate feature so a non-simulate build compiles it out rather than failing (matching test_simulate_sort.rs).
  • cargo ci-fmt, cargo ci-lint (clippy pedantic), and the aligner unit + integration tests all pass.

Scope

Ported from the feat-runall branch onto main, deliberately limited to the aligner command: the existing delegating region_to_bin is left untouched, the separate simulate sort command is not included, and only the aligner-relevant docs/simulate-cli.md additions are ported.

Risk: output changes only for the new fgumi simulate aligner command, pinned by verbatim replay of the supplied BAM; unsafe changes: none; memory, queue, and thread/backpressure policy: changed through concurrent FASTQ draining and block-buffered BAM output.

Adds fgumi simulate aligner with replay-file validation, trailing argument support, concurrent FASTQ draining, broken-pipe handling, and integration coverage. Updates the CLI documentation and command dispatch.

`fgumi simulate aligner` is a fake streaming aligner for benchmarking
`fgumi runall`'s align stage. Invoked as the aligner subprocess via
`--aligner::command`, it drains the interleaved FASTQ on stdin (discarding
it) while concurrently streaming a pre-captured aligned BAM (`--replay-bam`)
verbatim to stdout on independent threads, so it never deadlocks the align
stage the way a single-threaded lockstep replay does. It does no real
alignment; it replays alignments captured earlier (e.g. by bwa mem).

The emitter locks stdout once and block-buffers it: the replay is BGZF,
dense in newline bytes, and the default line-buffered stdout would split
nearly every copy chunk at its last newline, roughly doubling the write
syscall count in a tool whose whole purpose is to add minimal overhead.

Wires the command into SimulateCommand, documents it in docs/simulate-cli.md
(including the --replay-bam-before-positionals ordering constraint), and adds
unit plus subprocess integration coverage of the concurrent drain/emit,
broken-pipe, and error paths. The integration module is gated on the
simulate feature so a non-simulate build compiles it out rather than failing.
@nh13
nh13 deployed to github-actions September 7, 2026 11:10 — with GitHub Actions Active
@coderabbitai

coderabbitai Bot commented Sep 7, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

Note

Reviews paused

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: f22bc5c7-a1c7-44e1-9faa-428a34d813fe

📥 Commits

Reviewing files that changed from the base of the PR and between 215ef16 and 3fa2d82.

📒 Files selected for processing (5)
  • docs/simulate-cli.md
  • src/lib/commands/simulate/aligner.rs
  • src/lib/commands/simulate/mod.rs
  • tests/integration/main.rs
  • tests/integration/test_simulate_aligner.rs

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.


Walkthrough

Adds fgumi simulate aligner. The command drains FASTQ input concurrently and replays a captured BAM verbatim to stdout. It includes CLI dispatch, documentation, unit tests, and subprocess integration tests.

Changes

Simulated aligner command

Layer / File(s) Summary
CLI contract and dispatch
src/lib/commands/simulate/aligner.rs, src/lib/commands/simulate/mod.rs, docs/simulate-cli.md
Adds --replay-bam, accepts ignored trailing positional arguments, registers command dispatch, and documents ordering and replay behavior.
Concurrent replay implementation and validation
src/lib/commands/simulate/aligner.rs
Drains FASTQ input on a worker while buffered output replays BAM bytes. Tests cover backpressure, broken pipes, prompt errors, and BAM round-tripping.
Subprocess integration tests
tests/integration/main.rs, tests/integration/test_simulate_aligner.rs
Adds feature-gated tests for large concurrent streams, verbatim output, extra arguments, watchdog handling, and missing replay files.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~30 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 3fa2d

This adds a simulated aligner that concurrently drains FASTQ input while replaying BAM output for benchmarking. Its intended streaming, error, and argument-handling behavior is covered, with no remaining merge-blocking risk identified.

Sequence Diagram(s)

sequenceDiagram
  participant SimulateCommand
  participant Aligner
  participant StdinDrainer
  participant Stdout
  SimulateCommand->>Aligner: execute replay command
  Aligner->>StdinDrainer: drain FASTQ stdin concurrently
  Aligner->>Stdout: replay BAM bytes
  Stdout-->>Aligner: flush or report pipe result
  Aligner->>StdinDrainer: join after successful output
Loading

Possibly related PRs

  • fulcrumgenomics/fgumi#438: Adds the same simulated aligner command and related implementation, dispatch, documentation, and tests.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title uses the required Conventional Commit format. It accurately describes the addition of the simulate aligner replay subcommand. The description is lowercase, imperative, and has no trailing …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@nh13

nh13 commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai pause

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews paused.

@codecov

codecov Bot commented Sep 7, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.72611% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.06%. Comparing base (215ef16) to head (3fa2d82).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/lib/commands/simulate/aligner.rs 98.71% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #935      +/-   ##
==========================================
- Coverage   94.06%   94.06%   -0.01%     
==========================================
  Files         302      303       +1     
  Lines      152561   152718     +157     
==========================================
+ Hits       143503   143648     +145     
- Misses       9058     9070      +12     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nh13

nh13 commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 8, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nh13
nh13 added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 1da9603 Sep 8, 2026
17 checks passed
@nh13
nh13 deleted the nh/simulate-aligner branch September 8, 2026 22:20
@nh13 nh13 mentioned this pull request Sep 8, 2026

This branch was successfully deployed

1 active deployment
github-actions — 3fa2d82e Deployed Sep 7, 2026 by nh13 via coverage #4297
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant