Added stress testing; use of next_stable_chunk_boundary logic - #845
Merged
XciD merged 9 commits intoMay 21, 2026
Merged
Conversation
Bumps [openssl](https://github.com/rust-openssl/rust-openssl) from 0.10.76 to 0.10.79. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Di Xiao <seanses@users.noreply.github.com>
…843) The `hf-xet prerelease testing` workflow currently doesn't declare a `permissions:` block, so the workflow `GITHUB_TOKEN` falls back to the repository default. Every step in `trigger_rc_testing` authenticates via `TOKEN_HUGGINGFACE_HUB_AUTO_BY_XET` (a PAT scoped for the hf-hub auto-update flow): - the `actions/checkout` step pulls `huggingface/${{ matrix.target-repo }}` with `token: ${{ secrets.TOKEN_HUGGINGFACE_HUB_AUTO_BY_XET }}` - `git push` reuses the credentials persisted by checkout So the workflow's own `GITHUB_TOKEN` is unused. `permissions: {}` (workflow scope) pins that. Pattern matches the workflow-level permissions blocks already used in this repo. With it set: - the workflow token can't be widened by a future change to the repo default - the SLSA / OpenSSF Scorecard `Token-Permissions` check passes for this file - a hypothetical compromise of any third-party action reachable from this workflow (cf. `tj-actions/changed-files` CVE-2025-30066) has nothing to do with the workflow token Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
## Summary - Deduplicate MDBMinimalShard file infos by file hash during sync and async streaming parse. - Keep only the first file info seen for a duplicate file hash; async callbacks fire only for retained entries. - Add a focused streaming-shard test covering parse, async callbacks, and reserialization. ## Why Duplicate file infos can survive the minimal streaming shard parse/re-serialize path because it stores file entries as a Vec. This narrows canonicalization to that streaming path while leaving in-memory shard and set-operation behavior unchanged. ## Impact - MDBMinimalShard::num_files() now reports unique file hashes for parsed shards. - Later duplicate file infos are ignored even if they contain richer optional verification or metadata extension data. - Raw full-section readers, MDBInMemoryShard behavior, and shard set operations remain unchanged. ## Validation - cargo test -p xet-core-structures metadata_shard - cargo test -p xet-client test_global_dedup - git diff --check - rustfmt --edition 2024 --check xet_core_structures/src/metadata_shard/set_operations.rs xet_core_structures/src/metadata_shard/shard_in_memory.rs xet_core_structures/src/metadata_shard/streaming_shard.rs <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes shard streaming parse semantics by dropping duplicate `file_hash` entries, which can affect downstream counts/serialization and may hide later entries’ richer metadata/verification. > > **Overview** > `MDBMinimalShard` now **deduplicates file-info records by `file_hash`** during both sync (`from_reader`) and async (`from_reader_async_with_custom_callbacks`) streaming parses, keeping only the *first* occurrence. > > Adds a focused test that constructs a shard stream with duplicate file infos and asserts first-wins behavior, validates async parsing/callback behavior, and confirms re-serialization only emits the retained entry. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 1320ce3. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Human context: in integrating new hf-hub usage in tokenizers, tokenizers also generated a napi binary. so we should validate that hf-hub/hf-xet are napi compat (hf-hub is pretty trivial if given that hf-xet is compatible). ## Summary - Adds `examples/xet_pkg_napi/` — a minimal napi-rs binding that links `hf-xet` (the `xet` crate at `xet_pkg/`) into a Node.js native addon. - Exposes `initLogging(version)` and `smokeTest()`. The smoke test builds a `XetSession` synchronously and constructs upload-commit + file-download-group builders to exercise lazy runtime startup. - Crate is excluded from the xet-core workspace and carries its own `[workspace]` table so it stays standalone under git worktrees (where cargo would otherwise resolve through the canonical repo path). - Build artifacts (`*.node`, `index.js`, `index.d.ts`, `node_modules/`) are gitignored; `Cargo.lock` and `package-lock.json` are committed for reproducibility. The point of the smoke test is **not** a full JS API — it's to verify hf-xet compiles, links, and starts inside libuv (no pyo3, no host-owned tokio). If you can run `npm run smoke` and see `xet session built; runtime initialized`, the integration is ready for a fuller binding (async upload/download via `#[napi]` async fns, progress callbacks via `ThreadsafeFunction`). ## Test plan - [x] `npm install` (in `examples/xet_pkg_napi/`) - [x] `npm run build:debug` — compiles `hf-xet`, `xet-runtime`, `xet-client`, `xet-data`, `xet-core-structures` and the napi shim against napi 2.16 - [x] `npm run smoke` — outputs: ``` loaded addon, exports: [ 'initLogging', 'smokeTest' ] smokeTest: xet session built; runtime initialized ``` - [x] Verify on Linux (only tested on darwin-arm64 locally) - [x] Decide whether to wire into CI, or keep as a manual example <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Adds a standalone example project and build scripts without changing production crates; primary risk is repo bloat/noise from the committed lockfiles and an extra exclusion in the workspace. > > **Overview** > Adds a new standalone `examples/xet_pkg_napi` project to smoke-test that `hf-xet` can compile/link as a `napi-rs` Node native addon and perform a real file download via the blocking download APIs. > > Updates the root `Cargo.toml` to **exclude** this example from the workspace, and includes the example’s build/run scaffolding (`package.json`, `smoke.mjs`, `build.rs`) plus committed lockfiles and a `.gitignore` for generated artifacts. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit cb62895. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…og (#808) Two connected cleanups from the [2026-04-21 Julien upload-stuck investigation](https://www.notion.so/huggingface2/Julien-upload-stuck-upload_xorb-120s-timeouts-2026-04-21-3491384ebcac81a19d0af5394745cfff). Closes #807. Docs PR: huggingface/hub-docs#2419. ## Change 1 — raise `HF_XET_CLIENT_READ_TIMEOUT` default 120s → 300s **Files:** `xet_runtime/src/config/groups/client.rs`, `xet_client/src/cas_client/remote_client.rs` (stale comment). The 120s client read timeout was firing before legitimate `upload_xorb` requests could complete on high-latency / transatlantic / bursty links. Fleet-wide this produced a **chronic 30–50% xorb POST failure rate** (1,092–4,196 `error uploading xorb` events per hour sustained over 24h, peaking at 49.1% in the investigation window). 267 successful uploads in the same 24h had latency > 120s (max 37 min), so 120s wasn't protecting anything legitimate — it was only cutting off slow-but-healthy streams. 300s preserves stall-detection semantics (still an order of magnitude under the 3600s ALB idle). The env override `HF_XET_CLIENT_READ_TIMEOUT` is unchanged. ## Change 2 — log `query_dedup` 404 as cache miss, not "Fatal Error" **Files:** `xet_client/src/cas_client/retry_wrapper.rs`, `xet_client/src/cas_client/remote_client.rs`. A 404 from `cas::query_dedup` is an expected cache miss — the caller converts it to `Ok(None)` and proceeds to upload. Today the retry wrapper logs it as `Fatal Error: \"cas::query_dedup\" api call failed ... 404 Not Found`, producing **20+ alarming-looking lines per upload session** with no actual failure behind them (Hoyt flagged this in the incident Slack thread). Fix: add `RetryWrapper::with_expected_404()` — mirroring the existing `with_expected_416()` pattern — and opt `query_dedup` into it. The 404 still short-circuits retries and surfaces as a fatal error to the caller (preserving the existing `Ok(None)` conversion), but the log line now reads `Not Found (cache miss): \"cas::query_dedup\" api call failed ... 404 Not Found`. ## Test plan - [x] `cargo +nightly fmt --all --check` clean - [x] `cargo test -p xet-client --lib cas_client::retry_wrapper` — 5 passed (incl. new `test_404_expected_is_fatal_and_not_retried`) - [ ] Manually verify `HF_XET_CLIENT_READ_TIMEOUT=120` still overrides via env - [ ] Confirm a session run produces no `Fatal Error:` lines for the `query_dedup` 404s - [ ] Watch the xorb POST error-rate panel on the [CAS Grafana dashboard](https://grafana.huggingface.tech/d/dejp4w2hael1cb/cas) after release; expect the 120s-clustered p50 to disappear 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Adjusts client networking defaults (read timeout) and alters retry-wrapper handling/logging for HTTP 404s, which can change behavior and observability for slow uploads and cache-miss paths. > > **Overview** > Raises the default `HF_XET_CLIENT_READ_TIMEOUT` from 120s to 300s to better tolerate slow-but-progressing transfers. > > Adds `RetryWrapper::with_expected_404()` and opts `cas::query_dedup` into it so 404 responses are still non-retried/fatal to the caller but are logged as an expected *cache miss* (with a new unit test covering the no-retry behavior). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3e88f9c. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This PR adds a function, next_stable_chunk_boundary, that takes a list of chunk boundary positions and a starting cut point and returns the next chunk boundary after the cut point such that, for all possible alterations of the data up to the cut point, the chunk boundaries when chunking the entire file will always be the same starting at the stable chunk boundary. The implication of this is that to alter a specific range of a file `[a, b)`, we would do the following: 1. Locate the previous chunk boundary before a; call this `c_start`. 2. Take the full set of chunk boundary locations, call next_stable_chunk_boundary with b as the cut point. this will return the next stable chunk boundary. Call this `c_end`. 3. Make the replacement to `[a, b)`; prepend the original `data[c_start, a)` and append `data[b, c_end)`; chunk this segment. 4. Use the merkle hash subtrees for `[0, c_start)`, the new [c_start, c_end), and the original `[c_end, end)` to calculate the new file hash. This will be the same as chunking the entire new file. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Adds new public chunk-boundary selection logic used to make resumed/partial workflows deterministic; mistakes could cause misalignment or incorrect resume behavior in deduplication/chunking paths. Large new randomized/stress tests reduce risk but the algorithm’s correctness assumptions are subtle. > > **Overview** > Introduces a new public helper, `next_stable_chunk_boundary`, that computes a restart-safe/stable resume boundary *from existing chunk-boundary metadata* (no byte access) by scanning for two consecutive chunks that fall within a conservative size window derived from chunking constants. > > Updates `find_partitions` documentation to reflect the hash warmup/hidden-trigger verification approach and to reference the new helper, re-exports the function from `xet_data::deduplication`, and adds extensive edge-case and randomized mutation/stress tests to validate boundary stability under arbitrary prefix changes. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 9841160. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…g-correctness-update
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds additional stress testing to #717, causing the simulation server logic to properly use next_stable_chunk_boundary logic for the simulation. As a result, multiple requested ranges for editing could be merged into a single range on the server end, which required updating some checks on the client side.
Additional stress tests were added under the simulation feature flag, and smoke tests added as well for the cargo smoke-test feature.
Note
Medium Risk
Moderate risk: changes core dedup/chunk-window construction and relaxes client/server window shape assumptions, which can affect correctness of range uploads and hashing. Also bumps low-level deps (
ctor,openssl) and adds a Node napi smoke-test example, increasing build surface area.Overview
Adds a new public helper
next_stable_chunk_boundary(canonical inxet_core_structures, re-exported fromxet_data) and updates server-sidebuild_file_chunk_hashes_responseto extend dirty ranges to the next stable chunk boundary and coalesce overlaps before computing windows.Updates
upload_rangesto accept that the server may merge windows (validating onlywindowsnon-empty andhash_ranges.len() == windows.len() + 1) and adds targeted regression/stress tests plus a newxet_datatest suite validating stable-boundary behavior under random prefix mutations.Separately: improves retry logging by marking
query_dedup404s as expected cache misses, increases client read timeout to 300s, bumpsctorto v1 and updatesopensslcrates, tightens a few minor iterations/formatting, and adds anexamples/xet_pkg_napiNode addon smoke-test project (excluded from the workspace).Reviewed by Cursor Bugbot for commit 49427f0. Bugbot is set up for automated code reviews on this repo. Configure here.