Skip to content

docs(dedup): stop calling the duplication ladder a saturation curve - #980

Merged
nh13 merged 1 commit into
mainfrom
nh/ladder-coordinate-order-docs
Sep 23, 2026
Merged

nh13 merged 1 commit into
mainfrom
nh/ladder-coordinate-order-docs

Conversation

@nh13

@nh13 nh13 commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

The --duplication-ladder help text and the DuplicationLadderMetrics docs called the ladder a saturation curve, which it isn't.

dedup counts templates in coordinate order, and whether a template is a duplicate depends only on the other templates at its position. So the cumulative duplicate fraction after N templates is the duplicate rate of the genome covered so far. It is not the rate you'd see if the library were sequenced to N templates. A saturation curve needs templates in random order, as downsampling gives. A flattening ladder therefore doesn't mean the library is saturated, and its bumps come from regions with different duplicate rates.

This rewords the CLI help, the metric docs and the internal comments. No code changes.

Risk: Command output changes: none; unsafe changes: none, and the CLAUDE.md allowlist is unchanged; memory bounds, queue capacity, and thread/backpressure policy changes: none. Fix: clarify that the duplication ladder shows duplicate rates across templates in coordinate order, not library saturation.

Updated the --duplication-ladder help text, DuplicationLadderMetrics documentation, and internal comments to use “duplication ladder” and describe its cumulative and window fractions. No runtime behavior changes are described.

The ladder counts templates in coordinate order, and a template's duplicate
status is decided by the other templates at its own position. So the
cumulative fraction after N templates is the duplicate rate of the genome
covered so far, not the rate a library sequenced to N templates would show.
A flattening ladder says nothing about library saturation.

Reword the --duplication-ladder help, the DuplicationLadderMetrics docs and
the internal comments to describe what the ladder measures.
@nh13
nh13 deployed to github-actions September 23, 2026 09:29 — with GitHub Actions Active
@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: Repository: fulcrumgenomics/fgumi/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 574b562a-b007-4ef2-829d-527b58edabce

📥 Commits

Reviewing files that changed from the base of the PR and between fb2c977 and 917e5ed.

📒 Files selected for processing (4)
  • crates/fgumi-metrics/src/dedup.rs
  • src/lib/commands/dedup.rs
  • src/lib/pipeline/chains/builder.rs
  • src/lib/pipeline/chains/commands/dedup.rs

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


Walkthrough

Documentation and comments now describe the duplication ladder as coordinate-ordered cumulative duplicate fractions that show variation across genomic regions, not sequencing-depth saturation. Runtime behavior is unchanged.

Changes

Duplication ladder documentation

Layer / File(s) Summary
Update duplication ladder descriptions
crates/fgumi-metrics/src/dedup.rs, src/lib/commands/dedup.rs, src/lib/pipeline/chains/builder.rs, src/lib/pipeline/chains/commands/dedup.rs
Metric documentation and CLI help describe cumulative duplicate fractions across coordinate-ordered templates and clarify that the ladder shows regional variation rather than saturation. Related comments use the same terminology. No executable behavior changes.

Estimated code review effort: 2 (Simple) | ~5 minutes

Suggested labels: documentation

Merge Risk: ⚪ Minimal · up to 917e5

This change clarifies the duplication ladder’s meaning without changing runtime behavior; no actionable merge risk is identified.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title uses valid conventional-commit format with the allowed type docs, the relevant dedup scope, a lowercase imperative description, and no ending period. It accurately describes the document…
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 23, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai pause

@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews paused.

@codecov

codecov Bot commented Sep 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.13%. Comparing base (fb2c977) to head (917e5ed).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #980      +/-   ##
==========================================
- Coverage   96.15%   96.13%   -0.03%     
==========================================
  Files         293      293              
  Lines      147588   147588              
==========================================
- Hits       141919   141879      -40     
- Misses       5669     5709      +40     

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

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 23, 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 23, 2026
Merged via the queue into main with commit c43cda6 Sep 23, 2026
17 checks passed
@nh13
nh13 deleted the nh/ladder-coordinate-order-docs branch September 23, 2026 17:43
@nh13 nh13 mentioned this pull request Sep 18, 2026

This branch was successfully deployed

1 active deployment
github-actions — 917e5ed1 Deployed Sep 23, 2026 by nh13 via coverage #4593
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