Skip to content

Group small-entry transfers with div_ceil so exact-multiple sizes stop overshooting by a chunk - #853

Open
CleanCut wants to merge 2 commits into
mainfrom
cleancut/fetch-div-ceil
Open

Group small-entry transfers with div_ceil so exact-multiple sizes stop overshooting by a chunk#853
CleanCut wants to merge 2 commits into
mainfrom
cleancut/fetch-div-ceil

Conversation

@CleanCut

@CleanCut CleanCut commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Both pull_small_entries and download_small_entries sized their chunk groups with the (total / segment) + 1 idiom the repo guidelines warn against. When total_size is an exact multiple of the segment size it asked for one more group than needed, yielding marginally smaller groups.

Swapping in div_ceil on its own would have introduced a panic: it returns 0 when every entry is zero bytes, and the next line divides entries.len() by that count. The .max(1) floor keeps the divisor non-zero, which the + 1 form had been providing by accident.

Fixed a similar problem with chunk_size, where it should only be 0 when the number of entries is 0.

ENG-1229

…p overshooting by a chunk

Both `pull_small_entries` and `download_small_entries` sized their chunk groups with the
`(total / segment) + 1` idiom the repo guidelines warn against. When `total_size` is an exact
multiple of the segment size it asked for one more group than needed, yielding marginally smaller
groups. Not a correctness bug at these two sites — the `num_chunks > entries.len()` guard kept it
from ever requesting a zero-byte range — but it left two different idioms in the tree for the same
computation.

Swapping in `div_ceil` on its own would have introduced a panic: it returns 0 when every entry is
zero bytes, and the next line divides `entries.len()` by that count. The `.max(1)` floor keeps the
divisor non-zero, which the `+ 1` form had been providing by accident.

ENG-1229
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved downloads for empty and very small files.
    • Prevented download operations from calculating zero-sized chunks, ensuring reliable transfer behavior.

Walkthrough

The small-file pull and download paths now use ceiling division and enforce a minimum chunk count of one.

Changes

Small-file chunking

Layer / File(s) Summary
Update small-file chunk counts
crates/liboxen/src/core/v_latest/fetch.rs
pull_small_entries and download_small_entries now use ceiling division with a minimum of one chunk.

Estimated code review effort: 2 (Simple) | ~5 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly summarizes the main change: using ceiling division to prevent an unnecessary chunk for exact-multiple sizes.
Description check ✅ Passed The description directly explains the chunk-sizing changes, the zero-divisor safeguard, and the related edge cases.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cleancut/fetch-div-ceil

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@crates/liboxen/src/core/v_latest/fetch.rs`:
- Around line 731-732: Update pull_small_entries in
crates/liboxen/src/core/v_latest/fetch.rs at lines 731-732 so its
entries-per-group calculation uses ceiling division via
entries.len().div_ceil(num_chunks). Apply the same change in
download_small_entries at lines 974-975; both sites must use ceiling division to
avoid extra requests for non-divisible entry counts.
🪄 Autofix

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: d9ad1dc7-34d5-4d8c-834b-7345d98d5184

📥 Commits

Reviewing files that changed from the base of the PR and between ebab2cc and 45d58ea.

📒 Files selected for processing (1)
  • crates/liboxen/src/core/v_latest/fetch.rs

Comment thread crates/liboxen/src/core/v_latest/fetch.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant