Skip to content

refactor(consensus): remove dead code and improve efficiency in consensus commands - #154

Merged
nh13 merged 1 commit into
mainfrom
refactor/nh/simplify-fgumi-commands-consensus
Mar 3, 2026
Merged

nh13 merged 1 commit into
mainfrom
refactor/nh/simplify-fgumi-commands-consensus

Conversation

@nh13

@nh13 nh13 commented Mar 3, 2026

Copy link
Copy Markdown
Member

Summary

  • Remove ~115 lines of dead code from consensus_runner.rs (ConsensusResultTuple, ConsensusConfig, ConsensusExecutionContext and associated unused imports)
  • Replace local tag_from_string with library string_to_tag in duplex_metrics.rs and hoist tag lookups before the template loop to avoid per-template recomputation
  • Use ConsensusOutput::merge() instead of extend() in duplex.rs to move data via Vec::append rather than copying
  • Remove unused _mapq1/_mapq2 test parameters and empty impl Interval {} block in duplex_metrics.rs

Test plan

  • All 1838 tests pass (cargo ci-test)
  • Formatting clean (cargo ci-fmt)
  • Linting clean (cargo ci-lint)
  • Net removal of ~258 lines

…nsus commands

Remove ~115 lines of dead code from consensus_runner.rs (ConsensusResultTuple,
ConsensusConfig, ConsensusExecutionContext). Replace tag_from_string with library
string_to_tag in duplex_metrics.rs and hoist tag lookups before the template loop.
Use ConsensusOutput::merge() instead of extend() in duplex.rs to avoid unnecessary
copies. Remove unused test parameters and empty impl block in duplex_metrics.rs.
@nh13
nh13 temporarily deployed to github-actions March 3, 2026 07:01 — with GitHub Actions Inactive
@codecov

codecov Bot commented Mar 3, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.64%. Comparing base (3a460ea) to head (25c5ff6).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #154      +/-   ##
==========================================
+ Coverage   83.61%   83.64%   +0.03%     
==========================================
  Files         126      126              
  Lines       51510    51342     -168     
==========================================
- Hits        43069    42946     -123     
+ Misses       8441     8396      -45     

☔ View full report in Codecov by Sentry.
📢 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.

@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a460ea and 25c5ff6.

📒 Files selected for processing (3)
  • src/commands/consensus_runner.rs
  • src/commands/duplex.rs
  • src/commands/duplex_metrics.rs

📝 Walkthrough

Walkthrough

The changes refactor consensus and metrics handling across three modules. In consensus_runner.rs, public configuration and execution context structs are removed, replaced by a formalized ConsensusStatsOps trait defining merge and to_metrics methods. ConsensusCallingStats and CodecConsensusStats now implement this trait. The duplex.rs module updates ConsensusOutput's aggregation API from extend to merge, consuming owned input. The duplex_metrics.rs module consolidates tag resolution by replacing the tag_from_string helper with direct string_to_tag calls at the start of template processing, reducing redundant lookups.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main changes: removing dead code and improving efficiency in consensus-related commands.
Description check ✅ Passed The description directly relates to the changeset, detailing specific code removals, refactorings, and efficiency improvements across three files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/nh/simplify-fgumi-commands-consensus

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@nh13
nh13 merged commit a17ac3a into main Mar 3, 2026
7 checks passed
@nh13
nh13 deleted the refactor/nh/simplify-fgumi-commands-consensus branch March 3, 2026 07:42
This was referenced Mar 1, 2026

This branch was previously deployed

1 inactive deployment
github-actions — 25c5ff68 Deployed Mar 3, 2026 by nh13 via coverage #518
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