Skip to content

test(codec): read stats as fgbio KV metrics in pre-group-filter test - #585

Merged
nh13 merged 1 commit into
mainfrom
nh/fix-codec-stats-kv-test
Jul 14, 2026
Merged

nh13 merged 1 commit into
mainfrom
nh/fix-codec-stats-kv-test

Conversation

@nh13

@nh13 nh13 commented Jul 14, 2026 •

Copy link
Copy Markdown
Member

test_codec_allow_unmapped_gates_pregroup_filter parsed the codec stats file as a wide ConsensusMetrics TSV, but codec emits fgbio's vertical key-value format (ConsensusKvMetric). The header mismatch made read_tsv fail with Error parsing delimited data file header in all four cases (fast-path/threaded × default/allow-unmapped), failing test and coverage on main.

This was a semantic merge conflict, not a code regression: #513 switched codec stats to the KV format, then #509 (merged after) added this test against the old wide layout. Both merged cleanly as text, so the break only surfaced on the merged main tip — which is why the release PR (#454) and branches rebased on it (#519, #527) all inherit the same four failing cases.

The fix reads the rows back as ConsensusKvMetric and looks up the counts by key (raw_reads_considered, consensus_reads_emitted), mirroring the sibling simplex tests. The assertions' intent is unchanged: the supplementary alignment is filtered before grouping (2 raw reads considered) and the single FR molecule is emitted (1 consensus read).

Verified locally: all four cases pass; full codec module green; fmt and clippy --all-targets -D warnings clean.

Summary by CodeRabbit

  • Tests
    • Updated CODEC integration test coverage to validate the current --stats tab-separated output format.
    • Added checks for raw reads considered and consensus reads emitted statistics.

test_codec_allow_unmapped_gates_pregroup_filter parsed the codec stats
file as a wide ConsensusMetrics TSV, but codec emits fgbio's vertical
key-value format (ConsensusKvMetric). The header mismatch made read_tsv
fail with "Error parsing delimited data file header" in all four cases.

The break was a semantic merge conflict: #513 switched codec stats to the
KV format, then #509 (merged after) added this test using the old wide
layout. Both merged cleanly textually, so the failure only surfaced on the
merged main tip.

Read the rows back as ConsensusKvMetric and look up the counts by key
(raw_reads_considered, consensus_reads_emitted), mirroring the sibling
simplex tests. The assertions' intent is unchanged: the supplementary
alignment is filtered before grouping (2 raw reads considered) and the
single FR molecule is emitted (1 consensus read).
@nh13
nh13 temporarily deployed to github-actions July 14, 2026 14:39 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Jul 14, 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: dbcc4ef8-2fc8-4428-8d56-c5a0af7730eb

📥 Commits

Reviewing files that changed from the base of the PR and between 87b7607 and 0a84e98.

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

Walkthrough

The codec integration test now parses --stats output as fgbio-style seeded key-value metrics and verifies raw_reads_considered and consensus_reads_emitted.

Changes

Codec stats validation

Layer / File(s) Summary
Update codec stats assertions
src/lib/commands/codec.rs
The test imports ConsensusKvMetric, parses the stats TSV into keyed rows, and validates the expected raw-read and consensus counts.

Estimated code review effort: 2 (Simple) | ~5 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 accurately reflects the main change: the codec test now reads stats as fgbio key-value metrics in the pre-group-filter case.
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/fix-codec-stats-kv-test

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

@codecov

codecov Bot commented Jul 14, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.68%. Comparing base (60dd5e7) to head (0a84e98).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #585      +/-   ##
==========================================
- Coverage   92.69%   92.68%   -0.02%     
==========================================
  Files         166      166              
  Lines      100549   100719     +170     
==========================================
+ Hits        93205    93348     +143     
- Misses       7344     7371      +27     

☔ 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 Jul 14, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 14, 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 commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 14, 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 4f74a2c into main Jul 14, 2026
10 checks passed
@nh13
nh13 deleted the nh/fix-codec-stats-kv-test branch July 14, 2026 14:55
@nh13 nh13 mentioned this pull request Jul 14, 2026

This branch was previously deployed

1 inactive deployment
github-actions — 0a84e988 Deployed Jul 14, 2026 by nh13 via coverage #2558
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