fix(consensus): rejects fan-out deadlock and CODEC disagreement data loss (2/6)

[nh13]

Stack: 2 / 6 · base: nh/runall-fix-1-foundation

Two critical-path correctness fixes in the consensus/rejects flow.

• Rejects fan-out deadlock — all-clean batches now emit an empty (zero-byte) rejects block instead of taking the only_a path, keeping the rejects-branch ByItemOrdinal reorder serials dense so the pipeline can't wedge.
• CODEC disagreement data loss — consensus_reads_typed now captures duplex-disagreement source reads into rejected_reads and bumps the previously-dead reject counter before propagating the error, on both the threaded and single-threaded codec paths.

Both land with regression tests (the deadlock test is verified non-vacuous — it wedges with the fix reverted).

Summary by CodeRabbit

• Bug Fixes
  • Improved HDD duplex-disagreement rejection handling: disagreement errors now update HDD rejection statistics and, when reject tracking is enabled, capture the raw rejected inputs for downstream output.
  • Ensured --rejects fan-out always emits a per-batch rejects output (even when empty) across CODEC, duplex, simplex, and correction, preventing pipeline wedging and keeping batch serials dense.
• Tests
  • Expanded integration coverage for CODEC, duplex, simplex, and correction to validate non-wedging behavior and correct, EOF-terminated BGZF --rejects output with byte-identical RecordBufs.
  • Added an integration helper to read BAM records into RecordBuf values for stronger assertions.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b4496c3e-30ee-49c7-8190-77734d92b60b

📥 Commits

Reviewing files that changed from the base of the PR and between 8a852ff and 5749095.

📒 Files selected for processing (10)

• crates/fgumi-consensus/src/codec_caller.rs
• src/lib/pipeline/chains/commands/codec.rs
• src/lib/pipeline/chains/commands/duplex.rs
• src/lib/pipeline/chains/commands/simplex.rs
• src/lib/pipeline/steps/correct/mod.rs
• tests/integration/helpers/assertions.rs
• tests/integration/test_codec_command.rs
• tests/integration/test_correct_command.rs
• tests/integration/test_duplex_command.rs
• tests/integration/test_simplex_command.rs

---

Walkthrough

Reject accounting now records duplex-disagreement rejects before returning typed errors, and clean batches now emit explicit empty rejects blocks across codec, duplex, simplex, and correct paths. Integration tests now validate rejects BAM contents and pipeline completion for these cases.

Changes

Reject-path handling and output density

Layer / File(s)	Summary
HDD reject accounting crates/fgumi-consensus/src/codec_caller.rs	consensus_reads_typed now intercepts duplex-disagreement errors, increments consensus_reads_rejected_hdd, and stores rejected input records when reject tracking is enabled; the typed disagreement test asserts the counter value.
Dense rejects frames src/lib/pipeline/chains/commands/codec.rs, src/lib/pipeline/chains/commands/duplex.rs, src/lib/pipeline/chains/commands/simplex.rs	Codec, duplex, and simplex consensus steps now synthesize an empty rejects DecompressedBlock when a batch produces no rejects, so the rejects branch receives a value for every batch serial.
Correct rejects regression src/lib/pipeline/steps/correct/mod.rs, tests/integration/test_correct_command.rs	correct_step_with_rejects now emits an empty rejects block for clean batches, and the new regression covers the all-clean-leading-batch case while checking rejects stream validity and record identity.
Rejects BAM regressions tests/integration/helpers/assertions.rs, tests/integration/test_codec_command.rs, tests/integration/test_duplex_command.rs, tests/integration/test_simplex_command.rs	The codec, duplex, and simplex integration tests now enable --rejects outputs and assert rejects BAM existence, EOF validity, record counts, and record identity for disagreement and singleton cases.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• fulcrumgenomics/fgumi#342: This change extends the same typed duplex-disagreement path in CodecConsensusCaller::consensus_reads_typed.
• fulcrumgenomics/fgumi#332: Both PRs touch rejects fan-out and secondary-output plumbing in the pipeline commands.

Suggested labels

bug

🚥 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 summarizes the two main fixes: rejects fan-out deadlock and CODEC disagreement data loss.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

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

✨ 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/runall-fix-2-rejects-codec

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (feat-runall@41f9d3c). Learn more about missing BASE report.

Additional details and impacted files

@@              Coverage Diff               @@
##             feat-runall     #459   +/-   ##
==============================================
  Coverage               ?   93.96%           
==============================================
  Files                  ?      109           
  Lines                  ?    48479           
  Branches               ?        0           
==============================================
  Hits                   ?    45551           
  Misses                 ?     2928           
  Partials               ?        0           

☔ 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]

@coderabbitai review

[coderabbitai]

✅ 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]

@coderabbitai review

[coderabbitai]

✅ 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.

[coderabbitai]

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 `@tests/integration/test_codec_command.rs`:
- Around line 580-590: The reject assertion in the integration test only checks
shared QNAMEs, so it can still pass if the rejects list contains two copies of
the same mate. Update the test around the reject_names collection to assert mate
identity using the generated input records and the reject output from
rejects_reader, for example by checking FIRST_SEGMENT and LAST_SEGMENT (or
otherwise matching each reject record back to the exact original R1/R2 input
record). Keep the verification in test_codec_command.rs strong enough to ensure
the two distinct disagreement reads are emitted, not just two records with the
same name.

In `@tests/integration/test_correct_command.rs`:
- Around line 328-343: The reject verification in test_correct_command only
checks QNAME order via observed_order, which can miss mutation of rejected
records. In the test around the rejects BAM readback, keep the generated rej
records and compare each record’s full identity against the BAM output,
including raw fields and tags, not just the read name, so correctness-critical
behavior is asserted in the end-to-end test.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0f30c18a-7613-442a-9f58-925ff2dd68a7

