Skip to content

docs(codec): document the CODEC model and its quality-masking options - #616

Merged
nh13 merged 1 commit into
mainfrom
nh/codec-sam-hint-and-long-about
Jul 22, 2026
Merged

nh13 merged 1 commit into
mainfrom
nh/codec-sam-hint-and-long-about

Conversation

@nh13

@nh13 nh13 commented Jul 21, 2026 •

Copy link
Copy Markdown
Member

fgumi codec --help had long_about = None, so it showed only the one-line about. Nothing explained CODEC's defining property — that both strands of a source duplex molecule arrive in a single read pair, R1 carrying one strand and R2 the other, so duplex evidence comes from comparing the two reads rather than from grouping reads across the file the way duplex does. That is the thing a user needs in order to know when this command applies.

Nor was anything documented about --single-strand-qual, --outer-bases-qual / --outer-bases-length, --min-duplex-length, or the two --max-duplex-disagreement* flags — several of which mask quality rather than discard bases, which is not guessable from the flag name. The same hole appeared in the generated Tool Reference.

The new text also notes that --methylation-mode is unsupported for CODEC. I verified that rather than taking it on faith: the flag is absent from the command entirely.

What this PR no longer does

An earlier revision also added an error hint to codec, simplex and duplex telling users to pass --threads when the input is uncompressed SAM. That hint was wrong, and it has been dropped along with the three tests that asserted it.

Checked against the built binary on main: --threads does not select a SAM-capable reader. Both paths fail identically, because create_bam_reader_for_pipeline_with_opts reads its header through noodles_bgzf::io::Reader exactly as create_raw_bam_reader_with_opts does, and there is no SAM text path anywhere in fgumi-bam-io:

$ fgumi simplex -i in.sam -o out.bam --min-reads 1
Error: Failed to read header from: in.sam
Caused by:  invalid BGZF header

$ fgumi simplex -i in.sam -o out.bam --min-reads 1 --threads 4
Error: Failed to read header from: in.sam
Caused by:  invalid BGZF header

The wording came from upstream, where ReadSamChunks/ParseSamChunk genuinely exist — that capability did not survive the port to main, so the hint would have sent users to a flag that reproduces the identical error. That is strictly worse than the bare error, which at least already names the file and says invalid BGZF header.

The real fix is #642: every command that accepts BAM must accept SAM. That is implemented in a separate PR rather than papered over with a message here.

Removing the hint also restores three helper functions' doc comments, which the deleted tests had been inserted in front of (so e.g. /// Helper to create a Codec with specified input/output paths had ended up documenting a test).

cargo check --all-targets passes; the change is now confined to codec.rs.

Summary by CodeRabbit

  • Documentation
    • Expanded the fgumi codec command help text with detailed guidance on CODEC data assumptions.
    • Added documentation for quality masking and duplex agreement filters.

@nh13
nh13 temporarily deployed to github-actions July 21, 2026 01:45 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Jul 21, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c3aed1af-e40b-4763-b278-83cb5d780a98

📥 Commits

Reviewing files that changed from the base of the PR and between aced591 and af75797.

📒 Files selected for processing (1)
  • src/lib/commands/codec.rs

Walkthrough

The fgumi codec command now exposes detailed long-form help describing CODEC data assumptions, quality masking, and duplex agreement filter options. No runtime behavior or public declarations changed.

Changes

Codec command help

Layer / File(s) Summary
Codec help documentation
src/lib/commands/codec.rs
The long_about metadata now contains multi-line documentation for CODEC data interpretation and filtering options.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: expanded CODEC documentation and quality-masking options in the CLI help.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ 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/codec-sam-hint-and-long-about

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

@codecov

codecov Bot commented Jul 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.51%. Comparing base (f3b0c78) to head (af75797).
⚠️ Report is 18 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #616      +/-   ##
==========================================
+ Coverage   93.42%   93.51%   +0.08%     
==========================================
  Files         175      175              
  Lines      104807   105739     +932     
==========================================
+ Hits        97919    98880     +961     
+ Misses       6888     6859      -29     

☔ 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 force-pushed the nh/codec-sam-hint-and-long-about branch from f621649 to aced591 Compare July 21, 2026 02:06
@nh13
nh13 temporarily deployed to github-actions July 21, 2026 02:06 — with GitHub Actions Inactive
@nh13

nh13 commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 22, 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.

`fgumi codec --help` had `long_about = None`, so it showed only the
one-line about. Nothing explained CODEC's defining property -- that both
strands of a source duplex molecule arrive in a single read pair, R1
carrying one strand and R2 the other, so duplex evidence comes from
comparing the two reads rather than from grouping reads across the file
the way `duplex` does. That is what a user needs in order to know when
this command applies.

Nor was anything documented about --single-strand-qual, --outer-bases-qual
/ --outer-bases-length, --min-duplex-length, or the two
--max-duplex-disagreement* flags -- several of which mask quality rather
than discard bases, which is not guessable from the flag name. The same
hole appeared in the generated Tool Reference.

The new text also notes that --methylation-mode is unsupported for CODEC;
the flag is absent from the command entirely.
@nh13
nh13 force-pushed the nh/codec-sam-hint-and-long-about branch from aced591 to af75797 Compare July 22, 2026 20:14
@nh13
nh13 temporarily deployed to github-actions July 22, 2026 20:14 — with GitHub Actions Inactive
@nh13 nh13 changed the title docs(codec): document the CODEC model and point SAM input at --threads docs(codec): document the CODEC model and its quality-masking options Jul 22, 2026
@nh13

nh13 commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 22, 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 merged commit 2e235b8 into main Jul 22, 2026
24 of 25 checks passed
@nh13
nh13 deleted the nh/codec-sam-hint-and-long-about branch July 22, 2026 22:35
@nh13 nh13 mentioned this pull request Jul 22, 2026

This branch was previously deployed

1 inactive deployment
github-actions — af757978 Deployed Jul 22, 2026 by nh13 via coverage #2908
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