refactor(sort): introduce ChunkNamer to centralize temp file naming

[nh13]

Summary

Stacked on #178.

• Introduces a ChunkNamer struct that encapsulates both monotonic counters (chunk files and merged files) and the temp directory path, replacing 3 separate variables per sort method (chunk_counter, consolidation_count, temp_path passed to consolidation).
• Makes it impossible to forget to increment the counter — next_chunk_path() and next_merged_path() handle it atomically.
• Simplifies maybe_consolidate_temp_files signature from 3 parameters to 2.
• Removes the clippy::too_many_lines allow on sort_coordinate_with_index that fix(sort): use monotonic counter for chunk file naming during consolidation #178 added — no longer needed after the variable reduction.

Test plan

• All 1920 existing tests pass (cargo ci-test)
• cargo ci-fmt clean
• cargo ci-lint clean (including removal of too_many_lines allow)

[codecov]

Codecov Report

❌ Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.65%. Comparing base (e2abb5b) to head (890b933).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/lib/sort/raw.rs	96.66%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #179   +/-   ##
=======================================
  Coverage   85.65%   85.65%           
=======================================
  Files         128      128           
  Lines       51990    51983    -7     
=======================================
- Hits        44533    44528    -5     
+ Misses       7457     7455    -2     

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d0cbf06d-dfb2-4f78-835b-866c540b0e2d

📥 Commits

Reviewing files that changed from the base of the PR and between 3cafbc8 and 890b933.

📒 Files selected for processing (1)

• src/lib/sort/raw.rs

---

📝 Walkthrough

Walkthrough

Introduces an internal ChunkNamer that maintains separate monotonic counters and emits chunk_####.keyed and merged_####.keyed paths. Sorting phases (coordinate-optimized, coordinate-with-index, queryname, template-coordinate) now use a shared &mut ChunkNamer for spilled chunks and consolidations instead of local counters and external consolidation counts. maybe_consolidate_temp_files signature changed to accept &mut ChunkNamer. A clippy too_many_lines allow was removed and a consolidation test was adjusted to require at least five chunks.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title accurately summarizes the main change: introducing ChunkNamer to centralize temp file naming.
Description check	✅ Passed	Description clearly relates to the changeset, detailing the ChunkNamer struct, counter consolidation, and signature simplifications.
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 refactor/nh/chunk-namer

---

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

[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/sort/raw.rs (1)

2261-2265: ⚠️ Potential issue | 🟡 Minor

Make the regression assert the actual collision path.

With max_temp_files(4), the old overwrite only happens on the first spill after consolidation, so this needs at least 5 chunk writes. >= 4 can pass while never exercising the buggy naming path.

Suggested tweak

         assert!(
-            stats.chunks_written >= 4,
-            "expected at least 4 chunks to trigger consolidation, got {}",
+            stats.chunks_written >= 5,
+            "expected at least 5 chunks to exercise post-consolidation naming, got {}",
             stats.chunks_written
         );

🤖 Prompt for AI Agents

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

In `@src/lib/sort/raw.rs` around lines 2261 - 2265, The regression test's
assertion is too loose: with max_temp_files(4) the overwrite-only bug is
exercised only on the first spill after consolidation, which requires at least 5
chunk writes, so update the assertion that references stats.chunks_written in
raw.rs to require >= 5 (or otherwise assert the specific collision/overwrite
path was taken) so the test actually exercises the buggy naming path; locate the
check using stats.chunks_written in the failing test and change the expected
minimum from 4 to 5 (or add an explicit assertion that the consolidation+spill
collision code path ran).

🤖 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/sort/raw.rs`:
- Around line 2261-2265: The regression test's assertion is too loose: with
max_temp_files(4) the overwrite-only bug is exercised only on the first spill
after consolidation, which requires at least 5 chunk writes, so update the
assertion that references stats.chunks_written in raw.rs to require >= 5 (or
otherwise assert the specific collision/overwrite path was taken) so the test
actually exercises the buggy naming path; locate the check using
stats.chunks_written in the failing test and change the expected minimum from 4
to 5 (or add an explicit assertion that the consolidation+spill collision code
path ran).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: df7163d1-2a78-4ba1-9242-f48991483684

📥 Commits

Reviewing files that changed from the base of the PR and between e2abb5b and 3cafbc8.

📒 Files selected for processing (1)

• src/lib/sort/raw.rs