📥 Commits

Reviewing files that changed from the base of the PR and between 5b4d0ad and 2053f99.

📒 Files selected for processing (7)

• crates/fgumi-consensus/src/codec_caller.rs
• src/lib/pipeline/chains/commands/codec.rs
• src/lib/pipeline/chains/commands/duplex.rs
• src/lib/pipeline/chains/commands/simplex.rs
• src/lib/pipeline/steps/correct/mod.rs
• tests/integration/test_codec_command.rs
• tests/integration/test_correct_command.rs

[nh13]

@coderabbitai review

[coderabbitai]

✅ 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]

@coderabbitai review

[coderabbitai]

✅ 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.

[coderabbitai]

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 `@tests/integration/test_duplex_command.rs`:
- Around line 309-324: The reject-path assertion in the duplex integration test
is too weak because it only checks the number of records plus name/flag fields,
so changes to sequence, quality, or tags could slip through. Update the test
around the reject-record verification to compare full RecordBuf identity for the
singleton inputs, using the same pattern as the sibling “correct” assertion and
the existing to_record_buf helper, so the observed rejects are byte-identical to
the expected source records.

In `@tests/integration/test_simplex_command.rs`:
- Around line 207-213: The reject check in test_simplex_command is only
validating the record name, so it can miss corruption in the rejected BAM
payload. Update the assertion around reader.records() to compare the full reject
record(s) against the expected singleton input using the existing
reject/RecordBuf helpers in this test, and verify byte-identical identity
instead of just the QNAME. Use the reader, header, and to_record_buf flow from
this test to locate the change.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2365e51e-3bba-45d3-be38-20ffcf00c902

📥 Commits

Reviewing files that changed from the base of the PR and between 2053f99 and 3754cb2.

📒 Files selected for processing (9)

• crates/fgumi-consensus/src/codec_caller.rs
• src/lib/pipeline/chains/commands/codec.rs
• src/lib/pipeline/chains/commands/duplex.rs
• src/lib/pipeline/chains/commands/simplex.rs
• src/lib/pipeline/steps/correct/mod.rs
• tests/integration/test_codec_command.rs
• tests/integration/test_correct_command.rs
• tests/integration/test_duplex_command.rs
• tests/integration/test_simplex_command.rs

[nh13]

@coderabbitai review

[coderabbitai]

✅ 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.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@tests/integration/test_codec_command.rs`:
- Around line 573-605: The reject validation in the integration test is still
too weak because it only checks QNAME and mate flags, so it can miss changes to
sequence, qualities, CIGAR, or tags. Update the single-threaded and threaded
reject assertions in the test_codec_command test to compare the full observed
reject records against the original pair as RecordBuf values, using the existing
rejects_reader.records() flow as the locator, so the test verifies exact record
identity rather than just count/name/segment bits.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a51a679c-bcbe-47a8-a9c0-6a89ec39b22e

📥 Commits

Reviewing files that changed from the base of the PR and between de084a5 and 8a852ff.

📒 Files selected for processing (10)

• crates/fgumi-consensus/src/codec_caller.rs
• src/lib/pipeline/chains/commands/codec.rs
• src/lib/pipeline/chains/commands/duplex.rs
• src/lib/pipeline/chains/commands/simplex.rs
• src/lib/pipeline/steps/correct/mod.rs
• tests/integration/helpers/assertions.rs
• tests/integration/test_codec_command.rs
• tests/integration/test_correct_command.rs
• tests/integration/test_duplex_command.rs
• tests/integration/test_simplex_command.rs

[nh13]

@coderabbitai review

[coderabbitai]

✅ 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.

[coderabbitai]

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 `@tests/integration/test_duplex_command.rs`:
- Around line 251-260: The doc comment for the duplex regression test is
inconsistent with the actual test setup in test_duplex_command::regression case:
it claims a single large clean duplex molecule forms the leading batch, but the
construction uses many MI molecules and the batcher rationale depends on that.
Update the comment in the test body and surrounding header text so it accurately
describes the many-molecules setup that produces a reject-free leading batch and
preserves the deadlock repro, using the existing duplex --rejects /
ByItemOrdinal / try_pop_in_order context to locate it.

In `@tests/integration/test_simplex_command.rs`:
- Around line 156-162: The doc comment in the simplex integration test is stale
and no longer matches the constructed data flow. Update the header above the
simplex test near the X5-001 setup in test_simplex_command.rs to describe the
actual many-families prefix construction used by the test, and align it with the
batch behavior described later in the same test rather than the outdated “one
large clean family” wording. Use the surrounding test case and its
consensus/batch setup to keep the comment consistent with the current
reject-free prefix trigger.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5ef9e601-01f4-4359-8f8f-2f06901a6db2

📥 Commits

Reviewing files that changed from the base of the PR and between 8a852ff and fd54559.

📒 Files selected for processing (10)

• crates/fgumi-consensus/src/codec_caller.rs
• src/lib/pipeline/chains/commands/codec.rs
• src/lib/pipeline/chains/commands/duplex.rs
• src/lib/pipeline/chains/commands/simplex.rs
• src/lib/pipeline/steps/correct/mod.rs
• tests/integration/helpers/assertions.rs
• tests/integration/test_codec_command.rs
• tests/integration/test_correct_command.rs
• tests/integration/test_duplex_command.rs
• tests/integration/test_simplex_command.rs

[nh13]

@coderabbitai review

[coderabbitai]

✅ 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.