feat(sort): distribute spill files across multiple temp dirs

[nh13]

Summary

• fgumi sort -T/--tmp-dir is now repeatable; pass more than one path to spread spill chunks across multiple filesystems in free-space-aware round-robin order.
• FGUMI_TMP_DIRS (PATH-style list) is honoured as a fallback when no -T flags are passed on the command line. CLI flags take precedence.
• New TmpDirAllocator owns the rotation and re-checks free space every 8 spills; dirs below 1 GiB free are dropped from rotation automatically.
• Behaviour with zero or one -T flag is unchanged.

Usage

# Spread spill across multiple dirs
fgumi sort -i in.bam -o out.bam -T /mnt/ssd1 -T /mnt/ssd2

# Same via env var (useful for site defaults on HPC)
FGUMI_TMP_DIRS=/mnt/ssd1:/mnt/ssd2 fgumi sort -i in.bam -o out.bam

Design notes

• Distribution policy: free-space-aware round-robin. At startup, each supplied dir is probed for free space; dirs below the 1 GiB threshold are dropped. In-rotation dirs receive chunks in strict round-robin order. Every 8 calls, the allocator re-probes and prunes dirs whose free space has fallen below the threshold.
• Hot path: allocator work happens only at spill boundaries (typically 10s-100s of times per sort), never per-record. Per-record loops in the four sort_* fns are untouched.
• Scope: only fgumi sort spills today, so the option lives in src/lib/commands/sort.rs rather than in CompressionOptions-style shared common-options.

Known limitation

Mid-write ENOSPC is not auto-recovered — the startup filter and periodic recheck catch most cases before a write begins, but if a pooled spill writer hits ENOSPC mid-flight the sort fails with a clean error. TmpDirAllocator::mark_full() is wired and ready for a follow-up that intercepts the async PendingSpill error path.

New dependency

• fs4 = "0.13" (default-features off) — cross-platform available_space() query. Pulls in rustix + libc + errno + bitflags only; no async runtime.

Test plan

• cargo ci-fmt clean
• cargo ci-lint clean
• cargo ci-test all green (2559/2559)
• 18 new tests: 7 TmpDirAllocator unit tests (round-robin, mark_full, startup filter, periodic recheck, exhaustion), 4 env-var resolver tests, 1 rstest-parameterized clap parse test (4 cases), 1 e2e multi-tmpdir sort test (byte-equal to single-dir)
• fgumi sort --help renders new -T doc and EXAMPLES entries cleanly

[codecov]

Codecov Report

❌ Patch coverage is 96.21212% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.07%. Comparing base (5296085) to head (fa73333).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/lib/sort/tmp_dir_alloc.rs	96.78%	8 Missing ⚠️
src/lib/commands/sort.rs	86.53%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #284      +/-   ##
==========================================
+ Coverage   90.02%   90.07%   +0.04%     
==========================================
  Files         122      123       +1     
  Lines       61791    62144     +353     
==========================================
+ Hits        55629    55974     +345     
- Misses       6162     6170       +8     

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

📝 Walkthrough

Walkthrough

