docs: clear fgbio-parity doc tail across commands (W11) - #558
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
WalkthroughDocumentation-only changes update CLI help and module descriptions across commands, clarifying defaults, execution constraints, input formats, and behavioral differences from fgbio and Picard. No runtime logic, option wiring, or public declarations changed. ChangesCLI documentation alignment
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #558 +/- ##
=======================================
Coverage 92.84% 92.84%
=======================================
Files 166 166
Lines 102064 102064
=======================================
+ Hits 94765 94766 +1
+ Misses 7299 7298 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Corrects help text, long_about, and shared-option doc comments to match actual fgumi behavior and to document intentional divergences from fgbio/Picard. Documentation-only; no runtime, metric, or test changes. Findings addressed: - clip (CLIP3-06): long_about said "By default soft clipping is performed"; the actual default clipping mode is hard. - clip (CLIP3-07): note that --metrics is produced only by the single-threaded path and cannot be combined with --threads. - filter (FILT3-07): document that --min-base-quality is optional in fgumi (required in fgbio) and that omitting it performs no per-base quality masking. - group (GRP3-02): remove the stale "duplicate marking mode" note on --min-map-q (dedup handles marking); state the default is 1. - group (GRP3-07): document that --threads sizes the whole pipeline in fgumi, unlike fgbio where it sizes only UMI-comparison threads. - zipper (ZIP3-07, ZIP3-12): document comma-delimited tag lists (vs fgbio space-separated) and the silent skip of absent/unsupported tags. - downsample (DOWN3-01/02/03): document that fgumi downsample is a UMI-family sampler by design, not a Picard DownsampleSam port (sampling unit, decision function, determinism, fraction bound). - simplex (SIMPLEX3-02): note --max-reads downsampling uses a different PRNG than fgbio, so surviving reads are not bit-identical. - consensus shared options (SIMPLEX3-05, DUPLEX3-08): document --min-consensus-base-quality as an fgumi superset (default 2 = fgbio) and that --output-per-base-tags=false drops tags fgbio always writes. - duplex (DUPLEX3-09): document comma-delimited --min-reads. - codec (CODEC3-09): map --min-reads/--max-reads to fgbio's --min-read-pairs/--max-read-pairs. - duplex-metrics (DXM3-08, DXM3-09): document that templates missing the physical R2 record are skipped, and the "Sample" --description default. - simplex-metrics (SIMM3-04, SIMM3-05): document the "Sample" --description default and BED+Picard interval auto-detection.
c25d71c to
da72c12
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
W11 of the final-audit burn-down: the docs / wontfix sweep. This is a documentation-only PR that clears the low-value fgbio/Picard-parity doc tail — help text,
long_about, and shared-option doc comments — with no runtime, metric, or test changes. Each correction was verified against the actual code and the per-command audit files underreports/final-audit/.Doc changes (finding → file → what changed)
commands/clip.rslong_aboutsaid "By default soft clipping is performed"; the actual default clipping mode ishard. Corrected.commands/clip.rs--metricsis produced only by the single-threaded path and cannot be combined with--threads.commands/filter.rs--min-base-qualityis optional in fgumi (required in fgbio) and that omitting it performs no per-base quality masking.commands/group.rs--min-map-q(fgumi has no duplicate-marking mode ingroup;deduphandles it); state the default is 1.commands/group.rs--threadssizes the whole read/assign/write pipeline in fgumi, unlike fgbio where it sizes only the UMI-comparison worker pool.commands/zipper.rsZipperBams.commands/zipper.rscommands/downsample.rsdownsampleis a UMI-family sampler by design, not a PicardDownsampleSamport: different sampling unit (MI family vs read template), order-dependent per-family draw vs stateless name-hash, non-deterministic without--seed(vs Picard's fixed seed 1), and--fractionrestricted to(0.0, 1.0].commands/simplex.rs--max-readsdownsampling uses a different PRNG than fgbio's ScalaRandom, so surviving reads (and the consensus of a downsampled family) are not bit-identical to fgbio (deterministic within fgumi).commands/common.rs--min-consensus-base-qualityas an fgumi superset: default 2 matches fgbio, which hardcodes the minimum toMIN_PHRED(2) and defers masking tofilter.commands/common.rs--output-per-base-tagsdefault (true) matches fgbio and that setting it false drops per-base tags fgbio writes unconditionally.commands/duplex.rs--min-readsis comma-delimited in fgumi vs space-separated in fgbio.commands/codec.rs--min-reads/--max-readsto fgbio's--min-read-pairs/--max-read-pairsin the flag help.commands/duplex_metrics.rscommands/duplex_metrics.rs"Sample"default for--description(fgbio derives sample/library from the@RGheader, so plot titles differ unless set).commands/simplex_metrics.rs"Sample"default for--description(as DXM3-09).commands/simplex_metrics.rsExcluded / skipped (not doc-only, or wrong base)
These findings from the same audit files were not touched here because they are behavior changes, wrong-base, or already resolved:
src/lib/commands/runall.rson thefeat-runallbranch, notmain. Different base; excluded.lexicographic→lexicographical) — changes an emitted@HD SSheader value and a test that pins it (a behavior/round-trip change, marked "discuss"). Excluded; needs a disposition.cc > aberror-rate rejection) — the described over-restriction is not present in currentmain:validate_parametersonly enforcesab <= ba, which already matches fgbio. No change needed.am/bmmethylation MM-strings unclassified) — disposition is "verify EM-seq duplex need"; adding these tags to a transform set would be a behavior change. Excluded pending verification.cD/cM, base-case sensitivity flip, single-input quality clamp), all unreachable under defaults, with no natural user-facing help surface. Noted, not changed.--cell-tag; disposition "likely wontfix (opinionated standard-tag design)." Not a help-text correction.--min-consensus-base-qualityfor duplex) — disposition "remove or wire" (behavior). Excluded.--sort-orderoutput re-sort (behavior/discuss) and MT chain string-vs-typed error match (code fix). Excluded.Wontfix / not-a-gap set (recorded so it stops re-surfacing)
fgumi is correct or intentionally divergent in each of these; no change is warranted:
--umi-tag/--cell-tag/--sort, repurposed short flags); intentional opinionated standard-tags-only design; defaults keep output correct.Nonewhere fgbio emits an empty-stringRX; fgumi is arguably more correct./1,/2appended by default (unless-n): diverges from Picard but matches samtools; sensible for the pipe-to-aligner use.--skip-tc-tags); not an fgbio bug.seq_len > 0and keeps; fgbio drops onNaN. Unreachable for real consensus reads; fgumi safer.SUM_OF_BASE_QUALITIES(fgbio adds mapq); fgumi is a Picard drop-in.N/skip excluded from ref-consumption in the clip loop; deliberate to keep the raw and typed clippers byte-identical. Affects only spliced/RNA reads (rare in UMI workflows).samtools sort --template-coordinate.Testing
cargo ci-fmt— cleancargo ci-lint— clean (featurescompare,simulate,profile-adjacency)cargo ci-test— 2267 passed, 22 skipped, 0 failedcargo test --doc— 34 passed, 0 failedSummary by CodeRabbit