Repository navigation
refactor(codec): typed CodecConsensusError replaces substring matching - #342
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR replaces fragile substring matching of codec consensus errors with a typed 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 #342 +/- ##
==========================================
+ Coverage 90.87% 90.90% +0.02%
==========================================
Files 78 78
Lines 50775 50799 +24
==========================================
+ Hits 46143 46178 +35
+ Misses 4632 4621 -11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
982856e to
57e053a
Compare
57e053a to
3878064
Compare
3878064 to
6e76e4e
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/fgumi-consensus/src/codec_caller.rs`:
- Around line 98-106: The doc comment links the wrong method name: update the
reference from CodecConsensusCaller::try_consensus_reads to the actual exposed
method CodecConsensusCaller::consensus_reads_typed (or add an alias if
appropriate) so the documentation link points to the correct symbol and remains
accurate; modify the doc comment above the error enum to mention
CodecConsensusCaller::consensus_reads_typed instead of try_consensus_reads.
In `@src/lib/commands/codec.rs`:
- Around line 399-401: Update the outdated comment references to the old API
name `try_consensus_reads` to the current name `consensus_reads_typed`; locate
the explanatory comments near the `consensus_reads_typed` call in the file (the
blocks around the existing comment that mention distinguishing recoverable
duplex disagreements from failures) and replace the old identifier in both
occurrences so the comment matches the actual function being called.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d7ba61a-857e-43f3-931c-ef176bb7454b
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
crates/fgumi-consensus/Cargo.tomlcrates/fgumi-consensus/src/codec_caller.rscrates/fgumi-consensus/src/lib.rssrc/lib/commands/codec.rssrc/lib/consensus/mod.rstests/integration/test_codec_command.rs
6e76e4e to
0fa1659
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
The two duplex-disagreement reject paths in `src/lib/commands/codec.rs`
discriminated recoverable failures from genuine errors via
`e.to_string().contains("duplex disagreement")`, which silently coupled
recovery behavior to error-message wording in `codec_caller.rs`.
Introduce `CodecConsensusError` with `DuplexDisagreementCount`,
`DuplexDisagreementRate`, and `Other(anyhow::Error)` variants plus an
`is_duplex_disagreement()` helper. Expose `try_consensus_reads` as a
typed inherent method on `CodecConsensusCaller`; keep the
`ConsensusCaller::consensus_reads` trait surface unchanged by wrapping
the typed result via `anyhow::Error::from`. Both codec.rs call sites now
pattern-match on the variant. Reword-resistance is covered by a new test
that asserts the typed variant for the strict-disagreement path.
The single-thread err arm is also extracted to a small
`recover_or_propagate_codec_error` helper so its `Other`-variant
propagation branch is unit-testable — that branch is unreachable from
valid CLI input today (`consensus_reads_raw` only returns `Other` if
`ss_caller.consensus_call` propagates, and the public API gates against
the conditions that trigger it).
Two end-to-end integration tests drive the typed recovery path through
`Codec::run` (single-threaded) and `Codec::execute_threads_mode`
(--threads N) using an offset-position FR fixture whose single-strand
positions register as duplex disagreements; together they cover the
single-thread and parallel-mode lines that the existing test suite did
not exercise.
Audit (issue #338) confirmed these were the only production substring-
based error matches in the workspace; simplex, duplex, overlapping, and
the rest of the pipeline have no equivalent patterns to migrate.
Closes #338
0fa1659 to
3bd1b02
Compare
|
@coderabbitai resume |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
✅ Actions performedReviews resumed. |
|
@coderabbitai resume |
|
@coderabbitai review |
✅ Actions performedReviews resumed. |
✅ Actions performedReview triggered.
|
|
@coderabbitai resume |
|
@coderabbitai review |
✅ Actions performedReviews resumed. |
✅ Actions performedReview triggered.
|
|
@coderabbitai resume |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
✅ Actions performedReviews resumed. |
|
@coderabbitai resume |
|
@coderabbitai review |
✅ Actions performedReviews resumed. |
✅ Actions performedReview triggered.
|
|
@coderabbitai resume |
|
@coderabbitai review |
✅ Actions performedReviews resumed. |
✅ Actions performedReview triggered.
|
This branch was previously deployed
1 participant
Summary
e.to_string().contains("duplex disagreement")atsrc/lib/commands/codec.rs:392, 622, silently coupling reject-vs-error behavior to wording incrates/fgumi-consensus/src/codec_caller.rs:1161, 1164. Simplex, duplex, overlapping, and the rest of the pipeline have no equivalent patterns.CodecConsensusError(thiserror) withDuplexDisagreementCount,DuplexDisagreementRate, andOther(#[from] anyhow::Error)variants plus anis_duplex_disagreement()helper.CodecConsensusCaller::try_consensus_readsas the typed inherent entry point; keep theConsensusCaller::consensus_readstrait surface unchanged by wrapping the typed result viaanyhow::Error::from.#[error(...)]text inCodecConsensusErrorno longer affects recovery behavior.Closes #338
Test plan
cargo buildcleancargo ci-fmtcleancargo ci-lintcleancargo ci-test— 2067 passed, 23 skippedtest_try_consensus_reads_typed_disagreement_errorasserts the typed variant on the strict-disagreement pathtest_not_emit_consensus_high_disagreementstill passes (covers the wrappedanyhow::Resultsurface viaconsensus_reads_from_sam_records)