fix(review): keep dotted output prefixes intact

[nh13]

Summary

fgumi review built its output paths with Path::with_extension, which replaces the last dotted component of --output instead of appending to it. --output out.v1 wrote out.consensus.bam, out.grouped.bam, out.txt (and .bam.bai sidecars) instead of out.v1.*, so the outputs of out.v1 and out.v2 overwrote each other. Every output now goes through Review::output_path, which appends .{suffix} to the full prefix, as fgbio does.

review also had no output-vs-input guard: -o s -c s.consensus.bam truncated the consensus BAM while it was still being read. Before any writer opens, review now rejects any output (either BAM, its .bai, or the .txt) that is the same file as --input, --consensus-bam, --grouped-bam, --ref or an input BAM's index. It uses the existing reject_writes_aliasing_inputs (dev+inode), so symlinks and hard links are caught too.

The shared prefix helper moves from commands::group::with_extension to commands::common::append_suffix, next to the other output-path guards. It drops trailing separators (and a trailing . component), so out/ yields out.<suffix> beside the prefix, as fgbio's Path normalization does, instead of hidden out/.<suffix> files. It now returns an error for a prefix that names no file (., ./, .., out/.., /, empty), which previously wrote dot-named files such as /.txt. Every caller propagates that error early, before any input is read: review, group --metrics, simplex/duplex/codec --metrics, runall --all-metrics and per-stage --metrics, and simplex-/duplex-metrics --output.

Each metrics file set is now named in one place. group_metrics_paths covers group, and SimplexMetricsPaths/DuplexMetricsPaths in the inline metrics collector cover simplex/duplex/codec. The serial simplex-metrics/duplex-metrics paths use the collector's writer (write_simplex_metrics/write_duplex_metrics) instead of their own copy. They also keep paths as PathBuf: execute_r_script now takes AsRef<OsStr> args, so a non-UTF-8 prefix gets the same names on the serial path as on --threads. The --output/--metrics/--all-metrics help now states the append, separator-dropping and must-name-a-file rules.

Changes output for: review when the final component of --output contains a dot; metrics file names when the prefix ends in a path separator or . component (group --metrics, simplex/duplex/codec --metrics, runall --all-metrics and per-stage --metrics, simplex-/duplex-metrics --output including the PDF). A prefix that names no file is now an error for all of these.

fgbio parity (e51a661)

• ReviewConsensusVariants.scala:186-187,212 builds every output as output.getParent.resolve(s"${output.getFileName}${ext}"), which is the full file name plus the extension.
• ReviewConsensusVariantsTest.scala:188 uses the dotted prefix review_consensus.<n>.out and expects <outBase>.consensus.bam, .grouped.bam and .txt, all empty.
• The .bam.bai sidecars are the documented REV3-12 divergence.

Tests

• review_output_prefix_with_dots_is_preserved ports fgbio :188, plus a trailing-separator case. It checks the exact set of files written and that they are empty.
• test_review_output_path_extensions covers plain, dotted and trailing-separator prefixes. The integration test runs the CLI with a dotted prefix and checks that none of the old names are written.
• test_review_rejects_an_output_that_aliases_an_input checks the exact error for each aliasing combination: consensus/grouped BAM (plain, dotted, trailing separator), the review .txt vs --input/--ref, each output .bai vs --input, and an output .bai landing on an input BAM's .bai. The dotted cases collide only because the whole prefix is now kept. It also asserts that nothing is written. test_review_rejects_an_input_symlinked_to_an_output covers identity through a symlink.
• append_suffix_appends_to_the_full_prefix and append_suffix_rejects_a_prefix_that_names_no_file (in common.rs) assert exact strings and exact errors. test_review_rejects_a_prefix_that_names_no_file and all_metrics_rejects_a_prefix_that_names_no_file (runall) check the same error through the commands.
• all_metrics_fills_only_group_metrics_prefix covers --all-metrics all/ → all.group. test_metrics_prefix_with_trailing_separator_collides_with_output checks that group's --output collision guard sees --metrics out/ as out.*. The existing --metrics out/ naming tests for group and simplex-/duplex-metrics (serial and --threads) still pass.

Mutation-checked. Disabling the must-name-a-file check fails all 18 rejection cases. Disabling the review alias guard fails all 11 alias cases. Dropping the .bai outputs and input-index entries from the guard fails exactly the three sidecar/index cases.

