Skip to content

fix(runall): wire --methylation-mode/--ref into the fused filter stage - #944

Merged
nh13 merged 1 commit into
mainfrom
nh/runall-filter-methylation-wiring
Sep 11, 2026
Merged

nh13 merged 1 commit into
mainfrom
nh/runall-filter-methylation-wiring

Conversation

@nh13

@nh13 nh13 commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

Summary

fgumi runall's fused filter stage silently dropped the top-level --methylation-mode / --ref, so runall's methylation-aware filter option --filter::min-conversion-fraction was unreachable. build_stage_options_bag's Stage::Filter arm never threaded those flags into the filter stage the way the Simplex/Duplex arms do, so the chain builder's filter.validate_parameters() saw MethylationMode::Disabled and rejected a legitimately-set --methylation-mode — a fused … → filter chain with --methylation-mode em-seq failed with "requires --methylation-mode to be set" even though it was set.

What changed

  • The Stage::Filter arm now injects the resolved methylation_mode and, when methylation is requested, the top-level reference into the projected filter options before validation (mirrors the Simplex/Duplex arms). A user-supplied --filter::ref is preserved when no top-level --ref is given.
  • Filter consumes --methylation-mode only through --filter::min-conversion-fraction (the one filter option whose check reads the resolved mode; --require-strand-methylation-agreement and --min-methylation-depth use the reference/methylation tags but not the mode). So the up-front dead-flag guard treats a filter chain as a methylation consumer only when that option is set — a bare --methylation-mode on a filter chain stays rejected as inert rather than being silently accepted as a no-op.
  • Renamed the methylation-gated reference helper consensus_reference → methylation_reference, now that the filter stage uses it too.

Tests

  • Unit tests for the option-bag injection, the --filter::ref fallback, and both directions of the dead-flag guard (live when --filter::min-conversion-fraction is set; rejected-as-inert otherwise).
  • End-to-end fused consensus → filter parity test that runs a methylation filter through runall and asserts record + header equivalence against the staged standalone simplex + filter chain.

Risk: grouping output: none; consensus output: yes for methylation-aware fused runall, pinned by staged-chain parity tests; sort order: none; corrected UMI output: none; metrics output: none. unsafe: none; CLAUDE.md allowlist: unchanged. Memory bounds, queue capacity, and thread/backpressure policy: none.

  • Threads top-level --methylation-mode and --ref into fused filter stages.
  • Preserves --filter::ref when no top-level reference is set.
  • Rejects unused methylation options.
  • Renames consensus_reference to methylation_reference.
  • Adds unit and integration coverage for option wiring and fused/staged parity.

The Stage::Filter arm of build_stage_options_bag never threaded the
top-level --methylation-mode / --ref into the filter stage the way the
Simplex/Duplex arms do, so runall's methylation-aware filter option
--min-conversion-fraction was unreachable: the chain builder's
filter.validate_parameters() saw MethylationMode::Disabled and rejected a
legitimately-set --methylation-mode.

Thread the resolved methylation mode (and, when methylation is requested,
the top-level reference) into the filter stage before validation. Filter
consumes --methylation-mode only through --filter::min-conversion-fraction
(the sole filter option whose check reads the resolved mode), so the
dead-flag guard treats a filter chain as a methylation consumer only when
that option is set — a bare --methylation-mode on a filter chain stays
rejected as inert rather than silently accepted. The projected
--filter::ref is preserved when no top-level --ref is given. Rename the
methylation-gated reference helper consensus_reference -> methylation_reference
now that filter also uses it.

Add unit tests for the injection, the --filter::ref fallback, and both
directions of the dead-flag guard (consuming vs inert), plus an end-to-end
fused consensus->filter parity test that runs a methylation filter through
runall and matches the staged standalone chain.
@nh13
nh13 deployed to github-actions September 9, 2026 09:26 — with GitHub Actions Active
@coderabbitai

coderabbitai Bot commented Sep 9, 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: 9c98aea3-5669-4c82-a098-76ebae7dff8f

📥 Commits

Reviewing files that changed from the base of the PR and between 1da9603 and 928deb0.

📒 Files selected for processing (2)
  • src/lib/commands/runall.rs
  • tests/integration/test_runall_command.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

Runall now propagates top-level methylation mode and references to filter stages. Validation accepts methylation for conversion-fraction filtering and rejects inert filter-only usage. Tests cover precedence and fused versus staged execution.

Changes

Methylation wiring

Layer / File(s) Summary
Reference propagation
src/lib/commands/runall.rs
The reference helper now supports all methylation-consuming stages. Consensus and filter options receive top-level methylation settings while explicit filter references remain unchanged.
Validation and option-bag tests
src/lib/commands/runall.rs
Validation recognizes --filter::min-conversion-fraction as a methylation consumer. Tests cover active and inert configurations, injection, disabled behavior, and reference precedence.
Fused consensus and filter integration
tests/integration/test_runall_command.rs
The integration test compares fused simplex-consensus filtering with the equivalent staged pipeline. Documentation uses methylation_reference.

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

Sequence Diagram(s)

sequenceDiagram
  participant Runall
  participant SimplexConsensus
  participant Filter
  participant StagedPipeline
  Runall->>SimplexConsensus: apply top-level methylation mode and reference
  SimplexConsensus->>Filter: pass consensus output
  Runall->>Filter: inject top-level methylation options
  Filter-->>Runall: return fused filtered output
  StagedPipeline->>SimplexConsensus: run consensus stage
  StagedPipeline->>Filter: run filter with matching options
  Filter-->>StagedPipeline: return staged filtered output
  Runall->>StagedPipeline: compare records and headers
Loading

Merge Risk: ⚪ Minimal · up to 928de

Runall now forwards top-level methylation settings to fused filter stages while preserving reference precedence and rejecting inert flags. The supplied coverage shows the fused and staged paths agree, with no merge-blocking risk remaining.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title follows the required Conventional Commit format and accurately describes wiring top-level methylation options into the fused filter stage.
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 9, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai pause

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews paused.

@codecov

codecov Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.15966% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.37%. Comparing base (1da9603) to head (928deb0).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/lib/commands/runall.rs 99.15% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #944      +/-   ##
==========================================
- Coverage   94.38%   94.37%   -0.01%     
==========================================
  Files         304      304              
  Lines      150599   150713     +114     
==========================================
+ Hits       142144   142239      +95     
- Misses       8455     8474      +19     

☔ 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 10, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

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 Sep 10, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 10, 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 11, 2026
Merged via the queue into main with commit e27e453 Sep 11, 2026
17 checks passed
@nh13
nh13 deleted the nh/runall-filter-methylation-wiring branch September 11, 2026 08:23
@nh13 nh13 mentioned this pull request Sep 10, 2026

This branch was successfully deployed

1 active deployment
github-actions — 928deb0d Deployed Sep 9, 2026 by nh13 via coverage #4374
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