Adds support for multiple temporary directories for sorting. Cargo.toml gains fs4 for free-space probes. The CLI Sort option changed from tmp_dir: Option<PathBuf> to repeatable tmp_dirs: Vec<PathBuf> with FGUMI_TMP_DIRS env-var fallback and parsing. Introduces src/lib/sort/tmp_dir_alloc.rs providing TmpDirAllocator (round-robin selection, periodic free-space rechecks, mark_full removal). RawExternalSorter and naming/allocation paths were refactored to accept multiple temp dirs and propagate allocator errors. Tests updated/added for parsing, resolution, allocator behavior, and end-to-end equivalence.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title directly and concisely describes the main feature: enabling spill distribution across multiple temp directories.
Description check	✅ Passed	Description clearly relates to the changeset, covering the new multi-tempdir feature, usage, design, and testing.
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

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch nh/sort-tmpdir

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/sort/tmp_dir_alloc.rs`:
- Around line 60-68: The startup filtering in TmpDirAllocator::with_probe
currently hard-codes DEFAULT_MIN_FREE_BYTES when populating active, so calling
TmpDirAllocator::with_probe(...).with_min_free_bytes(0) can't re-include
directories; change with_probe to use a configurable threshold value (store
min_free_bytes on the TmpDirAllocator or accept it as a parameter) when deciding
which dirs to push into active, and/or defer the initial filtering (populate
with all dirs and apply the min_free_bytes filter later in with_min_free_bytes).
Specifically update the logic in with_probe that calls probe(&dir) and compares
to DEFAULT_MIN_FREE_BYTES so it references the allocator's min_free_bytes field
or the passed-in parameter, and ensure the duplicate code path at the other
occurrence (lines 94-99) is updated the same way.
- Around line 141-145: Removing an active dir can shift the rotation cursor and
break round-robin order: update the cursor in mark_full (and other pruning sites
around the same block) after removing at position pos by adjusting cursor =
(cursor - 1) if pos < cursor (or wrapping appropriately) and ensure cursor is
clamped modulo active.len() (or set to 0 when active becomes empty) so the next
allocation still returns the expected next element in the round-robin sequence;
locate the logic in the mark_full method and the other pruning functions that
remove from self.active and apply the same cursor rebase rule.

🪄 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: 31f05e46-deb6-46b0-a581-e5655b5c1e7e

📥 Commits

Reviewing files that changed from the base of the PR and between 5296085 and 4ab68cb.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (5)

• Cargo.toml
• src/lib/commands/sort.rs
• src/lib/sort/mod.rs
• src/lib/sort/raw.rs
• src/lib/sort/tmp_dir_alloc.rs

[nh13]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

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]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/lib/commands/sort.rs (1)

33-33: ⚠️ Potential issue | 🟡 Minor

Use var_os to preserve non-UTF-8 temp-dir values.

std::env::var silently ignores FGUMI_TMP_DIRS when it contains non-UTF-8 bytes. Switch to var_os and update resolve_tmp_dirs to accept Option<&OsStr> so Unix paths with non-UTF-8 sequences aren't lost.

Proposed fix

+use std::ffi::OsStr;
 use std::path::PathBuf;
@@
-pub(crate) fn resolve_tmp_dirs(cli: &[PathBuf], env_value: Option<&str>) -> Vec<PathBuf> {
+pub(crate) fn resolve_tmp_dirs(cli: &[PathBuf], env_value: Option<&OsStr>) -> Vec<PathBuf> {
     if !cli.is_empty() {
         return cli.to_vec();
     }
 
     let Some(value) = env_value else { return Vec::new() };
-    if value.is_empty() {
-        return Vec::new();
-    }
 
     std::env::split_paths(value)
         .filter(|p| !p.as_os_str().is_empty())
         .filter(|p| !p.to_string_lossy().trim().is_empty())
         .collect()
 }
@@
-        let env_value = std::env::var(TMP_DIRS_ENV).ok();
+        let env_value = std::env::var_os(TMP_DIRS_ENV);
         let resolved_tmp_dirs = resolve_tmp_dirs(&self.tmp_dirs, env_value.as_deref());

Update resolver tests to pass Some(OsStr::new(env)).

Also applies to: 335-348, 596-597

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/commands/sort.rs` at line 33, The code currently uses std::env::var
which drops FGUMI_TMP_DIRS values containing non-UTF-8 bytes; change calls to
std::env::var to std::env::var_os and update the signature of resolve_tmp_dirs
to accept Option<&OsStr> (instead of Option<&str> or String) so non-UTF-8
temp-dir values are preserved, then adapt all callers (including places
referenced by resolve_tmp_dirs and the other similar spots around the sort
logic) to pass the Option<&OsStr> returned by var_os; update the resolver tests
to call resolve_tmp_dirs with Some(OsStr::new(env)) where needed.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/lib/commands/sort.rs`:
- Line 33: The code currently uses std::env::var which drops FGUMI_TMP_DIRS
values containing non-UTF-8 bytes; change calls to std::env::var to
std::env::var_os and update the signature of resolve_tmp_dirs to accept
Option<&OsStr> (instead of Option<&str> or String) so non-UTF-8 temp-dir values
are preserved, then adapt all callers (including places referenced by
resolve_tmp_dirs and the other similar spots around the sort logic) to pass the
Option<&OsStr> returned by var_os; update the resolver tests to call
resolve_tmp_dirs with Some(OsStr::new(env)) where needed.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9849cd64-e2f3-4f88-9a1d-f54d734bfe59

📥 Commits

Reviewing files that changed from the base of the PR and between 0c3dbda and fa73333.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (5)

• Cargo.toml
• src/lib/commands/sort.rs
• src/lib/sort/mod.rs
• src/lib/sort/raw.rs
• src/lib/sort/tmp_dir_alloc.rs

✅ Files skipped from review due to trivial changes (2)

• Cargo.toml
• src/lib/sort/mod.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• src/lib/sort/raw.rs
• src/lib/sort/tmp_dir_alloc.rs