Skip to content

feat: add --includelist option to simulate fastq-reads - #198

Merged
nh13 merged 1 commit into
mainfrom
nh/simulate-fastq-includelist
Mar 30, 2026
Merged

nh13 merged 1 commit into
mainfrom
nh/simulate-fastq-includelist

Conversation

@nh13

@nh13 nh13 commented Mar 28, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds --includelist / -i option to fgumi simulate fastq-reads that samples UMIs from a whitelist file instead of generating them randomly
  • UMI length is inferred from the includelist, overriding --umi-length (with a warning if they differ)
  • Validates includelist format: one UMI per line, all same length, ACGT-only, blank lines skipped, lowercase uppercased

This enables generating synthetic FASTQ data that exercises the full correct → extract → ... → consensus pipeline in fgumi-benchmarks, since reads contain UMIs from a known whitelist that fgumi correct can match against.

Test plan

  • 7 unit tests for load_includelist (valid, lowercase, blank lines, invalid bases, mismatched lengths, empty file)
  • 1 integration test verifying molecule UMIs are sampled exclusively from the includelist
  • Manual smoke test: generated 100 molecules with 5-UMI whitelist, verified all truth UMIs are whitelist combinations
  • Pre-commit hooks pass (ci-fmt, ci-lint)
  • CI

@nh13
nh13 temporarily deployed to github-actions March 28, 2026 16:43 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Mar 28, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5184e645-301b-4dd4-bc51-e1ee5465964c

📥 Commits

Reviewing files that changed from the base of the PR and between 6e250d8 and 23dad2a.

📒 Files selected for processing (1)
  • src/commands/simulate/fastq_reads.rs

📝 Walkthrough

Walkthrough

Adds a new CLI option --includelist/-i to provide a file of UMIs and extends GenerationParams with an optional in-memory includelist. Implements load_includelist to read one UMI per non-blank line, trim/uppercase entries, require only A/C/G/T bases, reject empty lists, and enforce uniform UMI length with line-specific errors. When provided, the includelist's length overrides --umi-length; mismatches warn, and UMI length is validated against read length. During molecule generation UMIs are sampled with replacement from the includelist independently for R1 and R2. Unit tests added for parsing, validation, length checks, and generation sourcing.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding a --includelist option to the simulate fastq-reads command.
Description check ✅ Passed The description is directly related to the changeset, providing clear context on the feature, validation rules, test coverage, and motivation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nh/simulate-fastq-includelist

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@codecov

codecov Bot commented Mar 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.73171% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.00%. Comparing base (7442efd) to head (23dad2a).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/commands/simulate/fastq_reads.rs 90.73% 19 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #198      +/-   ##
==========================================
+ Coverage   86.12%   88.00%   +1.87%     
==========================================
  Files         110      111       +1     
  Lines       51974    52464     +490     
==========================================
+ Hits        44765    46173    +1408     
+ Misses       7209     6291     -918     

☔ View full report in Codecov by Sentry.
📢 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 Mar 28, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 28, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/commands/simulate/fastq_reads.rs`:
- Around line 266-278: When overriding umi_length from includelist (the
includelist variable -> list[0].len()), validate that the includelist UMI length
does not exceed the configured read length (e.g., self.common.read_length) and
reject the request if it does: instead of silently overriding and truncating
later, return an error (or bail) with a clear message referencing the
includelist UMI length and read length; update the same validation logic at the
other includelist-handling site around the second occurrence (the block at
~580-587) so both places enforce list[0].len() <= read_length before assigning
umi_length or using the UMI. Ensure you reference the variables includelist,
umi_length, self.common.umi_length, and self.common.read_length when
implementing the check and message.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7e8cde8d-ac67-435c-8de0-e565b260be94

📥 Commits

Reviewing files that changed from the base of the PR and between 7442efd and 6e250d8.

📒 Files selected for processing (1)
  • src/commands/simulate/fastq_reads.rs

Comment thread src/commands/simulate/fastq_reads.rs
@nh13
nh13 force-pushed the nh/simulate-fastq-includelist branch from 6e250d8 to 37156bb Compare March 28, 2026 23:10
@nh13
nh13 temporarily deployed to github-actions March 28, 2026 23:10 — with GitHub Actions Inactive
When provided, UMIs are sampled from the includelist file instead of
generated randomly. This enables generating synthetic FASTQ data that
exercises the full correct → extract → ... → consensus pipeline, since
the reads will contain UMIs from a known whitelist that fgumi correct
can match against.

The includelist file format is one UMI per line (same as fgumi correct).
All UMIs must be the same length, which overrides --umi-length with a
warning if they differ. Blank lines are skipped, lowercase is uppercased,
and non-ACGT bases are rejected.
@nh13
nh13 force-pushed the nh/simulate-fastq-includelist branch from 37156bb to 23dad2a Compare March 29, 2026 06:57
@nh13
nh13 temporarily deployed to github-actions March 29, 2026 06:58 — with GitHub Actions Inactive
@nh13

nh13 commented Mar 29, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 29, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

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 commented Mar 29, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 29, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

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 merged commit 8232495 into main Mar 30, 2026
7 checks passed
@nh13
nh13 deleted the nh/simulate-fastq-includelist branch March 30, 2026 06:27
@nh13 nh13 mentioned this pull request Mar 30, 2026

This branch was previously deployed

1 inactive deployment
github-actions — 23dad2a2 Deployed Mar 29, 2026 by nh13 via coverage #765
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