fix(range_upload): don't drop the tail of segments partially swallowed by window extension

[coyotte508]

The bug

upload_ranges snaps the caller's dirty ranges to segment boundaries before sending them to GET /v2/file-chunk-hashes, but the server (and the mirrored logic in chunk_window_builder.rs) extends each window past the requested end until two consecutive clean chunks have stable CDC sizes (is_stable_chunk_size) — always at least two chunks past the dirty range unless it hits EOF. So the returned dirty_byte_range end usually lands mid-segment.

compose_mdb then skipped original segments with:

while seg_idx < n_segs && seg_byte_starts[seg_idx] < w.end { seg_idx += 1; }

which skips every original segment whose start is < w.end — including a segment only partially covered by the window. The remainder of that segment, [w.end, next_segment_boundary), was silently dropped from the composed MDB: the composed segment list came out shorter than the file (e.g. 590,533 bytes of an 8 MB file in the new regression test).

Why existing tests missed it

• The test-module random_data generator (((i+seed)*2654435761) >> 16 bytes) is so weakly random that the CDC chunker only ever produces max-size (128 KB) chunks, which are all unstable per is_stable_chunk_size (stable range is [16 KB, 120 KB) for the 64 KB target). With no stable pair ever found, server windows always extend to EOF, where composition is trivially correct — the partial-swallow path was never exercised.
• The simulation server used by the tests does not verify file hashes against the shard, so a truncated composed MDB passed silently.
• Additionally, the existing tests' assert_eq!(result.hash(), clean_hash.hex()) pattern first performs a clean upload of the identical expected content, which registers the same file hash with a correct MDB — masking the truncated one for any subsequent get_file_reconstruction_info/download.

Production impact

Production xetcas does verify the file hash against the composed shard, so it rejects the shard and the whole edit operation fails loudly — i.e. mid-file range edits via upload_ranges fail on realistic (high-entropy) data whenever the window extension stops mid-segment, which is the common case.

The fix (client-side, no API change)

For each server window, compute effective_end = the first original segment boundary >= w.end (the trailing entry of seg_byte_starts is the file size, so it always exists):

1. Stream original bytes up to effective_end (not w.end) into the window's cleaner, with the matching middle_size. This is safe hash-wise: the server's stable-boundary rule guarantees the re-chunker has re-synced with the original chunk sequence by w.end, and the chunker resets its state at every chunk boundary, so the chunks produced for [w.end, effective_end) are bit-identical to the original file's chunks there.
2. Split the window's chunk list for the hash merge: the server's gap subtrees (hash_ranges) cover original chunks starting at w.end, so only the window chunks covering the first (w.end - w_start) + added - removed output bytes enter the window's MerkleHashSubtree::from_chunks; the tail chunks duplicate gap-covered chunks and are excluded. An internal error is returned if that split size doesn't land exactly on a chunk boundary (re-sync failure — should never happen).
3. Use effective_end in compose_mdb's segment-skip loop, so no segment is ever partially swallowed: the window's own mdb.segments (which now cover [w_start, effective_end)) supply data + verification for the whole span. This also keeps gap_verification consumption consistent with the server contract: the server emits no entry for a segment partially overlapped by one of its windows, and with effective_end the client re-uploads that whole segment, consuming no entry.
4. Guard: fail loudly (internal error) if effective_end would cross the next window's start, rather than compose a corrupt file.

Leftover-edit accounting and edit-to-window assignment still use the server's w.end (edits always lie within the requested range, which is <= w.end).

The same strategy is already implemented and validated against production xetcas in huggingface.js: huggingface/huggingface.js#2407

Tests