Checks

cargo ci-fmt, cargo ci-lint, cargo ci-doc, cargo ci-tag-literals, cargo nextest run --workspace (10331 passed, 31 skipped).

Risk: Output filenames change for review, grouping, consensus, and metrics; append_suffix plus targeted naming and collision tests pin the change. unsafe: not established from the supplied summary; CLAUDE.md allowlist status is also not established. Memory bounds, queue capacity, and thread/backpressure policy: none reported.

Fix: Append suffixes to the full output prefix and reject output paths that alias protected inputs before opening writers. The change also centralizes metrics paths and shares metric writers across serial and threaded paths.

Test results and current review findings were not supplied.

[coderabbitai]

Note

Reviews paused

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: Repository: fulcrumgenomics/fgumi/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Essentials
• Run ID: f5c280a6-97f7-4f18-a9c7-988035899ba8

📥 Commits

Reviewing files that changed from the base of the PR and between 62b98c0 and 470b712.

📒 Files selected for processing (14)

• src/lib/commands/codec.rs
• src/lib/commands/common.rs
• src/lib/commands/duplex.rs
• src/lib/commands/duplex_metrics.rs
• src/lib/commands/group.rs
• src/lib/commands/review.rs
• src/lib/commands/runall.rs
• src/lib/commands/shared_metrics.rs
• src/lib/commands/simplex.rs
• src/lib/commands/simplex_metrics.rs
• src/lib/inline_metrics_collector.rs
• tests/integration/test_consensus_metrics_parity.rs
• tests/integration/test_group_cutover_parity.rs
• tests/integration/test_review_command.rs

Included review availability: This review used your included allowance. 2 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour.

---

Walkthrough

Metrics and review outputs now append suffixes to full prefixes, preserve dotted names, and drop trailing separators. Shared path helpers reject prefixes that do not identify a file. Metrics writers use shared path and write helpers. Review checks output paths against inputs before opening writers.

Changes

Output path handling

Layer / File(s)	Summary
Shared suffix and metrics path helpers src/lib/commands/common.rs, src/lib/inline_metrics_collector.rs	Added validated suffix-path construction and shared simplex and duplex metrics path and writer helpers.
Metrics command output paths src/lib/commands/{codec,duplex,simplex,group}.rs, src/lib/commands/{duplex_metrics,simplex_metrics,shared_metrics}.rs, tests/integration/*parity.rs	Metrics commands use full-prefix suffix paths and shared writers. They propagate path errors, and tests cover trailing separators and dotted prefixes.
runall metrics path derivation src/lib/commands/runall.rs	runall uses shared suffix construction and propagates path-generation errors during option construction and collision-target collection.
Review output naming and collision checks src/lib/commands/review.rs, tests/integration/test_review_command.rs	Review output paths append suffixes to the full prefix. Execution checks for aliases with inputs and BAM indexes before writers open. Tests cover collisions, invalid prefixes, dotted names, and trailing separators.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~40 minutes

Change: Bug fix

Suggested labels: fgumi group

Merge Risk: ⚪ Minimal · up to 470b7

The change makes output naming preserve dotted prefixes and adds a guard against overwriting inputs. No concrete merge-blocking risk was identified in the supplied context.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows the required Conventional Commit format. Its lowercase imperative description accurately summarizes the main change: preserving dotted output prefixes in review.
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.

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[nh13]

@coderabbitai pause

[coderabbitai]

✅ Action performed

Reviews paused.

[codecov]

Codecov Report

❌ Patch coverage is 92.74924% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.45%. Comparing base (62b98c0) to head (470b712).

Files with missing lines	Patch %	Lines
src/lib/commands/simplex_metrics.rs	50.00%	8 Missing ⚠️
src/lib/commands/duplex_metrics.rs	58.82%	7 Missing ⚠️
src/lib/commands/shared_metrics.rs	0.00%	5 Missing ⚠️
src/lib/inline_metrics_collector.rs	94.28%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1026      +/-   ##
==========================================
- Coverage   96.47%   96.45%   -0.03%     
==========================================
  Files         299      299              
  Lines      152214   152321     +107     
==========================================
+ Hits       146854   146914      +60     
- Misses       5360     5407      +47     

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