• strong_random_data helper (splitmix64): high-entropy bytes giving a realistic CDC chunk-size distribution with stable-sized chunks, plus a warning comment on random_data explaining why it can't exercise these paths.
• test_second_edit_round_keeps_partially_swallowed_segment_tail: two rounds of edits on strong-random data; asserts the composed MDB's unpacked_segment_bytes sum equals the file size and the downloaded content matches byte-for-byte — before any clean upload of the same content (which would mask the bug). Fails on the old code with composed MDB is truncated: segments cover 590533 of 8388608 bytes.
• test_mid_file_edit_window_ends_mid_segment: queries get_file_chunk_hashes directly and asserts the returned window end is not on a segment boundary (proving the partial-swallow path is exercised), then asserts complete/correct composition. Also fails on the old code.
• cargo test -p xet-data --features simulation --lib range_upload: 30 passed, 0 failed, 3 ignored (stress tests). No new clippy or fmt diagnostics on the touched file.

cc @assafvayner

---

Note

High Risk
Changes core upload_ranges / compose_mdb behavior for partial-segment CDC windows; incorrect chunk trimming or segment skipping would corrupt file hashes and MDBs in production.

Overview
Fixes range upload composition when the CAS server extends each dirty window past the requested end to a stable CDC boundary, which usually stops mid-segment. Previously, compose_mdb skipped original segments using only w.end, so the tail [w.end, segment_boundary) was dropped from the composed MDB—production rejects those shards; the sim server did not catch it.

For each window, the client now computes effective_end (next original segment boundary ≥ w.end), re-streams original bytes through that boundary into the cleaner, trims duplicate tail chunks before the Merkle hash merge (gap subtrees still cover from w.end), and skips replaced segments using effective_end when splicing MDB segments. Internal errors guard crossed adjacent windows and missing chunk boundaries at w.end.

Adds strong_random_data and regression tests (multi-round edits, mid-segment window probes, two-window cases) that assert full MDB coverage before a clean upload would mask a truncated MDB; documents why the old random_data helper never exercised this path.

^{Reviewed by Cursor Bugbot for commit a771935. Bugbot is set up for automated code reviews on this repo. Configure here.}

[assafvayner]

Claude generated

For Assaf: context for chunker protocol protection

Question considered: we need re-chunking the composed file later to produce the same chunks (canonical chunking). Does this change break that?

No — it strengthens that guarantee rather than breaking it.

The composed file is canonical (a full re-chunk reproduces its chunk list) iff the cleaner's re-chunk of the window lands a boundary exactly at w_end. Reasoning, assuming the original file is canonical:

• Bytes before w_start are unchanged, so a full chunker reproduces the original boundaries there and is at a boundary at w_start (a segment boundary ⇒ an original chunk boundary). Boundary state = fresh state (set_hash(0), empty buffer), same as the cleaner's fresh chunker.
• From w_start, full chunker and cleaner see identical bytes with identical state ⇒ identical boundaries through the edited region.
• If both hit a boundary at w_end, the full chunker is now synced with the original chunker at w_end (same boundary, same trailing bytes) ⇒ from there it reproduces the original chunks: through the tail [w_end, effective_end) (which is why the cleaner's tail chunks are bit-identical to the originals and the forced cut at effective_end coincides with a real boundary), and on through every kept segment after.

So canonicity hinges entirely on the resync at w_end. Pre-PR that was assumed: the stream was force-cut at w_end, and if the chunker hadn't actually resynced you got a hash-consistent, verifiable, but non-canonical file — silently. Post-PR the acc == hash_split_size check verifies it and refuses to compose otherwise. A resync failure is precisely a canonicity violation, and this PR turns it from silent corruption of the invariant into a loud error.

Two consequences:

1. The check is load-bearing for the invariant — keep it; don't "fix" the error path by force-cutting at w_end again.
2. If a fallback is added for the error path, it must also be canonical: re-chunk from w_start to EOF in one cleaner (trivially matches a full chunker), or fall back to a full clean upload. Not a forced cut.

[coyotte508]

(btw the PR was fully LLM-generated, feel free to rework it / close & reopen / etc)

[coyotte508]

btw @assafvayner & xet team, please merge if ok with PR!

(I mean I'm not sure I should merge or not, leaving it up to you)

[seanses]

@coyotte508 Sorry for the delay. I'll review and merge